From 5ca8ea40c1649207997f5f7d7e221e9f4b99ef73 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Tue, 21 Feb 2023 11:21:34 +0100 Subject: [PATCH] AuthN: Cleanup authn package (#63456) * AuthN: Update comments for ClientParams * AuthN: Update flag name from SyncTeamMembers to SyncTeams * UserSync: rename function and fix order of parameters so it is correct * UserSync: Fix so we skip check if no authModule or authID is passed * UserSync: move quota check to create user function * UserSync: Move FetchSyncedUserHook to UserSync * UserSync: Move last seen user hook to user sync service * ApiKey: Implement last seen hook as a client hook instead --- pkg/services/authn/authn.go | 26 ++- pkg/services/authn/authnimpl/service.go | 11 +- .../authnimpl/sync/apikey_last_seen_sync.go | 38 ---- .../authn/authnimpl/sync/fetch_user_sync.go | 56 ------ .../authnimpl/sync/fetch_user_sync_test.go | 40 ----- .../authn/authnimpl/sync/oauth_token_sync.go | 2 +- .../authnimpl/sync/oauth_token_sync_test.go | 4 +- pkg/services/authn/authnimpl/sync/org_sync.go | 4 +- .../authn/authnimpl/sync/org_sync_test.go | 6 +- .../authnimpl/sync/user_last_seen_sync.go | 50 ------ .../authn/authnimpl/sync/user_sync.go | 170 +++++++++++++----- .../authn/authnimpl/sync/user_sync_test.go | 47 ++++- pkg/services/authn/clients/api_key.go | 21 +++ pkg/services/authn/clients/grafana.go | 2 +- pkg/services/authn/clients/grafana_test.go | 10 +- pkg/services/authn/clients/ldap.go | 2 +- pkg/services/authn/clients/ldap_test.go | 4 +- pkg/services/authn/clients/oauth.go | 2 +- pkg/services/authn/clients/oauth_test.go | 4 +- pkg/services/authn/clients/session.go | 1 - 20 files changed, 219 insertions(+), 281 deletions(-) delete mode 100644 pkg/services/authn/authnimpl/sync/apikey_last_seen_sync.go delete mode 100644 pkg/services/authn/authnimpl/sync/fetch_user_sync.go delete mode 100644 pkg/services/authn/authnimpl/sync/fetch_user_sync_test.go delete mode 100644 pkg/services/authn/authnimpl/sync/user_last_seen_sync.go diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index f552bdbb8c7..78de7ab075a 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -37,16 +37,16 @@ const ( // ClientParams are hints to the auth service about how to handle the identity management // from the authenticating client. type ClientParams struct { - // Update the internal representation of the entity from the identity provided + // SyncUser updates the internal representation of the identity from the identity provided SyncUser bool - // Add entity to teams - SyncTeamMembers bool - // Create entity in the DB if it doesn't exist + // AllowSignUp Adds identity to DB if it doesn't exist when, only work if SyncUser is enabled AllowSignUp bool - // EnableDisabledUsers is a hint to the auth service that it should re-enable disabled users + // EnableDisabledUsers will enable disabled user, only work if SyncUser is enabled EnableDisabledUsers bool // FetchSyncedUser ensure that all required information is added to the identity FetchSyncedUser bool + // SyncTeams will sync the groups from identity to teams in grafana, enterprise only feature + SyncTeams bool // CacheAuthProxyKey if this key is set we will try to cache the user id for proxy client CacheAuthProxyKey string // LookUpParams are the arguments used to look up the entity in the DB. @@ -222,26 +222,20 @@ func (i *Identity) Role() org.RoleType { return i.OrgRoles[i.OrgID] } -// TODO: improve error handling +// NamespacedID returns the namespace, e.g. "user" and the id for that namespace func (i *Identity) NamespacedID() (string, int64) { - var ( - id int64 - namespace string - ) - split := strings.Split(i.ID, ":") if len(split) != 2 { return "", -1 } - id, errI := strconv.ParseInt(split[1], 10, 64) - if errI != nil { + id, err := strconv.ParseInt(split[1], 10, 64) + if err != nil { + // FIXME (kalleep): Improve error handling return "", -1 } - namespace = split[0] - - return namespace, id + return split[0], id } // NamespacedID builds a namespaced ID from a namespace and an ID. diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 66d40f71c55..95eb83c3d79 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -143,16 +143,15 @@ func ProvideService( // FIXME (jguer): move to User package userSyncService := sync.ProvideUserSync(userService, userProtectionService, authInfoService, quotaService) orgUserSyncService := sync.ProvideOrgSync(userService, orgService, accessControlService) - s.RegisterPostAuthHook(userSyncService.SyncUser, 10) - s.RegisterPostAuthHook(orgUserSyncService.SyncOrgUser, 30) - s.RegisterPostAuthHook(sync.ProvideUserLastSeenSync(userService).SyncLastSeen, 40) - s.RegisterPostAuthHook(sync.ProvideAPIKeyLastSeenSync(apikeyService).SyncLastSeen, 50) + s.RegisterPostAuthHook(userSyncService.SyncUserHook, 10) + s.RegisterPostAuthHook(orgUserSyncService.SyncOrgRolesHook, 30) + s.RegisterPostAuthHook(userSyncService.SyncLastSeenHook, 40) if features.IsEnabled(featuremgmt.FlagAccessTokenExpirationCheck) { - s.RegisterPostAuthHook(sync.ProvideOauthTokenSync(oauthTokenService, sessionService).SyncOauthToken, 60) + s.RegisterPostAuthHook(sync.ProvideOauthTokenSync(oauthTokenService, sessionService).SyncOauthTokenHook, 60) } - s.RegisterPostAuthHook(sync.ProvideFetchUserSync(userService).FetchSyncedUserHook, 100) + s.RegisterPostAuthHook(userSyncService.FetchSyncedUserHook, 100) return s } diff --git a/pkg/services/authn/authnimpl/sync/apikey_last_seen_sync.go b/pkg/services/authn/authnimpl/sync/apikey_last_seen_sync.go deleted file mode 100644 index 7a58159981d..00000000000 --- a/pkg/services/authn/authnimpl/sync/apikey_last_seen_sync.go +++ /dev/null @@ -1,38 +0,0 @@ -package sync - -import ( - "context" - - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/apikey" - "github.com/grafana/grafana/pkg/services/authn" -) - -func ProvideAPIKeyLastSeenSync(service apikey.Service) *APIKeyLastSeenSync { - return &APIKeyLastSeenSync{log.New("apikeylastseen.sync"), service} -} - -type APIKeyLastSeenSync struct { - log log.Logger - service apikey.Service -} - -func (s *APIKeyLastSeenSync) SyncLastSeen(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { - namespace, id := identity.NamespacedID() - if namespace != authn.NamespaceAPIKey { - return nil - } - - go func(apikeyID int64) { - defer func() { - if err := recover(); err != nil { - s.log.Error("panic during user last seen sync", "err", err) - } - }() - if err := s.service.UpdateAPIKeyLastUsedDate(context.Background(), apikeyID); err != nil { - s.log.Warn("failed to update last use date for api key", "id", apikeyID) - } - }(id) - - return nil -} diff --git a/pkg/services/authn/authnimpl/sync/fetch_user_sync.go b/pkg/services/authn/authnimpl/sync/fetch_user_sync.go deleted file mode 100644 index 064326f2588..00000000000 --- a/pkg/services/authn/authnimpl/sync/fetch_user_sync.go +++ /dev/null @@ -1,56 +0,0 @@ -package sync - -import ( - "context" - - "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/org" - "github.com/grafana/grafana/pkg/services/user" - "github.com/grafana/grafana/pkg/util/errutil" -) - -var errFetchingSignedInUser = errutil.NewBase(errutil.StatusInternal, "user.sync.fetch", errutil.WithPublicMessage("Insufficient information to authenticate user")) - -func ProvideFetchUserSync(service user.Service) *FetchUserSync { - return &FetchUserSync{service} -} - -type FetchUserSync struct { - userService user.Service -} - -func (s *FetchUserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error { - if !identity.ClientParams.FetchSyncedUser { - return nil - } - namespace, id := identity.NamespacedID() - if namespace != authn.NamespaceUser { - return nil - } - - usr, err := s.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{ - UserID: id, - OrgID: r.OrgID, - }) - if err != nil { - return errFetchingSignedInUser.Errorf("failed to resolve user: %w", err) - } - - syncSignedInUserToIdentity(usr, identity) - return nil -} - -func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity) { - identity.Name = usr.Name - identity.Login = usr.Login - identity.Email = usr.Email - identity.OrgID = usr.OrgID - identity.OrgName = usr.OrgName - identity.OrgCount = usr.OrgCount - identity.OrgRoles = map[int64]org.RoleType{identity.OrgID: usr.OrgRole} - identity.HelpFlags1 = usr.HelpFlags1 - identity.Teams = usr.Teams - identity.LastSeenAt = usr.LastSeenAt - identity.IsDisabled = usr.IsDisabled - identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin -} diff --git a/pkg/services/authn/authnimpl/sync/fetch_user_sync_test.go b/pkg/services/authn/authnimpl/sync/fetch_user_sync_test.go deleted file mode 100644 index e7f1bac6fd9..00000000000 --- a/pkg/services/authn/authnimpl/sync/fetch_user_sync_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package sync - -import ( - "context" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/services/authn" -) - -func TestFetchUserSync_FetchSyncedUserHook(t *testing.T) { - type testCase struct { - desc string - req *authn.Request - identity *authn.Identity - expectedErr error - } - - tests := []testCase{ - { - desc: "should skip hook when flag is not enabled", - req: &authn.Request{}, - identity: &authn.Identity{ClientParams: authn.ClientParams{FetchSyncedUser: false}}, - }, - { - desc: "should skip hook when identity is not a user", - req: &authn.Request{}, - identity: &authn.Identity{ID: "apikey:1", ClientParams: authn.ClientParams{FetchSyncedUser: true}}, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - s := ProvideFetchUserSync(nil) - err := s.FetchSyncedUserHook(context.Background(), tt.identity, tt.req) - require.ErrorIs(t, err, tt.expectedErr) - }) - } -} diff --git a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go index 791f7aefac2..d1fa4bef1c3 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go @@ -31,7 +31,7 @@ type OauthTokenSync struct { sessionService auth.UserTokenService } -func (s *OauthTokenSync) SyncOauthToken(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { +func (s *OauthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { namespace, id := identity.NamespacedID() // only perform oauth token check if identity is a user if namespace != authn.NamespaceUser { 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 a180cab31a1..c7610e9c7d3 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go @@ -17,7 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/user" ) -func TestOauthTokenSync_SyncOauthToken(t *testing.T) { +func TestOauthTokenSync_SyncOauthTokenHook(t *testing.T) { type testCase struct { desc string identity *authn.Identity @@ -123,7 +123,7 @@ func TestOauthTokenSync_SyncOauthToken(t *testing.T) { sessionService: sessionService, } - err := sync.SyncOauthToken(context.Background(), tt.identity, nil) + err := sync.SyncOauthTokenHook(context.Background(), tt.identity, nil) assert.ErrorIs(t, err, tt.expectedErr) assert.Equal(t, tt.expectHasEntryCalled, hasEntryCalled) assert.Equal(t, tt.expectTryRefreshTokenCalled, tryRefreshCalled) diff --git a/pkg/services/authn/authnimpl/sync/org_sync.go b/pkg/services/authn/authnimpl/sync/org_sync.go index 6bbc708f228..47bbfc953ee 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -24,13 +24,13 @@ type OrgSync struct { log log.Logger } -func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error { +func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error { if !id.ClientParams.SyncUser { return nil } namespace, userID := id.NamespacedID() - if namespace != "user" || userID <= 0 { + if namespace != authn.NamespaceUser || userID <= 0 { s.log.Warn("invalid namespace %q for user ID %q", namespace, userID) return nil } diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index 4c4f6d6ea06..8c9852c0f6d 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -18,7 +18,7 @@ import ( "github.com/grafana/grafana/pkg/services/user/usertest" ) -func TestOrgSync_SyncOrgUser(t *testing.T) { +func TestOrgSync_SyncOrgRolesHook(t *testing.T) { orgService := &orgtest.FakeOrgService{ExpectedUserOrgDTO: []*org.UserOrgDTO{ { OrgID: 1, @@ -116,8 +116,8 @@ func TestOrgSync_SyncOrgUser(t *testing.T) { accessControl: tt.fields.accessControl, log: tt.fields.log, } - if err := s.SyncOrgUser(tt.args.ctx, tt.args.id, nil); (err != nil) != tt.wantErr { - t.Errorf("OrgSync.SyncOrgUser() error = %v, wantErr %v", err, tt.wantErr) + if err := s.SyncOrgRolesHook(tt.args.ctx, tt.args.id, nil); (err != nil) != tt.wantErr { + t.Errorf("OrgSync.SyncOrgRolesHook() error = %v, wantErr %v", err, tt.wantErr) } assert.EqualValues(t, tt.wantID, tt.args.id) diff --git a/pkg/services/authn/authnimpl/sync/user_last_seen_sync.go b/pkg/services/authn/authnimpl/sync/user_last_seen_sync.go deleted file mode 100644 index c4cd46af3a5..00000000000 --- a/pkg/services/authn/authnimpl/sync/user_last_seen_sync.go +++ /dev/null @@ -1,50 +0,0 @@ -package sync - -import ( - "context" - "time" - - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/user" -) - -func ProvideUserLastSeenSync(service user.Service) *UserLastSeenSync { - return &UserLastSeenSync{log.New("userlastseen.sync"), service} -} - -type UserLastSeenSync struct { - log log.Logger - service user.Service -} - -func (s *UserLastSeenSync) SyncLastSeen(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { - namespace, id := identity.NamespacedID() - - if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount { - // skip sync - return nil - } - - if !shouldUpdateLastSeen(identity.LastSeenAt) { - return nil - } - - go func(userID int64) { - defer func() { - if err := recover(); err != nil { - s.log.Error("panic during user last seen sync", "err", err) - } - }() - - if err := s.service.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{UserID: userID}); err != nil { - s.log.Error("failed to update last_seen_at", "err", err, "userId", userID) - } - }(id) - - return nil -} - -func shouldUpdateLastSeen(t time.Time) bool { - return time.Since(t) > time.Minute*5 -} diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index a054a7032ad..3a16ed13c6a 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/authn" @@ -15,12 +16,26 @@ import ( ) var ( - errSyncUserForbidden = errutil.NewBase(errutil.StatusForbidden, - "user.sync.forbidden", errutil.WithPublicMessage("User sync forbidden")) - errSyncUserInternal = errutil.NewBase(errutil.StatusInternal, - "user.sync.forbidden", errutil.WithPublicMessage("User sync failed")) - errUserProtection = errutil.NewBase(errutil.StatusForbidden, - "user.sync.protectedrole", errutil.WithPublicMessage("Unable to sync due to protected role")) + errSyncUserForbidden = errutil.NewBase( + errutil.StatusForbidden, + "user.sync.forbidden", + errutil.WithPublicMessage("User sync forbidden"), + ) + errSyncUserInternal = errutil.NewBase( + errutil.StatusInternal, + "user.sync.forbidden", + errutil.WithPublicMessage("User sync failed"), + ) + errUserProtection = errutil.NewBase( + errutil.StatusForbidden, + "user.sync.protected-role", + errutil.WithPublicMessage("Unable to sync due to protected role"), + ) + errFetchingSignedInUser = errutil.NewBase( + errutil.StatusInternal, + "user.sync.fetch", + errutil.WithPublicMessage("Insufficient information to authenticate user"), + ) ) func ProvideUserSync(userService user.Service, @@ -43,14 +58,14 @@ type UserSync struct { log log.Logger } -// SyncUser syncs a user with the database -func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error { +// SyncUserHook syncs a user with the database +func (s *UserSync) SyncUserHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error { if !id.ClientParams.SyncUser { return nil } // Does user exist in the database? - usr, errUserInDB := s.UserInDB(ctx, &id.AuthModule, &id.AuthID, id.ClientParams.LookUpParams) + usr, errUserInDB := s.getUser(ctx, id.AuthModule, id.AuthID, id.ClientParams.LookUpParams) if errUserInDB != nil && !errors.Is(errUserInDB, user.ErrUserNotFound) { s.log.Error("error retrieving user", "error", errUserInDB, "auth_module", id.AuthModule, "auth_id", id.AuthID, @@ -66,20 +81,6 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re return errSyncUserForbidden.Errorf("%w", login.ErrSignupNotAllowed) } - // quota check (FIXME: (jguer) this should be done in the user service) - // we may insert in both user and org_user tables - // therefore we need to query check quota for both user and org services - for _, srv := range []string{user.QuotaTargetSrv, org.QuotaTargetSrv} { - limitReached, errLimit := s.quotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil) - if errLimit != nil { - s.log.Error("error getting user quota", "error", errLimit) - return errSyncUserInternal.Errorf("%w", login.ErrGettingUserQuota) - } - if limitReached { - return errSyncUserForbidden.Errorf("%w", login.ErrUsersQuotaReached) - } - } - // create user var errCreate error usr, errCreate = s.createUser(ctx, id) @@ -108,7 +109,7 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re syncUserToIdentity(usr, id) - // persist latest auth info token + // persist the latest auth info token if errAuthInfo := s.updateAuthInfo(ctx, id); errAuthInfo != nil { s.log.Error("error creating user", "error", errAuthInfo, "auth_module", id.AuthModule, "auth_id", id.AuthID, @@ -119,14 +120,52 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re return nil } -// 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 = fmt.Sprintf("user:%d", usr.ID) - id.Login = usr.Login - id.Email = usr.Email - id.Name = usr.Name - id.IsGrafanaAdmin = &usr.IsAdmin +func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error { + if !identity.ClientParams.FetchSyncedUser { + return nil + } + namespace, id := identity.NamespacedID() + if namespace != authn.NamespaceUser { + return nil + } + + usr, err := s.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{ + UserID: id, + OrgID: r.OrgID, + }) + if err != nil { + return errFetchingSignedInUser.Errorf("failed to resolve user: %w", err) + } + + syncSignedInUserToIdentity(usr, identity) + return nil +} + +func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error { + namespace, id := identity.NamespacedID() + + if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount { + // skip sync + return nil + } + + if !shouldUpdateLastSeen(identity.LastSeenAt) { + return nil + } + + go func(userID int64) { + defer func() { + if err := recover(); err != nil { + s.log.Error("panic during user last seen sync", "err", err) + } + }() + + if err := s.userService.UpdateLastSeenAt(context.Background(), &user.UpdateUserLastSeenAtCommand{UserID: userID}); err != nil { + s.log.Error("failed to update last_seen_at", "err", err, "userId", userID) + } + }(id) + + return nil } func (s *UserSync) updateAuthInfo(ctx context.Context, id *authn.Identity) error { @@ -135,7 +174,7 @@ func (s *UserSync) updateAuthInfo(ctx context.Context, id *authn.Identity) error } namespace, userID := id.NamespacedID() - if namespace != "user" && userID <= 0 { // FIXME: constant namespace + if namespace != authn.NamespaceUser && userID <= 0 { return fmt.Errorf("invalid namespace %q for user ID %q", namespace, userID) } @@ -203,12 +242,25 @@ func (s *UserSync) updateUserAttributes(ctx context.Context, usr *user.User, id } func (s *UserSync) createUser(ctx context.Context, id *authn.Identity) (*user.User, error) { + // quota check (FIXME: (jguer) this should be done in the user service) + // we may insert in both user and org_user tables + // therefore we need to query check quota for both user and org services + for _, srv := range []string{user.QuotaTargetSrv, org.QuotaTargetSrv} { + limitReached, errLimit := s.quotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil) + if errLimit != nil { + s.log.Error("error getting user quota", "error", errLimit) + return nil, errSyncUserInternal.Errorf("%w", login.ErrGettingUserQuota) + } + if limitReached { + return nil, errSyncUserForbidden.Errorf("%w", login.ErrUsersQuotaReached) + } + } + isAdmin := false if id.IsGrafanaAdmin != nil { isAdmin = *id.IsGrafanaAdmin } - // TODO: add quota check usr, errCreateUser := s.userService.Create(ctx, &user.CreateUserCommand{ Login: id.Login, Email: id.Email, @@ -234,18 +286,12 @@ func (s *UserSync) createUser(ctx context.Context, id *authn.Identity) (*user.Us return usr, nil } -// Does user exist in the database? -// Check first authinfo table, then user table -// return user id if found, 0 if not found -func (s *UserSync) UserInDB(ctx context.Context, - authID *string, - authModule *string, - params login.UserLookupParams) (*user.User, error) { - // Check authinfo table - if authID != nil && authModule != nil { +func (s *UserSync) getUser(ctx context.Context, authModule, authID string, params login.UserLookupParams) (*user.User, error) { + // Check auth info fist + if authID != "" && authModule != "" { query := &login.GetAuthInfoQuery{ - AuthModule: *authModule, - AuthId: *authID, + AuthModule: authModule, + AuthId: authID, } errGetAuthInfo := s.authInfoService.GetAuthInfo(ctx, query) if errGetAuthInfo == nil { @@ -265,10 +311,10 @@ func (s *UserSync) UserInDB(ctx context.Context, } // Check user table to grab existing user - return s.LookupByOneOf(ctx, ¶ms) + return s.lookupByOneOf(ctx, ¶ms) } -func (s *UserSync) LookupByOneOf(ctx context.Context, params *login.UserLookupParams) (*user.User, error) { +func (s *UserSync) lookupByOneOf(ctx context.Context, params *login.UserLookupParams) (*user.User, error) { var usr *user.User var err error @@ -302,3 +348,33 @@ func (s *UserSync) LookupByOneOf(ctx context.Context, params *login.UserLookupPa return usr, nil } + +// 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 = fmt.Sprintf("user:%d", usr.ID) + id.Login = usr.Login + id.Email = usr.Email + id.Name = usr.Name + id.IsGrafanaAdmin = &usr.IsAdmin +} + +// syncSignedInUserToIdentity syncs a user to an identity. +func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity) { + identity.Name = usr.Name + identity.Login = usr.Login + identity.Email = usr.Email + identity.OrgID = usr.OrgID + identity.OrgName = usr.OrgName + identity.OrgCount = usr.OrgCount + identity.OrgRoles = map[int64]org.RoleType{identity.OrgID: usr.OrgRole} + identity.HelpFlags1 = usr.HelpFlags1 + identity.Teams = usr.Teams + identity.LastSeenAt = usr.LastSeenAt + identity.IsDisabled = usr.IsDisabled + identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin +} + +func shouldUpdateLastSeen(t time.Time) bool { + return time.Since(t) > time.Minute*5 +} diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 544419c76a8..f8e7c92e871 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -28,7 +28,7 @@ func ptrInt64(i int64) *int64 { return &i } -func TestUserSync_SyncUser(t *testing.T) { +func TestUserSync_SyncUserHook(t *testing.T) { userProtection := &authinfoservice.OSSUserProtectionImpl{} authFakeNil := &logintest.AuthInfoServiceFake{ @@ -266,12 +266,13 @@ func TestUserSync_SyncUser(t *testing.T) { }, args: args{ ctx: context.Background(), - id: &authn.Identity{ - ID: "", - Login: "test", - Name: "test", - Email: "test", + ID: "", + AuthID: "2032", + AuthModule: "oauth", + Login: "test", + Name: "test", + Email: "test", ClientParams: authn.ClientParams{ SyncUser: true, LookUpParams: login.UserLookupParams{ @@ -285,6 +286,8 @@ func TestUserSync_SyncUser(t *testing.T) { wantErr: false, wantID: &authn.Identity{ ID: "user:1", + AuthID: "2032", + AuthModule: "oauth", Login: "test", Name: "test", Email: "test", @@ -427,7 +430,7 @@ func TestUserSync_SyncUser(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := ProvideUserSync(tt.fields.userService, userProtection, tt.fields.authInfoService, tt.fields.quotaService) - err := s.SyncUser(tt.args.ctx, tt.args.id, nil) + err := s.SyncUserHook(tt.args.ctx, tt.args.id, nil) if tt.wantErr { require.Error(t, err) return @@ -438,3 +441,33 @@ func TestUserSync_SyncUser(t *testing.T) { }) } } + +func TestUserSync_FetchSyncedUserHook(t *testing.T) { + type testCase struct { + desc string + req *authn.Request + identity *authn.Identity + expectedErr error + } + + tests := []testCase{ + { + desc: "should skip hook when flag is not enabled", + req: &authn.Request{}, + identity: &authn.Identity{ClientParams: authn.ClientParams{FetchSyncedUser: false}}, + }, + { + desc: "should skip hook when identity is not a user", + req: &authn.Request{}, + identity: &authn.Identity{ID: "apikey:1", ClientParams: authn.ClientParams{FetchSyncedUser: true}}, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + s := UserSync{} + err := s.FetchSyncedUserHook(context.Background(), tt.identity, tt.req) + require.ErrorIs(t, err, tt.expectedErr) + }) + } +} diff --git a/pkg/services/authn/clients/api_key.go b/pkg/services/authn/clients/api_key.go index a5214b81654..1b712f2fe2f 100644 --- a/pkg/services/authn/clients/api_key.go +++ b/pkg/services/authn/clients/api_key.go @@ -23,6 +23,7 @@ var ( errAPIKeyRevoked = errutil.NewBase(errutil.StatusUnauthorized, "api-key.revoked", errutil.WithPublicMessage("Revoked API key")) ) +var _ authn.HookClient = new(APIKey) var _ authn.ContextAwareClient = new(APIKey) func ProvideAPIKey(apiKeyService apikey.Service, userService user.Service) *APIKey { @@ -141,6 +142,26 @@ func (s *APIKey) Priority() uint { return 30 } +func (s *APIKey) Hook(ctx context.Context, identity *authn.Identity, r *authn.Request) error { + namespace, id := identity.NamespacedID() + if namespace != authn.NamespaceAPIKey { + return nil + } + + go func(apikeyID int64) { + defer func() { + if err := recover(); err != nil { + s.log.Error("panic during user last seen sync", "err", err) + } + }() + if err := s.apiKeyService.UpdateAPIKeyLastUsedDate(context.Background(), apikeyID); err != nil { + s.log.Warn("failed to update last use date for api key", "id", apikeyID) + } + }(id) + + return nil +} + func looksLikeApiKey(token string) bool { return token != "" } diff --git a/pkg/services/authn/clients/grafana.go b/pkg/services/authn/clients/grafana.go index 067a1e5c8d1..de52d5c1530 100644 --- a/pkg/services/authn/clients/grafana.go +++ b/pkg/services/authn/clients/grafana.go @@ -36,7 +36,7 @@ func (c *Grafana) AuthenticateProxy(ctx context.Context, r *authn.Request, usern AuthID: username, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, FetchSyncedUser: true, AllowSignUp: c.cfg.AuthProxyAutoSignUp, }, diff --git a/pkg/services/authn/clients/grafana_test.go b/pkg/services/authn/clients/grafana_test.go index 48f3a16038d..55992829f4a 100644 --- a/pkg/services/authn/clients/grafana_test.go +++ b/pkg/services/authn/clients/grafana_test.go @@ -50,7 +50,7 @@ func TestGrafana_AuthenticateProxy(t *testing.T) { Groups: []string{"grp1", "grp2"}, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, AllowSignUp: true, FetchSyncedUser: true, LookUpParams: login.UserLookupParams{ @@ -71,9 +71,9 @@ func TestGrafana_AuthenticateProxy(t *testing.T) { AuthModule: "authproxy", AuthID: "test@test.com", ClientParams: authn.ClientParams{ - SyncUser: true, - SyncTeamMembers: true, - AllowSignUp: true, + SyncUser: true, + SyncTeams: true, + AllowSignUp: true, LookUpParams: login.UserLookupParams{ Email: strPtr("test@test.com"), Login: strPtr("test@test.com"), @@ -110,7 +110,7 @@ func TestGrafana_AuthenticateProxy(t *testing.T) { assert.Equal(t, tt.expectedIdentity.ClientParams.SyncUser, identity.ClientParams.SyncUser) assert.Equal(t, tt.expectedIdentity.ClientParams.AllowSignUp, identity.ClientParams.AllowSignUp) - assert.Equal(t, tt.expectedIdentity.ClientParams.SyncTeamMembers, identity.ClientParams.SyncTeamMembers) + assert.Equal(t, tt.expectedIdentity.ClientParams.SyncTeams, identity.ClientParams.SyncTeams) assert.Equal(t, tt.expectedIdentity.ClientParams.EnableDisabledUsers, identity.ClientParams.EnableDisabledUsers) assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Email, identity.ClientParams.LookUpParams.Email) diff --git a/pkg/services/authn/clients/ldap.go b/pkg/services/authn/clients/ldap.go index f2f546a96d3..66d05c977c9 100644 --- a/pkg/services/authn/clients/ldap.go +++ b/pkg/services/authn/clients/ldap.go @@ -82,7 +82,7 @@ func identityFromLDAPInfo(orgID int64, info *login.ExternalUserInfo, allowSignup Groups: info.Groups, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, EnableDisabledUsers: true, FetchSyncedUser: true, AllowSignUp: allowSignup, diff --git a/pkg/services/authn/clients/ldap_test.go b/pkg/services/authn/clients/ldap_test.go index eed01d6e291..9e174bcf5da 100644 --- a/pkg/services/authn/clients/ldap_test.go +++ b/pkg/services/authn/clients/ldap_test.go @@ -49,7 +49,7 @@ func TestLDAP_AuthenticateProxy(t *testing.T) { Groups: []string{"1", "2"}, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, EnableDisabledUsers: true, FetchSyncedUser: true, LookUpParams: login.UserLookupParams{ @@ -113,7 +113,7 @@ func TestLDAP_AuthenticatePassword(t *testing.T) { Groups: []string{"1", "2"}, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, EnableDisabledUsers: true, FetchSyncedUser: true, LookUpParams: login.UserLookupParams{ diff --git a/pkg/services/authn/clients/oauth.go b/pkg/services/authn/clients/oauth.go index 9dbbb27d106..0ae3549dc30 100644 --- a/pkg/services/authn/clients/oauth.go +++ b/pkg/services/authn/clients/oauth.go @@ -135,7 +135,7 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden OrgRoles: getOAuthOrgRole(userInfo, c.cfg), ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, FetchSyncedUser: true, AllowSignUp: c.connector.IsSignupAllowed(), LookUpParams: login.UserLookupParams{Email: &userInfo.Email}, diff --git a/pkg/services/authn/clients/oauth_test.go b/pkg/services/authn/clients/oauth_test.go index 1a3a10988bd..db8f8d4b866 100644 --- a/pkg/services/authn/clients/oauth_test.go +++ b/pkg/services/authn/clients/oauth_test.go @@ -136,7 +136,7 @@ func TestOAuth_Authenticate(t *testing.T) { OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, ClientParams: authn.ClientParams{ SyncUser: true, - SyncTeamMembers: true, + SyncTeams: true, AllowSignUp: true, FetchSyncedUser: true, LookUpParams: login.UserLookupParams{Email: strPtr("some@email.com")}, @@ -180,7 +180,7 @@ func TestOAuth_Authenticate(t *testing.T) { assert.Equal(t, tt.expectedIdentity.ClientParams.SyncUser, identity.ClientParams.SyncUser) assert.Equal(t, tt.expectedIdentity.ClientParams.AllowSignUp, identity.ClientParams.AllowSignUp) - assert.Equal(t, tt.expectedIdentity.ClientParams.SyncTeamMembers, identity.ClientParams.SyncTeamMembers) + assert.Equal(t, tt.expectedIdentity.ClientParams.SyncTeams, identity.ClientParams.SyncTeams) assert.Equal(t, tt.expectedIdentity.ClientParams.EnableDisabledUsers, identity.ClientParams.EnableDisabledUsers) assert.EqualValues(t, tt.expectedIdentity.ClientParams.LookUpParams.Email, identity.ClientParams.LookUpParams.Email) diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index d57daa71279..8a54a27eabf 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -65,7 +65,6 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id return nil, err } - // FIXME (jguer): oauth token refresh not implemented identity := authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{}) identity.SessionToken = token