Skip to content

Commit

Permalink
PMM-13315 Service account max length. (#3221)
Browse files Browse the repository at this point in the history
* 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 d812c2a.

* 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.
  • Loading branch information
JiriCtvrtka authored Nov 8, 2024
1 parent 14242e8 commit 84c8b60
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 12 deletions.
9 changes: 6 additions & 3 deletions api-tests/server/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down
15 changes: 7 additions & 8 deletions managed/services/grafana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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++
}
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}

Expand All @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion managed/services/grafana/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
37 changes: 37 additions & 0 deletions utils/grafana/serviceaccounts.go
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.

// 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
}
37 changes: 37 additions & 0 deletions utils/grafana/serviceaccounts_test.go
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.
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)
}
32 changes: 32 additions & 0 deletions utils/strings/generate.go
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.

// 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
}

0 comments on commit 84c8b60

Please sign in to comment.