diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 1d4c725f23c..c0aed770605 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -331,7 +331,7 @@ func TestLoginPostRedirect(t *testing.T) { HooksService: &hooks.HooksService{}, License: &licensing.OSSLicensingService{}, authnService: &authntest.FakeService{ - ExpectedIdentity: &authn.Identity{ID: "user:42", SessionToken: &usertoken.UserToken{}}, + ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:42"), SessionToken: &usertoken.UserToken{}}, }, AuthTokenService: authtest.NewFakeUserAuthTokenService(), Features: featuremgmt.WithFeatures(), diff --git a/pkg/api/org_test.go b/pkg/api/org_test.go index 39acc89cbd2..fcbc7f79bbf 100644 --- a/pkg/api/org_test.go +++ b/pkg/api/org_test.go @@ -266,7 +266,7 @@ func TestAPIEndpoint_GetOrg(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { expectedIdentity := &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), OrgID: 1, Permissions: map[int64]map[string][]string{ 0: accesscontrol.GroupScopesByAction(tt.permissions), diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index bc45f07ac9c..c5fc39ae508 100644 --- a/pkg/middleware/auth_test.go +++ b/pkg/middleware/auth_test.go @@ -94,7 +94,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedInNoAnonymous should return 200 for authenticated user", path: "/api/secure", authMiddleware: ReqSignedInNoAnonymous, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, expecedReached: true, expectedCode: http.StatusOK, }, @@ -102,7 +102,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "snapshot public mode disabled should return 200 for authenticated user", path: "/api/secure", authMiddleware: SnapshotPublicModeOrSignedIn(&setting.Cfg{SnapshotPublicMode: false}), - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, expecedReached: true, expectedCode: http.StatusOK, }, diff --git a/pkg/middleware/quota_test.go b/pkg/middleware/quota_test.go index 0cbc27da4e2..811871de525 100644 --- a/pkg/middleware/quota_test.go +++ b/pkg/middleware/quota_test.go @@ -52,7 +52,7 @@ func TestMiddlewareQuota(t *testing.T) { t.Run("with user logged in", func(t *testing.T) { setUp := func(sc *scenarioContext) { - sc.withIdentity(&authn.Identity{ID: "user:1", SessionToken: &auth.UserToken{UserId: 12}}) + sc.withIdentity(&authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{UserId: 12}}) } middlewareScenario(t, "global datasource quota reached", func(t *testing.T, sc *scenarioContext) { diff --git a/pkg/services/accesscontrol/authorize_in_org_test.go b/pkg/services/accesscontrol/authorize_in_org_test.go index 81d53dab2b8..c0c91734cba 100644 --- a/pkg/services/accesscontrol/authorize_in_org_test.go +++ b/pkg/services/accesscontrol/authorize_in_org_test.go @@ -186,7 +186,7 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/endpoint", nil) expectedIdentity := &authn.Identity{ - ID: fmt.Sprintf("user:%v", tc.ctxSignedInUser.UserID), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, tc.ctxSignedInUser.UserID), OrgID: tc.targetOrgId, Permissions: map[int64]map[string][]string{}, } diff --git a/pkg/services/auth/idimpl/service_test.go b/pkg/services/auth/idimpl/service_test.go index 37e89229460..94495f54873 100644 --- a/pkg/services/auth/idimpl/service_test.go +++ b/pkg/services/auth/idimpl/service_test.go @@ -69,7 +69,7 @@ func TestService_SignIdentity(t *testing.T) { featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) - token, err := s.SignIdentity(context.Background(), &authn.Identity{ID: "user:1"}) + token, err := s.SignIdentity(context.Background(), &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}) require.NoError(t, err) require.NotEmpty(t, token) }) @@ -80,7 +80,7 @@ func TestService_SignIdentity(t *testing.T) { featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) - token, err := s.SignIdentity(context.Background(), &authn.Identity{ID: "user:1", AuthenticatedBy: login.AzureADAuthModule}) + token, err := s.SignIdentity(context.Background(), &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), AuthenticatedBy: login.AzureADAuthModule}) require.NoError(t, err) parsed, err := jwt.ParseSigned(token) diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index ad8bb42c15f..bfd4698095a 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -188,14 +188,13 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i return nil, err } - namespace, namespaceID := id.GetNamespacedID() // Login is only supported for users - if namespace != authn.NamespaceUser { + if !id.ID.IsNamespace(authn.NamespaceUser) { s.metrics.failedLogin.WithLabelValues(client).Inc() - return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", namespace) + return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", id.ID.Namespace()) } - intId, err := identity.IntIdentifier(namespace, namespaceID) + userID, err := id.ID.ParseInt() if err != nil { return nil, err } @@ -206,7 +205,7 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i s.log.FromContext(ctx).Debug("Failed to parse ip from address", "client", c.Name(), "id", id.ID, "addr", addr, "error", err) } - sessionToken, err := s.sessionService.CreateToken(ctx, &user.User{ID: intId}, ip, r.HTTPRequest.UserAgent()) + sessionToken, err := s.sessionService.CreateToken(ctx, &user.User{ID: userID}, ip, r.HTTPRequest.UserAgent()) if err != nil { s.metrics.failedLogin.WithLabelValues(client).Inc() s.log.FromContext(ctx).Error("Failed to create session", "client", client, "id", id.ID, "err", err) @@ -340,7 +339,7 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID a if namespaceID.IsNamespace(authn.NamespaceUser) { return &authn.Identity{ OrgID: orgID, - ID: namespaceID.String(), + ID: namespaceID, ClientParams: authn.ClientParams{ AllowGlobalOrg: true, FetchSyncedUser: true, @@ -350,7 +349,7 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID a if namespaceID.IsNamespace(authn.NamespaceServiceAccount) { return &authn.Identity{ - ID: namespaceID.String(), + ID: namespaceID, OrgID: orgID, ClientParams: authn.ClientParams{ AllowGlobalOrg: true, diff --git a/pkg/services/authn/authnimpl/service_test.go b/pkg/services/authn/authnimpl/service_test.go index 045ebe6accf..c96147eb7eb 100644 --- a/pkg/services/authn/authnimpl/service_test.go +++ b/pkg/services/authn/authnimpl/service_test.go @@ -40,26 +40,26 @@ func TestService_Authenticate(t *testing.T) { { desc: "should succeed with authentication for configured client", clients: []authn.Client{ - &authntest.FakeClient{ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: "user:1"}}, + &authntest.FakeClient{ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}}, }, - expectedIdentity: &authn.Identity{ID: "user:1"}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, }, { desc: "should succeed with authentication for second client when first test fail", clients: []authn.Client{ &authntest.FakeClient{ExpectedName: "1", ExpectedPriority: 1, ExpectedTest: false}, - &authntest.FakeClient{ExpectedName: "2", ExpectedPriority: 2, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: "user:2"}}, + &authntest.FakeClient{ExpectedName: "2", ExpectedPriority: 2, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2")}}, }, - expectedIdentity: &authn.Identity{ID: "user:2"}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2")}, }, { desc: "should succeed with authentication for third client when error happened in first", clients: []authn.Client{ &authntest.FakeClient{ExpectedName: "1", ExpectedPriority: 2, ExpectedTest: false}, &authntest.FakeClient{ExpectedName: "2", ExpectedPriority: 1, ExpectedTest: true, ExpectedErr: errors.New("some error")}, - &authntest.FakeClient{ExpectedName: "3", ExpectedPriority: 3, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: "user:3"}}, + &authntest.FakeClient{ExpectedName: "3", ExpectedPriority: 3, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:3")}}, }, - expectedIdentity: &authn.Identity{ID: "user:3"}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:3")}, }, { desc: "should return error when no client could authenticate the request", @@ -214,10 +214,10 @@ func TestService_Login(t *testing.T) { client: "fake", expectedClientOK: true, expectedClientIdentity: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), }, expectedIdentity: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{UserId: 1}, }, }, @@ -230,7 +230,7 @@ func TestService_Login(t *testing.T) { desc: "should not login non user identity", client: "fake", expectedClientOK: true, - expectedClientIdentity: &authn.Identity{ID: "apikey:1"}, + expectedClientIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("api-key:1")}, expectedErr: authn.ErrUnsupportedIdentity, }, } @@ -318,31 +318,31 @@ func TestService_Logout(t *testing.T) { tests := []TestCase{ { desc: "should redirect to default redirect url when identity is not a user", - identity: &authn.Identity{ID: authn.NamespacedID(authn.NamespaceServiceAccount, 1)}, + identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceServiceAccount, 1)}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, }, { desc: "should redirect to default redirect url when no external provider was used to authenticate", - identity: &authn.Identity{ID: authn.NamespacedID(authn.NamespaceUser, 1)}, + identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1)}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, expectedTokenRevoked: true, }, { desc: "should redirect to default redirect url when client is not found", - identity: &authn.Identity{ID: authn.NamespacedID(authn.NamespaceUser, 1), AuthenticatedBy: "notfound"}, + identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "notfound"}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, expectedTokenRevoked: true, }, { desc: "should redirect to default redirect url when client do not implement logout extension", - identity: &authn.Identity{ID: authn.NamespacedID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"}, + identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, client: &authntest.FakeClient{ExpectedName: "auth.client.azuread"}, expectedTokenRevoked: true, }, { desc: "should redirect to client specific url", - identity: &authn.Identity{ID: authn.NamespacedID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"}, + identity: &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"}, expectedRedirect: &authn.Redirect{URL: "http://idp.com/logout"}, client: &authntest.MockClient{ NameFunc: func() string { return "auth.client.azuread" }, diff --git a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go index 935b4ec68c9..165613d45df 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go @@ -34,9 +34,8 @@ type OAuthTokenSync struct { } func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { - namespace, _ := identity.GetNamespacedID() // only perform oauth token check if identity is a user - if namespace != authn.NamespaceUser { + if !identity.ID.IsNamespace(authn.NamespaceUser) { return nil } @@ -50,8 +49,8 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn return nil } - _, err, _ := s.singleflightGroup.Do(identity.ID, func() (interface{}, error) { - s.log.Debug("Singleflight request for OAuth token sync", "key", identity.ID) + _, err, _ := s.singleflightGroup.Do(identity.ID.String(), func() (interface{}, error) { + s.log.Debug("Singleflight request for OAuth token sync", "key", identity.ID.String()) // FIXME: Consider using context.WithoutCancel instead of context.Background after Go 1.21 update updateCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) diff --git a/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go b/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go index 03db9ff1dd0..16b69dfc071 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go @@ -41,17 +41,17 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { tests := []testCase{ { desc: "should skip sync when identity is not a user", - identity: &authn.Identity{ID: "service-account:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("service-account:1")}, expectTryRefreshTokenCalled: false, }, { desc: "should skip sync when identity is a user but is not authenticated with session token", - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, expectTryRefreshTokenCalled: false, }, { desc: "should invalidate access token and session token if token refresh fails", - identity: &authn.Identity{ID: "user:1", SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: true, expectedTryRefreshErr: errors.New("some err"), expectTryRefreshTokenCalled: true, @@ -62,7 +62,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { }, { desc: "should refresh the token successfully", - identity: &authn.Identity{ID: "user:1", SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: false, expectTryRefreshTokenCalled: true, expectInvalidateOauthTokensCalled: false, @@ -70,7 +70,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { }, { desc: "should not invalidate the token if the token has already been refreshed by another request (singleflight)", - identity: &authn.Identity{ID: "user:1", SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: true, expectTryRefreshTokenCalled: true, expectInvalidateOauthTokensCalled: false, diff --git a/pkg/services/authn/authnimpl/sync/org_sync.go b/pkg/services/authn/authnimpl/sync/org_sync.go index d8d1146dab4..8492e44de09 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -8,7 +8,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" @@ -35,15 +34,14 @@ func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *a ctxLogger := s.log.FromContext(ctx).New("id", id.ID, "login", id.Login) - namespace, identifier := id.GetNamespacedID() - if namespace != authn.NamespaceUser { - ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "namespace", namespace) + if !id.ID.IsNamespace(authn.NamespaceUser) { + ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "namespace", id.ID.Namespace()) return nil } - userID, err := identity.IntIdentifier(namespace, identifier) + userID, err := id.ID.ParseInt() if err != nil { - ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "namespace", namespace, "err", err) + ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "namespace", id.ID.Namespace(), "err", err) return nil } @@ -139,15 +137,14 @@ func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn. ctxLogger := s.log.FromContext(ctx) - namespace, identifier := currentIdentity.GetNamespacedID() - if namespace != identity.NamespaceUser { - ctxLogger.Debug("Skipping default org sync, not a user", "namespace", namespace) + if !currentIdentity.ID.IsNamespace(authn.NamespaceUser) { + ctxLogger.Debug("Skipping default org sync, not a user", "namespace", currentIdentity.ID.Namespace()) return } - userID, err := identity.IntIdentifier(namespace, identifier) + userID, err := currentIdentity.ID.ParseInt() if err != nil { - ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "namespace", namespace, "err", err) + ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "namespace", currentIdentity.ID.Namespace(), "err", err) return } diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index 93d46e8f823..16c94171c4c 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -75,7 +75,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), Login: "test", Name: "test", Email: "test", @@ -91,7 +91,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { }, }, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), Login: "test", Name: "test", Email: "test", @@ -137,7 +137,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should set default org", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { userService.On("SetUsingOrg", mock.Anything, mock.MatchedBy(func(cmd *user.SetUsingOrgCommand) bool { return cmd.UserID == 1 && cmd.OrgID == 2 @@ -147,7 +147,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip setting the default org when default org is not set", defaultOrgSetting: -1, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, }, { name: "should skip setting the default org when identity is nil", @@ -157,28 +157,28 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip setting the default org when input err is not nil", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, inputErr: fmt.Errorf("error"), }, { name: "should skip setting the default org when identity is not a user", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "service-account:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("service-account:1")}, }, { name: "should skip setting the default org when user id is not valid", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "user:invalid"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:invalid")}, }, { name: "should skip setting the default org when user is not allowed to use the configured default org", defaultOrgSetting: 3, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, }, { name: "should skip setting the default org when validateUsingOrg returns error", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { orgService.ExpectedError = fmt.Errorf("error") }, @@ -186,7 +186,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip the hook when the user org update was unsuccessful", defaultOrgSetting: 2, - identity: &authn.Identity{ID: "user:1"}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { userService.On("SetUsingOrg", mock.Anything, mock.Anything).Return(fmt.Errorf("error")) }, diff --git a/pkg/services/authn/authnimpl/sync/rbac_sync.go b/pkg/services/authn/authnimpl/sync/rbac_sync.go index 121c2dcadbc..df10c2414ae 100644 --- a/pkg/services/authn/authnimpl/sync/rbac_sync.go +++ b/pkg/services/authn/authnimpl/sync/rbac_sync.go @@ -6,7 +6,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -98,13 +97,12 @@ func (s *RBACSync) SyncCloudRoles(ctx context.Context, ident *authn.Identity, r return nil } - namespace, id := ident.GetNamespacedID() - if namespace != authn.NamespaceUser { + if !ident.ID.IsNamespace(authn.NamespaceUser) { s.log.FromContext(ctx).Debug("Skip syncing cloud role", "id", ident.ID) return nil } - userID, err := identity.IntIdentifier(namespace, id) + userID, err := ident.ID.ParseInt() if err != nil { return err } diff --git a/pkg/services/authn/authnimpl/sync/rbac_sync_test.go b/pkg/services/authn/authnimpl/sync/rbac_sync_test.go index 27436998f4d..202dd58cadf 100644 --- a/pkg/services/authn/authnimpl/sync/rbac_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/rbac_sync_test.go @@ -24,14 +24,14 @@ func TestRBACSync_SyncPermission(t *testing.T) { testCases := []testCase{ { name: "enriches the identity successfully when SyncPermissions is true", - identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, expectedPermissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionUsersRead}, }, }, { name: "does not load the permissions when SyncPermissions is false", - identity: &authn.Identity{ID: "user:2", OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, expectedPermissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionUsersRead}, }, @@ -65,7 +65,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has viewer role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, }, @@ -76,7 +76,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has editor role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, }, @@ -87,7 +87,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has admin role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, }, @@ -98,7 +98,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should not call sync when authenticated with grafana com and has invalid role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleType("something else")}, }, @@ -109,7 +109,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should not call sync when not authenticated with grafana com", module: login.LDAPAuthModule, identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, }, diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index fb6d4d006e3..eb09115d580 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -394,7 +394,7 @@ func (s *UserSync) lookupByOneOf(ctx context.Context, params login.UserLookupPar // syncUserToIdentity syncs a user to an identity. // This is used to update the identity with the latest user information. func syncUserToIdentity(usr *user.User, id *authn.Identity) { - id.ID = authn.NamespacedID(authn.NamespaceUser, usr.ID) + id.ID = authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID) id.Login = usr.Login id.Email = usr.Email id.Name = usr.Name diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index f34b16c168d..4e861a6ad1f 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -110,7 +110,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test", @@ -124,7 +123,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test", @@ -146,7 +144,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test", @@ -161,7 +158,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), Login: "test", Name: "test", Email: "test", @@ -185,7 +182,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test", @@ -200,7 +196,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), Login: "test", Name: "test", Email: "test", @@ -224,7 +220,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", AuthID: "2032", AuthenticatedBy: "oauth", Login: "test", @@ -241,7 +236,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), AuthID: "2032", AuthenticatedBy: "oauth", Login: "test", @@ -267,7 +262,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test", @@ -294,7 +288,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test_create", Name: "test_create", IsGrafanaAdmin: ptrBool(true), @@ -314,7 +307,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:2", + ID: authn.MustParseNamespaceID("user:2"), Login: "test_create", Name: "test_create", Email: "test_create", @@ -342,7 +335,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test_mod", Name: "test_mod", Email: "test_mod", @@ -360,7 +352,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:3", + ID: authn.MustParseNamespaceID("user:3"), Login: "test_mod", Name: "test_mod", Email: "test_mod", @@ -386,7 +378,6 @@ func TestUserSync_SyncUserHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: "", Login: "test", Name: "test", Email: "test_mod@test.com", @@ -405,7 +396,7 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: "user:3", + ID: authn.MustParseNamespaceID("user:3"), Login: "test", Name: "test", Email: "test_mod@test.com", @@ -455,7 +446,7 @@ func TestUserSync_FetchSyncedUserHook(t *testing.T) { { desc: "should skip hook when identity is not a user", req: &authn.Request{}, - identity: &authn.Identity{ID: "apikey:1", ClientParams: authn.ClientParams{FetchSyncedUser: true}}, + identity: &authn.Identity{ID: authn.MustParseNamespaceID("api-key:1"), ClientParams: authn.ClientParams{FetchSyncedUser: true}}, }, } @@ -479,7 +470,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should skip if correct flag is not set", identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: false}, }, @@ -488,7 +479,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should skip if identity is not a user", identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceAPIKey, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceAPIKey, 1), IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: true}, }, @@ -497,7 +488,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should enabled disabled user", identity: &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, 1), + ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: true}, }, diff --git a/pkg/services/authn/clients/api_key.go b/pkg/services/authn/clients/api_key.go index c2679470fc7..4b36922cf99 100644 --- a/pkg/services/authn/clients/api_key.go +++ b/pkg/services/authn/clients/api_key.go @@ -256,7 +256,7 @@ func validateApiKey(orgID int64, key *apikey.APIKey) error { func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity { return &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceAPIKey, key.ID), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceAPIKey, key.ID), OrgID: key.OrgID, OrgRoles: map[int64]org.RoleType{key.OrgID: key.Role}, ClientParams: authn.ClientParams{SyncPermissions: true}, @@ -266,7 +266,7 @@ func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity { func newServiceAccountIdentity(key *apikey.APIKey) *authn.Identity { return &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceServiceAccount, *key.ServiceAccountId), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceServiceAccount, *key.ServiceAccountId), OrgID: key.OrgID, AuthenticatedBy: login.APIKeyAuthModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, diff --git a/pkg/services/authn/clients/api_key_test.go b/pkg/services/authn/clients/api_key_test.go index 2905503a633..c67c11b6cf0 100644 --- a/pkg/services/authn/clients/api_key_test.go +++ b/pkg/services/authn/clients/api_key_test.go @@ -47,7 +47,7 @@ func TestAPIKey_Authenticate(t *testing.T) { Role: org.RoleAdmin, }, expectedIdentity: &authn.Identity{ - ID: "api-key:1", + ID: authn.MustParseNamespaceID("api-key:1"), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, ClientParams: authn.ClientParams{ @@ -70,7 +70,7 @@ func TestAPIKey_Authenticate(t *testing.T) { ServiceAccountId: intPtr(1), }, expectedIdentity: &authn.Identity{ - ID: "service-account:1", + ID: authn.MustParseNamespaceID("service-account:1"), OrgID: 1, ClientParams: authn.ClientParams{ FetchSyncedUser: true, @@ -205,7 +205,7 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { ServiceAccountId: intPtr(1), }, expectedIdentity: &authn.Identity{ - ID: "service-account:1", + ID: authn.MustParseNamespaceID("service-account:1"), OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -221,7 +221,7 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: "api-key:2", + ID: authn.MustParseNamespaceID("api-key:2"), OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -237,7 +237,7 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: "user:2", + ID: authn.MustParseNamespaceID("user:2"), OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -253,7 +253,7 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: "service-account:2", + ID: authn.MustParseNamespaceID("service-account:2"), OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -351,7 +351,7 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedIdenity: &authn.Identity{ OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, - ID: "api-key:1", + ID: authn.MustParseNamespaceID("api-key:1"), AuthenticatedBy: login.APIKeyAuthModule, ClientParams: authn.ClientParams{SyncPermissions: true}, }, diff --git a/pkg/services/authn/clients/basic_test.go b/pkg/services/authn/clients/basic_test.go index fbf2a96a2d7..f92f29a3144 100644 --- a/pkg/services/authn/clients/basic_test.go +++ b/pkg/services/authn/clients/basic_test.go @@ -24,8 +24,8 @@ func TestBasic_Authenticate(t *testing.T) { { desc: "should success when password client return identity", req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}}, - client: authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "user:1"}}, - expectedIdentity: &authn.Identity{ID: "user:1"}, + client: authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, }, { desc: "should fail when basic auth header could not be decoded", diff --git a/pkg/services/authn/clients/ext_jwt.go b/pkg/services/authn/clients/ext_jwt.go index 303b14d9cad..446a7b0ee05 100644 --- a/pkg/services/authn/clients/ext_jwt.go +++ b/pkg/services/authn/clients/ext_jwt.go @@ -109,8 +109,13 @@ func (s *ExtendedJWT) authenticateAsUser(idTokenClaims, return nil, errJWTInvalid.Errorf("Failed to parse sub: %w", err) } + id, err := authn.ParseNamespaceID(idTokenClaims.Subject) + if err != nil { + return nil, err + } + return &authn.Identity{ - ID: idTokenClaims.Subject, + ID: id, OrgID: s.getDefaultOrgID(), AuthenticatedBy: login.ExtendedJWTModule, AuthID: accessTokenClaims.Subject, @@ -129,8 +134,13 @@ func (s *ExtendedJWT) authenticateAsService(claims *ExtendedJWTClaims) (*authn.I return nil, errJWTInvalid.Errorf("Failed to parse sub: %s", "invalid subject format") } + id, err := authn.ParseNamespaceID(claims.Subject) + if err != nil { + return nil, err + } + return &authn.Identity{ - ID: claims.Subject, + ID: id, OrgID: s.getDefaultOrgID(), AuthenticatedBy: login.ExtendedJWTModule, AuthID: claims.Subject, diff --git a/pkg/services/authn/clients/ext_jwt_test.go b/pkg/services/authn/clients/ext_jwt_test.go index fe8c1135897..9bbba9a684c 100644 --- a/pkg/services/authn/clients/ext_jwt_test.go +++ b/pkg/services/authn/clients/ext_jwt_test.go @@ -166,7 +166,7 @@ func TestExtendedJWT_Authenticate(t *testing.T) { orgID: 1, want: &authn.Identity{OrgID: 1, OrgName: "", OrgRoles: map[int64]roletype.RoleType(nil), - ID: "access-policy:this-uid", Login: "", Name: "", Email: "", + ID: authn.MustParseNamespaceID("access-policy:this-uid"), Login: "", Name: "", Email: "", IsGrafanaAdmin: (*bool)(nil), AuthenticatedBy: "extendedjwt", AuthID: "access-policy:this-uid", IsDisabled: false, HelpFlags1: 0x0, LastSeenAt: time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), @@ -196,7 +196,7 @@ func TestExtendedJWT_Authenticate(t *testing.T) { } }, want: &authn.Identity{OrgID: 1, OrgName: "", - OrgRoles: map[int64]roletype.RoleType(nil), ID: "user:2", + OrgRoles: map[int64]roletype.RoleType(nil), ID: authn.MustParseNamespaceID("user:2"), Login: "", Name: "", Email: "", IsGrafanaAdmin: (*bool)(nil), AuthenticatedBy: "extendedjwt", AuthID: "access-policy:this-uid", IsDisabled: false, HelpFlags1: 0x0, diff --git a/pkg/services/authn/clients/grafana.go b/pkg/services/authn/clients/grafana.go index 10e33008600..e95fb2f5828 100644 --- a/pkg/services/authn/clients/grafana.go +++ b/pkg/services/authn/clients/grafana.go @@ -104,12 +104,12 @@ func (c *Grafana) AuthenticatePassword(ctx context.Context, r *authn.Request, us return nil, errInvalidPassword.Errorf("invalid password") } - signedInUser, err := c.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{OrgID: r.OrgID, UserID: usr.ID}) - if err != nil { - return nil, err - } - - return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{SyncPermissions: true}, login.PasswordAuthModule), nil + return &authn.Identity{ + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID), + OrgID: r.OrgID, + ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, + AuthenticatedBy: login.PasswordAuthModule, + }, nil } func comparePassword(password, salt, hash string) bool { diff --git a/pkg/services/authn/clients/grafana_test.go b/pkg/services/authn/clients/grafana_test.go index b2cb45154ec..d77afb239a2 100644 --- a/pkg/services/authn/clients/grafana_test.go +++ b/pkg/services/authn/clients/grafana_test.go @@ -125,29 +125,25 @@ func TestGrafana_AuthenticateProxy(t *testing.T) { func TestGrafana_AuthenticatePassword(t *testing.T) { type testCase struct { - desc string - username string - password string - findUser bool - expectedErr error - expectedIdentity *authn.Identity - expectedSignedInUser *user.SignedInUser + desc string + username string + password string + findUser bool + expectedErr error + expectedIdentity *authn.Identity } tests := []testCase{ { - desc: "should successfully authenticate user with correct password", - username: "user", - password: "password", - findUser: true, - expectedSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: "Viewer"}, + desc: "should successfully authenticate user with correct password", + username: "user", + password: "password", + findUser: true, expectedIdentity: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), OrgID: 1, - OrgRoles: map[int64]org.RoleType{1: "Viewer"}, - IsGrafanaAdmin: boolPtr(false), - ClientParams: authn.ClientParams{SyncPermissions: true}, AuthenticatedBy: login.PasswordAuthModule, + ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, }, }, { @@ -169,8 +165,7 @@ func TestGrafana_AuthenticatePassword(t *testing.T) { t.Run(tt.desc, func(t *testing.T) { hashed, _ := util.EncodePassword("password", "salt") userService := &usertest.FakeUserService{ - ExpectedSignedInUser: tt.expectedSignedInUser, - ExpectedUser: &user.User{Password: user.Password(hashed), Salt: "salt"}, + ExpectedUser: &user.User{ID: 1, Password: user.Password(hashed), Salt: "salt"}, } if !tt.findUser { diff --git a/pkg/services/authn/clients/jwt_test.go b/pkg/services/authn/clients/jwt_test.go index f5d0dbd09ad..538ffb26e65 100644 --- a/pkg/services/authn/clients/jwt_test.go +++ b/pkg/services/authn/clients/jwt_test.go @@ -40,7 +40,6 @@ func TestAuthenticateJWT(t *testing.T) { OrgName: "", OrgRoles: map[int64]roletype.RoleType{1: roletype.RoleAdmin}, Groups: []string{"foo", "bar"}, - ID: "", Login: "eai-doe", Name: "Eai Doe", Email: "eai.doe@cor.po", @@ -92,7 +91,6 @@ func TestAuthenticateJWT(t *testing.T) { OrgID: 0, OrgName: "", OrgRoles: map[int64]roletype.RoleType{1: roletype.RoleAdmin}, - ID: "", Login: "eai-doe", Groups: []string{}, Name: "Eai Doe", diff --git a/pkg/services/authn/clients/password_test.go b/pkg/services/authn/clients/password_test.go index dda54325146..76d9d87dc00 100644 --- a/pkg/services/authn/clients/password_test.go +++ b/pkg/services/authn/clients/password_test.go @@ -29,16 +29,16 @@ func TestPassword_AuthenticatePassword(t *testing.T) { username: "test", password: "test", req: &authn.Request{}, - clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "user:1"}}}, - expectedIdentity: &authn.Identity{ID: "user:1"}, + clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}}}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")}, }, { desc: "should success when found in second client", username: "test", password: "test", req: &authn.Request{}, - clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "user:2"}}}, - expectedIdentity: &authn.Identity{ID: "user:2"}, + clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2")}}}, + expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2")}, }, { desc: "should fail for empty password", diff --git a/pkg/services/authn/clients/proxy.go b/pkg/services/authn/clients/proxy.go index 157af7ee0af..bbe96491533 100644 --- a/pkg/services/authn/clients/proxy.go +++ b/pkg/services/authn/clients/proxy.go @@ -125,7 +125,7 @@ func (c *Proxy) retrieveIDFromCache(ctx context.Context, cacheKey string, r *aut } return &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, uid), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, uid), OrgID: r.OrgID, // FIXME: This does not match the actual auth module used, but should not have any impact // Maybe caching the auth module used with the user ID would be a good idea diff --git a/pkg/services/authn/clients/proxy_test.go b/pkg/services/authn/clients/proxy_test.go index 1f980122476..d55bd68484e 100644 --- a/pkg/services/authn/clients/proxy_test.go +++ b/pkg/services/authn/clients/proxy_test.go @@ -202,8 +202,8 @@ func TestProxy_Hook(t *testing.T) { proxyFieldRole: "X-Role", } cache := &fakeCache{data: make(map[string][]byte)} - userId := 1 - userID := fmt.Sprintf("%s:%d", authn.NamespaceUser, userId) + userId := int64(1) + userID := authn.MustNewNamespaceID(authn.NamespaceUser, userId) // withRole creates a test case for a user with a specific role. withRole := func(role string) func(t *testing.T) { diff --git a/pkg/services/authn/clients/render.go b/pkg/services/authn/clients/render.go index 0a7b692029a..39f6922d258 100644 --- a/pkg/services/authn/clients/render.go +++ b/pkg/services/authn/clients/render.go @@ -42,7 +42,7 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide if renderUsr.UserID <= 0 { return &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceRenderService, 0), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceRenderService, 0), OrgID: renderUsr.OrgID, OrgRoles: map[int64]org.RoleType{renderUsr.OrgID: org.RoleType(renderUsr.OrgRole)}, ClientParams: authn.ClientParams{SyncPermissions: true}, @@ -52,7 +52,7 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide } return &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, renderUsr.UserID), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, renderUsr.UserID), LastSeenAt: time.Now(), AuthenticatedBy: login.RenderModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, diff --git a/pkg/services/authn/clients/render_test.go b/pkg/services/authn/clients/render_test.go index 24051db326f..8e815de266e 100644 --- a/pkg/services/authn/clients/render_test.go +++ b/pkg/services/authn/clients/render_test.go @@ -35,7 +35,7 @@ func TestRender_Authenticate(t *testing.T) { }, }, expectedIdentity: &authn.Identity{ - ID: "render:0", + ID: authn.MustParseNamespaceID("render:0"), OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, AuthenticatedBy: login.RenderModule, @@ -56,7 +56,7 @@ func TestRender_Authenticate(t *testing.T) { }, }, expectedIdentity: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), AuthenticatedBy: login.RenderModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, }, diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index 02ec093b74e..dc928107ba5 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -57,7 +57,7 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id } ident := &authn.Identity{ - ID: authn.NamespacedID(authn.NamespaceUser, token.UserId), + ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, token.UserId), SessionToken: token, ClientParams: authn.ClientParams{ FetchSyncedUser: true, diff --git a/pkg/services/authn/clients/session_test.go b/pkg/services/authn/clients/session_test.go index 0315083ea5b..c8fbc329277 100644 --- a/pkg/services/authn/clients/session_test.go +++ b/pkg/services/authn/clients/session_test.go @@ -96,7 +96,7 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), SessionToken: validToken, ClientParams: authn.ClientParams{ SyncPermissions: true, @@ -129,7 +129,7 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), SessionToken: validToken, ClientParams: authn.ClientParams{ SyncPermissions: true, @@ -148,7 +148,7 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), AuthID: "1", AuthenticatedBy: "oauth_azuread", SessionToken: validToken, @@ -170,7 +170,7 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: "user:1", + ID: authn.MustParseNamespaceID("user:1"), SessionToken: validToken, ClientParams: authn.ClientParams{ diff --git a/pkg/services/authn/identity.go b/pkg/services/authn/identity.go index 59b23faefbb..3f47c084c24 100644 --- a/pkg/services/authn/identity.go +++ b/pkg/services/authn/identity.go @@ -3,7 +3,6 @@ package authn import ( "fmt" "strconv" - "strings" "time" "golang.org/x/oauth2" @@ -21,17 +20,17 @@ const GlobalOrgID = int64(0) var _ identity.Requester = (*Identity)(nil) type Identity struct { + // ID is the unique identifier for the entity in the Grafana database. + // It is in the format : where namespace is one of the + // Namespace* constants. For example, "user:1" or "api-key:1". + // If the entity is not found in the DB or this entity is non-persistent, this field will be empty. + ID NamespaceID // OrgID is the active organization for the entity. OrgID int64 // OrgName is the name of the active organization. OrgName string // OrgRoles is the list of organizations the entity is a member of and their roles. OrgRoles map[int64]org.RoleType - // ID is the unique identifier for the entity in the Grafana database. - // It is in the format : where namespace is one of the - // Namespace* constants. For example, "user:1" or "api-key:1". - // If the entity is not found in the DB or this entity is non-persistent, this field will be empty. - ID string // Login is the shorthand identifier of the entity. Should be unique. Login string // Name is the display name of the entity. It is not guaranteed to be unique. @@ -74,15 +73,11 @@ type Identity struct { } func (i *Identity) GetID() string { - return i.ID + return i.ID.String() } func (i *Identity) GetNamespacedID() (namespace string, identifier string) { - split := strings.Split(i.GetID(), ":") - if len(split) != 2 { - return "", "" - } - return split[0], split[1] + return i.ID.Namespace(), i.ID.ID() } func (i *Identity) GetAuthID() string { @@ -224,7 +219,7 @@ func (i *Identity) SignedInUser() *user.SignedInUser { Teams: i.Teams, Permissions: i.Permissions, IDToken: i.IDToken, - NamespacedID: i.ID, + NamespacedID: i.ID.String(), } if namespace == NamespaceAPIKey { @@ -263,25 +258,3 @@ func (i *Identity) ExternalUserInfo() login.ExternalUserInfo { IsDisabled: i.IsDisabled, } } - -// IdentityFromSignedInUser creates an identity from a SignedInUser. -func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientParams, authenticatedBy string) *Identity { - return &Identity{ - ID: id, - OrgID: usr.OrgID, - OrgName: usr.OrgName, - OrgRoles: map[int64]org.RoleType{usr.OrgID: usr.OrgRole}, - Login: usr.Login, - Name: usr.Name, - Email: usr.Email, - AuthenticatedBy: authenticatedBy, - IsGrafanaAdmin: &usr.IsGrafanaAdmin, - IsDisabled: usr.IsDisabled, - HelpFlags1: usr.HelpFlags1, - LastSeenAt: usr.LastSeenAt, - Teams: usr.Teams, - ClientParams: params, - Permissions: usr.Permissions, - IDToken: usr.IDToken, - } -} diff --git a/pkg/services/authn/namespace.go b/pkg/services/authn/namespace.go index c46da890e86..a5324561963 100644 --- a/pkg/services/authn/namespace.go +++ b/pkg/services/authn/namespace.go @@ -15,9 +15,10 @@ const ( NamespaceAnonymous = identity.NamespaceAnonymous NamespaceRenderService = identity.NamespaceRenderService NamespaceAccessPolicy = identity.NamespaceAccessPolicy - AnonymousNamespaceID = NamespaceAnonymous + ":0" ) +var AnonymousNamespaceID = MustNewNamespaceID(NamespaceAnonymous, 0) + var namespaceLookup = map[string]struct{}{ NamespaceUser: {}, NamespaceAPIKey: {}, @@ -27,11 +28,6 @@ var namespaceLookup = map[string]struct{}{ NamespaceAccessPolicy: {}, } -// NamespacedID builds a namespaced ID from a namespace and an ID. -func NamespacedID(namespace string, id int64) string { - return fmt.Sprintf("%s:%d", namespace, id) -} - func ParseNamespaceID(str string) (NamespaceID, error) { var namespaceID NamespaceID @@ -62,6 +58,36 @@ func MustParseNamespaceID(str string) NamespaceID { return namespaceID } +// NewNamespaceID creates a new NamespaceID, will fail for invalid namespace. +func NewNamespaceID(namespace string, id int64) (NamespaceID, error) { + var namespaceID NamespaceID + if _, ok := namespaceLookup[namespace]; !ok { + return namespaceID, ErrInvalidNamepsaceID.Errorf("got invalid namespace %s", namespace) + } + namespaceID.id = strconv.FormatInt(id, 10) + namespaceID.namespace = namespace + return namespaceID, nil +} + +// MustNewNamespaceID creates a new NamespaceID, will panic for invalid namespace. +// Sutable to use in tests or when we can garantuee that we pass a correct format. +func MustNewNamespaceID(namespace string, id int64) NamespaceID { + namespaceID, err := NewNamespaceID(namespace, id) + if err != nil { + panic(err) + } + return namespaceID +} + +// NewNamespaceIDUnchecked creates a new NamespaceID without checking if namespace is valid. +// It us up to the caller to ensure that namespace is valid. +func NewNamespaceIDUnchecked(namespace string, id int64) NamespaceID { + return NamespaceID{ + id: strconv.FormatInt(id, 10), + namespace: namespace, + } +} + // FIXME: use this instead of encoded string through the codebase type NamespaceID struct { id string diff --git a/pkg/services/contexthandler/contexthandler_test.go b/pkg/services/contexthandler/contexthandler_test.go index f7c46f7208c..3bdede4be3b 100644 --- a/pkg/services/contexthandler/contexthandler_test.go +++ b/pkg/services/contexthandler/contexthandler_test.go @@ -44,7 +44,7 @@ func TestContextHandler(t *testing.T) { }) t.Run("should set identity on successful authentication", func(t *testing.T) { - identity := &authn.Identity{ID: authn.NamespacedID(authn.NamespaceUser, 1), OrgID: 1} + identity := &authn.Identity{ID: authn.MustNewNamespaceID(authn.NamespaceUser, 1), OrgID: 1} handler := contexthandler.ProvideService( setting.NewCfg(), tracing.InitializeTracerForTest(), @@ -150,7 +150,7 @@ func TestContextHandler(t *testing.T) { cfg, tracing.InitializeTracerForTest(), featuremgmt.WithFeatures(), - &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: id}}, + &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID(id)}}, ) server := webtest.NewServer(t, routing.NewRouteRegister()) diff --git a/pkg/services/oauthtoken/oauth_token_test.go b/pkg/services/oauthtoken/oauth_token_test.go index ebcbb76fb3f..873a25cc9ce 100644 --- a/pkg/services/oauthtoken/oauth_token_test.go +++ b/pkg/services/oauthtoken/oauth_token_test.go @@ -174,13 +174,13 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip sync when identity is not a user", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "service-account:1"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("service-account:1")} }, }, { desc: "should skip token refresh and return nil if namespace and id cannot be converted to user ID", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:invalidIdentifierFormat"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:invalidIdentifierFormat")} }, }, { @@ -203,28 +203,28 @@ func TestService_TryTokenRefresh(t *testing.T) { env.identity = &authn.Identity{ AuthenticatedBy: login.GenericOAuthModule, - ID: "user:1234", + ID: authn.MustParseNamespaceID("user:1234"), } }, }, { desc: "should skip token refresh if the expiration check has already been cached", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.cache.Set("oauth-refresh-token-1234", true, 1*time.Minute) }, }, { desc: "should skip token refresh if there's an unexpected error while looking up the user oauth entry, additionally, no error should be returned", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedError = errors.New("some error") }, }, { desc: "should skip token refresh if the user doesn't have an oauth entry", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.SAMLAuthModule, } @@ -233,7 +233,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should do token refresh if access token or id token have not expired yet", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, } @@ -242,7 +242,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when no oauth provider was found", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -252,7 +252,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when oauth provider token handling is disabled (UseRefreshToken is false)", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -265,7 +265,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when there is no refresh token", setup: func(env *environment) { - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -285,7 +285,7 @@ func TestService_TryTokenRefresh(t *testing.T) { Expiry: time.Now().Add(-time.Hour), TokenType: "Bearer", } - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{ UseRefreshToken: true, } @@ -310,7 +310,7 @@ func TestService_TryTokenRefresh(t *testing.T) { Expiry: time.Now().Add(time.Hour), TokenType: "Bearer", } - env.identity = &authn.Identity{ID: "user:1234"} + env.identity = &authn.Identity{ID: authn.MustParseNamespaceID("user:1234")} env.socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{ UseRefreshToken: true, } diff --git a/pkg/services/user/userimpl/verifier.go b/pkg/services/user/userimpl/verifier.go index 3017a9b6189..e4b62fa9d61 100644 --- a/pkg/services/user/userimpl/verifier.go +++ b/pkg/services/user/userimpl/verifier.go @@ -152,6 +152,6 @@ func (s *Verifier) Complete(ctx context.Context, cmd user.CompleteEmailVerifyCom // remove the current token, so a new one can be generated with correct values. return s.is.RemoveIDToken( ctx, - &user.SignedInUser{UserID: usr.ID, OrgID: usr.OrgID, NamespacedID: authn.NamespacedID(authn.NamespaceUser, usr.ID)}, + &authn.Identity{ID: authn.NewNamespaceIDUnchecked(authn.NamespaceUser, usr.ID), OrgID: usr.OrgID}, ) }