diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index b2083f3a3e4..93d46e8f823 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -84,9 +84,8 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncOrgRoles: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, @@ -102,9 +101,8 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncOrgRoles: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index 23d0d6f9042..2f56cac8884 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -368,14 +368,6 @@ func (s *UserSync) lookupByOneOf(ctx context.Context, params login.UserLookupPar var usr *user.User var err error - // If not found, try to find the user by id - if params.UserID != nil && *params.UserID != 0 { - usr, err = s.userService.GetByID(ctx, &user.GetUserByIDQuery{ID: *params.UserID}) - if err != nil && !errors.Is(err, user.ErrUserNotFound) { - return nil, err - } - } - // If not found, try to find the user by email address if usr == nil && params.Email != nil && *params.Email != "" { usr, err = s.userService.GetByEmail(ctx, &user.GetUserByEmailQuery{Email: *params.Email}) diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 9449547d75a..01c0866a205 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -25,10 +25,6 @@ func ptrBool(b bool) *bool { return &b } -func ptrInt64(i int64) *int64 { - return &i -} - func TestUserSync_SyncUserHook(t *testing.T) { userProtection := &authinfoimpl.OSSUserProtectionImpl{} @@ -120,9 +116,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { Email: "test", ClientParams: authn.ClientParams{ LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, @@ -135,9 +130,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { Email: "test", ClientParams: authn.ClientParams{ LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, @@ -159,9 +153,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, @@ -176,9 +169,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test"), - Login: nil, + Email: ptrString("test"), + Login: nil, }, }, }, @@ -200,9 +192,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: nil, - Login: ptrString("test"), + Email: nil, + Login: ptrString("test"), }, }, }, @@ -216,52 +207,10 @@ func TestUserSync_SyncUserHook(t *testing.T) { IsGrafanaAdmin: ptrBool(false), ClientParams: authn.ClientParams{ LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: nil, - Login: ptrString("test"), - }, - SyncUser: true, - }, - }, - }, - { - name: "sync - user found in DB - by ID", - fields: fields{ - userService: userService, - authInfoService: authFakeNil, - quotaService: "atest.FakeQuotaService{}, - }, - args: args{ - ctx: context.Background(), - id: &authn.Identity{ - ID: "", - Login: "test", - Name: "test", - Email: "test", - ClientParams: authn.ClientParams{ - SyncUser: true, - LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(1), - Email: nil, - Login: nil, - }, + Email: nil, + Login: ptrString("test"), }, - }, - }, - wantErr: false, - wantID: &authn.Identity{ - ID: "user:1", - Login: "test", - Name: "test", - Email: "test", - IsGrafanaAdmin: ptrBool(false), - ClientParams: authn.ClientParams{ SyncUser: true, - LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(1), - Email: nil, - Login: nil, - }, }, }, }, @@ -284,9 +233,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: nil, - Login: nil, + Email: nil, + Login: nil, }, }, }, @@ -303,9 +251,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: nil, - Login: nil, + Email: nil, + Login: nil, }, }, }, @@ -329,9 +276,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: nil, - Login: nil, + Email: nil, + Login: nil, }, }, }, @@ -360,9 +306,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { AllowSignUp: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test_create"), - Login: nil, + Email: ptrString("test_create"), + Login: nil, }, }, }, @@ -381,9 +326,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { AllowSignUp: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: ptrString("test_create"), - Login: nil, + Email: ptrString("test_create"), + Login: nil, }, }, }, @@ -408,9 +352,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { SyncUser: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(3), - Email: nil, - Login: nil, + Email: nil, + Login: ptrString("test"), }, }, }, @@ -427,9 +370,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { SyncUser: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(3), - Email: nil, - Login: nil, + Email: nil, + Login: ptrString("test"), }, }, }, @@ -455,9 +397,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { SyncUser: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(3), - Email: nil, - Login: nil, + Email: nil, + Login: ptrString("test"), }, }, }, @@ -475,9 +416,8 @@ func TestUserSync_SyncUserHook(t *testing.T) { SyncUser: true, EnableUser: true, LookUpParams: login.UserLookupParams{ - UserID: ptrInt64(3), - Email: nil, - Login: nil, + Email: nil, + Login: ptrString("test"), }, }, }, diff --git a/pkg/services/authn/clients/ext_jwt_test.go b/pkg/services/authn/clients/ext_jwt_test.go index 7ef5ecc7716..fe8c1135897 100644 --- a/pkg/services/authn/clients/ext_jwt_test.go +++ b/pkg/services/authn/clients/ext_jwt_test.go @@ -21,7 +21,6 @@ import ( "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/models/usertoken" "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/signingkeys" "github.com/grafana/grafana/pkg/services/signingkeys/signingkeystest" "github.com/grafana/grafana/pkg/services/user" @@ -176,8 +175,7 @@ func TestExtendedJWT_Authenticate(t *testing.T) { ClientParams: authn.ClientParams{SyncUser: false, AllowSignUp: false, EnableUser: false, FetchSyncedUser: false, SyncTeams: false, SyncOrgRoles: false, CacheAuthProxyKey: "", - LookUpParams: login.UserLookupParams{UserID: (*int64)(nil), - Email: (*string)(nil), Login: (*string)(nil)}, SyncPermissions: true, + SyncPermissions: true, FetchPermissionsParams: authn.FetchPermissionsParams{ActionsLookup: []string(nil), Roles: []string{"fixed:folders:reader"}}}, Permissions: map[int64]map[string][]string(nil), IDToken: ""}, wantErr: nil, @@ -208,7 +206,6 @@ func TestExtendedJWT_Authenticate(t *testing.T) { ClientParams: authn.ClientParams{SyncUser: false, AllowSignUp: false, EnableUser: false, FetchSyncedUser: true, SyncTeams: false, SyncOrgRoles: false, CacheAuthProxyKey: "", - LookUpParams: login.UserLookupParams{UserID: (*int64)(nil), Email: (*string)(nil), Login: (*string)(nil)}, SyncPermissions: true, FetchPermissionsParams: authn.FetchPermissionsParams{ActionsLookup: []string{"dashboards:create", "folders:read", "datasources:explore", "datasources.insights:read"}, diff --git a/pkg/services/authn/clients/grafana_test.go b/pkg/services/authn/clients/grafana_test.go index f77d233744f..b2cb45154ec 100644 --- a/pkg/services/authn/clients/grafana_test.go +++ b/pkg/services/authn/clients/grafana_test.go @@ -116,7 +116,6 @@ func TestGrafana_AuthenticateProxy(t *testing.T) { assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Email, identity.ClientParams.LookUpParams.Email) assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Login, identity.ClientParams.LookUpParams.Login) - assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.UserID, identity.ClientParams.LookUpParams.UserID) } else { assert.Nil(t, tt.expectedIdentity) } diff --git a/pkg/services/authn/clients/jwt_test.go b/pkg/services/authn/clients/jwt_test.go index d9476f37320..f5d0dbd09ad 100644 --- a/pkg/services/authn/clients/jwt_test.go +++ b/pkg/services/authn/clients/jwt_test.go @@ -57,9 +57,8 @@ func TestAuthenticateJWT(t *testing.T) { SyncPermissions: true, SyncTeams: true, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: stringPtr("eai.doe@cor.po"), - Login: stringPtr("eai-doe"), + Email: stringPtr("eai.doe@cor.po"), + Login: stringPtr("eai-doe"), }, }, }, @@ -111,9 +110,8 @@ func TestAuthenticateJWT(t *testing.T) { SyncPermissions: true, SyncTeams: false, LookUpParams: login.UserLookupParams{ - UserID: nil, - Email: stringPtr("eai.doe@cor.po"), - Login: stringPtr("eai-doe"), + Email: stringPtr("eai.doe@cor.po"), + Login: stringPtr("eai-doe"), }, }, }, diff --git a/pkg/services/authn/clients/oauth_test.go b/pkg/services/authn/clients/oauth_test.go index 65916f3b435..95838da0159 100644 --- a/pkg/services/authn/clients/oauth_test.go +++ b/pkg/services/authn/clients/oauth_test.go @@ -308,7 +308,6 @@ func TestOAuth_Authenticate(t *testing.T) { assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Email, identity.ClientParams.LookUpParams.Email) assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Login, identity.ClientParams.LookUpParams.Login) - assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.UserID, identity.ClientParams.LookUpParams.UserID) } else { assert.Nil(t, tt.expectedIdentity) } diff --git a/pkg/services/ldap/api/service.go b/pkg/services/ldap/api/service.go index 2d5de2a1da2..71c85c4fafe 100644 --- a/pkg/services/ldap/api/service.go +++ b/pkg/services/ldap/api/service.go @@ -331,9 +331,6 @@ func (s *Service) identityFromLDAPUser(user *login.ExternalUserInfo) *authn.Iden EnableUser: true, SyncOrgRoles: !s.cfg.LDAPSkipOrgRoleSync, AllowSignUp: s.cfg.LDAPAllowSignup, - LookUpParams: login.UserLookupParams{ - UserID: &user.UserId, - }, }, } } diff --git a/pkg/services/login/model.go b/pkg/services/login/model.go index 3ec4b55d99e..b11ec2161f3 100644 --- a/pkg/services/login/model.go +++ b/pkg/services/login/model.go @@ -104,9 +104,8 @@ type GetUserByAuthInfoQuery struct { type UserLookupParams struct { // Describes lookup order as well - UserID *int64 // if set, will try to find the user by id - Email *string // if set, will try to find the user by email - Login *string // if set, will try to find the user by login + Email *string // if set, will try to find the user by email + Login *string // if set, will try to find the user by login } type GetAuthInfoQuery struct { diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index 4dbe3e4114d..14c3be1ce7a 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -209,14 +209,7 @@ func (s *Service) Delete(ctx context.Context, cmd *user.DeleteUserCommand) error } func (s *Service) GetByID(ctx context.Context, query *user.GetUserByIDQuery) (*user.User, error) { - user, err := s.store.GetByID(ctx, query.ID) - if err != nil { - return nil, err - } - if err := s.store.CaseInsensitiveLoginConflict(ctx, user.Login, user.Email); err != nil { - return nil, err - } - return user, nil + return s.store.GetByID(ctx, query.ID) } func (s *Service) GetByLogin(ctx context.Context, query *user.GetUserByLoginQuery) (*user.User, error) {