From 417ee4d2ede2dada6c61badd33cdea9178f41ced Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Thu, 23 Jul 2026 11:37:55 +0530 Subject: [PATCH 1/4] fix(auth): reject PAT and service user when parent org is disabled Disabling an org flipped its state but left credentials owned by that org working. The PAT and service-user auth paths fetched the principal without looking at the org state. Each PAT and service-user authenticator now checks the parent org through a new organization.IsEnabled. A disabled or missing org returns 403. The platform org (bootstrap superuser) is skipped so break-glass credentials keep working. Part of #1585. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/serve.go | 1 + core/authenticate/authenticators.go | 30 ++++++ core/authenticate/mocks/org_service.go | 93 ++++++++++++++++ core/authenticate/service.go | 9 ++ core/authenticate/service_test.go | 112 ++++++++++++++++++++ core/organization/service.go | 12 +++ core/organization/service_test.go | 46 ++++++++ internal/api/v1beta1connect/authenticate.go | 2 + 8 files changed, 305 insertions(+) create mode 100644 core/authenticate/mocks/org_service.go diff --git a/cmd/serve.go b/cmd/serve.go index c73533d7f..f8bad0fba 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -473,6 +473,7 @@ func buildAPIDependencies( groupService := group.NewService(groupRepository, relationService, authnService, policyService) organizationService := organization.NewService(organizationRepository, relationService, userService, authnService, policyService, preferenceService, roleService) + authnService.SetOrgService(organizationService) projectRepository := postgres.NewProjectRepository(dbc) projectService := project.NewService(projectRepository, relationService, userService, policyService, authnService, serviceUserService, groupService, roleService) diff --git a/core/authenticate/authenticators.go b/core/authenticate/authenticators.go index 62805ddb7..25b4dcec0 100644 --- a/core/authenticate/authenticators.go +++ b/core/authenticate/authenticators.go @@ -54,6 +54,20 @@ func authenticateWithSession(ctx context.Context, s *Service) (Principal, error) return Principal{}, errSkip } +func (s *Service) ensureOrgEnabled(ctx context.Context, orgID string) error { + if s.orgService == nil || orgID == "" || orgID == schema.PlatformOrgID.String() { + return nil + } + enabled, err := s.orgService.IsEnabled(ctx, orgID) + if err != nil { + return err + } + if !enabled { + return errors.ErrForbidden + } + return nil +} + // authenticateWithPAT validates a personal access token. func authenticateWithPAT(ctx context.Context, s *Service) (Principal, error) { value, ok := GetTokenFromContext(ctx) @@ -70,6 +84,10 @@ func authenticateWithPAT(ctx context.Context, s *Service) (Principal, error) { return Principal{}, err } + if err := s.ensureOrgEnabled(ctx, pat.OrgID); err != nil { + return Principal{}, err + } + // resolve the owning user so downstream handlers can access principal.User currentUser, err := s.userService.GetByID(ctx, pat.UserID) if err != nil { @@ -125,6 +143,9 @@ func authenticateWithAccessToken(ctx context.Context, s *Service) (Principal, er s.log.DebugContext(ctx, "failed to get service user", "err", err) return Principal{}, err } + if err := s.ensureOrgEnabled(ctx, currentUser.OrgID); err != nil { + return Principal{}, err + } return Principal{ ID: currentUser.ID, Type: schema.ServiceUserPrincipal, @@ -143,6 +164,9 @@ func authenticateWithAccessToken(ctx context.Context, s *Service) (Principal, er s.log.DebugContext(ctx, "PAT has expired", "pat_id", patID) return Principal{}, errors.ErrUnauthenticated } + if err := s.ensureOrgEnabled(ctx, pat.OrgID); err != nil { + return Principal{}, err + } currentUser, err := s.userService.GetByID(ctx, pat.UserID) if err != nil { s.log.DebugContext(ctx, "failed to get PAT owner", "err", err) @@ -183,6 +207,9 @@ func authenticateWithJWTGrant(ctx context.Context, s *Service) (Principal, error serviceUser, err := s.serviceUserService.GetByJWT(ctx, userToken) if err == nil { + if err := s.ensureOrgEnabled(ctx, serviceUser.OrgID); err != nil { + return Principal{}, err + } return Principal{ ID: serviceUser.ID, Type: schema.ServiceUserPrincipal, @@ -226,6 +253,9 @@ func authenticateWithClientCredentials(ctx context.Context, s *Service) (Princip // extract user from secret if it's a service user serviceUser, err := s.serviceUserService.GetBySecret(ctx, clientID, clientSecret) if err == nil { + if err := s.ensureOrgEnabled(ctx, serviceUser.OrgID); err != nil { + return Principal{}, err + } return Principal{ ID: serviceUser.ID, Type: schema.ServiceUserPrincipal, diff --git a/core/authenticate/mocks/org_service.go b/core/authenticate/mocks/org_service.go new file mode 100644 index 000000000..ee8bf8be3 --- /dev/null +++ b/core/authenticate/mocks/org_service.go @@ -0,0 +1,93 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// OrgService is an autogenerated mock type for the OrgService type +type OrgService struct { + mock.Mock +} + +type OrgService_Expecter struct { + mock *mock.Mock +} + +func (_m *OrgService) EXPECT() *OrgService_Expecter { + return &OrgService_Expecter{mock: &_m.Mock} +} + +// IsEnabled provides a mock function with given fields: ctx, orgID +func (_m *OrgService) IsEnabled(ctx context.Context, orgID string) (bool, error) { + ret := _m.Called(ctx, orgID) + + if len(ret) == 0 { + panic("no return value specified for IsEnabled") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string) (bool, error)); ok { + return rf(ctx, orgID) + } + if rf, ok := ret.Get(0).(func(context.Context, string) bool); ok { + r0 = rf(ctx, orgID) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, orgID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// OrgService_IsEnabled_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'IsEnabled' +type OrgService_IsEnabled_Call struct { + *mock.Call +} + +// IsEnabled is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +func (_e *OrgService_Expecter) IsEnabled(ctx interface{}, orgID interface{}) *OrgService_IsEnabled_Call { + return &OrgService_IsEnabled_Call{Call: _e.mock.On("IsEnabled", ctx, orgID)} +} + +func (_c *OrgService_IsEnabled_Call) Run(run func(ctx context.Context, orgID string)) *OrgService_IsEnabled_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *OrgService_IsEnabled_Call) Return(_a0 bool, _a1 error) *OrgService_IsEnabled_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *OrgService_IsEnabled_Call) RunAndReturn(run func(context.Context, string) (bool, error)) *OrgService_IsEnabled_Call { + _c.Call.Return(run) + return _c +} + +// NewOrgService creates a new instance of OrgService. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewOrgService(t interface { + mock.TestingT + Cleanup(func()) +}) *OrgService { + mock := &OrgService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/authenticate/service.go b/core/authenticate/service.go index f045bac87..62f409196 100644 --- a/core/authenticate/service.go +++ b/core/authenticate/service.go @@ -95,6 +95,10 @@ type UserPATService interface { GetByID(ctx context.Context, id string) (patModels.PAT, error) } +type OrgService interface { + IsEnabled(ctx context.Context, orgID string) (bool, error) +} + type Service struct { log *slog.Logger cron *cron.Cron @@ -107,6 +111,7 @@ type Service struct { sessionService SessionService serviceUserService ServiceUserService userPATService UserPATService + orgService OrgService webAuth *webauthn.WebAuthn } @@ -136,6 +141,10 @@ func NewService(logger *slog.Logger, config Config, flowRepo FlowRepository, return r } +func (s *Service) SetOrgService(orgService OrgService) { + s.orgService = orgService +} + func (s Service) SupportedStrategies() []string { // add here strategies like mail link once implemented var strategies []string diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index 665cd443f..63748c594 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -26,6 +26,7 @@ import ( "github.com/raystack/frontier/core/user" patModels "github.com/raystack/frontier/core/userpat/models" "github.com/raystack/frontier/internal/bootstrap/schema" + frontiererrors "github.com/raystack/frontier/pkg/errors" mailerMock "github.com/raystack/frontier/pkg/mailer/mocks" pkgMetadata "github.com/raystack/frontier/pkg/metadata" "github.com/raystack/frontier/pkg/server/consts" @@ -510,3 +511,114 @@ func TestService_GetPrincipal_RestrictsByAuthVia(t *testing.T) { }) } } + +func TestService_GetPrincipal_OrgStateGate(t *testing.T) { + orgID := uuid.New().String() + userID := uuid.New().String() + patID := uuid.New().String() + secret := base64.StdEncoding.EncodeToString([]byte("user:password")) + + tests := []struct { + name string + ctx context.Context + assertions []authenticate.ClientAssertion + want authenticate.Principal + wantErr error + setup func(t *testing.T) *authenticate.Service + }{ + { + name: "reject PAT whose org is disabled or gone", + ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ + consts.UserTokenGatewayKey: {"pat-token"}, + }), + assertions: []authenticate.ClientAssertion{authenticate.PATClientAssertion}, + wantErr: frontiererrors.ErrForbidden, + setup: func(t *testing.T) *authenticate.Service { + pat := mocks.NewUserPATService(t) + pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, nil) + org := mocks.NewOrgService(t) + org.EXPECT().IsEnabled(mock.Anything, orgID).Return(false, nil) + s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, + nil, nil, nil, nil, mocks.NewUserService(t), nil, nil, pat) + s.SetOrgService(org) + return s + }, + }, + { + name: "allow PAT whose org is enabled", + ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ + consts.UserTokenGatewayKey: {"pat-token"}, + }), + assertions: []authenticate.ClientAssertion{authenticate.PATClientAssertion}, + want: authenticate.Principal{ + ID: patID, + Type: schema.PATPrincipal, + AuthVia: authenticate.PATClientAssertion, + PAT: &patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, + User: &user.User{ID: userID}, + }, + setup: func(t *testing.T) *authenticate.Service { + pat := mocks.NewUserPATService(t) + pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, nil) + usr := mocks.NewUserService(t) + usr.EXPECT().GetByID(mock.Anything, userID).Return(user.User{ID: userID}, nil) + org := mocks.NewOrgService(t) + org.EXPECT().IsEnabled(mock.Anything, orgID).Return(true, nil) + s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, + nil, nil, nil, nil, usr, nil, nil, pat) + s.SetOrgService(org) + return s + }, + }, + { + name: "reject service user (client credentials) whose org is disabled or gone", + ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ + consts.UserSecretGatewayKey: {secret}, + }), + assertions: []authenticate.ClientAssertion{authenticate.ClientCredentialsClientAssertion}, + wantErr: frontiererrors.ErrForbidden, + setup: func(t *testing.T) *authenticate.Service { + su := mocks.NewServiceUserService(t) + su.EXPECT().GetBySecret(mock.Anything, "user", "password").Return(serviceuser.ServiceUser{ID: userID, OrgID: orgID}, nil) + org := mocks.NewOrgService(t) + org.EXPECT().IsEnabled(mock.Anything, orgID).Return(false, nil) + s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, + nil, nil, nil, nil, nil, su, nil, nil) + s.SetOrgService(org) + return s + }, + }, + { + name: "reject service user (jwt grant) whose org is disabled or gone", + ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ + consts.UserTokenGatewayKey: {"grant-token"}, + }), + assertions: []authenticate.ClientAssertion{authenticate.JWTGrantClientAssertion}, + wantErr: frontiererrors.ErrForbidden, + setup: func(t *testing.T) *authenticate.Service { + su := mocks.NewServiceUserService(t) + su.EXPECT().GetByJWT(mock.Anything, "grant-token").Return(serviceuser.ServiceUser{ID: userID, OrgID: orgID}, nil) + org := mocks.NewOrgService(t) + org.EXPECT().IsEnabled(mock.Anything, orgID).Return(false, nil) + s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, + nil, nil, nil, nil, nil, su, nil, nil) + s.SetOrgService(org) + return s + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := tt.setup(t) + got, err := s.GetPrincipal(tt.ctx, tt.assertions...) + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) + return + } + assert.NoError(t, err) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("GetPrincipal() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/core/organization/service.go b/core/organization/service.go index ac55369af..2c230a7b7 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -125,6 +125,18 @@ func (s Service) Get(ctx context.Context, idOrName string) (Organization, error) return orgResp, nil } +func (s Service) IsEnabled(ctx context.Context, id string) (bool, error) { + _, err := s.Get(ctx, id) + switch { + case err == nil: + return true, nil + case errors.Is(err, ErrDisabled), errors.Is(err, ErrNotExist): + return false, nil + default: + return false, err + } +} + func (s Service) GetByIDs(ctx context.Context, ids []string) ([]Organization, error) { return s.repository.GetByIDs(ctx, ids) } diff --git a/core/organization/service_test.go b/core/organization/service_test.go index e88f8399e..913efaf78 100644 --- a/core/organization/service_test.go +++ b/core/organization/service_test.go @@ -73,6 +73,52 @@ func TestService_Get(t *testing.T) { }) } +func TestService_IsEnabled(t *testing.T) { + newSvc := func(t *testing.T) (*mocks.Repository, *organization.Service) { + mockRepo := mocks.NewRepository(t) + svc := organization.NewService(mockRepo, mocks.NewRelationService(t), mocks.NewUserService(t), + mocks.NewAuthnService(t), mocks.NewPolicyService(t), mocks.NewPreferencesService(t), mocks.NewRoleService(t)) + return mockRepo, svc + } + + t.Run("enabled org returns true", func(t *testing.T) { + mockRepo, svc := newSvc(t) + id := uuid.New().String() + mockRepo.On("GetByID", mock.Anything, id).Return(organization.Organization{ID: id, State: organization.Enabled}, nil).Once() + ok, err := svc.IsEnabled(context.Background(), id) + assert.NoError(t, err) + assert.True(t, ok) + }) + + t.Run("disabled org returns false", func(t *testing.T) { + mockRepo, svc := newSvc(t) + id := uuid.New().String() + mockRepo.On("GetByID", mock.Anything, id).Return(organization.Organization{ID: id, State: organization.Disabled}, nil).Once() + ok, err := svc.IsEnabled(context.Background(), id) + assert.NoError(t, err) + assert.False(t, ok) + }) + + t.Run("missing org returns false", func(t *testing.T) { + mockRepo, svc := newSvc(t) + id := uuid.New().String() + mockRepo.On("GetByID", mock.Anything, id).Return(organization.Organization{}, organization.ErrNotExist).Once() + ok, err := svc.IsEnabled(context.Background(), id) + assert.NoError(t, err) + assert.False(t, ok) + }) + + t.Run("unexpected error propagates", func(t *testing.T) { + mockRepo, svc := newSvc(t) + id := uuid.New().String() + repoErr := errors.New("db down") + mockRepo.On("GetByID", mock.Anything, id).Return(organization.Organization{}, repoErr).Once() + ok, err := svc.IsEnabled(context.Background(), id) + assert.ErrorIs(t, err, repoErr) + assert.False(t, ok) + }) +} + func TestService_GetRaw(t *testing.T) { mockRepo := mocks.NewRepository(t) mockRelationSvc := mocks.NewRelationService(t) diff --git a/internal/api/v1beta1connect/authenticate.go b/internal/api/v1beta1connect/authenticate.go index d48f62480..738f978b5 100644 --- a/internal/api/v1beta1connect/authenticate.go +++ b/internal/api/v1beta1connect/authenticate.go @@ -285,6 +285,8 @@ func (h *ConnectHandler) GetLoggedInPrincipal(ctx context.Context, via ...authen return principal, connect.NewError(connect.CodeNotFound, ErrUserNotExist) case errors.Is(err, errors.ErrUnauthenticated): return principal, connect.NewError(connect.CodeUnauthenticated, ErrUnauthenticated) + case errors.Is(err, errors.ErrForbidden): + return principal, connect.NewError(connect.CodePermissionDenied, ErrUnauthorized) case errors.Is(err, patErrors.ErrMalformedPAT), errors.Is(err, patErrors.ErrNotFound), errors.Is(err, patErrors.ErrExpired), From 0b3f533ac5e13e898cde2a566503187f30c922d2 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Thu, 23 Jul 2026 11:50:45 +0530 Subject: [PATCH 2/4] fix(auth): fail closed when credential has no org id An empty org id skipped the gate, so a PAT or service user with no resolvable org would authenticate. Return 403 for an empty id; only the platform org (a fixed UUID) is exempt. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/authenticate/authenticators.go | 5 ++++- core/authenticate/service_test.go | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/core/authenticate/authenticators.go b/core/authenticate/authenticators.go index 25b4dcec0..5624f41ae 100644 --- a/core/authenticate/authenticators.go +++ b/core/authenticate/authenticators.go @@ -55,9 +55,12 @@ func authenticateWithSession(ctx context.Context, s *Service) (Principal, error) } func (s *Service) ensureOrgEnabled(ctx context.Context, orgID string) error { - if s.orgService == nil || orgID == "" || orgID == schema.PlatformOrgID.String() { + if s.orgService == nil || orgID == schema.PlatformOrgID.String() { return nil } + if orgID == "" { + return errors.ErrForbidden + } enabled, err := s.orgService.IsEnabled(ctx, orgID) if err != nil { return err diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index 63748c594..a61d26121 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -544,6 +544,23 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { return s }, }, + { + name: "reject PAT with empty org id", + ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ + consts.UserTokenGatewayKey: {"pat-token"}, + }), + assertions: []authenticate.ClientAssertion{authenticate.PATClientAssertion}, + wantErr: frontiererrors.ErrForbidden, + setup: func(t *testing.T) *authenticate.Service { + pat := mocks.NewUserPATService(t) + pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: ""}, nil) + org := mocks.NewOrgService(t) + s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, + nil, nil, nil, nil, mocks.NewUserService(t), nil, nil, pat) + s.SetOrgService(org) + return s + }, + }, { name: "allow PAT whose org is enabled", ctx: metadata.NewIncomingContext(context.Background(), map[string][]string{ From a6d553e50b8a8a70cb5353cbce9643f0831d914b Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Thu, 23 Jul 2026 12:03:24 +0530 Subject: [PATCH 3/4] refactor(auth): gate org state once in GetPrincipal Move the parent-org check out of the individual authenticators into a single point in GetPrincipal, keyed off the resolved principal type. Same behavior, one check instead of five, and any future authenticator that returns a PAT or service user principal is covered automatically. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/authenticate/authenticators.go | 30 ++++++++++++++--------------- core/authenticate/service.go | 3 +++ core/authenticate/service_test.go | 8 ++++++-- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/core/authenticate/authenticators.go b/core/authenticate/authenticators.go index 5624f41ae..92c8dfae4 100644 --- a/core/authenticate/authenticators.go +++ b/core/authenticate/authenticators.go @@ -54,6 +54,20 @@ func authenticateWithSession(ctx context.Context, s *Service) (Principal, error) return Principal{}, errSkip } +func (s *Service) ensurePrincipalOrgEnabled(ctx context.Context, p Principal) error { + switch p.Type { + case schema.PATPrincipal: + if p.PAT != nil { + return s.ensureOrgEnabled(ctx, p.PAT.OrgID) + } + case schema.ServiceUserPrincipal: + if p.ServiceUser != nil { + return s.ensureOrgEnabled(ctx, p.ServiceUser.OrgID) + } + } + return nil +} + func (s *Service) ensureOrgEnabled(ctx context.Context, orgID string) error { if s.orgService == nil || orgID == schema.PlatformOrgID.String() { return nil @@ -87,10 +101,6 @@ func authenticateWithPAT(ctx context.Context, s *Service) (Principal, error) { return Principal{}, err } - if err := s.ensureOrgEnabled(ctx, pat.OrgID); err != nil { - return Principal{}, err - } - // resolve the owning user so downstream handlers can access principal.User currentUser, err := s.userService.GetByID(ctx, pat.UserID) if err != nil { @@ -146,9 +156,6 @@ func authenticateWithAccessToken(ctx context.Context, s *Service) (Principal, er s.log.DebugContext(ctx, "failed to get service user", "err", err) return Principal{}, err } - if err := s.ensureOrgEnabled(ctx, currentUser.OrgID); err != nil { - return Principal{}, err - } return Principal{ ID: currentUser.ID, Type: schema.ServiceUserPrincipal, @@ -167,9 +174,6 @@ func authenticateWithAccessToken(ctx context.Context, s *Service) (Principal, er s.log.DebugContext(ctx, "PAT has expired", "pat_id", patID) return Principal{}, errors.ErrUnauthenticated } - if err := s.ensureOrgEnabled(ctx, pat.OrgID); err != nil { - return Principal{}, err - } currentUser, err := s.userService.GetByID(ctx, pat.UserID) if err != nil { s.log.DebugContext(ctx, "failed to get PAT owner", "err", err) @@ -210,9 +214,6 @@ func authenticateWithJWTGrant(ctx context.Context, s *Service) (Principal, error serviceUser, err := s.serviceUserService.GetByJWT(ctx, userToken) if err == nil { - if err := s.ensureOrgEnabled(ctx, serviceUser.OrgID); err != nil { - return Principal{}, err - } return Principal{ ID: serviceUser.ID, Type: schema.ServiceUserPrincipal, @@ -256,9 +257,6 @@ func authenticateWithClientCredentials(ctx context.Context, s *Service) (Princip // extract user from secret if it's a service user serviceUser, err := s.serviceUserService.GetBySecret(ctx, clientID, clientSecret) if err == nil { - if err := s.ensureOrgEnabled(ctx, serviceUser.OrgID); err != nil { - return Principal{}, err - } return Principal{ ID: serviceUser.ID, Type: schema.ServiceUserPrincipal, diff --git a/core/authenticate/service.go b/core/authenticate/service.go index 62f409196..cd1f2a468 100644 --- a/core/authenticate/service.go +++ b/core/authenticate/service.go @@ -807,6 +807,9 @@ func (s Service) GetPrincipal(ctx context.Context, assertions ...ClientAssertion principal, err := authenticator(ctx, &s) if err == nil { principal.AuthVia = assertion + if err := s.ensurePrincipalOrgEnabled(ctx, principal); err != nil { + return Principal{}, err + } return principal, nil } if !errors.Is(err, errSkip) { diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index a61d26121..4b7c48823 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -536,10 +536,12 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { setup: func(t *testing.T) *authenticate.Service { pat := mocks.NewUserPATService(t) pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, nil) + usr := mocks.NewUserService(t) + usr.EXPECT().GetByID(mock.Anything, userID).Return(user.User{ID: userID}, nil) org := mocks.NewOrgService(t) org.EXPECT().IsEnabled(mock.Anything, orgID).Return(false, nil) s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, - nil, nil, nil, nil, mocks.NewUserService(t), nil, nil, pat) + nil, nil, nil, nil, usr, nil, nil, pat) s.SetOrgService(org) return s }, @@ -554,9 +556,11 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { setup: func(t *testing.T) *authenticate.Service { pat := mocks.NewUserPATService(t) pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: ""}, nil) + usr := mocks.NewUserService(t) + usr.EXPECT().GetByID(mock.Anything, userID).Return(user.User{ID: userID}, nil) org := mocks.NewOrgService(t) s := authenticate.NewService(slog.New(slog.NewTextHandler(io.Discard, nil)), authenticate.Config{}, - nil, nil, nil, nil, mocks.NewUserService(t), nil, nil, pat) + nil, nil, nil, nil, usr, nil, nil, pat) s.SetOrgService(org) return s }, From e0f3a0824efd49691dbaab10da6dbedc95e5bf13 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Thu, 23 Jul 2026 12:20:07 +0530 Subject: [PATCH 4/4] test(auth): satisfy thelper lint in org-state tests Use func() setup closures that capture the outer t (matching the existing GetPrincipal table) and add t.Helper() to the org test helper. Co-Authored-By: Claude Opus 4.8 (1M context) --- core/authenticate/service_test.go | 14 +++++++------- core/organization/service_test.go | 1 + 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/core/authenticate/service_test.go b/core/authenticate/service_test.go index 4b7c48823..90e69982d 100644 --- a/core/authenticate/service_test.go +++ b/core/authenticate/service_test.go @@ -524,7 +524,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { assertions []authenticate.ClientAssertion want authenticate.Principal wantErr error - setup func(t *testing.T) *authenticate.Service + setup func() *authenticate.Service }{ { name: "reject PAT whose org is disabled or gone", @@ -533,7 +533,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { }), assertions: []authenticate.ClientAssertion{authenticate.PATClientAssertion}, wantErr: frontiererrors.ErrForbidden, - setup: func(t *testing.T) *authenticate.Service { + setup: func() *authenticate.Service { pat := mocks.NewUserPATService(t) pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, nil) usr := mocks.NewUserService(t) @@ -553,7 +553,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { }), assertions: []authenticate.ClientAssertion{authenticate.PATClientAssertion}, wantErr: frontiererrors.ErrForbidden, - setup: func(t *testing.T) *authenticate.Service { + setup: func() *authenticate.Service { pat := mocks.NewUserPATService(t) pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: ""}, nil) usr := mocks.NewUserService(t) @@ -578,7 +578,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { PAT: &patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, User: &user.User{ID: userID}, }, - setup: func(t *testing.T) *authenticate.Service { + setup: func() *authenticate.Service { pat := mocks.NewUserPATService(t) pat.EXPECT().Validate(mock.Anything, "pat-token").Return(patModels.PAT{ID: patID, UserID: userID, OrgID: orgID}, nil) usr := mocks.NewUserService(t) @@ -598,7 +598,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { }), assertions: []authenticate.ClientAssertion{authenticate.ClientCredentialsClientAssertion}, wantErr: frontiererrors.ErrForbidden, - setup: func(t *testing.T) *authenticate.Service { + setup: func() *authenticate.Service { su := mocks.NewServiceUserService(t) su.EXPECT().GetBySecret(mock.Anything, "user", "password").Return(serviceuser.ServiceUser{ID: userID, OrgID: orgID}, nil) org := mocks.NewOrgService(t) @@ -616,7 +616,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { }), assertions: []authenticate.ClientAssertion{authenticate.JWTGrantClientAssertion}, wantErr: frontiererrors.ErrForbidden, - setup: func(t *testing.T) *authenticate.Service { + setup: func() *authenticate.Service { su := mocks.NewServiceUserService(t) su.EXPECT().GetByJWT(mock.Anything, "grant-token").Return(serviceuser.ServiceUser{ID: userID, OrgID: orgID}, nil) org := mocks.NewOrgService(t) @@ -630,7 +630,7 @@ func TestService_GetPrincipal_OrgStateGate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := tt.setup(t) + s := tt.setup() got, err := s.GetPrincipal(tt.ctx, tt.assertions...) if tt.wantErr != nil { assert.ErrorIs(t, err, tt.wantErr) diff --git a/core/organization/service_test.go b/core/organization/service_test.go index 913efaf78..428ef727f 100644 --- a/core/organization/service_test.go +++ b/core/organization/service_test.go @@ -75,6 +75,7 @@ func TestService_Get(t *testing.T) { func TestService_IsEnabled(t *testing.T) { newSvc := func(t *testing.T) (*mocks.Repository, *organization.Service) { + t.Helper() mockRepo := mocks.NewRepository(t) svc := organization.NewService(mockRepo, mocks.NewRelationService(t), mocks.NewUserService(t), mocks.NewAuthnService(t), mocks.NewPolicyService(t), mocks.NewPreferencesService(t), mocks.NewRoleService(t))