diff --git a/internal/api/v1beta1connect/domain.go b/internal/api/v1beta1connect/domain.go index 0e0d5385b..0994769cc 100644 --- a/internal/api/v1beta1connect/domain.go +++ b/internal/api/v1beta1connect/domain.go @@ -46,18 +46,6 @@ func (h *ConnectHandler) CreateOrganizationDomain(ctx context.Context, request * } func (h *ConnectHandler) DeleteOrganizationDomain(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteOrganizationDomainRequest]) (*connect.Response[frontierv1beta1.DeleteOrganizationDomainResponse], error) { - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteOrganizationDomain.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) - } - } - if err := h.domainService.Delete(ctx, request.Msg.GetId()); err != nil { switch err { case domain.ErrNotExist: @@ -72,18 +60,6 @@ func (h *ConnectHandler) DeleteOrganizationDomain(ctx context.Context, request * } func (h *ConnectHandler) GetOrganizationDomain(ctx context.Context, request *connect.Request[frontierv1beta1.GetOrganizationDomainRequest]) (*connect.Response[frontierv1beta1.GetOrganizationDomainResponse], error) { - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("GetOrganizationDomain.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) - } - } - domainResp, err := h.domainService.Get(ctx, request.Msg.GetId()) if err != nil { switch err { @@ -131,18 +107,6 @@ func (h *ConnectHandler) JoinOrganization(ctx context.Context, request *connect. } func (h *ConnectHandler) VerifyOrganizationDomain(ctx context.Context, request *connect.Request[frontierv1beta1.VerifyOrganizationDomainRequest]) (*connect.Response[frontierv1beta1.VerifyOrganizationDomainResponse], error) { - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("VerifyOrganizationDomain.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) - } - } - domainResp, err := h.domainService.VerifyDomain(ctx, request.Msg.GetId()) if err != nil { switch err { @@ -162,15 +126,14 @@ func (h *ConnectHandler) VerifyOrganizationDomain(ctx context.Context, request * } func (h *ConnectHandler) ListOrganizationDomains(ctx context.Context, request *connect.Request[frontierv1beta1.ListOrganizationDomainsRequest]) (*connect.Response[frontierv1beta1.ListOrganizationDomainsResponse], error) { - orgResp, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) + // org state is enforced on the authorization path; resolve state blind + orgResp, err := h.orgService.GetRaw(ctx, request.Msg.GetOrgId()) if err != nil { switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) case errors.Is(err, organization.ErrNotExist): return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationDomains.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) + return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationDomains.GetRaw: org_id=%s: %w", request.Msg.GetOrgId(), err)) } } diff --git a/internal/api/v1beta1connect/domain_test.go b/internal/api/v1beta1connect/domain_test.go index 4289d5872..80b4f3591 100644 --- a/internal/api/v1beta1connect/domain_test.go +++ b/internal/api/v1beta1connect/domain_test.go @@ -177,51 +177,14 @@ func TestHandler_CreateOrganizationDomain(t *testing.T) { func TestHandler_DeleteOrganizationDomain(t *testing.T) { tests := []struct { name string - setup func(os *mocks.OrganizationService, ds *mocks.DomainService) + setup func(ds *mocks.DomainService) request *connect.Request[frontierv1beta1.DeleteOrganizationDomainRequest] want *connect.Response[frontierv1beta1.DeleteOrganizationDomainResponse] wantErr error }{ - { - name: "should return internal error if org service return some error", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) - }, - request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteOrganizationDomain.Get: org_id=%s: %w", testOrgID, errors.New("test error"))), - }, - { - name: "should return not found error if org is disabled", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, - { - name: "should return not found error if org does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrNotFound), - }, { name: "should return not found error if domain does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Delete(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ @@ -233,8 +196,7 @@ func TestHandler_DeleteOrganizationDomain(t *testing.T) { }, { name: "should return internal error if domain service fails", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Delete(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(errors.New("domain service error")) }, request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ @@ -246,8 +208,7 @@ func TestHandler_DeleteOrganizationDomain(t *testing.T) { }, { name: "should delete domain successfully", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Delete(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationDomainRequest{ @@ -261,13 +222,11 @@ func TestHandler_DeleteOrganizationDomain(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockOrgService := new(mocks.OrganizationService) mockDomainService := new(mocks.DomainService) if tt.setup != nil { - tt.setup(mockOrgService, mockDomainService) + tt.setup(mockDomainService) } mockDep := &ConnectHandler{ - orgService: mockOrgService, domainService: mockDomainService, } resp, err := mockDep.DeleteOrganizationDomain(context.Background(), tt.request) @@ -280,51 +239,14 @@ func TestHandler_DeleteOrganizationDomain(t *testing.T) { func TestHandler_GetOrganizationDomain(t *testing.T) { tests := []struct { name string - setup func(os *mocks.OrganizationService, ds *mocks.DomainService) + setup func(ds *mocks.DomainService) request *connect.Request[frontierv1beta1.GetOrganizationDomainRequest] want *connect.Response[frontierv1beta1.GetOrganizationDomainResponse] wantErr error }{ - { - name: "should return internal error if org service return some error", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) - }, - request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, fmt.Errorf("GetOrganizationDomain.Get: org_id=%s: %w", testOrgID, errors.New("test error"))), - }, - { - name: "should return not found error if org is disabled", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, - { - name: "should return not found error if org does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrNotFound), - }, { name: "should return not found error if domain does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, domain.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ @@ -336,8 +258,7 @@ func TestHandler_GetOrganizationDomain(t *testing.T) { }, { name: "should return internal error if domain service fails", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, errors.New("domain service error")) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ @@ -349,8 +270,7 @@ func TestHandler_GetOrganizationDomain(t *testing.T) { }, { name: "should get domain successfully", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(testDomainMap[testDomainID1], nil) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationDomainRequest{ @@ -364,13 +284,11 @@ func TestHandler_GetOrganizationDomain(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockOrgService := new(mocks.OrganizationService) mockDomainService := new(mocks.DomainService) if tt.setup != nil { - tt.setup(mockOrgService, mockDomainService) + tt.setup(mockDomainService) } mockDep := &ConnectHandler{ - orgService: mockOrgService, domainService: mockDomainService, } resp, err := mockDep.GetOrganizationDomain(context.Background(), tt.request) @@ -481,29 +399,18 @@ func TestHandler_ListOrganizationDomains(t *testing.T) { { name: "should return internal error if org service return some error", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) }, request: connect.NewRequest(&frontierv1beta1.ListOrganizationDomainsRequest{ OrgId: testOrgID, }), want: nil, - wantErr: connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationDomains.Get: org_id=%s: %w", testOrgID, errors.New("test error"))), - }, - { - name: "should return not found error if org is disabled", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.ListOrganizationDomainsRequest{ - OrgId: testOrgID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), + wantErr: connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationDomains.GetRaw: org_id=%s: %w", testOrgID, errors.New("test error"))), }, { name: "should return not found error if org does not exist", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.ListOrganizationDomainsRequest{ OrgId: testOrgID, @@ -514,7 +421,7 @@ func TestHandler_ListOrganizationDomains(t *testing.T) { { name: "should return internal error if domain service fails", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) ds.EXPECT().List(mock.AnythingOfType("context.backgroundCtx"), domain.Filter{OrgID: testOrgID, State: domain.Status("")}).Return([]domain.Domain{}, errors.New("domain service error")) }, request: connect.NewRequest(&frontierv1beta1.ListOrganizationDomainsRequest{ @@ -526,7 +433,7 @@ func TestHandler_ListOrganizationDomains(t *testing.T) { { name: "should list domains successfully", setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) domains := []domain.Domain{testDomainMap[testDomainID1], testDomainMap[testDomainID2]} ds.EXPECT().List(mock.AnythingOfType("context.backgroundCtx"), domain.Filter{OrgID: testOrgID, State: domain.Status("")}).Return(domains, nil) }, @@ -561,51 +468,14 @@ func TestHandler_ListOrganizationDomains(t *testing.T) { func TestHandler_VerifyOrganizationDomain(t *testing.T) { tests := []struct { name string - setup func(os *mocks.OrganizationService, ds *mocks.DomainService) + setup func(ds *mocks.DomainService) request *connect.Request[frontierv1beta1.VerifyOrganizationDomainRequest] want *connect.Response[frontierv1beta1.VerifyOrganizationDomainResponse] wantErr error }{ - { - name: "should return internal error if org service return some error", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, errors.New("test error")) - }, - request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeInternal, fmt.Errorf("VerifyOrganizationDomain.Get: org_id=%s: %w", testOrgID, errors.New("test error"))), - }, - { - name: "should return not found error if org is disabled", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, - { - name: "should return not found error if org does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ - OrgId: testOrgID, - Id: testDomainID1, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrNotFound), - }, { name: "should return not found error if domain is invalid", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().VerifyDomain(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, domain.ErrInvalidDomain) }, request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ @@ -617,8 +487,7 @@ func TestHandler_VerifyOrganizationDomain(t *testing.T) { }, { name: "should return not found error if domain does not exist", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().VerifyDomain(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, domain.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ @@ -630,8 +499,7 @@ func TestHandler_VerifyOrganizationDomain(t *testing.T) { }, { name: "should return not found error if TXT record not found", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().VerifyDomain(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, domain.ErrTXTrecordNotFound) }, request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ @@ -643,8 +511,7 @@ func TestHandler_VerifyOrganizationDomain(t *testing.T) { }, { name: "should return internal error if domain service fails", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().VerifyDomain(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(domain.Domain{}, errors.New("domain service error")) }, request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ @@ -656,8 +523,7 @@ func TestHandler_VerifyOrganizationDomain(t *testing.T) { }, { name: "should verify domain successfully", - setup: func(os *mocks.OrganizationService, ds *mocks.DomainService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(ds *mocks.DomainService) { ds.EXPECT().VerifyDomain(mock.AnythingOfType("context.backgroundCtx"), testDomainID1).Return(testDomainMap[testDomainID1], nil) }, request: connect.NewRequest(&frontierv1beta1.VerifyOrganizationDomainRequest{ @@ -673,13 +539,11 @@ func TestHandler_VerifyOrganizationDomain(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockOrgService := new(mocks.OrganizationService) mockDomainService := new(mocks.DomainService) if tt.setup != nil { - tt.setup(mockOrgService, mockDomainService) + tt.setup(mockDomainService) } mockDep := &ConnectHandler{ - orgService: mockOrgService, domainService: mockDomainService, } resp, err := mockDep.VerifyOrganizationDomain(context.Background(), tt.request) diff --git a/internal/api/v1beta1connect/group.go b/internal/api/v1beta1connect/group.go index d832d56de..f6023cbd4 100644 --- a/internal/api/v1beta1connect/group.go +++ b/internal/api/v1beta1connect/group.go @@ -44,15 +44,13 @@ func (h *ConnectHandler) ListGroups(ctx context.Context, request *connect.Reques } func (h *ConnectHandler) ListOrganizationGroups(ctx context.Context, request *connect.Request[frontierv1beta1.ListOrganizationGroupsRequest]) (*connect.Response[frontierv1beta1.ListOrganizationGroupsResponse], error) { - orgResp, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) + orgResp, err := h.orgService.GetRaw(ctx, request.Msg.GetOrgId()) if err != nil { switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) case errors.Is(err, organization.ErrNotExist): return nil, connect.NewError(connect.CodeNotFound, ErrOrgNotFound) default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationGroups.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) + return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationGroups.GetRaw: org_id=%s: %w", request.Msg.GetOrgId(), err)) } } @@ -100,35 +98,18 @@ func (h *ConnectHandler) ListOrganizationGroups(ctx context.Context, request *co return connect.NewResponse(&frontierv1beta1.ListOrganizationGroupsResponse{Groups: groups}), nil } -// ensureOrgEnabled fails with FailedPrecondition when the org is disabled. -func (h *ConnectHandler) ensureOrgEnabled(ctx context.Context, orgID string) error { - if _, err := h.orgService.Get(ctx, orgID); err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return connect.NewError(connect.CodeNotFound, ErrOrgNotFound) - default: - return connect.NewError(connect.CodeInternal, fmt.Errorf("ensureOrgEnabled: org_id=%s: %w", orgID, err)) - } - } - return nil -} - -// getGroupInEnabledOrg loads the group and makes sure its own org is enabled. -func (h *ConnectHandler) getGroupInEnabledOrg(ctx context.Context, groupID string) (group.Group, error) { +// getGroup loads the group, mapping a disabled or missing group to not found. +// The group's org state is enforced centrally on the authorization path. +func (h *ConnectHandler) getGroup(ctx context.Context, groupID string) (group.Group, error) { grp, err := h.groupService.Get(ctx, groupID) if err != nil { switch { case errors.Is(err, group.ErrNotExist), errors.Is(err, group.ErrInvalidID), errors.Is(err, group.ErrInvalidUUID): return group.Group{}, connect.NewError(connect.CodeNotFound, ErrGroupNotFound) default: - return group.Group{}, connect.NewError(connect.CodeInternal, fmt.Errorf("getGroupInEnabledOrg: group_id=%s: %w", groupID, err)) + return group.Group{}, connect.NewError(connect.CodeInternal, fmt.Errorf("getGroup: group_id=%s: %w", groupID, err)) } } - if err := h.ensureOrgEnabled(ctx, grp.OrganizationID); err != nil { - return group.Group{}, err - } return grp, nil } @@ -211,7 +192,7 @@ func (h *ConnectHandler) CreateGroup(ctx context.Context, request *connect.Reque } func (h *ConnectHandler) GetGroup(ctx context.Context, request *connect.Request[frontierv1beta1.GetGroupRequest]) (*connect.Response[frontierv1beta1.GetGroupResponse], error) { - fetchedGroup, err := h.getGroupInEnabledOrg(ctx, request.Msg.GetId()) + fetchedGroup, err := h.getGroup(ctx, request.Msg.GetId()) if err != nil { return nil, err } @@ -257,7 +238,7 @@ func (h *ConnectHandler) UpdateGroup(ctx context.Context, request *connect.Reque return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadBodyMetaSchemaError) } - if _, err := h.getGroupInEnabledOrg(ctx, request.Msg.GetId()); err != nil { + if _, err := h.getGroup(ctx, request.Msg.GetId()); err != nil { return nil, err } @@ -294,7 +275,7 @@ func (h *ConnectHandler) UpdateGroup(ctx context.Context, request *connect.Reque } func (h *ConnectHandler) ListGroupUsers(ctx context.Context, request *connect.Request[frontierv1beta1.ListGroupUsersRequest]) (*connect.Response[frontierv1beta1.ListGroupUsersResponse], error) { - if _, err := h.getGroupInEnabledOrg(ctx, request.Msg.GetId()); err != nil { + if _, err := h.getGroup(ctx, request.Msg.GetId()); err != nil { return nil, err } @@ -344,7 +325,7 @@ func (h *ConnectHandler) ListGroupUsers(ctx context.Context, request *connect.Re } func (h *ConnectHandler) RemoveGroupUser(ctx context.Context, request *connect.Request[frontierv1beta1.RemoveGroupUserRequest]) (*connect.Response[frontierv1beta1.RemoveGroupUserResponse], error) { - if _, err := h.getGroupInEnabledOrg(ctx, request.Msg.GetId()); err != nil { + if _, err := h.getGroup(ctx, request.Msg.GetId()); err != nil { return nil, err } @@ -375,7 +356,7 @@ func (h *ConnectHandler) SetGroupMemberRole(ctx context.Context, request *connec principalType := request.Msg.GetPrincipalType() roleID := request.Msg.GetRoleId() - if _, err := h.getGroupInEnabledOrg(ctx, groupID); err != nil { + if _, err := h.getGroup(ctx, groupID); err != nil { return nil, err } @@ -413,9 +394,6 @@ func (h *ConnectHandler) EnableGroup(ctx context.Context, request *connect.Reque if len(grps) == 0 { return nil, connect.NewError(connect.CodeNotFound, ErrGroupNotFound) } - if err := h.ensureOrgEnabled(ctx, grps[0].OrganizationID); err != nil { - return nil, err - } if err := h.groupService.Enable(ctx, request.Msg.GetId()); err != nil { switch { case errors.Is(err, group.ErrNotExist): @@ -428,7 +406,7 @@ func (h *ConnectHandler) EnableGroup(ctx context.Context, request *connect.Reque } func (h *ConnectHandler) DisableGroup(ctx context.Context, request *connect.Request[frontierv1beta1.DisableGroupRequest]) (*connect.Response[frontierv1beta1.DisableGroupResponse], error) { - if _, err := h.getGroupInEnabledOrg(ctx, request.Msg.GetId()); err != nil { + if _, err := h.getGroup(ctx, request.Msg.GetId()); err != nil { return nil, err } if err := h.groupService.Disable(ctx, request.Msg.GetId()); err != nil { diff --git a/internal/api/v1beta1connect/group_test.go b/internal/api/v1beta1connect/group_test.go index 9b9fb6c1b..41d7e722c 100644 --- a/internal/api/v1beta1connect/group_test.go +++ b/internal/api/v1beta1connect/group_test.go @@ -457,44 +457,16 @@ func TestConnectHandler_GetGroup(t *testing.T) { someGroupID := utils.NewString() tests := []struct { name string - setup func(gs *mocks.GroupService, os *mocks.OrganizationService) + setup func(gs *mocks.GroupService) request *connect.Request[frontierv1beta1.GetGroupRequest] want *connect.Response[frontierv1beta1.GetGroupResponse] wantErr bool wantErrCode connect.Code wantErrMsg error }{ - { - name: "should return error if org does not exist", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ - Id: someGroupID, - }), - want: nil, - wantErr: true, - wantErrCode: connect.CodeNotFound, - wantErrMsg: ErrOrgNotFound, - }, - { - name: "should return error if org is disabled", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ - Id: someGroupID, - }), - want: nil, - wantErr: true, - wantErrCode: connect.CodeFailedPrecondition, - wantErrMsg: ErrOrgDisabled, - }, { name: "should return internal error if group service return some error", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { + setup: func(gs *mocks.GroupService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{}, errors.New("test error")) }, request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ @@ -503,11 +475,11 @@ func TestConnectHandler_GetGroup(t *testing.T) { want: nil, wantErr: true, wantErrCode: connect.CodeInternal, - wantErrMsg: fmt.Errorf("getGroupInEnabledOrg: group_id=%s: %w", someGroupID, errors.New("test error")), + wantErrMsg: fmt.Errorf("getGroup: group_id=%s: %w", someGroupID, errors.New("test error")), }, { name: "should return not found error if id is invalid", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { + setup: func(gs *mocks.GroupService) { gs.EXPECT().Get(mock.Anything, "").Return(group.Group{}, group.ErrInvalidID) }, request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ @@ -520,7 +492,7 @@ func TestConnectHandler_GetGroup(t *testing.T) { }, { name: "should return not found error if group not exist", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { + setup: func(gs *mocks.GroupService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{}, group.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ @@ -533,9 +505,8 @@ func TestConnectHandler_GetGroup(t *testing.T) { }, { name: "should return success if group service return nil", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { + setup: func(gs *mocks.GroupService) { gs.EXPECT().Get(mock.Anything, testGroupID).Return(testGroupMap[testGroupID], nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) }, request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ Id: testGroupID, @@ -558,14 +529,13 @@ func TestConnectHandler_GetGroup(t *testing.T) { }, { name: "should return internal error if group service return key as integer type", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { + setup: func(gs *mocks.GroupService) { gs.EXPECT().Get(mock.Anything, testGroupID).Return(group.Group{ OrganizationID: testOrgID, Metadata: metadata.Metadata{ "key": map[int]any{}, }, }, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) }, request: connect.NewRequest(&frontierv1beta1.GetGroupRequest{ Id: testGroupID, @@ -578,14 +548,12 @@ func TestConnectHandler_GetGroup(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockOrgSvc := new(mocks.OrganizationService) mockGroupSvc := new(mocks.GroupService) if tt.setup != nil { - tt.setup(mockGroupSvc, mockOrgSvc) + tt.setup(mockGroupSvc) } h := &ConnectHandler{ groupService: mockGroupSvc, - orgService: mockOrgSvc, } got, err := h.GetGroup(context.Background(), tt.request) if tt.wantErr { @@ -627,42 +595,6 @@ func TestConnectHandler_UpdateGroup(t *testing.T) { wantErrCode: connect.CodeInvalidArgument, wantErrMsg: ErrBadRequest, }, - { - name: "should return error if org does not exist", - setup: func(gs *mocks.GroupService, ms *mocks.MetaSchemaService, os *mocks.OrganizationService) { - ms.EXPECT().Validate(mock.AnythingOfType("metadata.Metadata"), groupMetaSchema).Return(nil) - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.UpdateGroupRequest{ - Id: someGroupID, - Body: &frontierv1beta1.GroupRequestBody{ - Name: "new-group", - }, - }), - want: nil, - wantErr: true, - wantErrCode: connect.CodeNotFound, - wantErrMsg: ErrOrgNotFound, - }, - { - name: "should return org is disabled", - setup: func(gs *mocks.GroupService, ms *mocks.MetaSchemaService, os *mocks.OrganizationService) { - ms.EXPECT().Validate(mock.AnythingOfType("metadata.Metadata"), groupMetaSchema).Return(nil) - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.UpdateGroupRequest{ - Id: someGroupID, - Body: &frontierv1beta1.GroupRequestBody{ - Name: "new-group", - }, - }), - want: nil, - wantErr: true, - wantErrCode: connect.CodeFailedPrecondition, - wantErrMsg: ErrOrgDisabled, - }, { name: "should return error if error in metadata validation", setup: func(gs *mocks.GroupService, ms *mocks.MetaSchemaService, os *mocks.OrganizationService) { @@ -924,7 +856,7 @@ func TestConnectHandler_ListOrganizationGroups(t *testing.T) { { name: "should return error if org does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, us *mocks.UserService, ms *mocks.MembershipService) { - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) + os.EXPECT().GetRaw(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.ListOrganizationGroupsRequest{ OrgId: testOrgID, @@ -934,23 +866,10 @@ func TestConnectHandler_ListOrganizationGroups(t *testing.T) { wantErrCode: connect.CodeNotFound, wantErrMsg: ErrOrgNotFound, }, - { - name: "should return error if org is disabled", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, us *mocks.UserService, ms *mocks.MembershipService) { - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.ListOrganizationGroupsRequest{ - OrgId: testOrgID, - }), - want: nil, - wantErr: true, - wantErrCode: connect.CodeFailedPrecondition, - wantErrMsg: ErrOrgDisabled, - }, { name: "should return empty groups list if organization with valid uuid is not found", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, us *mocks.UserService, ms *mocks.MembershipService) { - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) gs.EXPECT().List(mock.Anything, group.Filter{ OrganizationID: testOrgID, }).Return([]group.Group{}, nil) @@ -966,7 +885,7 @@ func TestConnectHandler_ListOrganizationGroups(t *testing.T) { { name: "should return success if list organization groups and group service return nil error", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, us *mocks.UserService, ms *mocks.MembershipService) { - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) var testGroupList []group.Group for _, u := range testGroupMap { testGroupList = append(testGroupList, u) @@ -1038,35 +957,10 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) { want *connect.Response[frontierv1beta1.ListGroupUsersResponse] wantErr error }{ - { - name: "should return error if org does not exist", - setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.ListGroupUsersRequest{ - Id: someGroupID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound), - }, - { - name: "should error if org is disabled", - setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.ListGroupUsersRequest{ - Id: someGroupID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, { name: "should return internal server error if error in listing group users", setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().ListPrincipalsByResource(mock.Anything, someGroupID, schema.GroupNamespace, membership.MemberFilter{ PrincipalType: schema.UserPrincipal, }).Return(nil, errors.New("some error")) @@ -1081,7 +975,6 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) { name: "should return error if metadata transformation fails in list of group users", setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) testUserList := []user.User{ { Metadata: metadata.Metadata{ @@ -1105,7 +998,6 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) { name: "should return success if list group users and group service return nil error", setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) var testUserList []user.User for _, u := range testUserMap { testUserList = append(testUserList, u) @@ -1149,7 +1041,6 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) { name: "should return error if membership service fails", setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().ListPrincipalsByResource(mock.Anything, someGroupID, schema.GroupNamespace, membership.MemberFilter{ PrincipalType: schema.UserPrincipal, }).Return(nil, errors.New("policy error")) @@ -1164,7 +1055,6 @@ func TestConnectHandler_ListGroupUsers(t *testing.T) { name: "should return success with roles", setup: func(gs *mocks.GroupService, us *mocks.UserService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) var testUserList []user.User for _, u := range testUserMap { testUserList = append(testUserList, u) @@ -1260,37 +1150,10 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { want *connect.Response[frontierv1beta1.RemoveGroupUserResponse] wantErr error }{ - { - name: "should return error if organization does not exist", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, _ *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ - Id: randomID, - UserId: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound), - }, - { - name: "should return error if organization is disabled", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, _ *mocks.MembershipService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ - Id: randomID, - UserId: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, { name: "should return not found if group does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(group.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1304,7 +1167,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return not found if user does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(user.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1318,7 +1180,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return failed precondition if user is not a group member", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrNotMember) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1332,7 +1193,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return invalid argument if user is the only owner", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrLastGroupOwnerRole) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1346,7 +1206,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return failed precondition if user is disabled", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(user.ErrDisabled) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1360,7 +1219,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return invalid argument if principal type is unsupported", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrInvalidPrincipalType) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1374,7 +1232,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return invalid argument if principal is invalid", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(membership.ErrInvalidPrincipal) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1388,7 +1245,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should return internal error for unknown errors", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(errors.New("unknown")) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1402,7 +1258,6 @@ func TestConnectHandler_RemoveGroupUser(t *testing.T) { name: "should remove user successfully", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService, ms *mocks.MembershipService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: randomID}, nil) - os.EXPECT().Get(mock.Anything, randomID).Return(organization.Organization{ID: randomID}, nil) ms.EXPECT().RemoveGroupMember(mock.Anything, randomID, randomID, schema.UserPrincipal).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.RemoveGroupUserRequest{ @@ -1449,35 +1304,10 @@ func TestConnectHandler_EnableGroup(t *testing.T) { want *connect.Response[frontierv1beta1.EnableGroupResponse] wantErr error }{ - { - name: "should return error if organization does not exist", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().GetByIDs(mock.Anything, []string{randomID}, group.Filter{IncludeDisabled: true}).Return([]group.Group{{ID: randomID, OrganizationID: testOrgID}}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.EnableGroupRequest{ - Id: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound), - }, - { - name: "should return error if organization is disabled", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().GetByIDs(mock.Anything, []string{randomID}, group.Filter{IncludeDisabled: true}).Return([]group.Group{{ID: randomID, OrganizationID: testOrgID}}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.EnableGroupRequest{ - Id: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, { name: "should return error if group does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { gs.EXPECT().GetByIDs(mock.Anything, []string{randomID}, group.Filter{IncludeDisabled: true}).Return([]group.Group{{ID: randomID, OrganizationID: testOrgID}}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{ID: testOrgID}, nil) gs.EXPECT().Enable(mock.Anything, randomID).Return(group.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.EnableGroupRequest{ @@ -1490,7 +1320,6 @@ func TestConnectHandler_EnableGroup(t *testing.T) { name: "should enable group successfully", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { gs.EXPECT().GetByIDs(mock.Anything, []string{randomID}, group.Filter{IncludeDisabled: true}).Return([]group.Group{{ID: randomID, OrganizationID: testOrgID}}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{ID: testOrgID}, nil) gs.EXPECT().Enable(mock.Anything, randomID).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.EnableGroupRequest{ @@ -1534,35 +1363,10 @@ func TestConnectHandler_DisableGroup(t *testing.T) { want *connect.Response[frontierv1beta1.DisableGroupResponse] wantErr error }{ - { - name: "should return error if organization does not exist", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: connect.NewRequest(&frontierv1beta1.DisableGroupRequest{ - Id: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound), - }, - { - name: "should return error if organization is disabled", - setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: connect.NewRequest(&frontierv1beta1.DisableGroupRequest{ - Id: randomID, - }), - want: nil, - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, { name: "should return error if group does not exist", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{ID: testOrgID}, nil) gs.EXPECT().Disable(mock.Anything, randomID).Return(group.ErrNotExist) }, request: connect.NewRequest(&frontierv1beta1.DisableGroupRequest{ @@ -1575,7 +1379,6 @@ func TestConnectHandler_DisableGroup(t *testing.T) { name: "should disable group successfully", setup: func(gs *mocks.GroupService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, randomID).Return(group.Group{ID: randomID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{ID: testOrgID}, nil) gs.EXPECT().Disable(mock.Anything, randomID).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.DisableGroupRequest{ @@ -1686,29 +1489,10 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { want *connect.Response[frontierv1beta1.SetGroupMemberRoleResponse] wantErr error }{ - { - name: "should return not found if org does not exist", - setup: func(gs *mocks.GroupService, _ *mocks.MembershipService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrNotExist) - }, - request: baseRequest(), - wantErr: connect.NewError(connect.CodeNotFound, ErrOrgNotFound), - }, - { - name: "should return not found if org is disabled", - setup: func(gs *mocks.GroupService, _ *mocks.MembershipService, os *mocks.OrganizationService) { - gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(organization.Organization{}, organization.ErrDisabled) - }, - request: baseRequest(), - wantErr: connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled), - }, { name: "should return not found if group does not exist", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(group.ErrNotExist) }, request: baseRequest(), @@ -1718,7 +1502,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return not found if user does not exist", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(user.ErrNotExist) }, request: baseRequest(), @@ -1728,7 +1511,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return not found if role does not exist", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(role.ErrNotExist) }, request: baseRequest(), @@ -1738,7 +1520,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return invalid argument if role is not valid for group scope", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(membership.ErrInvalidGroupRole) }, request: baseRequest(), @@ -1748,7 +1529,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return invalid argument if principal type is unsupported", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) // handler must forward the unsupported principal_type to the service unchanged ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.ServiceUserPrincipal, someRoleID).Return(membership.ErrInvalidPrincipalType) }, @@ -1764,7 +1544,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return failed precondition if principal is not a member of the org", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(membership.ErrNotOrgMember) }, request: baseRequest(), @@ -1774,7 +1553,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return failed precondition if demoting last group owner", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(membership.ErrLastGroupOwnerRole) }, request: baseRequest(), @@ -1784,7 +1562,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return internal error for unknown errors", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(errors.New("unknown")) }, request: baseRequest(), @@ -1794,7 +1571,6 @@ func TestConnectHandler_SetGroupMemberRole(t *testing.T) { name: "should return success on valid request", setup: func(gs *mocks.GroupService, ms *mocks.MembershipService, os *mocks.OrganizationService) { gs.EXPECT().Get(mock.Anything, someGroupID).Return(group.Group{ID: someGroupID, OrganizationID: testOrgID}, nil) - os.EXPECT().Get(mock.Anything, testOrgID).Return(testOrgMap[testOrgID], nil) ms.EXPECT().SetGroupMemberRole(mock.Anything, someGroupID, somePrincipalID, schema.UserPrincipal, someRoleID).Return(nil) }, request: baseRequest(), diff --git a/internal/api/v1beta1connect/invitations.go b/internal/api/v1beta1connect/invitations.go index e262d9734..60c8decf1 100644 --- a/internal/api/v1beta1connect/invitations.go +++ b/internal/api/v1beta1connect/invitations.go @@ -17,15 +17,13 @@ import ( ) func (h *ConnectHandler) ListOrganizationInvitations(ctx context.Context, request *connect.Request[frontierv1beta1.ListOrganizationInvitationsRequest]) (*connect.Response[frontierv1beta1.ListOrganizationInvitationsResponse], error) { - orgResp, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) + orgResp, err := h.orgService.GetRaw(ctx, request.Msg.GetOrgId()) if err != nil { switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) case errors.Is(err, organization.ErrNotExist): return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationInvitations.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) + return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("ListOrganizationInvitations.GetRaw: org_id=%s: %w", request.Msg.GetOrgId(), err)) } } @@ -166,18 +164,6 @@ func (h *ConnectHandler) CreateOrganizationInvitation(ctx context.Context, reque } func (h *ConnectHandler) GetOrganizationInvitation(ctx context.Context, request *connect.Request[frontierv1beta1.GetOrganizationInvitationRequest]) (*connect.Response[frontierv1beta1.GetOrganizationInvitationResponse], error) { - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("GetOrganizationInvitation.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) - } - } - inviteID, err := uuid.Parse(request.Msg.GetId()) if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) @@ -231,18 +217,6 @@ func (h *ConnectHandler) AcceptOrganizationInvitation(ctx context.Context, reque } func (h *ConnectHandler) DeleteOrganizationInvitation(ctx context.Context, request *connect.Request[frontierv1beta1.DeleteOrganizationInvitationRequest]) (*connect.Response[frontierv1beta1.DeleteOrganizationInvitationResponse], error) { - _, err := h.orgService.Get(ctx, request.Msg.GetOrgId()) - if err != nil { - switch { - case errors.Is(err, organization.ErrDisabled): - return nil, connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) - case errors.Is(err, organization.ErrNotExist): - return nil, connect.NewError(connect.CodeNotFound, ErrNotFound) - default: - return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("DeleteOrganizationInvitation.Get: org_id=%s: %w", request.Msg.GetOrgId(), err)) - } - } - inviteID, err := uuid.Parse(request.Msg.GetId()) if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, ErrBadRequest) diff --git a/internal/api/v1beta1connect/invitations_test.go b/internal/api/v1beta1connect/invitations_test.go index da97fbfbd..23bb7474f 100644 --- a/internal/api/v1beta1connect/invitations_test.go +++ b/internal/api/v1beta1connect/invitations_test.go @@ -80,7 +80,7 @@ func TestHandler_ListOrganizationInvitations(t *testing.T) { { name: "should return an error if listing invitation returns an error", setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) is.EXPECT().List(mock.AnythingOfType("context.backgroundCtx"), invitation.Filter{ OrgID: testOrgID, }).Return(nil, errors.New("new-error")) @@ -94,7 +94,7 @@ func TestHandler_ListOrganizationInvitations(t *testing.T) { { name: "should return the list of invitations belonging to an org on success", setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + os.EXPECT().GetRaw(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) var testInvitationList []invitation.Invitation for _, u := range testInvitationMap { if u.OrgID == testOrgID { @@ -445,15 +445,14 @@ func TestHandler_CreateOrganizationInvitation(t *testing.T) { func TestHandler_GetOrganizationInvitation(t *testing.T) { tests := []struct { name string - setup func(is *mocks.InvitationService, os *mocks.OrganizationService) + setup func(is *mocks.InvitationService) request *connect.Request[frontierv1beta1.GetOrganizationInvitationRequest] want *connect.Response[frontierv1beta1.GetOrganizationInvitationResponse] wantErr error }{ { name: "should return an invitation", - setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(is *mocks.InvitationService) { is.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(testInvitationMap[testInvitation1ID.String()], nil) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationInvitationRequest{ @@ -479,8 +478,7 @@ func TestHandler_GetOrganizationInvitation(t *testing.T) { }, { name: "should return an error if the invitation service fails", - setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(is *mocks.InvitationService) { is.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(invitation.Invitation{}, errors.New("test error")) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationInvitationRequest{ @@ -492,8 +490,7 @@ func TestHandler_GetOrganizationInvitation(t *testing.T) { }, { name: "should return an error if the invitation is not found", - setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testOrgID).Return(testOrgMap[testOrgID], nil) + setup: func(is *mocks.InvitationService) { is.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(invitation.Invitation{}, invitation.ErrNotFound) }, request: connect.NewRequest(&frontierv1beta1.GetOrganizationInvitationRequest{ @@ -508,13 +505,11 @@ func TestHandler_GetOrganizationInvitation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { is := &mocks.InvitationService{} - os := &mocks.OrganizationService{} if tt.setup != nil { - tt.setup(is, os) + tt.setup(is) } h := &ConnectHandler{ invitationService: is, - orgService: os, } got, err := h.GetOrganizationInvitation(context.Background(), tt.request) assert.Equal(t, tt.wantErr, err) @@ -620,15 +615,14 @@ func TestHandler_DeleteOrganizationInvitation(t *testing.T) { randomOrgID := uuid.New().String() tests := []struct { name string - setup func(is *mocks.InvitationService, os *mocks.OrganizationService) + setup func(is *mocks.InvitationService) request *connect.Request[frontierv1beta1.DeleteOrganizationInvitationRequest] want *connect.Response[frontierv1beta1.DeleteOrganizationInvitationResponse] wantErr error }{ { name: "should return an internal server error if invitation service fails to delete the invite", - setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), randomOrgID).Return(testOrgMap[randomOrgID], nil) + setup: func(is *mocks.InvitationService) { is.EXPECT().Delete(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(errors.New("test error")) }, request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationInvitationRequest{ @@ -640,8 +634,7 @@ func TestHandler_DeleteOrganizationInvitation(t *testing.T) { }, { name: "should delete an invitation on success", - setup: func(is *mocks.InvitationService, os *mocks.OrganizationService) { - os.EXPECT().Get(mock.AnythingOfType("context.backgroundCtx"), randomOrgID).Return(testOrgMap[randomOrgID], nil) + setup: func(is *mocks.InvitationService) { is.EXPECT().Delete(mock.AnythingOfType("context.backgroundCtx"), testInvitation1ID).Return(nil) }, request: connect.NewRequest(&frontierv1beta1.DeleteOrganizationInvitationRequest{ @@ -656,13 +649,11 @@ func TestHandler_DeleteOrganizationInvitation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { is := &mocks.InvitationService{} - os := &mocks.OrganizationService{} if tt.setup != nil { - tt.setup(is, os) + tt.setup(is) } h := &ConnectHandler{ invitationService: is, - orgService: os, } got, err := h.DeleteOrganizationInvitation(context.Background(), tt.request) assert.Equal(t, tt.wantErr, err) diff --git a/internal/api/v1beta1connect/org_state.go b/internal/api/v1beta1connect/org_state.go index ea10c9e8d..f922339c5 100644 --- a/internal/api/v1beta1connect/org_state.go +++ b/internal/api/v1beta1connect/org_state.go @@ -10,6 +10,7 @@ import ( "github.com/raystack/frontier/core/authenticate" "github.com/raystack/frontier/core/group" "github.com/raystack/frontier/core/invitation" + "github.com/raystack/frontier/core/organization" "github.com/raystack/frontier/core/project" "github.com/raystack/frontier/core/relation" "github.com/raystack/frontier/core/resource" @@ -156,3 +157,19 @@ func (h *ConnectHandler) resolveObjectOrg(ctx context.Context, object relation.O } return proj.Organization.ID, true, nil } + +// ensureOrgEnabled blocks with FailedPrecondition when the org is disabled. +// It backs both the object gate and the PAT org check above. +func (h *ConnectHandler) ensureOrgEnabled(ctx context.Context, orgID string) error { + if _, err := h.orgService.Get(ctx, orgID); err != nil { + switch { + case errors.Is(err, organization.ErrDisabled): + return connect.NewError(connect.CodeFailedPrecondition, ErrOrgDisabled) + case errors.Is(err, organization.ErrNotExist): + return connect.NewError(connect.CodeNotFound, ErrOrgNotFound) + default: + return connect.NewError(connect.CodeInternal, fmt.Errorf("ensureOrgEnabled: org_id=%s: %w", orgID, err)) + } + } + return nil +}