From 854b7dff3729efc9265b1871108b92b94c95cc13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Fri, 27 Dec 2024 09:41:51 +0100 Subject: [PATCH 1/9] PMM-13633 Nil response instead error in case of it is not SA. --- pkg/services/serviceaccounts/api/token.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index e06b7001fbf2f..f044eab56e081 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -114,7 +114,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *contextmodel.ReqContext) response // @PERCONA // swagger:route GET /auth/serviceaccount serviceaccounts retrieveServiceAccount // -// # CurrentServiceAccount get current service account info +// # CurrentServiceAccount get current service account info, in case of nil it is not service account. // // Requires service account token authentication. // @@ -126,7 +126,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *contextmodel.ReqContext) response // 500: internalServerError func (api *ServiceAccountsAPI) CurrentServiceAccount(ctx *contextmodel.ReqContext) response.Response { if !ctx.IsServiceAccount { - return response.Error(http.StatusBadRequest, "Auth method is not service account token", errors.New("failed to get service account info")) + return response.JSON(http.StatusOK, nil) } serviceAccount, err := api.service.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgID, ctx.UserID) From 62f230f81c33d9b6215624e12f9d43d40236a4a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 8 Jan 2025 11:34:00 +0100 Subject: [PATCH 2/9] PMM-13633 Add hashing to too long Service accounts names. --- pkg/services/user/userimpl/user.go | 25 +++++++++++++++++---- pkg/services/user/userimpl/user_test.go | 30 ++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index e7168251986e1..ab12891747121 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -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" @@ -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 { @@ -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( @@ -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, diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 346f854526721..88ff2403d2f6c 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -2,13 +2,13 @@ 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" @@ -16,6 +16,8 @@ import ( "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) { @@ -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 From 4b11b5d548573fcf933fb3e07bbb4a4ab90033d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 8 Jan 2025 14:25:36 +0100 Subject: [PATCH 3/9] PMM-13633 Add hashing even for tokens. --- .../serviceaccounts/database/token_store.go | 3 +- pkg/services/user/userimpl/user.go | 21 ++---------- pkg/services/user/userimpl/user_test.go | 25 --------------- pkg/util/nameutil/serviceaccount.go | 23 +++++++++++++ pkg/util/nameutil/serviceaccount_test.go | 32 +++++++++++++++++++ 5 files changed, 59 insertions(+), 45 deletions(-) create mode 100644 pkg/util/nameutil/serviceaccount.go create mode 100644 pkg/util/nameutil/serviceaccount_test.go diff --git a/pkg/services/serviceaccounts/database/token_store.go b/pkg/services/serviceaccounts/database/token_store.go index 173f7a27177d1..1bc6346eacdd2 100644 --- a/pkg/services/serviceaccounts/database/token_store.go +++ b/pkg/services/serviceaccounts/database/token_store.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/util/nameutil" ) const maxRetrievedTokens = 300 @@ -50,7 +51,7 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s } addKeyCmd := &apikey.AddCommand{ - Name: cmd.Name, + Name: nameutil.SanitizeSAName(cmd.Name), Role: org.RoleViewer, OrgID: cmd.OrgId, Key: cmd.Key, diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index ab12891747121..b04310763545f 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -2,7 +2,6 @@ package userimpl import ( "context" - "crypto/md5" "encoding/json" "fmt" "strings" @@ -21,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" + "github.com/grafana/grafana/pkg/util/nameutil" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -400,23 +400,6 @@ 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( @@ -433,7 +416,7 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser // create user usr := &user.User{ Email: cmd.Email, - Name: SanitizeSAName(cmd.Name), + Name: nameutil.SanitizeSAName(cmd.Name), Login: cmd.Login, IsDisabled: cmd.IsDisabled, OrgID: cmd.OrgID, diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 88ff2403d2f6c..956d0e4f6f29a 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -2,10 +2,7 @@ package userimpl import ( "context" - "crypto/rand" - "encoding/base64" "errors" - "fmt" "testing" "time" @@ -266,28 +263,6 @@ 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 diff --git a/pkg/util/nameutil/serviceaccount.go b/pkg/util/nameutil/serviceaccount.go new file mode 100644 index 0000000000000..e2d6b6ae601a3 --- /dev/null +++ b/pkg/util/nameutil/serviceaccount.go @@ -0,0 +1,23 @@ +package nameutil + +import ( + "crypto/md5" + "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. +// +// 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 +} diff --git a/pkg/util/nameutil/serviceaccount_test.go b/pkg/util/nameutil/serviceaccount_test.go new file mode 100644 index 0000000000000..b24714719ef6d --- /dev/null +++ b/pkg/util/nameutil/serviceaccount_test.go @@ -0,0 +1,32 @@ +package nameutil + +import ( + "crypto/rand" + "encoding/base64" + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +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) +} + +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 +} From 20c04b56c672cfb123462b2d90d2c3cbe041e9c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 8 Jan 2025 17:01:01 +0100 Subject: [PATCH 4/9] PMM-13633 Changes, level above. --- pkg/services/serviceaccounts/database/store.go | 7 ++++--- pkg/services/user/userimpl/user.go | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/services/serviceaccounts/database/store.go b/pkg/services/serviceaccounts/database/store.go index ff8105401dc9a..dbc579577df84 100644 --- a/pkg/services/serviceaccounts/database/store.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util/nameutil" ) type ServiceAccountsStoreImpl struct { @@ -58,7 +59,7 @@ func generateLogin(prefix string, orgId int64, name string) string { // CreateServiceAccount creates service account func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { - name := saForm.Name + name := nameutil.SanitizeSAName(saForm.Name) login := generateLogin(serviceaccounts.ServiceAccountPrefix, orgId, saForm.Name) isDisabled := false role := org.RoleViewer @@ -481,8 +482,8 @@ func (s *ServiceAccountsStoreImpl) MigrateApiKey(ctx context.Context, orgId int6 func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Context, key *apikey.APIKey) error { prefix := "sa-autogen" cmd := user.CreateUserCommand{ - Login: generateLogin(prefix, key.OrgID, key.Name), - Name: fmt.Sprintf("%v-%v", prefix, key.Name), + Login: nameutil.SanitizeSAName(generateLogin(prefix, key.OrgID, key.Name)), + Name: nameutil.SanitizeSAName(fmt.Sprintf("%v-%v", prefix, key.Name)), OrgID: key.OrgID, DefaultOrgRole: string(key.Role), IsServiceAccount: true, diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index b04310763545f..5b331decb8467 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -20,7 +20,6 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" - "github.com/grafana/grafana/pkg/util/nameutil" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -416,7 +415,7 @@ func (s *Service) CreateServiceAccount(ctx context.Context, cmd *user.CreateUser // create user usr := &user.User{ Email: cmd.Email, - Name: nameutil.SanitizeSAName(cmd.Name), + Name: cmd.Name, Login: cmd.Login, IsDisabled: cmd.IsDisabled, OrgID: cmd.OrgID, From 87acd3a679dfa1993fadfc5cd68278369fd85687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 8 Jan 2025 17:29:43 +0100 Subject: [PATCH 5/9] PMM-13633 Apply on login in create too. --- pkg/services/serviceaccounts/database/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/serviceaccounts/database/store.go b/pkg/services/serviceaccounts/database/store.go index dbc579577df84..a96bd4bec1994 100644 --- a/pkg/services/serviceaccounts/database/store.go +++ b/pkg/services/serviceaccounts/database/store.go @@ -60,7 +60,7 @@ func generateLogin(prefix string, orgId int64, name string) string { // CreateServiceAccount creates service account func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { name := nameutil.SanitizeSAName(saForm.Name) - login := generateLogin(serviceaccounts.ServiceAccountPrefix, orgId, saForm.Name) + login := nameutil.SanitizeSAName(generateLogin(serviceaccounts.ServiceAccountPrefix, orgId, saForm.Name)) isDisabled := false role := org.RoleViewer if saForm.IsDisabled != nil { From 6abaf7e718dfbff95814e9cb5ef51d1892170f4c Mon Sep 17 00:00:00 2001 From: Matej Kubinec Date: Tue, 7 Jan 2025 13:39:46 +0100 Subject: [PATCH 6/9] PMM-13633 Add apikey migration modal --- .../app/core/components/Login/LoginCtrl.tsx | 18 +++++-- .../PerconaBootstrapper.tsx | 7 ++- .../PerconaMigrator/PerconaMigrator.tsx | 19 +++++++ .../components/FailedMigrationRow.messages.ts | 5 ++ .../components/FailedMigrationRow.tsx | 33 ++++++++++++ .../components/FailedMigrationRow.types.ts | 4 ++ .../components/FailedMigrationRow.utils.ts | 17 +++++++ .../components/MigrationSummary.constants.ts | 4 ++ .../components/MigrationSummary.messages.ts | 12 +++++ .../components/MigrationSummary.styles.ts | 31 +++++++++++ .../components/MigrationSummary.tsx | 51 +++++++++++++++++++ .../PerconaMigrator/index.ts | 3 ++ .../app/percona/shared/core/hooks/migrator.ts | 31 +++++++++++ .../percona/shared/core/reducers/user/user.ts | 11 ++++ .../shared/core/reducers/user/user.types.ts | 1 + .../shared/core/reducers/user/user.utils.ts | 1 + .../shared/services/user/User.service.ts | 4 ++ .../shared/services/user/User.types.ts | 2 + 18 files changed, 250 insertions(+), 4 deletions(-) create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/PerconaMigrator.tsx create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.messages.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.types.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.utils.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.messages.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.styles.ts create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.tsx create mode 100644 public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/index.ts create mode 100644 public/app/percona/shared/core/hooks/migrator.ts diff --git a/public/app/core/components/Login/LoginCtrl.tsx b/public/app/core/components/Login/LoginCtrl.tsx index 7adf963eed4ac..9f2395b586ab3 100644 --- a/public/app/core/components/Login/LoginCtrl.tsx +++ b/public/app/core/components/Login/LoginCtrl.tsx @@ -118,15 +118,27 @@ export class LoginCtrl extends PureComponent { }; toGrafana = () => { + // @PERCONA + // trigger apikey migration on login + const constructUrl = (url: string) => { + const forceApiKeyMigration = 'force-apikey-migration=true'; + + if (url.includes('?')) { + return url + '&' + forceApiKeyMigration; + } else { + return url + '?' + forceApiKeyMigration; + } + }; + // Use window.location.href to force page reload if (this.result?.redirectUrl) { if (config.appSubUrl !== '' && !this.result.redirectUrl.startsWith(config.appSubUrl)) { - window.location.assign(config.appSubUrl + this.result.redirectUrl); + window.location.assign(constructUrl(config.appSubUrl + this.result.redirectUrl)); } else { - window.location.assign(this.result.redirectUrl); + window.location.assign(constructUrl(this.result.redirectUrl)); } } else { - window.location.assign(config.appSubUrl + '/'); + window.location.assign(constructUrl(config.appSubUrl + '/')); } }; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaBootstrapper.tsx b/public/app/percona/shared/components/PerconaBootstrapper/PerconaBootstrapper.tsx index c107e57b1a0e8..9263af97f972c 100644 --- a/public/app/percona/shared/components/PerconaBootstrapper/PerconaBootstrapper.tsx +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaBootstrapper.tsx @@ -19,6 +19,7 @@ import { useAppDispatch } from 'app/store/store'; import { useSelector } from 'app/types'; import { Telemetry } from '../../../ui-events/components/Telemetry'; +import { useMigrator } from '../../core/hooks/migrator'; import usePerconaTour from '../../core/hooks/tour'; import { checkUpdatesAction } from '../../core/reducers/updates'; import { logger } from '../../helpers/logger'; @@ -27,6 +28,7 @@ import { isPmmAdmin } from '../../helpers/permissions'; import { Messages } from './PerconaBootstrapper.messages'; import { getStyles } from './PerconaBootstrapper.styles'; import { PerconaBootstrapperProps } from './PerconaBootstrapper.types'; +import PerconaMigrator from './PerconaMigrator'; import PerconaNavigation from './PerconaNavigation/PerconaNavigation'; import PerconaTourBootstrapper from './PerconaTour'; import PerconaUpdateVersion from './PerconaUpdateVersion/PerconaUpdateVersion'; @@ -42,6 +44,7 @@ export const PerconaBootstrapper = ({ onReady }: PerconaBootstrapperProps) => { const { user } = config.bootData; const { isSignedIn } = user; const theme = useTheme2(); + const { migrationSummaryVisible } = useMigrator(); const dismissModal = () => { setModalIsOpen(false); @@ -108,7 +111,9 @@ export const PerconaBootstrapper = ({ onReady }: PerconaBootstrapperProps) => { {isSignedIn && } - {updateAvailable && showUpdateModal && !isLoadingUpdates ? ( + {isSignedIn && isPmmAdmin(user) && migrationSummaryVisible ? ( + + ) : updateAvailable && showUpdateModal && !isLoadingUpdates ? ( ) : ( isSignedIn && diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/PerconaMigrator.tsx b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/PerconaMigrator.tsx new file mode 100644 index 0000000000000..08cc390a37482 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/PerconaMigrator.tsx @@ -0,0 +1,19 @@ +import React, { FC } from 'react'; + +import { useMigrator } from 'app/percona/shared/core/hooks/migrator'; +import { useSelector } from 'app/types'; + +import MigrationSummary from './components/MigrationSummary'; + +const PerconaMigrator: FC = () => { + const migrationResult = useSelector((state) => state.apiKeys.migrationResult); + const { migrationSummaryVisible, dismissSummary } = useMigrator(); + + if (!migrationResult || !migrationSummaryVisible) { + return; + } + + return ; +}; + +export default PerconaMigrator; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.messages.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.messages.ts new file mode 100644 index 0000000000000..e0c8eee362238 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.messages.ts @@ -0,0 +1,5 @@ +export const Messages = { + id: (id: number) => `API Key ID ${id}`, + name: 'Name: ', + error: 'Error: ', +}; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx new file mode 100644 index 0000000000000..888f83ee11167 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx @@ -0,0 +1,33 @@ +import React, { FC } from 'react'; + +import { Text } from '@grafana/ui'; + +import { Messages } from './FailedMigrationRow.messages'; +import { FailedMigrationRowProps } from './FailedMigrationRow.types'; +import { parseDetail } from './FailedMigrationRow.utils'; + +const FailedMigrationRow: FC = ({ id, details }) => { + const { name, error } = parseDetail(details); + + return ( +
  • + {Messages.id(id)} +
      +
    • + + {Messages.name} + {name} + +
      • +
    • + + {Messages.error} + {error} + +
      • +
    +
    • + ); +}; + +export default FailedMigrationRow; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.types.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.types.ts new file mode 100644 index 0000000000000..3855a4a010679 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.types.ts @@ -0,0 +1,4 @@ +export interface FailedMigrationRowProps { + id: number; + details: string; +} diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.utils.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.utils.ts new file mode 100644 index 0000000000000..b902c22c5d473 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.utils.ts @@ -0,0 +1,17 @@ +import { capitalizeText } from 'app/percona/shared/helpers/capitalizeText'; + +export const parseDetail = (detail: string) => { + if (!detail.includes('- Error: ')) { + return { + name: '', + error: detail, + }; + } + + const [name, error] = detail.split('- Error: '); + + return { + name: name.replace('API key name: ', ''), + error: capitalizeText(error), + }; +}; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts new file mode 100644 index 0000000000000..caaee4555b119 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts @@ -0,0 +1,4 @@ +export const SERVICE_ACCOUNTS_DOCS_LINK = + 'https://docs.percona.com/percona-monitoring-and-management/3/api/authentication.html?h=service+accounts'; + +export const CONTACT_SUPPORT_LINK = 'https://www.percona.com/about/contact'; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.messages.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.messages.ts new file mode 100644 index 0000000000000..055732fd31514 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.messages.ts @@ -0,0 +1,12 @@ +export const Messages = { + title: 'API migration status', + description: + 'Migration of API keys complete. The following keys could not be converted to service accounts but remain fully functional:', + failed: (failed: number, total: number) => `Failed conversions (${failed} of ${total} total)`, + needHelp: 'Need help? See our ', + documentation: 'documentation', + or: ' or ', + contactSupport: 'contact support', + dot: '.', + close: 'Close', +}; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.styles.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.styles.ts new file mode 100644 index 0000000000000..e24c6fd858151 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.styles.ts @@ -0,0 +1,31 @@ +import { css } from '@emotion/css'; + +import { GrafanaTheme2 } from '@grafana/data'; + +export const getStyles = (theme: GrafanaTheme2) => ({ + modal: css` + width: auto; + min-width: 750px; + max-width: 75vw; + `, + migrationSummary: css` + padding: ${theme.spacing(2)}; + padding-top: 0; + `, + list: css` + margin-top: ${theme.spacing(0.5)}; + margin-bottom: ${theme.spacing(2)}; + + ol, + ul { + margin-left: ${theme.spacing(2)}; + + li > * { + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + } + `, +}); diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.tsx b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.tsx new file mode 100644 index 0000000000000..b0cb1d5c73ec9 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.tsx @@ -0,0 +1,51 @@ +import React, { FC } from 'react'; + +import { Button, Modal, TextLink, useStyles2 } from '@grafana/ui'; +import { MigrationSummaryProps } from 'app/features/api-keys/ApiKeysPage'; + +import FailedMigrationRow from './FailedMigrationRow'; +import { CONTACT_SUPPORT_LINK, SERVICE_ACCOUNTS_DOCS_LINK } from './MigrationSummary.constants'; +import { Messages } from './MigrationSummary.messages'; +import { getStyles } from './MigrationSummary.styles'; + +const MigrationSummary: FC = ({ visible, data, onDismiss }) => { + const styles = useStyles2(getStyles); + + return ( + +

    {Messages.description}

    + {Messages.failed(data.failed, data.total)} +
    +
      + {data.failedApikeyIDs.map((id, idx) => ( + + ))} +
    +
    +

    + {Messages.needHelp} + + {Messages.documentation} + + {Messages.or} + + {Messages.contactSupport} + + {Messages.dot} +

    + + + +     + ); +}; + +export default MigrationSummary; diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/index.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/index.ts new file mode 100644 index 0000000000000..e56e803db8a71 --- /dev/null +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/index.ts @@ -0,0 +1,3 @@ +import PerconaMigrator from './PerconaMigrator'; + +export default PerconaMigrator; diff --git a/public/app/percona/shared/core/hooks/migrator.ts b/public/app/percona/shared/core/hooks/migrator.ts new file mode 100644 index 0000000000000..2493f6fe1c546 --- /dev/null +++ b/public/app/percona/shared/core/hooks/migrator.ts @@ -0,0 +1,31 @@ +import { useEffect } from 'react'; +import { useLocation } from 'react-router'; + +import { migrateAll } from 'app/features/api-keys/state/actions'; +import { useAppDispatch } from 'app/store/store'; +import { useSelector } from 'app/types'; + +import { snoozeApiKeyMigrationSummary } from '../reducers/user/user'; +import { getPerconaUser } from '../selectors'; + +export const useMigrator = () => { + const migrationResult = useSelector((state) => state.apiKeys.migrationResult); + const { snoozedApiKeysMigration } = useSelector(getPerconaUser); + const dispatch = useAppDispatch(); + const location = useLocation(); + const migrationSummaryVisible = !snoozedApiKeysMigration && migrationResult && migrationResult.failed > 0; + + useEffect(() => { + if (!location.search.includes('force-apikey-migration=true')) { + return; + } + + dispatch(migrateAll()); + }, [location.search, dispatch]); + + const dismissSummary = () => { + dispatch(snoozeApiKeyMigrationSummary(true)); + }; + + return { migrationSummaryVisible, dismissSummary }; +}; diff --git a/public/app/percona/shared/core/reducers/user/user.ts b/public/app/percona/shared/core/reducers/user/user.ts index d8391e0f948fe..022a179881857 100644 --- a/public/app/percona/shared/core/reducers/user/user.ts +++ b/public/app/percona/shared/core/reducers/user/user.ts @@ -9,6 +9,7 @@ import { toUserDetailsModel } from './user.utils'; export const initialUserState: PerconaUserState = { userId: 0, + snoozedApiKeysMigration: true, productTourCompleted: true, alertingTourCompleted: true, isAuthorized: false, @@ -86,6 +87,16 @@ export const setAlertingTourCompleted = createAsyncThunk( } ); +export const snoozeApiKeyMigrationSummary = createAsyncThunk( + 'percona/snoozeApiKeyMigrationSummary', + async (dismiss: boolean, thunkAPI): Promise => { + const res = await UserService.setApiKeyMigrationSnoozed(dismiss); + const details = toUserDetailsModel(res); + thunkAPI.dispatch(setUserDetails(details)); + return details; + } +); + export const { setAuthorized, setIsPlatformUser, setUserDetails } = perconaUserSlice.actions; export default perconaUserSlice.reducer; diff --git a/public/app/percona/shared/core/reducers/user/user.types.ts b/public/app/percona/shared/core/reducers/user/user.types.ts index c8a950d900c5a..ed9a3fe1e6545 100644 --- a/public/app/percona/shared/core/reducers/user/user.types.ts +++ b/public/app/percona/shared/core/reducers/user/user.types.ts @@ -1,5 +1,6 @@ export interface UserDetails { userId: number; + snoozedApiKeysMigration: boolean; productTourCompleted: boolean; alertingTourCompleted: boolean; snoozedPmmVersion?: string; diff --git a/public/app/percona/shared/core/reducers/user/user.utils.ts b/public/app/percona/shared/core/reducers/user/user.utils.ts index 5e972077d1bd8..d539c4a993246 100644 --- a/public/app/percona/shared/core/reducers/user/user.utils.ts +++ b/public/app/percona/shared/core/reducers/user/user.utils.ts @@ -6,5 +6,6 @@ export const toUserDetailsModel = (res: UserDetailsResponse): UserDetails => ({ userId: res.user_id, productTourCompleted: !!res.product_tour_completed, alertingTourCompleted: !!res.alerting_tour_completed, + snoozedApiKeysMigration: !!res.snoozed_api_keys_migration, snoozedPmmVersion: res.snoozed_pmm_version, }); diff --git a/public/app/percona/shared/services/user/User.service.ts b/public/app/percona/shared/services/user/User.service.ts index eeefbe6be117c..030c54a462c50 100644 --- a/public/app/percona/shared/services/user/User.service.ts +++ b/public/app/percona/shared/services/user/User.service.ts @@ -26,6 +26,10 @@ export const UserService = { const payload: UserDetailsPutPayload = { snoozed_pmm_version: version }; return await api.put('/v1/users/me', payload); }, + async setApiKeyMigrationSnoozed(shouldSnooze: boolean): Promise { + const payload: UserDetailsPutPayload = { snoozed_api_keys_migration: shouldSnooze }; + return await api.put('/v1/users/me', payload); + }, async getUsersList(): Promise { return await api.get('/v1/users'); }, diff --git a/public/app/percona/shared/services/user/User.types.ts b/public/app/percona/shared/services/user/User.types.ts index a31cadb6fc239..e143824d3b319 100644 --- a/public/app/percona/shared/services/user/User.types.ts +++ b/public/app/percona/shared/services/user/User.types.ts @@ -6,6 +6,7 @@ export interface UserDetailsResponse { user_id: number; product_tour_completed?: boolean; alerting_tour_completed?: boolean; + snoozed_api_keys_migration?: boolean; snoozed_pmm_version?: string; } @@ -13,6 +14,7 @@ export interface UserDetailsPutPayload { product_tour_completed?: boolean; alerting_tour_completed?: boolean; snoozed_pmm_version?: string; + snoozed_api_keys_migration?: boolean; } export interface UserListItemResponse { From 64ddbff152e6e90bea2f6d335766bc6e0f0496c8 Mon Sep 17 00:00:00 2001 From: Matej Kubinec Date: Thu, 9 Jan 2025 07:48:17 +0100 Subject: [PATCH 7/9] PMM-13633 Add comments and name check --- .../components/FailedMigrationRow.tsx | 14 ++++++++------ .../components/MigrationSummary.constants.ts | 2 ++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx index 888f83ee11167..5c096ac39f125 100644 --- a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/FailedMigrationRow.tsx @@ -13,12 +13,14 @@ const FailedMigrationRow: FC = ({ id, details }) => {
  • {Messages.id(id)}
      -
    • - - {Messages.name} - {name} - -
      • + {!!name && ( +
    • + + {Messages.name} + {name} + +
      • + )}
    • {Messages.error} diff --git a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts index caaee4555b119..158894dc3de31 100644 --- a/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts +++ b/public/app/percona/shared/components/PerconaBootstrapper/PerconaMigrator/components/MigrationSummary.constants.ts @@ -1,4 +1,6 @@ +// todo: use a shortened link export const SERVICE_ACCOUNTS_DOCS_LINK = 'https://docs.percona.com/percona-monitoring-and-management/3/api/authentication.html?h=service+accounts'; +// todo: use a shortened link export const CONTACT_SUPPORT_LINK = 'https://www.percona.com/about/contact'; From 3e9b49cff74db9df6cff5fe81bad9483bcd6a0dc Mon Sep 17 00:00:00 2001 From: Matej Kubinec Date: Thu, 9 Jan 2025 13:27:09 +0100 Subject: [PATCH 8/9] PMM-13633 Add migration success message --- public/app/percona/shared/core/hooks/migrator.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/public/app/percona/shared/core/hooks/migrator.ts b/public/app/percona/shared/core/hooks/migrator.ts index 2493f6fe1c546..8f2979fac3fd0 100644 --- a/public/app/percona/shared/core/hooks/migrator.ts +++ b/public/app/percona/shared/core/hooks/migrator.ts @@ -1,6 +1,8 @@ import { useEffect } from 'react'; import { useLocation } from 'react-router'; +import { AppEvents } from '@grafana/data'; +import { appEvents } from 'app/core/core'; import { migrateAll } from 'app/features/api-keys/state/actions'; import { useAppDispatch } from 'app/store/store'; import { useSelector } from 'app/types'; @@ -23,6 +25,15 @@ export const useMigrator = () => { dispatch(migrateAll()); }, [location.search, dispatch]); + useEffect(() => { + if (migrationResult && migrationResult.total > 0 && migrationResult.failed === 0) { + // give some time for the app to load + setTimeout(() => { + appEvents.emit(AppEvents.alertSuccess, ['All api keys successfully migrated']); + }, 1000); + } + }, [migrationResult]); + const dismissSummary = () => { dispatch(snoozeApiKeyMigrationSummary(true)); }; From 4f68045b9931b306e99c2d7c9e8cadf1bd86d983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Mon, 13 Jan 2025 11:14:22 +0100 Subject: [PATCH 9/9] PMM-13633 Required changes. --- pkg/services/serviceaccounts/api/token.go | 4 ++-- pkg/services/user/userimpl/user.go | 5 +++-- pkg/services/user/userimpl/user_test.go | 5 +++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go index f044eab56e081..e06b7001fbf2f 100644 --- a/pkg/services/serviceaccounts/api/token.go +++ b/pkg/services/serviceaccounts/api/token.go @@ -114,7 +114,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *contextmodel.ReqContext) response // @PERCONA // swagger:route GET /auth/serviceaccount serviceaccounts retrieveServiceAccount // -// # CurrentServiceAccount get current service account info, in case of nil it is not service account. +// # CurrentServiceAccount get current service account info // // Requires service account token authentication. // @@ -126,7 +126,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *contextmodel.ReqContext) response // 500: internalServerError func (api *ServiceAccountsAPI) CurrentServiceAccount(ctx *contextmodel.ReqContext) response.Response { if !ctx.IsServiceAccount { - return response.JSON(http.StatusOK, nil) + return response.Error(http.StatusBadRequest, "Auth method is not service account token", errors.New("failed to get service account info")) } serviceAccount, err := api.service.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgID, ctx.UserID) diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 5b331decb8467..e7168251986e1 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -7,6 +7,9 @@ import ( "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" @@ -20,8 +23,6 @@ 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 { diff --git a/pkg/services/user/userimpl/user_test.go b/pkg/services/user/userimpl/user_test.go index 956d0e4f6f29a..346f854526721 100644 --- a/pkg/services/user/userimpl/user_test.go +++ b/pkg/services/user/userimpl/user_test.go @@ -6,6 +6,9 @@ import ( "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" @@ -13,8 +16,6 @@ import ( "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) {