From 8796d2d30750319b916baacf08c37ceff3bf86a0 Mon Sep 17 00:00:00 2001 From: Misi Date: Fri, 5 Apr 2024 16:03:51 +0200 Subject: [PATCH] Auth: Convert SetDefaultOrgHook to PostLoginHook (#85649) * Convert SetDefaultOrgHook to PostLoginHook --- pkg/services/anonymous/anonimpl/impl.go | 2 +- pkg/services/authn/authnimpl/registration.go | 2 +- pkg/services/authn/authnimpl/sync/org_sync.go | 21 ++++++++----------- .../authn/authnimpl/sync/org_sync_test.go | 18 +++++++++------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/services/anonymous/anonimpl/impl.go b/pkg/services/anonymous/anonimpl/impl.go index e1cc21415f7..b34048be407 100644 --- a/pkg/services/anonymous/anonimpl/impl.go +++ b/pkg/services/anonymous/anonimpl/impl.go @@ -112,7 +112,7 @@ func (a *AnonDeviceService) untagDevice(ctx context.Context, errD := a.anonStore.DeleteDevice(ctx, deviceID) if errD != nil { - a.log.Debug("Failed to untag device", "error", err) + a.log.Debug("Failed to untag device", "error", errD) } } diff --git a/pkg/services/authn/authnimpl/registration.go b/pkg/services/authn/authnimpl/registration.go index 9ed6bf80e18..f3dd550fb5d 100644 --- a/pkg/services/authn/authnimpl/registration.go +++ b/pkg/services/authn/authnimpl/registration.go @@ -110,7 +110,7 @@ func ProvideRegistration( } authnSvc.RegisterPostAuthHook(rbacSync.SyncPermissionsHook, 120) - authnSvc.RegisterPostAuthHook(orgSync.SetDefaultOrgHook, 130) + authnSvc.RegisterPostLoginHook(orgSync.SetDefaultOrgHook, 140) return Registration{} } diff --git a/pkg/services/authn/authnimpl/sync/org_sync.go b/pkg/services/authn/authnimpl/sync/org_sync.go index 0b9ce6f7e80..d8d1146dab4 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -132,9 +132,9 @@ func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *a return nil } -func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn.Identity, r *authn.Request) error { - if s.cfg.LoginDefaultOrgId < 1 || currentIdentity == nil { - return nil +func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn.Identity, r *authn.Request, err error) { + if s.cfg.LoginDefaultOrgId < 1 || currentIdentity == nil || err != nil { + return } ctxLogger := s.log.FromContext(ctx) @@ -142,33 +142,30 @@ func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn. namespace, identifier := currentIdentity.GetNamespacedID() if namespace != identity.NamespaceUser { ctxLogger.Debug("Skipping default org sync, not a user", "namespace", namespace) - return nil + return } userID, err := identity.IntIdentifier(namespace, identifier) if err != nil { ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "namespace", namespace, "err", err) - return nil + return } hasAssignedToOrg, err := s.validateUsingOrg(ctx, userID, s.cfg.LoginDefaultOrgId) if err != nil { ctxLogger.Error("Skipping default org sync, failed to validate user's organizations", "id", currentIdentity.ID, "err", err) - return nil + return } if !hasAssignedToOrg { ctxLogger.Debug("Skipping default org sync, user is not assigned to org", "id", currentIdentity.ID, "org", s.cfg.LoginDefaultOrgId) - return nil + return } cmd := user.SetUsingOrgCommand{UserID: userID, OrgID: s.cfg.LoginDefaultOrgId} - if err := s.userService.SetUsingOrg(ctx, &cmd); err != nil { - ctxLogger.Error("Failed to set default org", "id", currentIdentity.ID, "err", err) - return err + if svcErr := s.userService.SetUsingOrg(ctx, &cmd); svcErr != nil { + ctxLogger.Error("Failed to set default org", "id", currentIdentity.ID, "err", svcErr) } - - return nil } func (s *OrgSync) validateUsingOrg(ctx context.Context, userID int64, orgID int64) (bool, error) { diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index 9288f4c1958..b2083f3a3e4 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -134,8 +134,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { defaultOrgSetting int64 identity *authn.Identity setupMock func(*usertest.MockService, *orgtest.FakeOrgService) - - wantErr bool + inputErr error }{ { name: "should set default org", @@ -157,6 +156,12 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { defaultOrgSetting: -1, identity: nil, }, + { + name: "should skip setting the default org when input err is not nil", + defaultOrgSetting: 2, + identity: &authn.Identity{ID: "user:1"}, + inputErr: fmt.Errorf("error"), + }, { name: "should skip setting the default org when identity is not a user", defaultOrgSetting: 2, @@ -181,13 +186,12 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { }, }, { - name: "should return error when the user org update was unsuccessful", + name: "should skip the hook when the user org update was unsuccessful", defaultOrgSetting: 2, identity: &authn.Identity{ID: "user:1"}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { userService.On("SetUsingOrg", mock.Anything, mock.Anything).Return(fmt.Errorf("error")) }, - wantErr: true, }, } for _, tt := range testCases { @@ -214,9 +218,9 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { cfg: cfg, } - if err := s.SetDefaultOrgHook(context.Background(), tt.identity, nil); (err != nil) != tt.wantErr { - t.Errorf("OrgSync.SetDefaultOrgHook() error = %v, wantErr %v", err, tt.wantErr) - } + s.SetDefaultOrgHook(context.Background(), tt.identity, nil, tt.inputErr) + + userService.AssertExpectations(t) }) } }