diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index 1f6362ec1a5..3d092bb968d 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -156,8 +156,8 @@ func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, id *authn.I // Validate the provisioned user.ExternalUID with the authinfo.ExternalUID if usr.IsProvisioned { - // The user is provisioned via SAML and the identity is empty, meaning this request is not from the SAML auth flow - if authInfo.AuthModule == login.SAMLAuthModule && authInfo.ExternalUID != "" && id.ExternalUID == "" { + // Allow non-SAML requests for SAML-provisioned users to proceed if incoming ExternalUID is empty (e.g. session access). + if authInfo.AuthModule == login.SAMLAuthModule && id.AuthenticatedBy != login.SAMLAuthModule && authInfo.ExternalUID != "" && id.ExternalUID == "" { log.Debug("Skipping ExternalUID validation for non-SAML request to SAML-provisioned user") return nil } diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 266ba3a739a..6f565a974fc 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -439,6 +439,94 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, }, }, + { + name: "SyncUserHook: Provisioned user, Incoming ExternalUID is empty, DB ExternalUID non-empty - expect errEmptyExternalUID", + fields: fields{ + userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}, + authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}}, + quotaService: "atest.FakeQuotaService{}, + }, + args: args{ + ctx: context.Background(), + id: &authn.Identity{ + AuthID: "1", + AuthenticatedBy: login.SAMLAuthModule, + ExternalUID: "", + ClientParams: authn.ClientParams{SyncUser: true}, + }, + }, + wantErr: true, // Expecting errEmptyExternalUID + }, + { + name: "SyncUserHook: Provisioned user, Incoming ExternalUID is empty, DB ExternalUID also empty - expect errEmptyExternalUID", + fields: fields{ + userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}, + authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}}, // DB empty + quotaService: "atest.FakeQuotaService{}, + }, + args: args{ + ctx: context.Background(), + id: &authn.Identity{ + AuthID: "1", + AuthenticatedBy: login.SAMLAuthModule, + ExternalUID: "", + ClientParams: authn.ClientParams{SyncUser: true}, + }, + }, + wantErr: true, // Expecting errEmptyExternalUID + }, + { + name: "SyncUserHook: Provisioned user, Incoming and DB ExternalUIDs non-empty and mismatch - expect errMismatchedExternalUID", + fields: fields{ + userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}, + authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}}, + quotaService: "atest.FakeQuotaService{}, + }, + args: args{ + ctx: context.Background(), + id: &authn.Identity{ + AuthID: "1", + AuthenticatedBy: login.SAMLAuthModule, + ExternalUID: "incoming-uid", + ClientParams: authn.ClientParams{SyncUser: true}, + }, + }, + wantErr: true, // Expecting errMismatchedExternalUID + }, + { + name: "SyncUserHook: Provisioned user, Incoming and DB ExternalUIDs non-empty and match - expect success", + fields: fields{ + userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, Login: "user1", Email: "user1@test.com", Name: "User One", IsProvisioned: true}}, + authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, AuthId: "1", ExternalUID: "matching-uid"}}, + quotaService: "atest.FakeQuotaService{}, + }, + args: args{ + ctx: context.Background(), + id: &authn.Identity{ + AuthID: "1", + AuthenticatedBy: login.SAMLAuthModule, + Login: "user1", + Email: "user1@test.com", + Name: "User One", + ExternalUID: "matching-uid", + ClientParams: authn.ClientParams{SyncUser: true}, + }, + }, + wantErr: false, + wantID: &authn.Identity{ + ID: "1", + UID: "", + Type: claims.TypeUser, + AuthID: "1", + AuthenticatedBy: login.SAMLAuthModule, + Login: "user1", + Email: "user1@test.com", + Name: "User One", + ExternalUID: "matching-uid", + IsGrafanaAdmin: ptrBool(false), + ClientParams: authn.ClientParams{SyncUser: true}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -812,12 +900,118 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) { return userSyncService }, identity: &authn.Identity{ - AuthenticatedBy: login.SAMLAuthModule, + AuthenticatedBy: login.GenericOAuthModule, + AuthID: "1", + ExternalUID: "", + }, + expectedErr: nil, + }, + { + desc: "ValidateProvisioning: DB ExternalUID is empty, Incoming ExternalUID is empty - expect mismatch (stricter logic)", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} + userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}} + return userSyncService + }, + identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: ""}, + expectedErr: errUserExternalUIDMismatch, + }, + { + desc: "ValidateProvisioning: DB ExternalUID non-empty, Incoming ExternalUID is empty - expect mismatch", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} + userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "valid-uid"}} + return userSyncService + }, + identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: ""}, + expectedErr: errUserExternalUIDMismatch, + }, + { + desc: "ValidateProvisioning: DB ExternalUID is empty, Incoming ExternalUID non-empty - expect mismatch (stricter logic)", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} + userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}} + return userSyncService + }, + identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: "valid-uid"}, + expectedErr: errUserExternalUIDMismatch, + }, + { + desc: "ValidateProvisioning: DB and Incoming ExternalUIDs non-empty and mismatch - expect mismatch", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}} + userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}} + return userSyncService + }, + identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: "incoming-uid"}, + expectedErr: errUserExternalUIDMismatch, + }, + { + desc: "it should skip ExternalUID validation for a SAML-provisioned user accessed by a non-SAML method with an empty incoming ExternalUID", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.allowNonProvisionedUsers = false + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ + ExpectedUser: &user.User{ + ID: 1, + IsProvisioned: true, + }, + } + userSyncService.authInfoService = &authinfotest.FakeService{ + ExpectedUserAuth: &login.UserAuth{ + UserId: 1, + AuthModule: login.SAMLAuthModule, + AuthId: "1", + ExternalUID: "saml-originated-uid", + }, + } + return userSyncService + }, + identity: &authn.Identity{ + AuthenticatedBy: login.GenericOAuthModule, AuthID: "1", ExternalUID: "", }, expectedErr: nil, }, + { + desc: "it should fail validation when a provisioned user is accessed by SAML with an empty incoming ExternalUID", + userSyncServiceSetup: func() *UserSync { + userSyncService := initUserSyncService() + userSyncService.allowNonProvisionedUsers = false + userSyncService.isUserProvisioningEnabled = true + userSyncService.userService = &usertest.FakeUserService{ + ExpectedUser: &user.User{ + ID: 1, + IsProvisioned: true, + }, + } + userSyncService.authInfoService = &authinfotest.FakeService{ + ExpectedUserAuth: &login.UserAuth{ + UserId: 1, + AuthModule: login.SAMLAuthModule, + AuthId: "1", + ExternalUID: "saml-originated-uid", + }, + } + return userSyncService + }, + identity: &authn.Identity{ + AuthenticatedBy: login.SAMLAuthModule, + AuthID: "1", + ExternalUID: "", + }, + expectedErr: errUserExternalUIDMismatch.Errorf("the provisioned user.ExternalUID does not match the authinfo.ExternalUID"), + }, } for _, tt := range tests {