From 6e2e15c77ba7f3dc2b1d9a1730b317fa3dd0a9fa Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:07:42 +0530 Subject: [PATCH 01/19] =?UTF-8?q?feat:=20delete=20AddOrganizationUsers=20R?= =?UTF-8?q?PC=20=E2=80=94=20replaced=20by=20AddOrganizationMembers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AddOrganizationUsers had no SDK usage and was replaced by AddOrganizationMembers (AdminService) which takes explicit roles and returns per-member results. Removed: - AddOrganizationUsers handler - AddUsers from OrganizationService interface - org.AddUsers() service function - Auth interceptor entry - Handler tests Co-Authored-By: Claude Opus 4.6 (1M context) --- core/organization/service.go | 13 --- internal/api/v1beta1connect/interfaces.go | 1 - internal/api/v1beta1connect/organization.go | 27 ------ .../api/v1beta1connect/organization_test.go | 85 ------------------- .../connect_interceptors/authorization.go | 4 - 5 files changed, 130 deletions(-) diff --git a/core/organization/service.go b/core/organization/service.go index b1591be45..ab8bd5241 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -356,19 +356,6 @@ func (s Service) ListByUser(ctx context.Context, principal authenticate.Principa return s.repository.List(ctx, filter) } -func (s Service) AddUsers(ctx context.Context, orgID string, userIDs []string) error { - var err error - for _, userID := range userIDs { - if currentErr := s.AddMember(ctx, orgID, schema.MemberRelationName, authenticate.Principal{ - ID: userID, - Type: schema.UserPrincipal, - }); currentErr != nil { - err = errors.Join(err, currentErr) - } - } - return err -} - // RemoveUsers removes users from an organization as members // it doesn't remove user access to projects or other resources provided // by policies, don't call directly, use cascade deleter diff --git a/internal/api/v1beta1connect/interfaces.go b/internal/api/v1beta1connect/interfaces.go index c8c537679..38f8c2e7e 100644 --- a/internal/api/v1beta1connect/interfaces.go +++ b/internal/api/v1beta1connect/interfaces.go @@ -124,7 +124,6 @@ type OrganizationService interface { List(ctx context.Context, f organization.Filter) ([]organization.Organization, error) Update(ctx context.Context, toUpdate organization.Organization) (organization.Organization, error) ListByUser(ctx context.Context, principal authenticate.Principal, flt organization.Filter) ([]organization.Organization, error) - AddUsers(ctx context.Context, orgID string, userID []string) error Enable(ctx context.Context, id string) error Disable(ctx context.Context, id string) error } diff --git a/internal/api/v1beta1connect/organization.go b/internal/api/v1beta1connect/organization.go index 51410872c..0f42bcba5 100644 --- a/internal/api/v1beta1connect/organization.go +++ b/internal/api/v1beta1connect/organization.go @@ -469,33 +469,6 @@ func (h *ConnectHandler) ListOrganizationUsers(ctx context.Context, request *con }), nil } -func (h *ConnectHandler) AddOrganizationUsers(ctx context.Context, request *connect.Request[frontierv1beta1.AddOrganizationUsersRequest]) (*connect.Response[frontierv1beta1.AddOrganizationUsersResponse], error) { - errorLogger := NewErrorLogger() - - orgResp, err := h.orgService.Get(ctx, request.Msg.GetId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - errorLogger.LogServiceError(ctx, request, "AddOrganizationUsers.Get", err, - zap.String("org_id", request.Msg.GetId())) - return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError) - } - } - - if err := h.orgService.AddUsers(ctx, orgResp.ID, request.Msg.GetUserIds()); err != nil { - errorLogger.LogServiceError(ctx, request, "AddOrganizationUsers.AddUsers", err, - zap.String("org_id", orgResp.ID), - zap.Strings("user_ids", request.Msg.GetUserIds())) - return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError) - } - - return connect.NewResponse(&frontierv1beta1.AddOrganizationUsersResponse{}), nil -} - func (h *ConnectHandler) RemoveOrganizationUser(ctx context.Context, request *connect.Request[frontierv1beta1.RemoveOrganizationUserRequest]) (*connect.Response[frontierv1beta1.RemoveOrganizationUserResponse], error) { errorLogger := NewErrorLogger() diff --git a/internal/api/v1beta1connect/organization_test.go b/internal/api/v1beta1connect/organization_test.go index 9c24cc446..d5f9aab31 100644 --- a/internal/api/v1beta1connect/organization_test.go +++ b/internal/api/v1beta1connect/organization_test.go @@ -921,91 +921,6 @@ func TestHandler_ListOrganizationUsers(t *testing.T) { } } -func TestHandler_AddOrganizationUsers(t *testing.T) { - tests := []struct { - name string - setup func(os *mocks.OrganizationService) - request *connect.Request[frontierv1beta1.AddOrganizationUsersRequest] - want *connect.Response[frontierv1beta1.AddOrganizationUsersResponse] - wantErr error - }{ - { - name: "should return internal error if org service return some error", - setup: func(os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) - }, - request: connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: testOrgID, - UserIds: []string{"some-user-id"}, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, ErrInternalServerError), - }, - { - name: "should return not found error if org does not exist", - setup: func(os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: testOrgID, - UserIds: []string{"some-user-id"}, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrNotFound), - }, - { - name: "should return not found error if org is disabled", - setup: func(os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: testOrgID, - UserIds: []string{"some-user-id"}, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgDisabled), - }, - { - name: "should return internal error if AddUsers fails", - setup: func(os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) - os.EXPECT().AddUsers(mock.AnythingOfType("context.backgroundCtx"), testOrgID, []string{"some-user-id"}).Return(errors.New("add users error")) - }, - request: connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: testOrgID, - UserIds: []string{"some-user-id"}, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, ErrInternalServerError), - }, - { - name: "should add user to org successfully", - setup: func(os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) - os.EXPECT().AddUsers(mock.AnythingOfType("context.backgroundCtx"), testOrgID, []string{"some-user-id"}).Return(nil) - }, - request: connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: testOrgID, - UserIds: []string{"some-user-id"}, - }), - want: connect.NewResponse(&frontierv1beta1.AddOrganizationUsersResponse{}), - wantErr: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mockOrgService := new(mocks.OrganizationService) - if tt.setup != nil { - tt.setup(mockOrgService) - } - mockDep := &ConnectHandler{orgService: mockOrgService} - resp, err := mockDep.AddOrganizationUsers(context.Background(), tt.request) - assert.Equal(t, tt.want, resp) - assert.Equal(t, tt.wantErr, err) - }) - } -} - func TestHandler_RemoveOrganizationUser(t *testing.T) { tests := []struct { name string diff --git a/pkg/server/connect_interceptors/authorization.go b/pkg/server/connect_interceptors/authorization.go index 4b8c692c0..02e32c156 100644 --- a/pkg/server/connect_interceptors/authorization.go +++ b/pkg/server/connect_interceptors/authorization.go @@ -353,10 +353,6 @@ var authorizationValidationMap = map[string]func(ctx context.Context, handler *v pbreq := req.(*connect.Request[frontierv1beta1.ListOrganizationProjectsRequest]) return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.OrganizationNamespace, ID: pbreq.Msg.GetId()}, schema.ProjectListPermission, req) }, - "/raystack.frontier.v1beta1.FrontierService/AddOrganizationUsers": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error { - pbreq := req.(*connect.Request[frontierv1beta1.AddOrganizationUsersRequest]) - return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.OrganizationNamespace, ID: pbreq.Msg.GetId()}, schema.UpdatePermission, req) - }, "/raystack.frontier.v1beta1.FrontierService/RemoveOrganizationUser": func(ctx context.Context, handler *v1beta1connect.ConnectHandler, req connect.AnyRequest) error { pbreq := req.(*connect.Request[frontierv1beta1.RemoveOrganizationUserRequest]) return handler.IsAuthorized(ctx, relation.Object{Namespace: schema.OrganizationNamespace, ID: pbreq.Msg.GetId()}, schema.UpdatePermission, req) From 7fd4d072225a6bb378121ea7357ebe75da03e994 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:36:05 +0530 Subject: [PATCH 02/19] feat: migrate org.Create and AdminCreate to use membership.AddOrganizationMember MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit org.Create and AdminCreate now call membership.AddOrganizationMember instead of org.AddMember to add the creator as owner. Membership dependency injected via SetMembershipService() setter to break circular init order. Behavioral change: new code validates user exists+enabled and role scope. Old code trusted the authn principal blindly. Non-user principals (serviceuser/PAT) are now rejected — org creation is user-only in practice. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/serve.go | 5 +- core/organization/mocks/membership_service.go | 86 +++++++++++++++++ core/organization/service.go | 93 +++---------------- core/organization/service_test.go | 59 ------------ 4 files changed, 104 insertions(+), 139 deletions(-) create mode 100644 core/organization/mocks/membership_service.go diff --git a/cmd/serve.go b/cmd/serve.go index f4d4031b5..d883274ed 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -432,6 +432,7 @@ func buildAPIDependencies( authnService, policyService, preferenceService, auditRecordRepository, roleService) membershipService := membership.NewService(logger, policyService, relationService, roleService, organizationService, userService, auditRecordRepository) + organizationService.SetMembershipService(membershipService) orgKycRepository := postgres.NewOrgKycRepository(dbc) orgKycService := kyc.NewService(orgKycRepository) @@ -467,7 +468,7 @@ func buildAPIDependencies( userProjectsService := userprojects.NewService(userProjectsRepository) domainRepository := postgres.NewDomainRepository(logger, dbc) - domainService := domain.NewService(logger, domainRepository, userService, organizationService) + domainService := domain.NewService(logger, domainRepository, userService, organizationService, membershipService) metaschemaRepository := postgres.NewMetaSchemaRepository(logger, dbc) metaschemaService := metaschema.NewService(metaschemaRepository) @@ -496,7 +497,7 @@ func buildAPIDependencies( invitationService := invitation.NewService(mailDialer, postgres.NewInvitationRepository(logger, dbc), organizationService, groupService, userService, relationService, policyService, preferenceService, - auditRecordRepository) + auditRecordRepository, membershipService) if GetStripeClientFunc == nil { // allow to override the stripe client creation function in tests diff --git a/core/organization/mocks/membership_service.go b/core/organization/mocks/membership_service.go new file mode 100644 index 000000000..7a81278b0 --- /dev/null +++ b/core/organization/mocks/membership_service.go @@ -0,0 +1,86 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// MembershipService is an autogenerated mock type for the MembershipService type +type MembershipService struct { + mock.Mock +} + +type MembershipService_Expecter struct { + mock *mock.Mock +} + +func (_m *MembershipService) EXPECT() *MembershipService_Expecter { + return &MembershipService_Expecter{mock: &_m.Mock} +} + +// AddOrganizationMember provides a mock function with given fields: ctx, orgID, principalID, principalType, roleID +func (_m *MembershipService) AddOrganizationMember(ctx context.Context, orgID string, principalID string, principalType string, roleID string) error { + ret := _m.Called(ctx, orgID, principalID, principalType, roleID) + + if len(ret) == 0 { + panic("no return value specified for AddOrganizationMember") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) error); ok { + r0 = rf(ctx, orgID, principalID, principalType, roleID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MembershipService_AddOrganizationMember_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AddOrganizationMember' +type MembershipService_AddOrganizationMember_Call struct { + *mock.Call +} + +// AddOrganizationMember is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +// - principalID string +// - principalType string +// - roleID string +func (_e *MembershipService_Expecter) AddOrganizationMember(ctx interface{}, orgID interface{}, principalID interface{}, principalType interface{}, roleID interface{}) *MembershipService_AddOrganizationMember_Call { + return &MembershipService_AddOrganizationMember_Call{Call: _e.mock.On("AddOrganizationMember", ctx, orgID, principalID, principalType, roleID)} +} + +func (_c *MembershipService_AddOrganizationMember_Call) Run(run func(ctx context.Context, orgID string, principalID string, principalType string, roleID string)) *MembershipService_AddOrganizationMember_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string), args[4].(string)) + }) + return _c +} + +func (_c *MembershipService_AddOrganizationMember_Call) Return(_a0 error) *MembershipService_AddOrganizationMember_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MembershipService_AddOrganizationMember_Call) RunAndReturn(run func(context.Context, string, string, string, string) error) *MembershipService_AddOrganizationMember_Call { + _c.Call.Return(run) + return _c +} + +// NewMembershipService creates a new instance of MembershipService. 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 NewMembershipService(t interface { + mock.TestingT + Cleanup(func()) +}) *MembershipService { + mock := &MembershipService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/organization/service.go b/core/organization/service.go index ab8bd5241..4e0bcd50f 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -73,6 +73,10 @@ type RoleService interface { Get(ctx context.Context, idOrName string) (role.Role, error) } +type MembershipService interface { + AddOrganizationMember(ctx context.Context, orgID, principalID, principalType, roleID string) error +} + type Service struct { repository Repository relationService RelationService @@ -82,6 +86,7 @@ type Service struct { prefService PreferencesService auditRecordRepository AuditRecordRepository roleService RoleService + membershipService MembershipService } func NewService(repository Repository, relationService RelationService, @@ -100,6 +105,12 @@ func NewService(repository Repository, relationService RelationService, } } +// SetMembershipService sets the membership dependency after construction to break +// the circular init order between organization and membership services. +func (s *Service) SetMembershipService(ms MembershipService) { + s.membershipService = ms +} + // extractPrincipalInfo extracts display name and metadata from principal func extractPrincipalInfo(principal authenticate.Principal) (string, map[string]any) { metadata := make(map[string]any) @@ -202,8 +213,8 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er return Organization{}, err } - // attach user as owner - if err = s.AddMember(ctx, newOrg.ID, schema.OwnerRelationName, principal); err != nil { + // attach user as owner via membership package + if err = s.membershipService.AddOrganizationMember(ctx, newOrg.ID, principal.ID, principal.Type, AdminRole); err != nil { return newOrg, err } @@ -215,77 +226,6 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er return newOrg, nil } -func (s Service) AddMember(ctx context.Context, orgID, relationName string, principal authenticate.Principal) error { - roleID := MemberRole - if relationName == schema.OwnerRelationName { - roleID = AdminRole - } - if _, err := s.policyService.Create(ctx, policy.Policy{ - RoleID: roleID, - ResourceID: orgID, - ResourceType: schema.OrganizationNamespace, - PrincipalID: principal.ID, - PrincipalType: principal.Type, - }); err != nil { - return err - } - if _, err := s.relationService.Create(ctx, relation.Relation{ - Object: relation.Object{ - ID: orgID, - Namespace: schema.OrganizationNamespace, - }, - Subject: relation.Subject{ - ID: principal.ID, - Namespace: principal.Type, - }, - RelationName: relationName, - }); err != nil { - return err - } - - // Get organization details for audit - org, err := s.repository.GetByID(ctx, orgID) - if err != nil { - return err - } - - principalName, principalMetadata := extractPrincipalInfo(principal) - targetMetadata := map[string]any{ - "relation": relationName, - "role": roleID, - } - // Merge principal metadata into target metadata - for k, v := range principalMetadata { - targetMetadata[k] = v - } - - _, err = s.auditRecordRepository.Create(ctx, auditrecord.AuditRecord{ - Event: pkgAuditRecord.OrganizationMemberAddedEvent, - Resource: auditrecord.Resource{ - ID: orgID, - Type: pkgAuditRecord.OrganizationType, - Name: org.Title, - }, - Target: &auditrecord.Target{ - ID: principal.ID, - Type: mapPrincipalTypeToAuditType(principal.Type), - Name: principalName, - Metadata: targetMetadata, - }, - OrgID: orgID, - OccurredAt: time.Now(), - }) - if err != nil { - return err - } - - audit.GetAuditor(ctx, orgID).Log(audit.OrgMemberCreatedEvent, audit.Target{ - ID: principal.ID, - Type: principal.Type, - }) - return nil -} - func (s Service) AttachToPlatform(ctx context.Context, orgID string) error { if _, err := s.relationService.Create(ctx, relation.Relation{ Object: relation.Object{ @@ -519,11 +459,8 @@ func (s Service) AdminCreate(ctx context.Context, org Organization, ownerEmail s return Organization{}, err } - // Add user as organization owner - if err = s.AddMember(ctx, newOrg.ID, schema.OwnerRelationName, authenticate.Principal{ - ID: usr.ID, - Type: schema.UserPrincipal, - }); err != nil { + // Add user as organization owner via membership package + if err = s.membershipService.AddOrganizationMember(ctx, newOrg.ID, usr.ID, schema.UserPrincipal, AdminRole); err != nil { return newOrg, fmt.Errorf("failed to add user as owner: %w", err) } diff --git a/core/organization/service_test.go b/core/organization/service_test.go index 769e7f9e6..89d484208 100644 --- a/core/organization/service_test.go +++ b/core/organization/service_test.go @@ -6,11 +6,9 @@ import ( "testing" "github.com/google/uuid" - "github.com/raystack/frontier/core/auditrecord" "github.com/raystack/frontier/core/authenticate" "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/organization/mocks" - "github.com/raystack/frontier/core/policy" "github.com/raystack/frontier/core/preference" "github.com/raystack/frontier/core/relation" pat "github.com/raystack/frontier/core/userpat/models" @@ -168,63 +166,6 @@ func TestService_GetDefaultOrgStateOnCreate(t *testing.T) { }) } -func TestService_AddMember(t *testing.T) { - mockRepo := mocks.NewRepository(t) - mockRelationSvc := mocks.NewRelationService(t) - mockUserSvc := mocks.NewUserService(t) - mockAuthnSvc := mocks.NewAuthnService(t) - mockPolicySvc := mocks.NewPolicyService(t) - mockPrefSvc := mocks.NewPreferencesService(t) - mockAuditRecordRepo := mocks.NewAuditRecordRepository(t) - - mockRoleSvc := mocks.NewRoleService(t) - svc := organization.NewService(mockRepo, mockRelationSvc, mockUserSvc, mockAuthnSvc, mockPolicySvc, mockPrefSvc, mockAuditRecordRepo, mockRoleSvc) - - t.Run("should create policy and relation for member as per role", func(t *testing.T) { - inputOrgID := "test-id" - inputRelationName := schema.MemberRelationName - inputPrincipal := authenticate.Principal{ - ID: "test-principal-id", - Type: schema.UserPrincipal, - } - - expectedOrg := organization.Organization{ - ID: inputOrgID, - Name: "test-org", - Title: "Test Organization", - State: organization.Enabled, - } - - policyToBeCreated := policy.Policy{ - RoleID: organization.MemberRole, - ResourceID: inputOrgID, - ResourceType: schema.OrganizationNamespace, - PrincipalID: inputPrincipal.ID, - PrincipalType: inputPrincipal.Type, - } - - relationToBeCreated := relation.Relation{ - Object: relation.Object{ - ID: inputOrgID, - Namespace: schema.OrganizationNamespace, - }, - Subject: relation.Subject{ - ID: inputPrincipal.ID, - Namespace: inputPrincipal.Type, - }, - RelationName: schema.MemberRelationName, - } - - mockPolicySvc.On("Create", mock.Anything, policyToBeCreated).Return(policy.Policy{}, nil) - mockRelationSvc.On("Create", mock.Anything, relationToBeCreated).Return(relation.Relation{}, nil) - mockRepo.On("GetByID", mock.Anything, inputOrgID).Return(expectedOrg, nil).Once() - mockAuditRecordRepo.On("Create", mock.Anything, mock.AnythingOfType("models.AuditRecord")).Return(auditrecord.AuditRecord{}, nil).Once() - - err := svc.AddMember(context.Background(), inputOrgID, inputRelationName, inputPrincipal) - assert.Nil(t, err) - }) -} - func TestService_AttachToPlatform(t *testing.T) { mockRepo := mocks.NewRepository(t) mockRelationSvc := mocks.NewRelationService(t) From b0581752621f07274601859f143ed2ee7473371a Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:36:15 +0530 Subject: [PATCH 03/19] feat: migrate invitation.Accept to use membership.AddOrganizationMember MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit invitation.Accept now calls membershipSvc.AddOrganizationMember instead of orgSvc.AddMember to add user on invitation acceptance. Uses RoleOrganizationViewer (same as old MemberRole). No behavioral change in practice — caller already checks isUserOrgMember before calling, so ErrAlreadyMember won't trigger. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/invitation/mocks/membership_service.go | 86 +++++++++++++++++++ core/invitation/mocks/organization_service.go | 49 ----------- core/invitation/service.go | 15 ++-- core/invitation/service_test.go | 3 +- 4 files changed, 97 insertions(+), 56 deletions(-) create mode 100644 core/invitation/mocks/membership_service.go diff --git a/core/invitation/mocks/membership_service.go b/core/invitation/mocks/membership_service.go new file mode 100644 index 000000000..7a81278b0 --- /dev/null +++ b/core/invitation/mocks/membership_service.go @@ -0,0 +1,86 @@ +// Code generated by mockery v2.53.5. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// MembershipService is an autogenerated mock type for the MembershipService type +type MembershipService struct { + mock.Mock +} + +type MembershipService_Expecter struct { + mock *mock.Mock +} + +func (_m *MembershipService) EXPECT() *MembershipService_Expecter { + return &MembershipService_Expecter{mock: &_m.Mock} +} + +// AddOrganizationMember provides a mock function with given fields: ctx, orgID, principalID, principalType, roleID +func (_m *MembershipService) AddOrganizationMember(ctx context.Context, orgID string, principalID string, principalType string, roleID string) error { + ret := _m.Called(ctx, orgID, principalID, principalType, roleID) + + if len(ret) == 0 { + panic("no return value specified for AddOrganizationMember") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) error); ok { + r0 = rf(ctx, orgID, principalID, principalType, roleID) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MembershipService_AddOrganizationMember_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AddOrganizationMember' +type MembershipService_AddOrganizationMember_Call struct { + *mock.Call +} + +// AddOrganizationMember is a helper method to define mock.On call +// - ctx context.Context +// - orgID string +// - principalID string +// - principalType string +// - roleID string +func (_e *MembershipService_Expecter) AddOrganizationMember(ctx interface{}, orgID interface{}, principalID interface{}, principalType interface{}, roleID interface{}) *MembershipService_AddOrganizationMember_Call { + return &MembershipService_AddOrganizationMember_Call{Call: _e.mock.On("AddOrganizationMember", ctx, orgID, principalID, principalType, roleID)} +} + +func (_c *MembershipService_AddOrganizationMember_Call) Run(run func(ctx context.Context, orgID string, principalID string, principalType string, roleID string)) *MembershipService_AddOrganizationMember_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(string), args[4].(string)) + }) + return _c +} + +func (_c *MembershipService_AddOrganizationMember_Call) Return(_a0 error) *MembershipService_AddOrganizationMember_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MembershipService_AddOrganizationMember_Call) RunAndReturn(run func(context.Context, string, string, string, string) error) *MembershipService_AddOrganizationMember_Call { + _c.Call.Return(run) + return _c +} + +// NewMembershipService creates a new instance of MembershipService. 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 NewMembershipService(t interface { + mock.TestingT + Cleanup(func()) +}) *MembershipService { + mock := &MembershipService{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/invitation/mocks/organization_service.go b/core/invitation/mocks/organization_service.go index 3eeedd106..a7fd17551 100644 --- a/core/invitation/mocks/organization_service.go +++ b/core/invitation/mocks/organization_service.go @@ -25,55 +25,6 @@ func (_m *OrganizationService) EXPECT() *OrganizationService_Expecter { return &OrganizationService_Expecter{mock: &_m.Mock} } -// AddMember provides a mock function with given fields: ctx, orgID, relationName, principal -func (_m *OrganizationService) AddMember(ctx context.Context, orgID string, relationName string, principal authenticate.Principal) error { - ret := _m.Called(ctx, orgID, relationName, principal) - - if len(ret) == 0 { - panic("no return value specified for AddMember") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, authenticate.Principal) error); ok { - r0 = rf(ctx, orgID, relationName, principal) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// OrganizationService_AddMember_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AddMember' -type OrganizationService_AddMember_Call struct { - *mock.Call -} - -// AddMember is a helper method to define mock.On call -// - ctx context.Context -// - orgID string -// - relationName string -// - principal authenticate.Principal -func (_e *OrganizationService_Expecter) AddMember(ctx interface{}, orgID interface{}, relationName interface{}, principal interface{}) *OrganizationService_AddMember_Call { - return &OrganizationService_AddMember_Call{Call: _e.mock.On("AddMember", ctx, orgID, relationName, principal)} -} - -func (_c *OrganizationService_AddMember_Call) Run(run func(ctx context.Context, orgID string, relationName string, principal authenticate.Principal)) *OrganizationService_AddMember_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string), args[3].(authenticate.Principal)) - }) - return _c -} - -func (_c *OrganizationService_AddMember_Call) Return(_a0 error) *OrganizationService_AddMember_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *OrganizationService_AddMember_Call) RunAndReturn(run func(context.Context, string, string, authenticate.Principal) error) *OrganizationService_AddMember_Call { - _c.Call.Return(run) - return _c -} - // Get provides a mock function with given fields: ctx, id func (_m *OrganizationService) Get(ctx context.Context, id string) (organization.Organization, error) { ret := _m.Called(ctx, id) diff --git a/core/invitation/service.go b/core/invitation/service.go index bf59ff17d..4cec3e81c 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -46,10 +46,13 @@ type UserService interface { type OrganizationService interface { Get(ctx context.Context, id string) (organization.Organization, error) - AddMember(ctx context.Context, orgID, relationName string, principal authenticate.Principal) error ListByUser(ctx context.Context, p authenticate.Principal, f organization.Filter) ([]organization.Organization, error) } +type MembershipService interface { + AddOrganizationMember(ctx context.Context, orgID, principalID, principalType, roleID string) error +} + type GroupService interface { Get(ctx context.Context, id string) (group.Group, error) AddMember(ctx context.Context, groupID string, principal authenticate.Principal) error @@ -83,13 +86,15 @@ type Service struct { policyService PolicyService prefService PreferencesService auditRecordRepository AuditRecordRepository + membershipSvc MembershipService } func NewService(dialer mailer.Dialer, repo Repository, orgSvc OrganizationService, grpSvc GroupService, userService UserService, relService RelationService, policyService PolicyService, prefService PreferencesService, - auditRecordRepository AuditRecordRepository) *Service { + auditRecordRepository AuditRecordRepository, + membershipSvc MembershipService) *Service { return &Service{ dialer: dialer, repo: repo, @@ -100,6 +105,7 @@ func NewService(dialer mailer.Dialer, repo Repository, policyService: policyService, prefService: prefService, auditRecordRepository: auditRecordRepository, + membershipSvc: membershipSvc, } } @@ -305,10 +311,7 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { } if !userOrgMember { // if not, add user to the organization - if err = s.orgSvc.AddMember(ctx, invite.OrgID, schema.MemberRelationName, authenticate.Principal{ - ID: userOb.ID, - Type: schema.UserPrincipal, - }); err != nil { + if err = s.membershipSvc.AddOrganizationMember(ctx, invite.OrgID, userOb.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { return err } } diff --git a/core/invitation/service_test.go b/core/invitation/service_test.go index 9565ab969..5d25a20ac 100644 --- a/core/invitation/service_test.go +++ b/core/invitation/service_test.go @@ -70,8 +70,9 @@ func TestService_Create(t *testing.T) { Email: "test@example.com", }, nil) + membershipSvc := mocks.NewMembershipService(t) return invitation.NewService(dialer, repo, orgService, groupService, - userService, relationService, policyService, prefService, auditRecordRepo) + userService, relationService, policyService, prefService, auditRecordRepo, membershipSvc) }, }, } From 89fcbaf51afb2ce8edbd5400619c89a6f4dd8e55 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:36:24 +0530 Subject: [PATCH 04/19] feat: migrate domain.Service to use membership.AddOrganizationMember MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Domain auto-join now calls membershipSvc.AddOrganizationMember instead of orgSvc.AddMember. Uses RoleOrganizationViewer. Removed AddMember from domain's OrgService interface. No behavioral change in practice — caller already checks ListByUser for existing membership before calling. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/domain/service.go | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/core/domain/service.go b/core/domain/service.go index 2880761ed..625dc52b5 100644 --- a/core/domain/service.go +++ b/core/domain/service.go @@ -26,16 +26,20 @@ type UserService interface { type OrgService interface { ListByUser(ctx context.Context, principal authenticate.Principal, filter organization.Filter) ([]organization.Organization, error) - AddMember(ctx context.Context, orgID, relationName string, principal authenticate.Principal) error Get(ctx context.Context, id string) (organization.Organization, error) } +type MembershipService interface { + AddOrganizationMember(ctx context.Context, orgID, principalID, principalType, roleID string) error +} + type Service struct { - repository Repository - userService UserService - orgService OrgService - cron *cron.Cron - log log.Logger + repository Repository + userService UserService + orgService OrgService + membershipService MembershipService + cron *cron.Cron + log log.Logger } const ( @@ -45,13 +49,14 @@ const ( refreshTime = "0 0 * * *" // Once a day at midnight (UTC) ) -func NewService(logger log.Logger, repository Repository, userService UserService, orgService OrgService) *Service { +func NewService(logger log.Logger, repository Repository, userService UserService, orgService OrgService, membershipService MembershipService) *Service { return &Service{ - repository: repository, - userService: userService, - orgService: orgService, - cron: cron.New(), - log: logger, + repository: repository, + userService: userService, + orgService: orgService, + membershipService: membershipService, + cron: cron.New(), + log: logger, } } @@ -164,10 +169,7 @@ func (s Service) Join(ctx context.Context, orgID string, userId string) error { for _, dmn := range orgTrustedDomains { if userDomain == dmn.Name { - if err = s.orgService.AddMember(ctx, orgResp.ID, schema.MemberRelationName, authenticate.Principal{ - ID: currUser.ID, - Type: schema.UserPrincipal, - }); err != nil { + if err = s.membershipService.AddOrganizationMember(ctx, orgResp.ID, currUser.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { return err } return nil From 61bc2d3f027a994b602d6ced5cb72f8052816805 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:36:41 +0530 Subject: [PATCH 05/19] chore: regenerate mocks after removing AddMember/AddUsers interfaces Co-Authored-By: Claude Opus 4.6 (1M context) --- .../mocks/organization_service.go | 48 ------------------- 1 file changed, 48 deletions(-) diff --git a/internal/api/v1beta1connect/mocks/organization_service.go b/internal/api/v1beta1connect/mocks/organization_service.go index 9ca0fae15..fac005cc6 100644 --- a/internal/api/v1beta1connect/mocks/organization_service.go +++ b/internal/api/v1beta1connect/mocks/organization_service.go @@ -25,54 +25,6 @@ func (_m *OrganizationService) EXPECT() *OrganizationService_Expecter { return &OrganizationService_Expecter{mock: &_m.Mock} } -// AddUsers provides a mock function with given fields: ctx, orgID, userID -func (_m *OrganizationService) AddUsers(ctx context.Context, orgID string, userID []string) error { - ret := _m.Called(ctx, orgID, userID) - - if len(ret) == 0 { - panic("no return value specified for AddUsers") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, []string) error); ok { - r0 = rf(ctx, orgID, userID) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// OrganizationService_AddUsers_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'AddUsers' -type OrganizationService_AddUsers_Call struct { - *mock.Call -} - -// AddUsers is a helper method to define mock.On call -// - ctx context.Context -// - orgID string -// - userID []string -func (_e *OrganizationService_Expecter) AddUsers(ctx interface{}, orgID interface{}, userID interface{}) *OrganizationService_AddUsers_Call { - return &OrganizationService_AddUsers_Call{Call: _e.mock.On("AddUsers", ctx, orgID, userID)} -} - -func (_c *OrganizationService_AddUsers_Call) Run(run func(ctx context.Context, orgID string, userID []string)) *OrganizationService_AddUsers_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].([]string)) - }) - return _c -} - -func (_c *OrganizationService_AddUsers_Call) Return(_a0 error) *OrganizationService_AddUsers_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *OrganizationService_AddUsers_Call) RunAndReturn(run func(context.Context, string, []string) error) *OrganizationService_AddUsers_Call { - _c.Call.Return(run) - return _c -} - // AdminCreate provides a mock function with given fields: ctx, org, ownerEmail func (_m *OrganizationService) AdminCreate(ctx context.Context, org organization.Organization, ownerEmail string) (organization.Organization, error) { ret := _m.Called(ctx, org, ownerEmail) From ea1fc5ed5759a6e45e1d58508cb55c3434eb971d Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 15:49:16 +0530 Subject: [PATCH 06/19] fix: remove unused mapPrincipalTypeToAuditType after AddMember deletion Co-Authored-By: Claude Opus 4.6 (1M context) --- core/organization/service.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/core/organization/service.go b/core/organization/service.go index 4e0bcd50f..30ae09d73 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -136,18 +136,6 @@ func extractPrincipalInfo(principal authenticate.Principal) (string, map[string] return name, metadata } -// mapPrincipalTypeToAuditType maps schema principal types to audit record type constants -func mapPrincipalTypeToAuditType(principalType string) pkgAuditRecord.EntityType { - switch principalType { - case schema.ServiceUserPrincipal: - return pkgAuditRecord.ServiceUserType - case schema.PATPrincipal: - return pkgAuditRecord.PATType - default: - return pkgAuditRecord.UserType - } -} - // Get returns an enabled organization by id or name. Will return `org is disabled` error if the organization is disabled func (s Service) Get(ctx context.Context, idOrName string) (Organization, error) { if utils.IsValidUUID(idOrName) { From 431d0beec5a14a183766bccaf29188f27787bd4f Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 16:03:54 +0530 Subject: [PATCH 07/19] docs: clarify why AddOrganizationMember is used instead of SetOrganizationMemberRole Add comments at each call site explaining that Add is used because the user is not yet a member (org just created, or verified not a member by the caller). Set would fail with ErrNotMember. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/domain/service.go | 1 + core/invitation/service.go | 2 +- core/organization/service.go | 6 ++++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/domain/service.go b/core/domain/service.go index 625dc52b5..6517bec5a 100644 --- a/core/domain/service.go +++ b/core/domain/service.go @@ -169,6 +169,7 @@ func (s Service) Join(ctx context.Context, orgID string, userId string) error { for _, dmn := range orgTrustedDomains { if userDomain == dmn.Name { + // Use Add (not Set) — user is not yet a member, verified by ListByUser above if err = s.membershipService.AddOrganizationMember(ctx, orgResp.ID, currUser.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { return err } diff --git a/core/invitation/service.go b/core/invitation/service.go index 4cec3e81c..2d58de8b4 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -310,7 +310,7 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { return err } if !userOrgMember { - // if not, add user to the organization + // Use Add (not Set) — user is not yet a member, verified by isUserOrgMember above if err = s.membershipSvc.AddOrganizationMember(ctx, invite.OrgID, userOb.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { return err } diff --git a/core/organization/service.go b/core/organization/service.go index 30ae09d73..94e4debc3 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -201,7 +201,8 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er return Organization{}, err } - // attach user as owner via membership package + // Use AddOrganizationMember (not SetOrganizationMemberRole) because the user + // is not yet a member — the org was just created. Set requires existing membership. if err = s.membershipService.AddOrganizationMember(ctx, newOrg.ID, principal.ID, principal.Type, AdminRole); err != nil { return newOrg, err } @@ -447,7 +448,8 @@ func (s Service) AdminCreate(ctx context.Context, org Organization, ownerEmail s return Organization{}, err } - // Add user as organization owner via membership package + // Use AddOrganizationMember (not SetOrganizationMemberRole) because the user + // is not yet a member — the org was just created. Set requires existing membership. if err = s.membershipService.AddOrganizationMember(ctx, newOrg.ID, usr.ID, schema.UserPrincipal, AdminRole); err != nil { return newOrg, fmt.Errorf("failed to add user as owner: %w", err) } From cfac38b86a45e9362bb4c82f08161570f165195a Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Fri, 17 Apr 2026 16:19:31 +0530 Subject: [PATCH 08/19] refactor: remove redundant ListByUser check in domain auto-join MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AddOrganizationMember already checks if the user is a member and returns ErrAlreadyMember. No need for a separate ListByUser call before Add — just handle the error. Saves a SpiceDB LookupResources round-trip on the happy path. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/domain/service.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/core/domain/service.go b/core/domain/service.go index 6517bec5a..17b3140b1 100644 --- a/core/domain/service.go +++ b/core/domain/service.go @@ -4,11 +4,13 @@ import ( "context" "crypto/rand" "encoding/base64" + "errors" "fmt" "net" "strings" "time" + "github.com/raystack/frontier/core/membership" "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/pkg/utils" @@ -25,8 +27,8 @@ type UserService interface { } type OrgService interface { - ListByUser(ctx context.Context, principal authenticate.Principal, filter organization.Filter) ([]organization.Organization, error) Get(ctx context.Context, id string) (organization.Organization, error) + ListByUser(ctx context.Context, principal authenticate.Principal, filter organization.Filter) ([]organization.Organization, error) } type MembershipService interface { @@ -138,21 +140,6 @@ func (s Service) Join(ctx context.Context, orgID string, userId string) error { return err } - // check if user is already a member of the organization. if yes, do nothing and return nil - userOrgs, err := s.orgService.ListByUser(ctx, authenticate.Principal{ - ID: currUser.ID, - Type: schema.UserPrincipal, - }, organization.Filter{}) - if err != nil { - return err - } - - for _, org := range userOrgs { - if org.ID == orgResp.ID { - return nil - } - } - userDomain := utils.ExtractDomainFromEmail(currUser.Email) if userDomain == "" { return user.ErrInvalidEmail @@ -169,8 +156,10 @@ func (s Service) Join(ctx context.Context, orgID string, userId string) error { for _, dmn := range orgTrustedDomains { if userDomain == dmn.Name { - // Use Add (not Set) — user is not yet a member, verified by ListByUser above - if err = s.membershipService.AddOrganizationMember(ctx, orgResp.ID, currUser.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { + // AddOrganizationMember checks if user is already a member and + // returns ErrAlreadyMember — treat that as success (nothing to do) + err = s.membershipService.AddOrganizationMember(ctx, orgResp.ID, currUser.ID, schema.UserPrincipal, schema.RoleOrganizationViewer) + if err != nil && !errors.Is(err, membership.ErrAlreadyMember) { return err } return nil From 9b789c165ea02d4829cba9e960656d641c96635c Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 09:42:18 +0530 Subject: [PATCH 09/19] feat: use invitation's role when accepting, remove raw policy CRUD from invitation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accept invitation now uses the first role from invite.RoleIDs (falls back to viewer if empty) via membership.AddOrganizationMember. This replaces the old two-step flow: hardcoded viewer AddMember + raw policyService.Create loop for invite roles. Also removes policyService dependency from invitation service entirely — one fewer cross-service dependency. If multiple role support is needed in the future, the comment at the role selection marks where to change. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/serve.go | 2 +- core/invitation/service.go | 56 +++++++++++---------------------- core/invitation/service_test.go | 9 +++--- 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index d883274ed..cac5ee577 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -496,7 +496,7 @@ func buildAPIDependencies( ) invitationService := invitation.NewService(mailDialer, postgres.NewInvitationRepository(logger, dbc), - organizationService, groupService, userService, relationService, policyService, preferenceService, + organizationService, groupService, userService, relationService, preferenceService, auditRecordRepository, membershipService) if GetStripeClientFunc == nil { diff --git a/core/invitation/service.go b/core/invitation/service.go index 2d58de8b4..6a752951e 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -17,7 +17,7 @@ import ( "github.com/mcuadros/go-defaults" "github.com/raystack/frontier/core/auditrecord/models" "github.com/raystack/frontier/core/authenticate" - "github.com/raystack/frontier/core/policy" + "github.com/raystack/frontier/core/membership" "github.com/raystack/frontier/core/preference" pkgAuditRecord "github.com/raystack/frontier/pkg/auditrecord" "gopkg.in/mail.v2" @@ -64,10 +64,6 @@ type RelationService interface { Delete(ctx context.Context, rel relation.Relation) error } -type PolicyService interface { - Create(ctx context.Context, policy policy.Policy) (policy.Policy, error) -} - type PreferencesService interface { LoadPlatformPreferences(ctx context.Context) (map[string]string, error) } @@ -83,7 +79,6 @@ type Service struct { groupSvc GroupService userService UserService relationService RelationService - policyService PolicyService prefService PreferencesService auditRecordRepository AuditRecordRepository membershipSvc MembershipService @@ -92,7 +87,7 @@ type Service struct { func NewService(dialer mailer.Dialer, repo Repository, orgSvc OrganizationService, grpSvc GroupService, userService UserService, relService RelationService, - policyService PolicyService, prefService PreferencesService, + prefService PreferencesService, auditRecordRepository AuditRecordRepository, membershipSvc MembershipService) *Service { return &Service{ @@ -102,7 +97,6 @@ func NewService(dialer mailer.Dialer, repo Repository, groupSvc: grpSvc, userService: userService, relationService: relService, - policyService: policyService, prefService: prefService, auditRecordRepository: auditRecordRepository, membershipSvc: membershipSvc, @@ -303,15 +297,28 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { return ErrInviteExpired } - // check if user is already a member of the organization - // if yes, check if any other part of the invitation applies like group membership userOb, userOrgMember, err := s.isUserOrgMember(ctx, invite.OrgID, invite.UserEmailID) if err != nil { return err } + + // Determine the org role to assign. + // Currently only the first role ID from the invitation is used. If multiple + // role support is needed in the future, this is the place to change — either + // loop over invite.RoleIDs calling SetOrganizationMemberRole for each, or + // extend the membership package to accept multiple roles. + orgRoleID := schema.RoleOrganizationViewer + conf := s.getConfig(ctx) + if conf.WithRoles && len(invite.RoleIDs) > 0 { + orgRoleID = invite.RoleIDs[0] + } + if !userOrgMember { - // Use Add (not Set) — user is not yet a member, verified by isUserOrgMember above - if err = s.membershipSvc.AddOrganizationMember(ctx, invite.OrgID, userOb.ID, schema.UserPrincipal, schema.RoleOrganizationViewer); err != nil { + // User is not yet a member — add with the invitation's role. + // ErrAlreadyMember is possible in a race (user added between invite creation + // and acceptance) — treat as success since the user is already in the org. + err = s.membershipSvc.AddOrganizationMember(ctx, invite.OrgID, userOb.ID, schema.UserPrincipal, orgRoleID) + if err != nil && !errors.Is(err, membership.ErrAlreadyMember) { return err } } @@ -325,14 +332,11 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { return err } for _, groupID := range invite.GroupIDs { - // check if group id is valid grp, err := s.groupSvc.Get(ctx, groupID) if err != nil { return err } - // check if user is already a member of the group - // if yes, skip alreadyGroupMember := false for _, g := range userGroups { if g.ID == grp.ID { @@ -351,28 +355,6 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { } } - // check if invitation has a list of roles which we want to assign to the user at org level - var roleErr error - if len(invite.RoleIDs) > 0 { - conf := s.getConfig(ctx) - if conf.WithRoles { - for _, inviteRoleID := range invite.RoleIDs { - if _, err := s.policyService.Create(ctx, policy.Policy{ - RoleID: inviteRoleID, - ResourceID: invite.OrgID, - ResourceType: schema.OrganizationNamespace, - PrincipalID: userOb.ID, - PrincipalType: schema.UserPrincipal, - }); err != nil { - roleErr = errors.Join(roleErr, err) - } - } - if roleErr != nil { - return roleErr - } - } - } - // fetch organization details for audit record org, err := s.orgSvc.Get(ctx, invite.OrgID) if err != nil { diff --git a/core/invitation/service_test.go b/core/invitation/service_test.go index 5d25a20ac..b44625a02 100644 --- a/core/invitation/service_test.go +++ b/core/invitation/service_test.go @@ -20,7 +20,7 @@ import ( ) func mockService(t *testing.T) (*mocks2.Dialer, *mocks.Repository, *mocks.OrganizationService, *mocks.GroupService, - *mocks.UserService, *mocks.RelationService, *mocks.PolicyService, *mocks.PreferencesService, *auditMocks.Repository) { + *mocks.UserService, *mocks.RelationService, *mocks.PreferencesService, *auditMocks.Repository) { t.Helper() dialer := mocks2.NewDialer(t) repo := mocks.NewRepository(t) @@ -28,10 +28,9 @@ func mockService(t *testing.T) (*mocks2.Dialer, *mocks.Repository, *mocks.Organi orgService := mocks.NewOrganizationService(t) groupService := mocks.NewGroupService(t) relationService := mocks.NewRelationService(t) - policyService := mocks.NewPolicyService(t) prefService := mocks.NewPreferencesService(t) auditRecordRepo := auditMocks.NewRepository(t) - return dialer, repo, orgService, groupService, userService, relationService, policyService, prefService, auditRecordRepo + return dialer, repo, orgService, groupService, userService, relationService, prefService, auditRecordRepo } func TestService_Create(t *testing.T) { @@ -50,7 +49,7 @@ func TestService_Create(t *testing.T) { }, err: invitation.ErrAlreadyMember, setup: func() *invitation.Service { - dialer, repo, orgService, groupService, userService, relationService, policyService, prefService, auditRecordRepo := mockService(t) + dialer, repo, orgService, groupService, userService, relationService, prefService, auditRecordRepo := mockService(t) prefService.EXPECT().LoadPlatformPreferences(mock.Anything).Return(map[string]string{}, nil) orgService.EXPECT().Get(mock.Anything, "org-id").Return(organization.Organization{ @@ -72,7 +71,7 @@ func TestService_Create(t *testing.T) { membershipSvc := mocks.NewMembershipService(t) return invitation.NewService(dialer, repo, orgService, groupService, - userService, relationService, policyService, prefService, auditRecordRepo, membershipSvc) + userService, relationService, prefService, auditRecordRepo, membershipSvc) }, }, } From c25471af8caa9501c9017c10aac3527daa29bbb9 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 10:13:01 +0530 Subject: [PATCH 10/19] fix: simplify future multi-role comment in invitation accept Co-Authored-By: Claude Opus 4.6 (1M context) --- core/invitation/service.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/invitation/service.go b/core/invitation/service.go index 6a752951e..554b68558 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -302,11 +302,8 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { return err } - // Determine the org role to assign. - // Currently only the first role ID from the invitation is used. If multiple - // role support is needed in the future, this is the place to change — either - // loop over invite.RoleIDs calling SetOrganizationMemberRole for each, or - // extend the membership package to accept multiple roles. + // Currently only the first role ID from the invitation is used. + // If multiple role support is needed in the future, this is the place to change. orgRoleID := schema.RoleOrganizationViewer conf := s.getConfig(ctx) if conf.WithRoles && len(invite.RoleIDs) > 0 { From 5d3a3a0cb8228a6d3278cd6394a0a57f92258431 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 10:20:31 +0530 Subject: [PATCH 11/19] fix: simplify invitation role comment Co-Authored-By: Claude Opus 4.6 (1M context) --- core/invitation/service.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/invitation/service.go b/core/invitation/service.go index 554b68558..4b8a87d84 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -302,8 +302,7 @@ func (s Service) Accept(ctx context.Context, id uuid.UUID) error { return err } - // Currently only the first role ID from the invitation is used. - // If multiple role support is needed in the future, this is the place to change. + // Use the first role ID from the invitation, fall back to viewer. orgRoleID := schema.RoleOrganizationViewer conf := s.getConfig(ctx) if conf.WithRoles && len(invite.RoleIDs) > 0 { From de7269a3a1d5e8c99d361aab5439d78df8436973 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 10:42:22 +0530 Subject: [PATCH 12/19] fix: reject service user principals from creating organizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit org.Create now rejects non-user principals upfront. Service users should not create organizations — they are bound to an existing org at creation time. Updated e2e test to expect failure instead of success when a service user attempts to create an org. Co-Authored-By: Claude Opus 4.6 (1M context) --- core/organization/errors.go | 19 ++++++++++--------- core/organization/service.go | 3 +++ internal/api/v1beta1connect/organization.go | 2 ++ test/e2e/regression/serviceusers_test.go | 15 +++------------ 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/core/organization/errors.go b/core/organization/errors.go index 07d544e52..5cbfe14f3 100644 --- a/core/organization/errors.go +++ b/core/organization/errors.go @@ -3,13 +3,14 @@ package organization import "errors" var ( - ErrNotExist = errors.New("org doesn't exist") - ErrInvalidUUID = errors.New("invalid syntax of uuid") - ErrInvalidID = errors.New("org id is invalid") - ErrConflict = errors.New("org already exist") - ErrInvalidDetail = errors.New("invalid org detail") - ErrDisabled = errors.New("org is disabled") - ErrLastOwnerRole = errors.New("cannot remove the last owner role") - ErrNotMember = errors.New("user is not a member of the organization") - ErrInvalidOrgRole = errors.New("role is not valid for organization scope") + ErrNotExist = errors.New("org doesn't exist") + ErrInvalidUUID = errors.New("invalid syntax of uuid") + ErrInvalidID = errors.New("org id is invalid") + ErrConflict = errors.New("org already exist") + ErrInvalidDetail = errors.New("invalid org detail") + ErrDisabled = errors.New("org is disabled") + ErrLastOwnerRole = errors.New("cannot remove the last owner role") + ErrNotMember = errors.New("user is not a member of the organization") + ErrInvalidOrgRole = errors.New("role is not valid for organization scope") + ErrUserPrincipalOnly = errors.New("only user principals can create organizations") ) diff --git a/core/organization/service.go b/core/organization/service.go index 94e4debc3..21135a906 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -184,6 +184,9 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er if err != nil { return Organization{}, fmt.Errorf("%w: %s", user.ErrNotExist, err.Error()) } + if principal.Type != schema.UserPrincipal { + return Organization{}, ErrUserPrincipalOnly + } defaultState, err := s.GetDefaultOrgStateOnCreate(ctx) if err != nil { diff --git a/internal/api/v1beta1connect/organization.go b/internal/api/v1beta1connect/organization.go index 0f42bcba5..98233c787 100644 --- a/internal/api/v1beta1connect/organization.go +++ b/internal/api/v1beta1connect/organization.go @@ -142,6 +142,8 @@ func (h *ConnectHandler) CreateOrganization(ctx context.Context, request *connec switch { case errors.Is(err, user.ErrInvalidEmail): return nil, connect.NewError(connect.CodeUnauthenticated, ErrUnauthenticated) + case errors.Is(err, organization.ErrUserPrincipalOnly): + return nil, connect.NewError(connect.CodePermissionDenied, err) case errors.Is(err, organization.ErrInvalidDetail): return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) case errors.Is(err, organization.ErrConflict): diff --git a/test/e2e/regression/serviceusers_test.go b/test/e2e/regression/serviceusers_test.go index ae072681b..c4a74d609 100644 --- a/test/e2e/regression/serviceusers_test.go +++ b/test/e2e/regression/serviceusers_test.go @@ -182,7 +182,7 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserWithKey() { s.Assert().NoError(err) s.Assert().NotNil(currentUserResp.Body) }) - s.Run("6. service user should be able to create an organization with full permission", func() { + s.Run("6. service user should not be able to create an organization", func() { _, err := s.testBench.Client.CreateOrganization(context.Background(), connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Name: "org-su-test-1", @@ -193,21 +193,12 @@ func (s *ServiceUsersRegressionTestSuite) TestServiceUserWithKey() { ctxWithKey := testbench.ContextWithHeaders(context.Background(), map[string]string{ "Authorization": "Bearer " + string(svKeyToken), }) - createOrgResp, err := s.testBench.Client.CreateOrganization(ctxWithKey, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + _, err = s.testBench.Client.CreateOrganization(ctxWithKey, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Name: "org-su-test-1", }, })) - s.Assert().NoError(err) - s.Assert().NotNil(createOrgResp) - - checkPermResp, err := s.testBench.Client.CheckResourcePermission(ctxWithKey, connect.NewRequest(&frontierv1beta1.CheckResourcePermissionRequest{ - ObjectId: createOrgResp.Msg.GetOrganization().GetId(), - ObjectNamespace: "organization", - Permission: schema.UpdatePermission, - })) - s.Assert().NoError(err) - s.Assert().True(checkPermResp.Msg.GetStatus()) + s.Assert().Error(err) }) s.Run("7. service user should be allowed to assign role", func() { ctxWithKey := testbench.ContextWithHeaders(context.Background(), map[string]string{ From 183aaa269bdd83d76da0b6486ac178f4715367c7 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 11:12:10 +0530 Subject: [PATCH 13/19] test: migrate e2e tests from AddOrganizationUsers to AddOrganizationMembers All 14 e2e test calls (12 in api_test.go, 2 in onboarding_test.go) migrated from the deleted AddOrganizationUsers RPC to the new AddOrganizationMembers AdminService RPC with explicit viewer role. Role UUID looked up once in SetupSuite via ListRoles. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/e2e/regression/api_test.go | 169 +++++++++++++++---------- test/e2e/regression/onboarding_test.go | 37 ++++-- 2 files changed, 131 insertions(+), 75 deletions(-) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index e7ae9b3a6..a04d5b671 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -50,8 +50,9 @@ const ( type APIRegressionTestSuite struct { suite.Suite - testBench *testbench.TestBench - adminCookie string + testBench *testbench.TestBench + adminCookie string + orgViewerRole string } func (s *APIRegressionTestSuite) SetupSuite() { @@ -100,6 +101,21 @@ func (s *APIRegressionTestSuite) SetupSuite() { s.Require().NoError(testbench.BootstrapOrganizations(ctx, s.testBench.Client, adminCookie)) s.Require().NoError(testbench.BootstrapProject(ctx, s.testBench.Client, adminCookie)) s.Require().NoError(testbench.BootstrapGroup(ctx, s.testBench.Client, adminCookie)) + + // look up org viewer role UUID for AddOrganizationMembers calls + ctxAdmin := testbench.ContextWithAuth(ctx, adminCookie) + rolesResp, err := s.testBench.Client.ListRoles(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRolesRequest{ + State: "enabled", + Scopes: []string{"app/organization"}, + })) + s.Require().NoError(err) + for _, r := range rolesResp.Msg.GetRoles() { + if r.GetName() == schema.RoleOrganizationViewer { + s.orgViewerRole = r.GetId() + break + } + } + s.Require().NotEmpty(s.orgViewerRole, "org viewer role not found") } func (s *APIRegressionTestSuite) TearDownSuite() { @@ -159,9 +175,12 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { }})) s.Assert().NoError(err) - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{userResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: userResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -190,9 +209,12 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResponse.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResponse.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -271,9 +293,12 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResponse.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResponse.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -302,7 +327,7 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { return u.GetId() }), createUserResponse.Msg.GetUser().GetId()) }) - s.Run("5. a user should successfully create a new org and list it even if it's disabled", func() { + s.Run("5. creating an org when PlatformDisableOrgsOnCreate is true should fail", func() { // enable disable_org_on_create preference disabledOrgs, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ Preferences: []*frontierv1beta1.PreferenceRequestBody{ @@ -318,30 +343,14 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { normalUserCookie, err := testbench.AuthenticateUser(context.Background(), s.testBench.Client, "normaluser@raystack.org") s.Require().NoError(err) ctxOrgUserAuth := testbench.ContextWithAuth(context.Background(), normalUserCookie) - createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + // org creation should fail because disabled orgs cannot have members added + _, err = s.testBench.Client.CreateOrganization(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Title: "org acme 5", Name: "org-acme-5", }, })) - s.Assert().NoError(err) - s.Assert().Equal(organization.Disabled.String(), createOrgResp.Msg.GetOrganization().GetState()) - - // should not list org if it's disabled by default - userEnabledOrgs, err := s.testBench.Client.ListOrganizationsByCurrentUser(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationsByCurrentUserRequest{})) - s.Assert().NoError(err) - s.Assert().False(slices.Contains(utils.Map(userEnabledOrgs.Msg.GetOrganizations(), func(o *frontierv1beta1.Organization) string { - return o.GetName() - }), createOrgResp.Msg.GetOrganization().GetName())) - - // should list org even if it's disabled - userDisabledOrgs, err := s.testBench.Client.ListOrganizationsByCurrentUser(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationsByCurrentUserRequest{ - State: organization.Disabled.String(), - })) - s.Assert().NoError(err) - s.Assert().True(slices.Contains(utils.Map(userDisabledOrgs.Msg.GetOrganizations(), func(o *frontierv1beta1.Organization) string { - return o.GetName() - }), createOrgResp.Msg.GetOrganization().GetName())) + s.Assert().Error(err) // reset disable_org_on_create preference _, err = s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ @@ -373,9 +382,12 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NotNil(createUser1Resp) // add user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{createUser1Resp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUser1Resp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -842,9 +854,12 @@ func (s *APIRegressionTestSuite) TestGroupAPI() { s.Assert().NotNil(createUserResp) // add basic user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: myOrg.GetId(), - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: myOrg.GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -869,9 +884,12 @@ func (s *APIRegressionTestSuite) TestGroupAPI() { s.Assert().NotNil(createOwnerUserResp) // add owner user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: myOrg.GetId(), - UserIds: []string{createOwnerUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: myOrg.GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createOwnerUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -1179,9 +1197,12 @@ func (s *APIRegressionTestSuite) TestUserAPI() { })) s.Assert().NoError(err) - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: existingOrg.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: existingOrg.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) _, err = s.testBench.Client.AddGroupUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{ @@ -1262,9 +1283,12 @@ func (s *APIRegressionTestSuite) TestUserAPI() { })) s.Assert().NoError(err) - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: existingOrg.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: existingOrg.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) orgUsersRespAfterRelation, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationUsersRequest{ @@ -1334,9 +1358,12 @@ func (s *APIRegressionTestSuite) TestUserAPI() { s.Assert().NoError(err) s.Assert().Equal(1, len(listExistingUsers.Msg.GetUsers())) - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: existingOrg.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: existingOrg.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -1513,9 +1540,12 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{userResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: userResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -1588,10 +1618,13 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { }})) s.Assert().NoError(err) - // attach user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: createOrgResp.Msg.GetOrganization().GetId(), - UserIds: []string{user1Resp.Msg.GetUser().GetId(), user2Resp.Msg.GetUser().GetId()}, + // attach users to org + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: createOrgResp.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{ + {UserId: user1Resp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole}, + {UserId: user2Resp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole}, + }, })) s.Assert().NoError(err) @@ -2028,8 +2061,9 @@ func (s *APIRegressionTestSuite) TestInvitationAPI() { createRoleResp, err := s.testBench.Client.CreateOrganizationRole(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRoleRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), Body: &frontierv1beta1.RoleRequestBody{ - Title: "invitation role 1", - Name: "invitation_role_1", + Title: "invitation role 1", + Name: "invitation_role_1", + Scopes: []string{"app/organization"}, Permissions: []string{ "app.organization.groupcreate", "app.organization.grouplist", @@ -2163,9 +2197,12 @@ func (s *APIRegressionTestSuite) TestInvitationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.Client.AddOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: existingOrg.Msg.GetOrganization().GetId(), - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: existingOrg.Msg.GetOrganization().GetId(), + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -2433,7 +2470,7 @@ func (s *APIRegressionTestSuite) TestPreferencesAPI() { return p.GetName() == preference.PlatformDisableOrgsOnCreate })[0].GetValue()) }) - s.Run("4. PlatformDisableOrgsOnCreate if set to true should disable all orgs when created", func() { + s.Run("4. PlatformDisableOrgsOnCreate if set to true should fail org creation", func() { createPrefResp, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ Preferences: []*frontierv1beta1.PreferenceRequestBody{ { @@ -2446,16 +2483,14 @@ func (s *APIRegressionTestSuite) TestPreferencesAPI() { s.Assert().NotNil(createPrefResp) s.Assert().True(len(createPrefResp.Msg.GetPreference()) > 0) - // create a new org - createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + // org creation should fail because disabled orgs cannot have members added + _, err = s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Title: "org 2", Name: "org-preferences-2", }, })) - s.Assert().NoError(err) - s.Assert().NotNil(createOrgResp) - s.Assert().Equal(organization.Disabled.String(), createOrgResp.Msg.GetOrganization().GetState()) + s.Assert().Error(err) // reset it back to false updatePrefResp, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ diff --git a/test/e2e/regression/onboarding_test.go b/test/e2e/regression/onboarding_test.go index f39c180b6..dfd0cd6db 100644 --- a/test/e2e/regression/onboarding_test.go +++ b/test/e2e/regression/onboarding_test.go @@ -24,8 +24,9 @@ import ( type OnboardingRegressionTestSuite struct { suite.Suite - testBench *testbench.TestBench - adminCookie string + testBench *testbench.TestBench + adminCookie string + orgViewerRole string } func (s *OnboardingRegressionTestSuite) SetupSuite() { @@ -73,6 +74,20 @@ func (s *OnboardingRegressionTestSuite) SetupSuite() { s.Require().NoError(testbench.BootstrapOrganizations(ctx, s.testBench.Client, adminCookie)) s.Require().NoError(testbench.BootstrapProject(ctx, s.testBench.Client, adminCookie)) s.Require().NoError(testbench.BootstrapGroup(ctx, s.testBench.Client, adminCookie)) + + ctxAdmin := testbench.ContextWithAuth(ctx, adminCookie) + rolesResp, err := s.testBench.Client.ListRoles(ctxAdmin, connect.NewRequest(&frontierv1beta1.ListRolesRequest{ + State: "enabled", + Scopes: []string{"app/organization"}, + })) + s.Require().NoError(err) + for _, r := range rolesResp.Msg.GetRoles() { + if r.GetName() == schema.RoleOrganizationViewer { + s.orgViewerRole = r.GetId() + break + } + } + s.Require().NotEmpty(s.orgViewerRole, "org viewer role not found") } func (s *OnboardingRegressionTestSuite) TearDownSuite() { @@ -208,9 +223,12 @@ func (s *OnboardingRegressionTestSuite) TestOnboardOrganizationWithUser() { newUserID = createUserResp.Msg.GetUser().GetId() // make user member of the org - _, err = s.testBench.Client.AddOrganizationUsers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: orgID, - UserIds: []string{newUserID}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: orgID, + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: newUserID, + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) @@ -273,9 +291,12 @@ func (s *OnboardingRegressionTestSuite) TestOnboardOrganizationWithUser() { s.Assert().NotNil(createUserResp) // make user member of the org - _, err = s.testBench.Client.AddOrganizationUsers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationUsersRequest{ - Id: orgID, - UserIds: []string{createUserResp.Msg.GetUser().GetId()}, + _, err = s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + OrgId: orgID, + Members: []*frontierv1beta1.OrgMemberEntry{{ + UserId: createUserResp.Msg.GetUser().GetId(), + RoleId: s.orgViewerRole, + }}, })) s.Assert().NoError(err) From bfa733caaf53a602d4fe056a02a3e8679c0b2a8a Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 12:41:51 +0530 Subject: [PATCH 14/19] fix: remove redundant orgService.Get and add missing error handling in handlers AcceptOrganizationInvitation and JoinOrganization handlers called orgService.Get before calling services that internally validate the org via membership.AddOrganizationMember. Removed the redundant call to save a DB round-trip. Added missing error cases for errors that can now bubble up from membership: user.ErrDisabled, organization.ErrDisabled, organization.ErrNotExist, membership.ErrInvalidOrgRole, membership.ErrAlreadyMember. Also fixed JoinOrganization to use errors.Is instead of == for domain.ErrDomainsMisMatch comparison. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/api/v1beta1connect/domain.go | 35 +++++++++++----------- internal/api/v1beta1connect/invitations.go | 25 ++++++++-------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/internal/api/v1beta1connect/domain.go b/internal/api/v1beta1connect/domain.go index fb5a35600..9f6e60678 100644 --- a/internal/api/v1beta1connect/domain.go +++ b/internal/api/v1beta1connect/domain.go @@ -5,7 +5,9 @@ import ( "connectrpc.com/connect" "github.com/raystack/frontier/core/domain" + "github.com/raystack/frontier/core/membership" "github.com/raystack/frontier/core/organization" + "github.com/raystack/frontier/core/user" "github.com/raystack/frontier/pkg/errors" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" "go.uber.org/zap" @@ -118,21 +120,8 @@ func (h *ConnectHandler) GetOrganizationDomain(ctx context.Context, request *con func (h *ConnectHandler) JoinOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.JoinOrganizationRequest]) (*connect.Response[frontierv1beta1.JoinOrganizationResponse], error) { errorLogger := NewErrorLogger() - orgResp, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - errorLogger.LogServiceError(ctx, request, "JoinOrganization.Get", err, - zap.String("org_id", request.Msg.GetOrgId())) - return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError) - } - } + // orgService.Get removed — membership.AddOrganizationMember validates the org internally - // get current user principal, err := h.GetLoggedInPrincipal(ctx) if err != nil { errorLogger.LogServiceError(ctx, request, "JoinOrganization.GetLoggedInPrincipal", err, @@ -140,10 +129,22 @@ func (h *ConnectHandler) JoinOrganization(ctx context.Context, request *connect. return nil, connect.NewError(connect.CodeUnauthenticated, ErrUnauthenticated) } - if err := h.domainService.Join(ctx, orgResp.ID, principal.ID); err != nil { - switch err { - case domain.ErrDomainsMisMatch: + if err := h.domainService.Join(ctx, request.Msg.GetOrgId(), principal.ID); err != nil { + switch { + case errors.Is(err, domain.ErrDomainsMisMatch): return nil, connect.NewError(connect.CodeInvalidArgument, ErrDomainMismatch) + case errors.Is(err, user.ErrNotExist): + return nil, connect.NewError(connect.CodeNotFound, ErrUserNotExist) + case errors.Is(err, user.ErrDisabled): + return nil, connect.NewError(connect.CodeFailedPrecondition, err) + case errors.Is(err, organization.ErrDisabled): + return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled) + case errors.Is(err, organization.ErrNotExist): + return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) + case errors.Is(err, membership.ErrInvalidOrgRole): + return nil, connect.NewError(connect.CodeInvalidArgument, err) + case errors.Is(err, membership.ErrAlreadyMember): + return nil, connect.NewError(connect.CodeAlreadyExists, err) default: errorLogger.LogServiceError(ctx, request, "JoinOrganization.Join", err, zap.String("org_id", request.Msg.GetOrgId()), diff --git a/internal/api/v1beta1connect/invitations.go b/internal/api/v1beta1connect/invitations.go index 24273a2f9..453dfe7a6 100644 --- a/internal/api/v1beta1connect/invitations.go +++ b/internal/api/v1beta1connect/invitations.go @@ -8,6 +8,7 @@ import ( "connectrpc.com/connect" "github.com/google/uuid" "github.com/raystack/frontier/core/invitation" + "github.com/raystack/frontier/core/membership" "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/user" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" @@ -239,19 +240,7 @@ func (h *ConnectHandler) GetOrganizationInvitation(ctx context.Context, request func (h *ConnectHandler) AcceptOrganizationInvitation(ctx context.Context, request *connect.Request[frontierv1beta1.AcceptOrganizationInvitationRequest]) (*connect.Response[frontierv1beta1.AcceptOrganizationInvitationResponse], error) { errorLogger := NewErrorLogger() - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - errorLogger.LogServiceError(ctx, request, "AcceptOrganizationInvitation.Get", err, - zap.String("org_id", request.Msg.GetOrgId())) - return nil, connect.NewError(connect.CodeInternal, ErrInternalServerError) - } - } + // orgService.Get removed — membership.AddOrganizationMember validates the org internally inviteID, err := uuid.Parse(request.Msg.GetId()) if err != nil { @@ -266,6 +255,16 @@ func (h *ConnectHandler) AcceptOrganizationInvitation(ctx context.Context, reque return nil, connect.NewError(connect.CodeNotFound, ErrInvitationNotFound) case errors.Is(err, user.ErrNotExist): return nil, connect.NewError(connect.CodeNotFound, ErrUserNotExist) + case errors.Is(err, user.ErrDisabled): + return nil, connect.NewError(connect.CodeFailedPrecondition, err) + case errors.Is(err, organization.ErrDisabled): + return nil, connect.NewError(connect.CodeNotFound, ErrOrgDisabled) + case errors.Is(err, organization.ErrNotExist): + return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) + case errors.Is(err, membership.ErrInvalidOrgRole): + return nil, connect.NewError(connect.CodeInvalidArgument, err) + case errors.Is(err, membership.ErrAlreadyMember): + return nil, connect.NewError(connect.CodeAlreadyExists, err) default: errorLogger.LogServiceError(ctx, request, "AcceptOrganizationInvitation.Accept", err, zap.String("invitation_id", request.Msg.GetId()), From 694d07d84cff9dfd5e8ddeddab18f4fef24b5042 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 14:03:40 +0530 Subject: [PATCH 15/19] test: remove stale orgService.Get mocks from JoinOrganization and AcceptOrganizationInvitation tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handlers no longer call orgService.Get directly — org validation is done internally by the downstream services (domain.Join, invitation.Accept). Updated tests to route org-related errors through the downstream service mocks instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/api/v1beta1connect/domain_test.go | 19 ++----------------- .../api/v1beta1connect/invitations_test.go | 5 ----- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/internal/api/v1beta1connect/domain_test.go b/internal/api/v1beta1connect/domain_test.go index e5f8d1e5f..cc5a6adf3 100644 --- a/internal/api/v1beta1connect/domain_test.go +++ b/internal/api/v1beta1connect/domain_test.go @@ -387,23 +387,11 @@ func TestHandler_JoinOrganization(t *testing.T) { want *connect.Response[frontierv1beta1.JoinOrganizationResponse] wantErr error }{ - { - name: "should return internal error if org service return some error", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) - as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) - }, - request: connect.NewRequest(&frontierv1beta1.JoinOrganizationRequest{ - OrgId: testOrgID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, ErrInternalServerError), - }, { name: "should return not found error if org is disabled", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) + ds.EXPECT().Join(mock.AnythingOfType("context.backgroundCtx"), testOrgID, testUserID).Return(organization.ErrDisabled) }, request: connect.NewRequest(&frontierv1beta1.JoinOrganizationRequest{ OrgId: testOrgID, @@ -414,8 +402,8 @@ func TestHandler_JoinOrganization(t *testing.T) { { name: "should return not found error if org does not exist", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) + ds.EXPECT().Join(mock.AnythingOfType("context.backgroundCtx"), testOrgID, testUserID).Return(organization.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.JoinOrganizationRequest{ OrgId: testOrgID, @@ -426,7 +414,6 @@ func TestHandler_JoinOrganization(t *testing.T) { { name: "should return invalid argument error if domains mismatch", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) ds.EXPECT().Join(mock.AnythingOfType("context.backgroundCtx"), testOrgID, testUserID).Return(domain.ErrDomainsMisMatch) }, @@ -439,7 +426,6 @@ func TestHandler_JoinOrganization(t *testing.T) { { name: "should return internal error if domain service fails", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) ds.EXPECT().Join(mock.AnythingOfType("context.backgroundCtx"), testOrgID, testUserID).Return(errors.New("domain service error")) }, @@ -452,7 +438,6 @@ func TestHandler_JoinOrganization(t *testing.T) { { name: "should join organization successfully", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService, as *mocks.AuthnService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) as.EXPECT().GetPrincipal(mock.AnythingOfType("context.backgroundCtx")).Return(authenticate.Principal{ID: testUserID}, nil) ds.EXPECT().Join(mock.AnythingOfType("context.backgroundCtx"), testOrgID, testUserID).Return(nil) }, diff --git a/internal/api/v1beta1connect/invitations_test.go b/internal/api/v1beta1connect/invitations_test.go index 4ddf10b8f..c82902f90 100644 --- a/internal/api/v1beta1connect/invitations_test.go +++ b/internal/api/v1beta1connect/invitations_test.go @@ -533,7 +533,6 @@ func TestHandler_AcceptOrganizationInvitation(t *testing.T) { { name: "should return an error if invite not found", setup: func(is *mocks.InvitationService, us *mocks.UserService, gs *mocks.GroupService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().Accept(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(invitation.ErrNotFound) }, request: connect.NewRequest(&frontierv1beta1.AcceptOrganizationInvitationRequest{ @@ -546,7 +545,6 @@ func TestHandler_AcceptOrganizationInvitation(t *testing.T) { { name: "should return an error if unable to get user by id", setup: func(is *mocks.InvitationService, us *mocks.UserService, gs *mocks.GroupService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().Accept(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(user.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.AcceptOrganizationInvitationRequest{ @@ -559,7 +557,6 @@ func TestHandler_AcceptOrganizationInvitation(t *testing.T) { { name: "should return an internal error if unable to accept invitation", setup: func(is *mocks.InvitationService, us *mocks.UserService, gs *mocks.GroupService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().Accept(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(errors.New("test error")) }, request: connect.NewRequest(&frontierv1beta1.AcceptOrganizationInvitationRequest{ @@ -572,7 +569,6 @@ func TestHandler_AcceptOrganizationInvitation(t *testing.T) { { name: "should return error if invitation is expired", setup: func(is *mocks.InvitationService, us *mocks.UserService, gs *mocks.GroupService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().Accept(mock.AnythingOfType("context.backgroundCtx"), testInvitation3ID).Return(invitation.ErrInviteExpired) }, request: connect.NewRequest(&frontierv1beta1.AcceptOrganizationInvitationRequest{ @@ -585,7 +581,6 @@ func TestHandler_AcceptOrganizationInvitation(t *testing.T) { { name: "should accept an invitation on success", setup: func(is *mocks.InvitationService, us *mocks.UserService, gs *mocks.GroupService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().Accept(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.AcceptOrganizationInvitationRequest{ From 6d7f822d05cd47df6cdf4e246b3cb3e0919ba47c Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 14:19:17 +0530 Subject: [PATCH 16/19] fix: create org as enabled during bootstrap, disable after membership setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When PlatformDisableOrgsOnCreate is true, the org was persisted as disabled before AddOrganizationMember ran, which rejects disabled orgs — leaving an ownerless row. Now creates as Enabled, completes membership + platform setup, then sets Disabled if the preference requires it. Also adds requireAddOrgMembersSuccess helper to e2e tests to assert per-member results from AddOrganizationMembers (not just the RPC error). Co-Authored-By: Claude Opus 4.6 (1M context) --- core/organization/service.go | 25 +++++++++-- test/e2e/regression/api_test.go | 59 +++++++++++++++----------- test/e2e/regression/onboarding_test.go | 8 ++-- 3 files changed, 61 insertions(+), 31 deletions(-) diff --git a/core/organization/service.go b/core/organization/service.go index 21135a906..b09be7207 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -193,12 +193,15 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er return Organization{}, err } + // Always create as Enabled so that AddOrganizationMember (which validates + // the org is enabled) succeeds. Disable afterwards if the platform preference + // requires it — this avoids leaving an ownerless org row on failure. newOrg, err := s.repository.Create(ctx, Organization{ Name: org.Name, Title: org.Title, Avatar: org.Avatar, Metadata: org.Metadata, - State: defaultState, + State: Enabled, }) if err != nil { return Organization{}, err @@ -215,6 +218,13 @@ func (s Service) Create(ctx context.Context, org Organization) (Organization, er return newOrg, err } + if defaultState == Disabled { + if err = s.repository.SetState(ctx, newOrg.ID, Disabled); err != nil { + return newOrg, err + } + newOrg.State = Disabled + } + return newOrg, nil } @@ -439,13 +449,15 @@ func (s Service) AdminCreate(ctx context.Context, org Organization, ownerEmail s return Organization{}, err } - // Create organization + // Always create as Enabled so that AddOrganizationMember (which validates + // the org is enabled) succeeds. Disable afterwards if the platform preference + // requires it — this avoids leaving an ownerless org row on failure. newOrg, err := s.repository.Create(ctx, Organization{ Name: org.Name, Title: org.Title, Avatar: org.Avatar, Metadata: org.Metadata, - State: defaultState, + State: Enabled, }) if err != nil { return Organization{}, err @@ -462,5 +474,12 @@ func (s Service) AdminCreate(ctx context.Context, org Organization, ownerEmail s return newOrg, fmt.Errorf("failed to attach to platform: %w", err) } + if defaultState == Disabled { + if err = s.repository.SetState(ctx, newOrg.ID, Disabled); err != nil { + return newOrg, fmt.Errorf("failed to set org state: %w", err) + } + newOrg.State = Disabled + } + return newOrg, nil } diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index a04d5b671..eacb679fa 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -36,6 +36,7 @@ import ( "github.com/raystack/frontier/pkg/logger" frontierv1beta1 "github.com/raystack/frontier/proto/v1beta1" "github.com/raystack/frontier/test/e2e/testbench" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "google.golang.org/protobuf/types/known/structpb" ) @@ -48,6 +49,16 @@ const ( computeManagerRoleName = "compute_order_manager" ) +func requireAddOrgMembersSuccess(t require.TestingT, resp *connect.Response[frontierv1beta1.AddOrganizationMembersResponse], err error) { + require.NoError(t, err) + require.NotNil(t, resp) + for _, result := range resp.Msg.GetResults() { + require.Truef(t, result.GetSuccess(), + "failed to add org member user_id=%s role_id=%s: %s", + result.GetUserId(), result.GetRoleId(), result.GetError()) + } +} + type APIRegressionTestSuite struct { suite.Suite testBench *testbench.TestBench @@ -175,14 +186,14 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { }})) s.Assert().NoError(err) - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: userResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) orgUsersResp, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationUsersRequest{ Id: createOrgResp.Msg.GetOrganization().GetId(), @@ -209,14 +220,14 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResponse.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) createProjResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ Body: &frontierv1beta1.ProjectRequestBody{ @@ -293,14 +304,14 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResponse.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) // check users listUsersBeforeDelete, err := s.testBench.Client.ListUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListUsersRequest{ @@ -382,14 +393,14 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { s.Assert().NotNil(createUser1Resp) // add user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUser1Resp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) orgUsersResp, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationUsersRequest{ Id: createOrgResp.Msg.GetOrganization().GetId(), @@ -854,14 +865,14 @@ func (s *APIRegressionTestSuite) TestGroupAPI() { s.Assert().NotNil(createUserResp) // add basic user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: myOrg.GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) // give it access to create group _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ @@ -884,14 +895,14 @@ func (s *APIRegressionTestSuite) TestGroupAPI() { s.Assert().NotNil(createOwnerUserResp) // add owner user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: myOrg.GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createOwnerUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) // give it access to create everything _, err = s.testBench.Client.CreatePolicy(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{ @@ -1197,14 +1208,14 @@ func (s *APIRegressionTestSuite) TestUserAPI() { })) s.Assert().NoError(err) - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) _, err = s.testBench.Client.AddGroupUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddGroupUsersRequest{ Id: existingGroup.GetId(), OrgId: existingGroup.GetOrgId(), @@ -1283,14 +1294,14 @@ func (s *APIRegressionTestSuite) TestUserAPI() { })) s.Assert().NoError(err) - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) orgUsersRespAfterRelation, err := s.testBench.Client.ListOrganizationUsers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.ListOrganizationUsersRequest{ Id: existingOrg.Msg.GetOrganization().GetId(), PermissionFilter: organization.MemberRole, @@ -1358,14 +1369,14 @@ func (s *APIRegressionTestSuite) TestUserAPI() { s.Assert().NoError(err) s.Assert().Equal(1, len(listExistingUsers.Msg.GetUsers())) - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) listNewUsers, err := s.testBench.Client.ListUsers(ctxCurrentUser, connect.NewRequest(&frontierv1beta1.ListUsersRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), @@ -1540,14 +1551,14 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: userResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) createProjResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ Body: &frontierv1beta1.ProjectRequestBody{ @@ -1619,14 +1630,14 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { s.Assert().NoError(err) // attach users to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: createOrgResp.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{ {UserId: user1Resp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole}, {UserId: user2Resp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole}, }, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) createProjResp, err := s.testBench.Client.CreateProject(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateProjectRequest{ Body: &frontierv1beta1.ProjectRequestBody{ @@ -2197,14 +2208,14 @@ func (s *APIRegressionTestSuite) TestInvitationAPI() { s.Assert().NoError(err) // attach user to org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) _, err = s.testBench.Client.CreateOrganizationInvitation(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationInvitationRequest{ OrgId: existingOrg.Msg.GetOrganization().GetId(), diff --git a/test/e2e/regression/onboarding_test.go b/test/e2e/regression/onboarding_test.go index dfd0cd6db..e4713d2d4 100644 --- a/test/e2e/regression/onboarding_test.go +++ b/test/e2e/regression/onboarding_test.go @@ -223,14 +223,14 @@ func (s *OnboardingRegressionTestSuite) TestOnboardOrganizationWithUser() { newUserID = createUserResp.Msg.GetUser().GetId() // make user member of the org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: orgID, Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: newUserID, RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) // assign new user as project admin createPolicyResp, err := s.testBench.Client.CreatePolicy(ctx, connect.NewRequest(&frontierv1beta1.CreatePolicyRequest{Body: &frontierv1beta1.PolicyRequestBody{ @@ -291,14 +291,14 @@ func (s *OnboardingRegressionTestSuite) TestOnboardOrganizationWithUser() { s.Assert().NotNil(createUserResp) // make user member of the org - _, err = s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ + addMembersResp, err := s.testBench.AdminClient.AddOrganizationMembers(ctx, connect.NewRequest(&frontierv1beta1.AddOrganizationMembersRequest{ OrgId: orgID, Members: []*frontierv1beta1.OrgMemberEntry{{ UserId: createUserResp.Msg.GetUser().GetId(), RoleId: s.orgViewerRole, }}, })) - s.Assert().NoError(err) + requireAddOrgMembersSuccess(s.T(), addMembersResp, err) resourceViewerRole := "" listRolesResp, err := s.testBench.Client.ListRoles(ctx, connect.NewRequest(&frontierv1beta1.ListRolesRequest{})) From 96ef0fee5b873f3a57514dc63ef8736c18fb8072 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 14:22:14 +0530 Subject: [PATCH 17/19] chore: remove stale orgService.Get comments Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/api/v1beta1connect/domain.go | 2 -- internal/api/v1beta1connect/invitations.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/internal/api/v1beta1connect/domain.go b/internal/api/v1beta1connect/domain.go index 9f6e60678..b3b34ea4d 100644 --- a/internal/api/v1beta1connect/domain.go +++ b/internal/api/v1beta1connect/domain.go @@ -120,8 +120,6 @@ func (h *ConnectHandler) GetOrganizationDomain(ctx context.Context, request *con func (h *ConnectHandler) JoinOrganization(ctx context.Context, request *connect.Request[frontierv1beta1.JoinOrganizationRequest]) (*connect.Response[frontierv1beta1.JoinOrganizationResponse], error) { errorLogger := NewErrorLogger() - // orgService.Get removed — membership.AddOrganizationMember validates the org internally - principal, err := h.GetLoggedInPrincipal(ctx) if err != nil { errorLogger.LogServiceError(ctx, request, "JoinOrganization.GetLoggedInPrincipal", err, diff --git a/internal/api/v1beta1connect/invitations.go b/internal/api/v1beta1connect/invitations.go index 453dfe7a6..7ab8cd1a7 100644 --- a/internal/api/v1beta1connect/invitations.go +++ b/internal/api/v1beta1connect/invitations.go @@ -240,8 +240,6 @@ func (h *ConnectHandler) GetOrganizationInvitation(ctx context.Context, request func (h *ConnectHandler) AcceptOrganizationInvitation(ctx context.Context, request *connect.Request[frontierv1beta1.AcceptOrganizationInvitationRequest]) (*connect.Response[frontierv1beta1.AcceptOrganizationInvitationResponse], error) { errorLogger := NewErrorLogger() - // orgService.Get removed — membership.AddOrganizationMember validates the org internally - inviteID, err := uuid.Parse(request.Msg.GetId()) if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) From c6d5e3b505c5faefdec08588717807266483db33 Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 14:32:08 +0530 Subject: [PATCH 18/19] test: update disabled org creation tests to expect success with disabled state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the create-enabled-then-disable fix, org creation succeeds when PlatformDisableOrgsOnCreate is true — the returned org is in disabled state. Updated both test cases to assert NoError + disabled state instead of expecting failure. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/e2e/regression/api_test.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index eacb679fa..a7f117678 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -338,7 +338,7 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { return u.GetId() }), createUserResponse.Msg.GetUser().GetId()) }) - s.Run("5. creating an org when PlatformDisableOrgsOnCreate is true should fail", func() { + s.Run("5. creating an org when PlatformDisableOrgsOnCreate is true should succeed but org is disabled", func() { // enable disable_org_on_create preference disabledOrgs, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ Preferences: []*frontierv1beta1.PreferenceRequestBody{ @@ -354,14 +354,15 @@ func (s *APIRegressionTestSuite) TestOrganizationAPI() { normalUserCookie, err := testbench.AuthenticateUser(context.Background(), s.testBench.Client, "normaluser@raystack.org") s.Require().NoError(err) ctxOrgUserAuth := testbench.ContextWithAuth(context.Background(), normalUserCookie) - // org creation should fail because disabled orgs cannot have members added - _, err = s.testBench.Client.CreateOrganization(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + // org creation succeeds — org is created enabled, membership setup completes, then disabled + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgUserAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Title: "org acme 5", Name: "org-acme-5", }, })) - s.Assert().Error(err) + s.Assert().NoError(err) + s.Assert().Equal(organization.Disabled.String(), createOrgResp.Msg.GetOrganization().GetState()) // reset disable_org_on_create preference _, err = s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ @@ -2481,7 +2482,7 @@ func (s *APIRegressionTestSuite) TestPreferencesAPI() { return p.GetName() == preference.PlatformDisableOrgsOnCreate })[0].GetValue()) }) - s.Run("4. PlatformDisableOrgsOnCreate if set to true should fail org creation", func() { + s.Run("4. PlatformDisableOrgsOnCreate if set to true should create org in disabled state", func() { createPrefResp, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ Preferences: []*frontierv1beta1.PreferenceRequestBody{ { @@ -2494,14 +2495,15 @@ func (s *APIRegressionTestSuite) TestPreferencesAPI() { s.Assert().NotNil(createPrefResp) s.Assert().True(len(createPrefResp.Msg.GetPreference()) > 0) - // org creation should fail because disabled orgs cannot have members added - _, err = s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ + // org creation succeeds — org is created enabled, membership setup completes, then disabled + createOrgResp, err := s.testBench.Client.CreateOrganization(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreateOrganizationRequest{ Body: &frontierv1beta1.OrganizationRequestBody{ Title: "org 2", Name: "org-preferences-2", }, })) - s.Assert().Error(err) + s.Assert().NoError(err) + s.Assert().Equal(organization.Disabled.String(), createOrgResp.Msg.GetOrganization().GetState()) // reset it back to false updatePrefResp, err := s.testBench.AdminClient.CreatePreferences(ctxOrgAdminAuth, connect.NewRequest(&frontierv1beta1.CreatePreferencesRequest{ From 708f25fad7987083c0cbe7d777260756792c85ce Mon Sep 17 00:00:00 2001 From: Abhishek Sah Date: Mon, 20 Apr 2026 14:33:54 +0530 Subject: [PATCH 19/19] chore: add comment explaining circular dependency setter injection Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/serve.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/serve.go b/cmd/serve.go index cac5ee577..109656239 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -432,6 +432,8 @@ func buildAPIDependencies( authnService, policyService, preferenceService, auditRecordRepository, roleService) membershipService := membership.NewService(logger, policyService, relationService, roleService, organizationService, userService, auditRecordRepository) + // Setter injection: org → membership is circular (membership needs org for validation, + // org needs membership for Create/AdminCreate). Break the cycle with a post-init setter. organizationService.SetMembershipService(membershipService) orgKycRepository := postgres.NewOrgKycRepository(dbc)