From 827f893ba04d177dda92fb2e47a773939ec579e0 Mon Sep 17 00:00:00 2001 From: Kush Sharma Date: Sun, 6 Oct 2024 18:16:31 +0530 Subject: [PATCH] fix: missing org ids in service user tokens Signed-off-by: Kush Sharma --- core/authenticate/authenticate.go | 5 ++- core/deleter/service.go | 9 ++++- core/domain/service.go | 12 ++++-- core/invitation/mocks/organization_service.go | 30 +++++++-------- core/invitation/service.go | 7 +++- core/organization/service.go | 11 ++++-- internal/api/v1beta1/authenticate.go | 2 +- .../api/v1beta1/mocks/organization_service.go | 37 ++++++++++--------- internal/api/v1beta1/org.go | 4 +- internal/api/v1beta1/user.go | 7 +++- test/e2e/regression/api_test.go | 21 +++++++++++ .../regression/testdata/resource/compute.yml | 6 +++ 12 files changed, 103 insertions(+), 48 deletions(-) diff --git a/core/authenticate/authenticate.go b/core/authenticate/authenticate.go index a100e8ab7..553cb6288 100644 --- a/core/authenticate/authenticate.go +++ b/core/authenticate/authenticate.go @@ -128,7 +128,10 @@ type RegistrationFinishResponse struct { } type Principal struct { - ID string + // ID is the unique identifier of principal + ID string + // Type is the namespace of principal + // E.g. app/user, app/serviceuser Type string User *user.User diff --git a/core/deleter/service.go b/core/deleter/service.go index af4596b50..1ccfec9eb 100644 --- a/core/deleter/service.go +++ b/core/deleter/service.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "github.com/raystack/frontier/core/authenticate" + "github.com/raystack/frontier/billing/invoice" "github.com/raystack/frontier/billing/customer" @@ -39,7 +41,7 @@ type OrganizationService interface { Get(ctx context.Context, id string) (organization.Organization, error) DeleteModel(ctx context.Context, id string) error RemoveUsers(ctx context.Context, orgID string, userIDs []string) error - ListByUser(ctx context.Context, userID string, f organization.Filter) ([]organization.Organization, error) + ListByUser(ctx context.Context, principal authenticate.Principal, f organization.Filter) ([]organization.Organization, error) } type RoleService interface { @@ -320,7 +322,10 @@ func (d Service) RemoveUsersFromOrg(ctx context.Context, orgID string, userIDs [ } func (d Service) DeleteUser(ctx context.Context, userID string) error { - userOrgs, err := d.orgService.ListByUser(ctx, userID, organization.Filter{}) + userOrgs, err := d.orgService.ListByUser(ctx, authenticate.Principal{ + ID: userID, + Type: schema.UserPrincipal, + }, organization.Filter{}) if err != nil { return err } diff --git a/core/domain/service.go b/core/domain/service.go index d6130bce1..2880761ed 100644 --- a/core/domain/service.go +++ b/core/domain/service.go @@ -25,7 +25,7 @@ type UserService interface { } type OrgService interface { - ListByUser(ctx context.Context, userID string, filter organization.Filter) ([]organization.Organization, error) + 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) } @@ -134,7 +134,10 @@ func (s Service) Join(ctx context.Context, orgID string, userId string) error { } // check if user is already a member of the organization. if yes, do nothing and return nil - userOrgs, err := s.orgService.ListByUser(ctx, currUser.ID, organization.Filter{}) + userOrgs, err := s.orgService.ListByUser(ctx, authenticate.Principal{ + ID: currUser.ID, + Type: schema.UserPrincipal, + }, organization.Filter{}) if err != nil { return err } @@ -190,7 +193,10 @@ func (s Service) ListJoinableOrgsByDomain(ctx context.Context, email string) ([] return nil, err } - userOrgs, err := s.orgService.ListByUser(ctx, currUser.ID, organization.Filter{}) + userOrgs, err := s.orgService.ListByUser(ctx, authenticate.Principal{ + ID: currUser.ID, + Type: schema.UserPrincipal, + }, organization.Filter{}) if err != nil { return nil, err } diff --git a/core/invitation/mocks/organization_service.go b/core/invitation/mocks/organization_service.go index 5f8e64fac..090d3157d 100644 --- a/core/invitation/mocks/organization_service.go +++ b/core/invitation/mocks/organization_service.go @@ -131,9 +131,9 @@ func (_c *OrganizationService_Get_Call) RunAndReturn(run func(context.Context, s return _c } -// ListByUser provides a mock function with given fields: ctx, userID, f -func (_m *OrganizationService) ListByUser(ctx context.Context, userID string, f organization.Filter) ([]organization.Organization, error) { - ret := _m.Called(ctx, userID, f) +// ListByUser provides a mock function with given fields: ctx, p, f +func (_m *OrganizationService) ListByUser(ctx context.Context, p authenticate.Principal, f organization.Filter) ([]organization.Organization, error) { + ret := _m.Called(ctx, p, f) if len(ret) == 0 { panic("no return value specified for ListByUser") @@ -141,19 +141,19 @@ func (_m *OrganizationService) ListByUser(ctx context.Context, userID string, f var r0 []organization.Organization var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, organization.Filter) ([]organization.Organization, error)); ok { - return rf(ctx, userID, f) + if rf, ok := ret.Get(0).(func(context.Context, authenticate.Principal, organization.Filter) ([]organization.Organization, error)); ok { + return rf(ctx, p, f) } - if rf, ok := ret.Get(0).(func(context.Context, string, organization.Filter) []organization.Organization); ok { - r0 = rf(ctx, userID, f) + if rf, ok := ret.Get(0).(func(context.Context, authenticate.Principal, organization.Filter) []organization.Organization); ok { + r0 = rf(ctx, p, f) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]organization.Organization) } } - if rf, ok := ret.Get(1).(func(context.Context, string, organization.Filter) error); ok { - r1 = rf(ctx, userID, f) + if rf, ok := ret.Get(1).(func(context.Context, authenticate.Principal, organization.Filter) error); ok { + r1 = rf(ctx, p, f) } else { r1 = ret.Error(1) } @@ -168,15 +168,15 @@ type OrganizationService_ListByUser_Call struct { // ListByUser is a helper method to define mock.On call // - ctx context.Context -// - userID string +// - p authenticate.Principal // - f organization.Filter -func (_e *OrganizationService_Expecter) ListByUser(ctx interface{}, userID interface{}, f interface{}) *OrganizationService_ListByUser_Call { - return &OrganizationService_ListByUser_Call{Call: _e.mock.On("ListByUser", ctx, userID, f)} +func (_e *OrganizationService_Expecter) ListByUser(ctx interface{}, p interface{}, f interface{}) *OrganizationService_ListByUser_Call { + return &OrganizationService_ListByUser_Call{Call: _e.mock.On("ListByUser", ctx, p, f)} } -func (_c *OrganizationService_ListByUser_Call) Run(run func(ctx context.Context, userID string, f organization.Filter)) *OrganizationService_ListByUser_Call { +func (_c *OrganizationService_ListByUser_Call) Run(run func(ctx context.Context, p authenticate.Principal, f organization.Filter)) *OrganizationService_ListByUser_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(organization.Filter)) + run(args[0].(context.Context), args[1].(authenticate.Principal), args[2].(organization.Filter)) }) return _c } @@ -186,7 +186,7 @@ func (_c *OrganizationService_ListByUser_Call) Return(_a0 []organization.Organiz return _c } -func (_c *OrganizationService_ListByUser_Call) RunAndReturn(run func(context.Context, string, organization.Filter) ([]organization.Organization, error)) *OrganizationService_ListByUser_Call { +func (_c *OrganizationService_ListByUser_Call) RunAndReturn(run func(context.Context, authenticate.Principal, organization.Filter) ([]organization.Organization, error)) *OrganizationService_ListByUser_Call { _c.Call.Return(run) return _c } diff --git a/core/invitation/service.go b/core/invitation/service.go index 61ddfbeef..9372e7c20 100644 --- a/core/invitation/service.go +++ b/core/invitation/service.go @@ -45,7 +45,7 @@ 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, userID string, f organization.Filter) ([]organization.Organization, error) + ListByUser(ctx context.Context, p authenticate.Principal, f organization.Filter) ([]organization.Organization, error) } type GroupService interface { @@ -263,7 +263,10 @@ func (s Service) isUserOrgMember(ctx context.Context, orgID, userID string) (use return userOb, false, err } - orgs, err := s.orgSvc.ListByUser(ctx, userOb.ID, organization.Filter{}) + orgs, err := s.orgSvc.ListByUser(ctx, authenticate.Principal{ + ID: userOb.ID, + Type: schema.UserPrincipal, + }, organization.Filter{}) if err != nil { return userOb, false, err } diff --git a/core/organization/service.go b/core/organization/service.go index 20a4daa49..24af40bbd 100644 --- a/core/organization/service.go +++ b/core/organization/service.go @@ -206,7 +206,10 @@ func (s Service) AttachToPlatform(ctx context.Context, orgID string) error { func (s Service) List(ctx context.Context, f Filter) ([]Organization, error) { if f.UserID != "" { - return s.ListByUser(ctx, f.UserID, f) + return s.ListByUser(ctx, authenticate.Principal{ + ID: f.UserID, + Type: schema.UserPrincipal, + }, f) } // state gets filtered in db @@ -220,14 +223,14 @@ func (s Service) Update(ctx context.Context, org Organization) (Organization, er return s.repository.UpdateByName(ctx, org) } -func (s Service) ListByUser(ctx context.Context, userID string, filter Filter) ([]Organization, error) { +func (s Service) ListByUser(ctx context.Context, principal authenticate.Principal, filter Filter) ([]Organization, error) { subjectIDs, err := s.relationService.LookupResources(ctx, relation.Relation{ Object: relation.Object{ Namespace: schema.OrganizationNamespace, }, Subject: relation.Subject{ - ID: userID, - Namespace: schema.UserPrincipal, + ID: principal.ID, + Namespace: principal.Type, }, RelationName: schema.MembershipPermission, }) diff --git a/internal/api/v1beta1/authenticate.go b/internal/api/v1beta1/authenticate.go index ced8dfe75..896d67743 100644 --- a/internal/api/v1beta1/authenticate.go +++ b/internal/api/v1beta1/authenticate.go @@ -277,7 +277,7 @@ func (h Handler) getAccessToken(ctx context.Context, principal authenticate.Prin if h.authConfig.Token.Claims.AddOrgIDsClaim { // get orgs a user belongs to - orgs, err := h.orgService.ListByUser(ctx, principal.ID, organization.Filter{}) + orgs, err := h.orgService.ListByUser(ctx, principal, organization.Filter{}) if err != nil { return nil, err } diff --git a/internal/api/v1beta1/mocks/organization_service.go b/internal/api/v1beta1/mocks/organization_service.go index c7a2d277d..db25a805e 100644 --- a/internal/api/v1beta1/mocks/organization_service.go +++ b/internal/api/v1beta1/mocks/organization_service.go @@ -1,12 +1,15 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.45.0. DO NOT EDIT. package mocks import ( context "context" - organization "github.com/raystack/frontier/core/organization" + authenticate "github.com/raystack/frontier/core/authenticate" + mock "github.com/stretchr/testify/mock" + + organization "github.com/raystack/frontier/core/organization" ) // OrganizationService is an autogenerated mock type for the OrganizationService type @@ -394,9 +397,9 @@ func (_c *OrganizationService_List_Call) RunAndReturn(run func(context.Context, return _c } -// ListByUser provides a mock function with given fields: ctx, userID, flt -func (_m *OrganizationService) ListByUser(ctx context.Context, userID string, flt organization.Filter) ([]organization.Organization, error) { - ret := _m.Called(ctx, userID, flt) +// ListByUser provides a mock function with given fields: ctx, principal, flt +func (_m *OrganizationService) ListByUser(ctx context.Context, principal authenticate.Principal, flt organization.Filter) ([]organization.Organization, error) { + ret := _m.Called(ctx, principal, flt) if len(ret) == 0 { panic("no return value specified for ListByUser") @@ -404,19 +407,19 @@ func (_m *OrganizationService) ListByUser(ctx context.Context, userID string, fl var r0 []organization.Organization var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, organization.Filter) ([]organization.Organization, error)); ok { - return rf(ctx, userID, flt) + if rf, ok := ret.Get(0).(func(context.Context, authenticate.Principal, organization.Filter) ([]organization.Organization, error)); ok { + return rf(ctx, principal, flt) } - if rf, ok := ret.Get(0).(func(context.Context, string, organization.Filter) []organization.Organization); ok { - r0 = rf(ctx, userID, flt) + if rf, ok := ret.Get(0).(func(context.Context, authenticate.Principal, organization.Filter) []organization.Organization); ok { + r0 = rf(ctx, principal, flt) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]organization.Organization) } } - if rf, ok := ret.Get(1).(func(context.Context, string, organization.Filter) error); ok { - r1 = rf(ctx, userID, flt) + if rf, ok := ret.Get(1).(func(context.Context, authenticate.Principal, organization.Filter) error); ok { + r1 = rf(ctx, principal, flt) } else { r1 = ret.Error(1) } @@ -431,15 +434,15 @@ type OrganizationService_ListByUser_Call struct { // ListByUser is a helper method to define mock.On call // - ctx context.Context -// - userID string +// - principal authenticate.Principal // - flt organization.Filter -func (_e *OrganizationService_Expecter) ListByUser(ctx interface{}, userID interface{}, flt interface{}) *OrganizationService_ListByUser_Call { - return &OrganizationService_ListByUser_Call{Call: _e.mock.On("ListByUser", ctx, userID, flt)} +func (_e *OrganizationService_Expecter) ListByUser(ctx interface{}, principal interface{}, flt interface{}) *OrganizationService_ListByUser_Call { + return &OrganizationService_ListByUser_Call{Call: _e.mock.On("ListByUser", ctx, principal, flt)} } -func (_c *OrganizationService_ListByUser_Call) Run(run func(ctx context.Context, userID string, flt organization.Filter)) *OrganizationService_ListByUser_Call { +func (_c *OrganizationService_ListByUser_Call) Run(run func(ctx context.Context, principal authenticate.Principal, flt organization.Filter)) *OrganizationService_ListByUser_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(organization.Filter)) + run(args[0].(context.Context), args[1].(authenticate.Principal), args[2].(organization.Filter)) }) return _c } @@ -449,7 +452,7 @@ func (_c *OrganizationService_ListByUser_Call) Return(_a0 []organization.Organiz return _c } -func (_c *OrganizationService_ListByUser_Call) RunAndReturn(run func(context.Context, string, organization.Filter) ([]organization.Organization, error)) *OrganizationService_ListByUser_Call { +func (_c *OrganizationService_ListByUser_Call) RunAndReturn(run func(context.Context, authenticate.Principal, organization.Filter) ([]organization.Organization, error)) *OrganizationService_ListByUser_Call { _c.Call.Return(run) return _c } diff --git a/internal/api/v1beta1/org.go b/internal/api/v1beta1/org.go index 0a2de5762..819d62835 100644 --- a/internal/api/v1beta1/org.go +++ b/internal/api/v1beta1/org.go @@ -3,6 +3,8 @@ package v1beta1 import ( "context" + "github.com/raystack/frontier/core/authenticate" + "go.uber.org/zap" "github.com/raystack/frontier/core/audit" @@ -41,7 +43,7 @@ type OrganizationService interface { Create(ctx context.Context, org organization.Organization) (organization.Organization, error) List(ctx context.Context, f organization.Filter) ([]organization.Organization, error) Update(ctx context.Context, toUpdate organization.Organization) (organization.Organization, error) - ListByUser(ctx context.Context, userID string, flt organization.Filter) ([]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/v1beta1/user.go b/internal/api/v1beta1/user.go index f60924ee7..31da6f90d 100644 --- a/internal/api/v1beta1/user.go +++ b/internal/api/v1beta1/user.go @@ -450,7 +450,10 @@ func (h Handler) ListCurrentUserGroups(ctx context.Context, request *frontierv1b } func (h Handler) ListOrganizationsByUser(ctx context.Context, request *frontierv1beta1.ListOrganizationsByUserRequest) (*frontierv1beta1.ListOrganizationsByUserResponse, error) { - orgList, err := h.orgService.ListByUser(ctx, request.GetId(), organization.Filter{}) + orgList, err := h.orgService.ListByUser(ctx, authenticate.Principal{ + ID: request.GetId(), + Type: schema.UserPrincipal, + }, organization.Filter{}) if err != nil { return nil, err } @@ -497,7 +500,7 @@ func (h Handler) ListOrganizationsByCurrentUser(ctx context.Context, request *fr if request.GetState() != "" { orgFilter.State = organization.State(request.GetState()) } - orgList, err := h.orgService.ListByUser(ctx, principal.ID, orgFilter) + orgList, err := h.orgService.ListByUser(ctx, principal, orgFilter) if err != nil { return nil, err } diff --git a/test/e2e/regression/api_test.go b/test/e2e/regression/api_test.go index 2d07c1e4d..c74e9de8a 100644 --- a/test/e2e/regression/api_test.go +++ b/test/e2e/regression/api_test.go @@ -42,6 +42,7 @@ import ( const ( fixturesDir = "testdata" computeOrderNamespace = "compute/order" + computeDiskNamespace = "compute/disk" computeViewerRoleName = "compute_order_viewer" ) @@ -1460,12 +1461,32 @@ func (s *APIRegressionTestSuite) TestResourceAPI() { }) s.Assert().NoError(err) s.Assert().NotNil(createResourceResp) + createResourceResp2, err := s.testBench.Client.CreateProjectResource(ctxOrgAdminAuth, &frontierv1beta1.CreateProjectResourceRequest{ + ProjectId: createProjResp.GetProject().GetId(), + Body: &frontierv1beta1.ResourceRequestBody{ + Name: "res-2", + Namespace: computeDiskNamespace, + Principal: userResp.GetUser().GetId(), + Metadata: &structpb.Struct{}, + }, + }) + s.Assert().NoError(err) + s.Assert().NotNil(createResourceResp2) listResourcesResp, err := s.testBench.Client.ListProjectResources(ctxOrgAdminAuth, &frontierv1beta1.ListProjectResourcesRequest{ ProjectId: createProjResp.GetProject().GetId(), }) s.Assert().NoError(err) s.Assert().Equal("res-1", listResourcesResp.GetResources()[0].GetName()) + + // filter user by namespace + listAllResourcesResp, err := s.testBench.AdminClient.ListResources(ctxOrgAdminAuth, &frontierv1beta1.ListResourcesRequest{ + ProjectId: createProjResp.GetProject().GetId(), + Namespace: computeDiskNamespace, + }) + s.Assert().NoError(err) + s.Assert().Len(listAllResourcesResp.GetResources(), 1) + s.Assert().Equal("res-2", listAllResourcesResp.GetResources()[0].GetName()) }) } diff --git a/test/e2e/regression/testdata/resource/compute.yml b/test/e2e/regression/testdata/resource/compute.yml index 617f78079..8ade548e0 100644 --- a/test/e2e/regression/testdata/resource/compute.yml +++ b/test/e2e/regression/testdata/resource/compute.yml @@ -8,6 +8,12 @@ permissions: - name: create namespace: compute/order - key: compute.order.configure + - name: get + namespace: compute/disk + - name: create + namespace: compute/disk + - name: delete + namespace: compute/disk roles: - name: compute_order_manager permissions: