apitech
/
grafana
mirror of https://github.com/grafana/grafana
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

User: Fix GetByID (#86282)

Browse Source 
* Auth: Remove unused lookup param

* Remove case sensitive lookup for GetByID
pull/86331/head
Karl Persson 1 year ago committed by GitHub
parent b9285e320a
commit 8520892923
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 42 additions and 130 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 10
      pkg/services/authn/authnimpl/sync/org_sync_test.go
  2. 8
      pkg/services/authn/authnimpl/sync/user_sync.go
  3. 120
      pkg/services/authn/authnimpl/sync/user_sync_test.go
  4. 5
      pkg/services/authn/clients/ext_jwt_test.go
  5. 1
      pkg/services/authn/clients/grafana_test.go
  6. 10
      pkg/services/authn/clients/jwt_test.go
  7. 1
      pkg/services/authn/clients/oauth_test.go
  8. 3
      pkg/services/ldap/api/service.go
  9. 5
      pkg/services/login/model.go
  10. 9
      pkg/services/user/userimpl/user.go

10
pkg/services/authn/authnimpl/sync/org_sync_test.go
Unescape View File

@ -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,
},
},
},

8
pkg/services/authn/authnimpl/sync/user_sync.go
Unescape View File

@ -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})

120
pkg/services/authn/authnimpl/sync/user_sync_test.go
Unescape View File

@ -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: &quotatest.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"),
},
},
},

5
pkg/services/authn/clients/ext_jwt_test.go
Unescape View File

@ -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"},

1
pkg/services/authn/clients/grafana_test.go
Unescape View File

@ -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)
}

10
pkg/services/authn/clients/jwt_test.go
Unescape View File

@ -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"),
},
},
},

1
pkg/services/authn/clients/oauth_test.go
Unescape View File

@ -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)
}

3
pkg/services/ldap/api/service.go
Unescape View File

@ -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,
},
},
}
}

5
pkg/services/login/model.go
Unescape View File

@ -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 {

9
pkg/services/user/userimpl/user.go
Unescape View File

@ -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) {

Write Preview
Loading…
Cancel
Save