From 84c8b60faa42d0b01ee44e178fcc938eed4c7cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= <62988319+JiriCtvrtka@users.noreply.github.com> Date: Fri, 8 Nov 2024 10:51:48 +0100 Subject: [PATCH] PMM-13315 Service account max length. (#3221) * PMM-13315 Remove comment about SA max length. * PMM-13315 Long postfix for tests. * trigger * Revert "PMM-13315 Remove comment about SA max length." This reverts commit d812c2a4326eb8967f46338e99d4401b13b210d6. * PMM-13315 Long SA names hashing. * PMM-13315 Lint. * PMM-11315 Changes, refactor. * PMM-13315 Format. * PMM-13315 Remove forgotten prints. * PMM-13315 Remove another print. * PMM-13315 Fix problem with big orgIDs. * PMM-13315 Move SanitizeSAName to global utils. * PMM-13315 Headers. * PMM-13315 Licence year. * PMM-13315 Lint. --- api-tests/server/auth_test.go | 9 ++++-- managed/services/grafana/client.go | 15 +++++----- managed/services/grafana/client_test.go | 6 +++- utils/grafana/serviceaccounts.go | 37 +++++++++++++++++++++++++ utils/grafana/serviceaccounts_test.go | 37 +++++++++++++++++++++++++ utils/strings/generate.go | 32 +++++++++++++++++++++ 6 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 utils/grafana/serviceaccounts.go create mode 100644 utils/grafana/serviceaccounts_test.go create mode 100644 utils/strings/generate.go diff --git a/api-tests/server/auth_test.go b/api-tests/server/auth_test.go index a51d60b3dd..7529f89538 100644 --- a/api-tests/server/auth_test.go +++ b/api-tests/server/auth_test.go @@ -35,6 +35,8 @@ import ( pmmapitests "github.com/percona/pmm/api-tests" serverClient "github.com/percona/pmm/api/server/v1/json/client" server "github.com/percona/pmm/api/server/v1/json/client/server_service" + "github.com/percona/pmm/utils/grafana" + stringsgen "github.com/percona/pmm/utils/strings" ) const ( @@ -362,7 +364,8 @@ func TestServiceAccountPermissions(t *testing.T) { // service token role options: editor, admin // basic auth format is skipped, endpoint /auth/serviceaccount (to get info about currently used token in request) requires Bearer authorization // service_token:token format could be used in pmm-agent and pmm-admin (its transformed into Bearer authorization) - nodeName := "test-node" + nodeName, err := stringsgen.GenerateRandomString(256) + require.NoError(t, err) viewerNodeName := fmt.Sprintf("%s-viewer", nodeName) viewerAccountID := createServiceAccountWithRole(t, "Viewer", viewerNodeName) @@ -520,7 +523,7 @@ func createServiceAccountWithRole(t *testing.T, role, nodeName string) int { name := fmt.Sprintf("%s-%s", pmmServiceAccountName, nodeName) data, err := json.Marshal(map[string]string{ - "name": name, + "name": grafana.SanitizeSAName(name), "role": role, }) require.NoError(t, err) @@ -582,7 +585,7 @@ func createServiceToken(t *testing.T, serviceAccountID int, nodeName string) (in name := fmt.Sprintf("%s-%s", pmmServiceTokenName, nodeName) data, err := json.Marshal(map[string]string{ - "name": name, + "name": grafana.SanitizeSAName(name), }) require.NoError(t, err) diff --git a/managed/services/grafana/client.go b/managed/services/grafana/client.go index 707a5df63e..791c24a7ab 100644 --- a/managed/services/grafana/client.go +++ b/managed/services/grafana/client.go @@ -40,6 +40,7 @@ import ( "github.com/percona/pmm/managed/services" "github.com/percona/pmm/managed/utils/auth" "github.com/percona/pmm/managed/utils/irt" + "github.com/percona/pmm/utils/grafana" ) // ErrFailedToGetToken means it failed to get user's token. Most likely due to the fact user is not logged in using Percona Account. @@ -350,7 +351,7 @@ type serviceAccountSearch struct { func (c *Client) getServiceAccountIDFromName(ctx context.Context, nodeName string, authHeaders http.Header) (int, error) { var res serviceAccountSearch - serviceAccountName := fmt.Sprintf("%s-%s", pmmServiceAccountName, nodeName) + serviceAccountName := grafana.SanitizeSAName(fmt.Sprintf("%s-%s", pmmServiceAccountName, nodeName)) if err := c.do(ctx, http.MethodGet, "/api/serviceaccounts/search", fmt.Sprintf("query=%s", serviceAccountName), authHeaders, nil, &res); err != nil { return 0, err } @@ -383,7 +384,7 @@ func (c *Client) getNotPMMAgentTokenCountForServiceAccount(ctx context.Context, count := 0 for _, token := range tokens { serviceTokenName := fmt.Sprintf("%s-%s", pmmServiceTokenName, nodeName) - if !strings.HasPrefix(token.Name, serviceTokenName) { + if !strings.HasPrefix(token.Name, grafana.SanitizeSAName(serviceTokenName)) { count++ } } @@ -677,10 +678,8 @@ func (c *Client) createServiceAccount(ctx context.Context, role role, nodeName s return 0, errors.New("you cannot create service account with empty role") } - // Max length of service account name is 190 chars (default limit in Grafana). In reality it is 187. - // Some tests could fail if you pass a name longer than 187 chars. serviceAccountName := fmt.Sprintf("%s-%s", pmmServiceAccountName, nodeName) - b, err := json.Marshal(serviceAccount{Name: serviceAccountName, Role: role.String(), Force: reregister}) + b, err := json.Marshal(serviceAccount{Name: grafana.SanitizeSAName(serviceAccountName), Role: role.String(), Force: reregister}) if err != nil { return 0, errors.WithStack(err) } @@ -714,7 +713,7 @@ func (c *Client) createServiceToken(ctx context.Context, serviceAccountID int, n } } - b, err := json.Marshal(serviceToken{Name: serviceTokenName, Role: admin.String()}) + b, err := json.Marshal(serviceToken{Name: grafana.SanitizeSAName(serviceTokenName), Role: admin.String()}) if err != nil { return 0, "", errors.WithStack(err) } @@ -737,7 +736,7 @@ func (c *Client) serviceTokenExists(ctx context.Context, serviceAccountID int, n serviceTokenName := fmt.Sprintf("%s-%s", pmmServiceTokenName, nodeName) for _, token := range tokens { - if !strings.HasPrefix(token.Name, serviceTokenName) { + if !strings.HasPrefix(token.Name, grafana.SanitizeSAName(serviceTokenName)) { continue } @@ -755,7 +754,7 @@ func (c *Client) deletePMMAgentServiceToken(ctx context.Context, serviceAccountI serviceTokenName := fmt.Sprintf("%s-%s", pmmServiceTokenName, nodeName) for _, token := range tokens { - if strings.HasPrefix(token.Name, serviceTokenName) { + if strings.HasPrefix(token.Name, grafana.SanitizeSAName(serviceTokenName)) { if err := c.do(ctx, "DELETE", fmt.Sprintf("/api/serviceaccounts/%d/tokens/%d", serviceAccountID, token.ID), "", authHeaders, nil, nil); err != nil { return err } diff --git a/managed/services/grafana/client_test.go b/managed/services/grafana/client_test.go index 2b315a7275..50d28ed1a5 100644 --- a/managed/services/grafana/client_test.go +++ b/managed/services/grafana/client_test.go @@ -26,6 +26,8 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + stringsgen "github.com/percona/pmm/utils/strings" ) func TestClient(t *testing.T) { @@ -144,7 +146,9 @@ func TestClient(t *testing.T) { }) t.Run(fmt.Sprintf("Service token auth %s", role.String()), func(t *testing.T) { - nodeName := fmt.Sprintf("test-node-%s", role) + name, err := stringsgen.GenerateRandomString(256) + require.NoError(t, err) + nodeName := fmt.Sprintf("%s-%s", name, role) serviceAccountID, err := c.createServiceAccount(ctx, role, nodeName, true, authHeaders) require.NoError(t, err) defer func() { diff --git a/utils/grafana/serviceaccounts.go b/utils/grafana/serviceaccounts.go new file mode 100644 index 0000000000..4bcbe0570e --- /dev/null +++ b/utils/grafana/serviceaccounts.go @@ -0,0 +1,37 @@ +// Copyright (C) 2023 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// Package grafana provides util functions related to grafana functionality. +package grafana + +import ( + "crypto/md5" //nolint:gosec + "fmt" +) + +// SanitizeSAName is used for sanitize name and it's length for service accounts. +// Max length of service account name is 190 chars (limit in Grafana Postgres DB). +// However, prefix added by grafana is counted too. Prefix is sa-{orgID}-. +// Bare minimum is 5 chars reserved (orgID is <10, like sa-1-) and could be more depends +// on orgID number. Let's reserve 10 chars. It will cover almost one million orgIDs. +// Sanitizing, ensure its length by hashing postfix when length is exceeded. +// MD5 is used because it has fixed length 32 chars. +func SanitizeSAName(name string) string { + if len(name) <= 180 { + return name + } + + return fmt.Sprintf("%s%x", name[:148], md5.Sum([]byte(name[148:]))) //nolint:gosec +} diff --git a/utils/grafana/serviceaccounts_test.go b/utils/grafana/serviceaccounts_test.go new file mode 100644 index 0000000000..ddea7f833e --- /dev/null +++ b/utils/grafana/serviceaccounts_test.go @@ -0,0 +1,37 @@ +// Copyright (C) 2023 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . +package grafana + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + stringsgen "github.com/percona/pmm/utils/strings" +) + +func Test_sanitizeSAName(t *testing.T) { + // max possible length without hashing + len180, err := stringsgen.GenerateRandomString(180) + require.NoError(t, err) + require.Equal(t, len180, SanitizeSAName(len180)) + + // too long length - postfix hashed + len200, err := stringsgen.GenerateRandomString(200) + require.NoError(t, err) + len200sanitized := SanitizeSAName(len200) + require.Equal(t, fmt.Sprintf("%s%s", len200[:148], len200sanitized[148:]), len200sanitized) +} diff --git a/utils/strings/generate.go b/utils/strings/generate.go new file mode 100644 index 0000000000..81a62f0f9b --- /dev/null +++ b/utils/strings/generate.go @@ -0,0 +1,32 @@ +// Copyright (C) 2023 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// Package stringsgen provides functions to generate random strings. +package stringsgen + +import ( + "crypto/rand" + "encoding/base64" +) + +// GenerateRandomString returns random string with given length. +func GenerateRandomString(length int) (string, error) { + buffer := make([]byte, length) + _, err := rand.Read(buffer) + if err != nil { + return "", err + } + return base64.URLEncoding.EncodeToString(buffer)[:length], nil +}