Skip to content

Commit

Permalink
PMM-13633 Add hashing to too long Service accounts names.
Browse files Browse the repository at this point in the history
  • Loading branch information
JiriCtvrtka committed Jan 8, 2025
1 parent 854b7df commit 62f230f
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 7 deletions.
25 changes: 21 additions & 4 deletions pkg/services/user/userimpl/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ package userimpl

import (
"context"
"crypto/md5"
"encoding/json"
"fmt"
"strings"
"time"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/tracing"
Expand All @@ -23,6 +21,8 @@ import (
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)

type Service struct {
Expand Down Expand Up @@ -400,6 +400,23 @@ func (s *Service) GetProfile(ctx context.Context, query *user.GetUserProfileQuer
return s.store.GetProfile(ctx, query)
}

// 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.
//
// Be aware that the same method is implemented in the PMM repo, and all changes should be reflected there as well!
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
}

// CreateServiceAccount creates a service account in the user table and adds service account to an organisation in the org_user table
func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) {
ctx, span := s.tracer.Start(ctx, "user.CreateServiceAccount", trace.WithAttributes(
Expand All @@ -416,7 +433,7 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser
// create user
usr := &user.User{
Email: cmd.Email,
Name: cmd.Name,
Name: SanitizeSAName(cmd.Name),
Login: cmd.Login,
IsDisabled: cmd.IsDisabled,
OrgID: cmd.OrgID,
Expand Down
30 changes: 27 additions & 3 deletions pkg/services/user/userimpl/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,22 @@ package userimpl

import (
"context"
"crypto/rand"
"encoding/base64"
"errors"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/team/teamtest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestUserService(t *testing.T) {
Expand Down Expand Up @@ -264,6 +266,28 @@ func TestMetrics(t *testing.T) {
})
}

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
}

func TestSanitizeSAName(t *testing.T) {
// max possible length without hashing
len180, err := generateRandomString(180)
require.NoError(t, err)
require.Equal(t, len180, SanitizeSAName(len180))

// too long length - postfix hashed
len200, err := generateRandomString(200)
require.NoError(t, err)
len200sanitized := SanitizeSAName(len200)
require.Equal(t, fmt.Sprintf("%s%s", len200[:148], len200sanitized[148:]), len200sanitized)
}

type FakeUserStore struct {
ExpectedUser *user.User
ExpectedSignedInUser *user.SignedInUser
Expand Down

0 comments on commit 62f230f

Please sign in to comment.