diff --git a/internal/api/grpc/admin/import.go b/internal/api/grpc/admin/import.go index 67802777e92..8d03b8eec95 100644 --- a/internal/api/grpc/admin/import.go +++ b/internal/api/grpc/admin/import.go @@ -866,7 +866,7 @@ func importOrgDomains(ctx context.Context, s *Server, errors *[]*admin_pb.Import Verified: domainR.IsVerified, Primary: domainR.IsPrimary, } - _, err := s.command.AddOrgDomain(ctx, org.GetOrgId(), domainR.DomainName, []string{}) + _, err := s.command.AddOrgDomain(ctx, org.GetOrgId(), domainR.DomainName, []string{}, nil) if err != nil { *errors = append(*errors, &admin_pb.ImportDataError{Type: "domain", Id: org.GetOrgId() + "_" + domainR.DomainName, Message: errorToImportError(err)}) if isCtxTimeout(ctx) { diff --git a/internal/api/grpc/admin/org.go b/internal/api/grpc/admin/org.go index ca5731c4327..2b2137a6ff3 100644 --- a/internal/api/grpc/admin/org.go +++ b/internal/api/grpc/admin/org.go @@ -28,7 +28,7 @@ func (s *Server) SetDefaultOrg(ctx context.Context, req *admin_pb.SetDefaultOrgR } func (s *Server) RemoveOrg(ctx context.Context, req *admin_pb.RemoveOrgRequest) (*admin_pb.RemoveOrgResponse, error) { - details, err := s.command.RemoveOrg(ctx, req.OrgId) + details, err := s.command.RemoveOrg(ctx, req.OrgId, nil, true) if err != nil { return nil, err } @@ -87,7 +87,7 @@ func (s *Server) SetUpOrg(ctx context.Context, req *admin_pb.SetUpOrgRequest) (* Roles: req.Roles, }, }, - }, true, userIDs...) + }, true, nil, userIDs...) if err != nil { return nil, err } diff --git a/internal/api/grpc/management/org.go b/internal/api/grpc/management/org.go index 092dcdd6e49..f2a811b40a1 100644 --- a/internal/api/grpc/management/org.go +++ b/internal/api/grpc/management/org.go @@ -98,7 +98,7 @@ func (s *Server) AddOrg(ctx context.Context, req *mgmt_pb.AddOrgRequest) (*mgmt_ func (s *Server) UpdateOrg(ctx context.Context, req *mgmt_pb.UpdateOrgRequest) (*mgmt_pb.UpdateOrgResponse, error) { ctxData := authz.GetCtxData(ctx) - org, err := s.command.ChangeOrg(ctx, ctxData.OrgID, req.Name) + org, err := s.command.ChangeOrg(ctx, ctxData.OrgID, req.Name, nil) if err != nil { return nil, err } @@ -112,7 +112,7 @@ func (s *Server) UpdateOrg(ctx context.Context, req *mgmt_pb.UpdateOrgRequest) ( } func (s *Server) DeactivateOrg(ctx context.Context, req *mgmt_pb.DeactivateOrgRequest) (*mgmt_pb.DeactivateOrgResponse, error) { - objectDetails, err := s.command.DeactivateOrg(ctx, authz.GetCtxData(ctx).OrgID) + objectDetails, err := s.command.DeactivateOrg(ctx, authz.GetCtxData(ctx).OrgID, nil) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func (s *Server) DeactivateOrg(ctx context.Context, req *mgmt_pb.DeactivateOrgRe } func (s *Server) ReactivateOrg(ctx context.Context, req *mgmt_pb.ReactivateOrgRequest) (*mgmt_pb.ReactivateOrgResponse, error) { - objectDetails, err := s.command.ReactivateOrg(ctx, authz.GetCtxData(ctx).OrgID) + objectDetails, err := s.command.ReactivateOrg(ctx, authz.GetCtxData(ctx).OrgID, nil) if err != nil { return nil, err } @@ -132,7 +132,7 @@ func (s *Server) ReactivateOrg(ctx context.Context, req *mgmt_pb.ReactivateOrgRe } func (s *Server) RemoveOrg(ctx context.Context, req *mgmt_pb.RemoveOrgRequest) (*mgmt_pb.RemoveOrgResponse, error) { - details, err := s.command.RemoveOrg(ctx, authz.GetCtxData(ctx).OrgID) + details, err := s.command.RemoveOrg(ctx, authz.GetCtxData(ctx).OrgID, nil, true) if err != nil { return nil, err } @@ -187,7 +187,7 @@ func (s *Server) AddOrgDomain(ctx context.Context, req *mgmt_pb.AddOrgDomainRequ if err != nil { return nil, err } - details, err := s.command.AddOrgDomain(ctx, orgID, req.Domain, userIDs) + details, err := s.command.AddOrgDomain(ctx, orgID, req.Domain, userIDs, nil) if err != nil { return nil, err } @@ -197,7 +197,7 @@ func (s *Server) AddOrgDomain(ctx context.Context, req *mgmt_pb.AddOrgDomainRequ } func (s *Server) RemoveOrgDomain(ctx context.Context, req *mgmt_pb.RemoveOrgDomainRequest) (*mgmt_pb.RemoveOrgDomainResponse, error) { - details, err := s.command.RemoveOrgDomain(ctx, RemoveOrgDomainRequestToDomain(ctx, req)) + details, err := s.command.RemoveOrgDomain(ctx, RemoveOrgDomainRequestToDomain(ctx, req), nil) if err != nil { return nil, err } @@ -207,7 +207,7 @@ func (s *Server) RemoveOrgDomain(ctx context.Context, req *mgmt_pb.RemoveOrgDoma } func (s *Server) GenerateOrgDomainValidation(ctx context.Context, req *mgmt_pb.GenerateOrgDomainValidationRequest) (*mgmt_pb.GenerateOrgDomainValidationResponse, error) { - token, url, err := s.command.GenerateOrgDomainValidation(ctx, GenerateOrgDomainValidationRequestToDomain(ctx, req)) + token, url, err := s.command.GenerateOrgDomainValidation(ctx, GenerateOrgDomainValidationRequestToDomain(ctx, req), nil) if err != nil { return nil, err } @@ -232,7 +232,7 @@ func (s *Server) ValidateOrgDomain(ctx context.Context, req *mgmt_pb.ValidateOrg if err != nil { return nil, err } - details, err := s.command.ValidateOrgDomain(ctx, ValidateOrgDomainRequestToDomain(ctx, req), userIDs) + details, err := s.command.ValidateOrgDomain(ctx, ValidateOrgDomainRequestToDomain(ctx, req), userIDs, nil) if err != nil { return nil, err } @@ -359,7 +359,7 @@ func (s *Server) SetOrgMetadata(ctx context.Context, req *mgmt_pb.SetOrgMetadata } func (s *Server) BulkSetOrgMetadata(ctx context.Context, req *mgmt_pb.BulkSetOrgMetadataRequest) (*mgmt_pb.BulkSetOrgMetadataResponse, error) { - result, err := s.command.BulkSetOrgMetadata(ctx, authz.GetCtxData(ctx).OrgID, BulkSetOrgMetadataToDomain(req)...) + result, err := s.command.BulkSetOrgMetadata(ctx, authz.GetCtxData(ctx).OrgID, nil, BulkSetOrgMetadataToDomain(req)...) if err != nil { return nil, err } @@ -379,7 +379,7 @@ func (s *Server) RemoveOrgMetadata(ctx context.Context, req *mgmt_pb.RemoveOrgMe } func (s *Server) BulkRemoveOrgMetadata(ctx context.Context, req *mgmt_pb.BulkRemoveOrgMetadataRequest) (*mgmt_pb.BulkRemoveOrgMetadataResponse, error) { - result, err := s.command.BulkRemoveOrgMetadata(ctx, authz.GetCtxData(ctx).OrgID, req.Keys...) + result, err := s.command.BulkRemoveOrgMetadata(ctx, authz.GetCtxData(ctx).OrgID, nil, req.Keys...) if err != nil { return nil, err } diff --git a/internal/api/grpc/org/v2/org.go b/internal/api/grpc/org/v2/org.go index 42832d147f5..423ff92e4a2 100644 --- a/internal/api/grpc/org/v2/org.go +++ b/internal/api/grpc/org/v2/org.go @@ -17,7 +17,7 @@ func (s *Server) AddOrganization(ctx context.Context, request *connect.Request[o if err != nil { return nil, err } - createdOrg, err := s.command.SetUpOrg(ctx, orgSetup, false) + createdOrg, err := s.command.SetUpOrg(ctx, orgSetup, false, s.command.CheckPermissionOrganizationCreate) if err != nil { return nil, err } diff --git a/internal/api/grpc/org/v2beta/integration_test/org_test.go b/internal/api/grpc/org/v2beta/integration_test/org_test.go index 5952e7bfcfc..d8b12f2b38c 100644 --- a/internal/api/grpc/org/v2beta/integration_test/org_test.go +++ b/internal/api/grpc/org/v2beta/integration_test/org_test.go @@ -17,17 +17,21 @@ import ( "github.com/zitadel/zitadel/internal/integration" "github.com/zitadel/zitadel/pkg/grpc/admin" + "github.com/zitadel/zitadel/pkg/grpc/filter/v2beta" + metadata "github.com/zitadel/zitadel/pkg/grpc/metadata/v2beta" v2beta_object "github.com/zitadel/zitadel/pkg/grpc/object/v2beta" + "github.com/zitadel/zitadel/pkg/grpc/org/v2" v2beta_org "github.com/zitadel/zitadel/pkg/grpc/org/v2beta" "github.com/zitadel/zitadel/pkg/grpc/user/v2" user_v2beta "github.com/zitadel/zitadel/pkg/grpc/user/v2beta" ) var ( - CTX context.Context - Instance *integration.Instance - Client v2beta_org.OrganizationServiceClient - User *user.AddHumanUserResponse + CTX context.Context + Instance *integration.Instance + Client v2beta_org.OrganizationServiceClient + User *user.AddHumanUserResponse + OtherOrganization *org.AddOrganizationResponse ) func TestMain(m *testing.M) { @@ -40,6 +44,7 @@ func TestMain(m *testing.M) { CTX = Instance.WithAuthorizationToken(ctx, integration.UserTypeIAMOwner) User = Instance.CreateHumanUser(CTX) + OtherOrganization = Instance.CreateOrganization(CTX, integration.OrganizationName(), integration.Email()) return m.Run() }()) } @@ -339,6 +344,15 @@ func TestServer_UpdateOrganization(t *testing.T) { }, wantErr: true, }, + { + name: "no permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + req: &v2beta_org.UpdateOrganizationRequest{ + Id: OtherOrganization.GetOrganizationId(), + Name: integration.OrganizationName(), + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -376,33 +390,42 @@ func TestServer_ListOrganizations(t *testing.T) { name string ctx context.Context query []*v2beta_org.OrganizationSearchFilter - want []*v2beta_org.Organization + want *v2beta_org.ListOrganizationsResponse err error }{ { name: "list organizations, without required permissions", ctx: ListOrgIinstance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission), - err: errors.New("membership not found"), + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 4, + }, + }, }, { name: "list organizations happy path, no filter", ctx: listOrgIAmOwnerCtx, - want: []*v2beta_org.Organization{ - { - // default org - Name: "testinstance", + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 4, }, - { - Id: orgs[0].Id, - Name: orgsName[0], - }, - { - Id: orgs[1].Id, - Name: orgsName[1], - }, - { - Id: orgs[2].Id, - Name: orgsName[2], + Organizations: []*v2beta_org.Organization{ + { + Id: ListOrgIinstance.DefaultOrg.Id, + Name: ListOrgIinstance.DefaultOrg.Name, + }, + { + Id: orgs[0].Id, + Name: orgsName[0], + }, + { + Id: orgs[1].Id, + Name: orgsName[1], + }, + { + Id: orgs[2].Id, + Name: orgsName[2], + }, }, }, }, @@ -418,10 +441,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -437,18 +465,23 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - // default org - Name: "testinstance", + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 3, }, - { - Id: orgs[0].Id, - Name: orgsName[0], - }, - { - Id: orgs[2].Id, - Name: orgsName[2], + Organizations: []*v2beta_org.Organization{ + { + Id: ListOrgIinstance.DefaultOrg.Id, + Name: ListOrgIinstance.DefaultOrg.Name, + }, + { + Id: orgs[0].Id, + Name: orgsName[0], + }, + { + Id: orgs[2].Id, + Name: orgsName[2], + }, }, }, }, @@ -464,10 +497,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -483,6 +521,12 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 0, + }, + Organizations: nil, + }, }, { name: "list organizations specify org name equals", @@ -497,10 +541,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -519,10 +568,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -541,10 +595,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -561,10 +620,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -581,10 +645,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -601,10 +670,15 @@ func TestServer_ListOrganizations(t *testing.T) { }, }, }, - want: []*v2beta_org.Organization{ - { - Id: orgs[1].Id, - Name: orgsName[1], + want: &v2beta_org.ListOrganizationsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Organizations: []*v2beta_org.Organization{ + { + Id: orgs[1].Id, + Name: orgsName[1], + }, }, }, }, @@ -615,6 +689,10 @@ func TestServer_ListOrganizations(t *testing.T) { require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := listOrgClient.ListOrganizations(tt.ctx, &v2beta_org.ListOrganizationsRequest{ Filter: tt.query, + Pagination: &filter.PaginationRequest{ + Asc: true, + }, + SortingColumn: v2beta_org.OrgFieldName_ORG_FIELD_NAME_CREATION_DATE, }) if tt.err != nil { require.ErrorContains(ttt, err, tt.err.Error()) @@ -622,32 +700,20 @@ func TestServer_ListOrganizations(t *testing.T) { } require.NoError(ttt, err) - require.Equal(ttt, uint64(len(tt.want)), got.Pagination.GetTotalResult()) + require.Equal(ttt, tt.want.GetPagination(), got.GetPagination()) - foundOrgs := 0 - for _, got := range got.Organizations { - for _, org := range tt.want { + require.Len(ttt, got.Organizations, len(tt.want.Organizations)) + for i, got := range got.Organizations { + // created/chagned date + gotCD := got.GetCreationDate().AsTime() + now := time.Now() + assert.WithinRange(ttt, gotCD, testStartTimestamp, now.Add(time.Minute)) + gotCD = got.GetChangedDate().AsTime() + assert.WithinRange(ttt, gotCD, testStartTimestamp, now.Add(time.Minute)) - // created/chagned date - gotCD := got.GetCreationDate().AsTime() - now := time.Now() - assert.WithinRange(ttt, gotCD, testStartTimestamp, now.Add(time.Minute)) - gotCD = got.GetChangedDate().AsTime() - assert.WithinRange(ttt, gotCD, testStartTimestamp, now.Add(time.Minute)) - - // default org - if org.Name == got.Name && got.Name == "testinstance" { - foundOrgs += 1 - continue - } - - if org.Name == got.Name && - org.Id == got.Id { - foundOrgs += 1 - } - } + assert.Equal(ttt, tt.want.Organizations[i].Id, got.Id) + assert.Equal(ttt, tt.want.Organizations[i].Name, got.Name) } - require.Equal(ttt, len(tt.want), foundOrgs) }, retryDuration, tick, "timeout waiting for expected organizations being created") }) } @@ -665,7 +731,7 @@ func TestServer_DeleteOrganization(t *testing.T) { }{ { name: "delete org no permission", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission), + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), createOrgFunc: func() string { orgs, _, _ := createOrgs(CTX, t, Client, 1) return orgs[0].Id @@ -793,13 +859,12 @@ func TestServer_ActivateOrganization(t *testing.T) { }, { name: "Activate, no permission", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission), + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), testFunc: func() string { orgs, _, _ := createOrgs(CTX, t, Client, 1) orgId := orgs[0].Id return orgId }, - // BUG: this needs changing err: errors.New("membership not found"), }, { @@ -860,13 +925,12 @@ func TestServer_DeactivateOrganization(t *testing.T) { }, { name: "Deactivate, no permission", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeNoPermission), + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), testFunc: func() string { orgs, _, _ := createOrgs(CTX, t, Client, 1) orgId := orgs[0].Id return orgId }, - // BUG: this needs changing err: errors.New("membership not found"), }, { @@ -944,6 +1008,7 @@ func TestServer_AddOrganizationDomain(t *testing.T) { }{ { name: "add org domain, happy path", + ctx: CTX, domain: integration.DomainName(), testFunc: func() string { orgs, _, _ := createOrgs(CTX, t, Client, 1) @@ -952,9 +1017,22 @@ func TestServer_AddOrganizationDomain(t *testing.T) { }, }, { - name: "add org domain, twice", + name: "no permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), domain: integration.DomainName(), testFunc: func() string { + orgs, _, _ := createOrgs(CTX, t, Client, 1) + orgId := orgs[0].Id + return orgId + }, + err: errors.New("membership not found"), + }, + { + name: "add org domain, twice", + ctx: CTX, + domain: integration.DomainName(), + testFunc: func() string { + t.Helper() // 1. create organization orgs, _, _ := createOrgs(CTX, t, Client, 1) orgId := orgs[0].Id @@ -992,33 +1070,32 @@ func TestServer_AddOrganizationDomain(t *testing.T) { }, { name: "add org domain to non existent org", + ctx: CTX, domain: integration.DomainName(), testFunc: func() string { return "non-existing-org-id" }, - // BUG: should return a error - err: nil, + err: errors.New("Organisation not found"), }, } for _, tt := range tests { - var orgId string t.Run(tt.name, func(t *testing.T) { - orgId = tt.testFunc() + orgId := tt.testFunc() + addOrgDomainRes, err := Client.AddOrganizationDomain(tt.ctx, &v2beta_org.AddOrganizationDomainRequest{ + OrganizationId: orgId, + Domain: tt.domain, + }) + if tt.err != nil { + require.Contains(t, err.Error(), tt.err.Error()) + } else { + require.NoError(t, err) + // check details + gotCD := addOrgDomainRes.GetCreationDate().AsTime() + now := time.Now() + assert.WithinRange(t, gotCD, now.Add(-time.Minute), now.Add(time.Minute)) + } }) - addOrgDomainRes, err := Client.AddOrganizationDomain(CTX, &v2beta_org.AddOrganizationDomainRequest{ - OrganizationId: orgId, - Domain: tt.domain, - }) - if tt.err != nil { - require.Contains(t, err.Error(), tt.err.Error()) - } else { - require.NoError(t, err) - // check details - gotCD := addOrgDomainRes.GetCreationDate().AsTime() - now := time.Now() - assert.WithinRange(t, gotCD, now.Add(-time.Minute), now.Add(time.Minute)) - } } } @@ -1090,60 +1167,155 @@ func TestServer_AddOrganizationDomain_ClaimDomain(t *testing.T) { func TestServer_ListOrganizationDomains(t *testing.T) { domain := integration.DomainName() + + orgs, _, _ := createOrgs(CTX, t, Client, 1) + orgId := orgs[0].Id + + var primaryDomain string + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Second) + require.EventuallyWithT(t, func(t *assert.CollectT) { + organizations, err := Client.ListOrganizations(CTX, &v2beta_org.ListOrganizationsRequest{ + Filter: []*v2beta_org.OrganizationSearchFilter{ + {Filter: &v2beta_org.OrganizationSearchFilter_IdFilter{ + IdFilter: &v2beta_org.OrgIDFilter{Id: orgId}, + }}, + }, + }) + require.NoError(t, err) + require.Len(t, organizations.GetOrganizations(), 1) + primaryDomain = organizations.GetOrganizations()[0].GetPrimaryDomain() + }, retryDuration, tick, "could not find primary domain") + + _, err := Client.AddOrganizationDomain(CTX, &v2beta_org.AddOrganizationDomainRequest{ + OrganizationId: orgId, + Domain: domain, + }) + require.NoError(t, err) + + type args struct { + ctx context.Context + request *v2beta_org.ListOrganizationDomainsRequest + } + type want struct { + response *v2beta_org.ListOrganizationDomainsResponse + err bool + } + tests := []struct { - name string - ctx context.Context - domain string - testFunc func() string - err error + name string + args args + want want }{ { - name: "list org domain, happy path", - domain: domain, - testFunc: func() string { - // 1. create organization - orgs, _, _ := createOrgs(CTX, t, Client, 1) - orgId := orgs[0].Id - // 2. add domain - addOrgDomainRes, err := Client.AddOrganizationDomain(CTX, &v2beta_org.AddOrganizationDomainRequest{ + name: "non existing organization", + args: args{ + ctx: CTX, + request: &v2beta_org.ListOrganizationDomainsRequest{OrganizationId: "not-existing"}, + }, + want: want{ + response: &v2beta_org.ListOrganizationDomainsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 0, + }, + Domains: nil, + }, + }, + }, + { + name: "no permission (different organization), error", + args: args{ + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + request: &v2beta_org.ListOrganizationDomainsRequest{ OrganizationId: orgId, - Domain: domain, - }) - require.NoError(t, err) - // check details - gotCD := addOrgDomainRes.GetCreationDate().AsTime() - now := time.Now() - assert.WithinRange(t, gotCD, now.Add(-time.Minute), now.Add(time.Minute)) - - return orgId + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationDomainsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 0, + }, + Domains: nil, + }, + }, + }, + { + name: "list org domain, all domains", + args: args{ + ctx: CTX, + request: &v2beta_org.ListOrganizationDomainsRequest{ + OrganizationId: orgId, + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationDomainsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 2, + }, + Domains: []*v2beta_org.Domain{ + { + OrganizationId: orgId, + DomainName: domain, + IsVerified: true, + IsPrimary: false, + ValidationType: 0, + }, + { + OrganizationId: orgId, + DomainName: primaryDomain, + IsVerified: true, + IsPrimary: true, + ValidationType: 0, + }, + }, + }, + }, + }, + { + name: "list specific domain", + args: args{ + ctx: CTX, + request: &v2beta_org.ListOrganizationDomainsRequest{ + OrganizationId: orgId, + Filters: []*v2beta_org.DomainSearchFilter{ + {Filter: &v2beta_org.DomainSearchFilter_DomainNameFilter{DomainNameFilter: &v2beta_org.DomainNameFilter{Name: domain}}}, + }, + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationDomainsResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + }, + Domains: []*v2beta_org.Domain{ + { + OrganizationId: orgId, + DomainName: domain, + IsVerified: true, + IsPrimary: false, + ValidationType: 0, + }, + }, + }, }, }, } for _, tt := range tests { - var orgId string t.Run(tt.name, func(t *testing.T) { - orgId = tt.testFunc() - }) - - var err error - var queryRes *v2beta_org.ListOrganizationDomainsResponse - - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) - require.EventuallyWithT(t, func(ttt *assert.CollectT) { - queryRes, err = Client.ListOrganizationDomains(CTX, &v2beta_org.ListOrganizationDomainsRequest{ - OrganizationId: orgId, - }) - require.NoError(ttt, err) - found := false - for _, res := range queryRes.Domains { - if res.DomainName == tt.domain { - found = true + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + queryRes, err := Client.ListOrganizationDomains(tt.args.ctx, tt.args.request) + if tt.want.err { + require.Error(ttt, err) + return } - } - require.True(ttt, found, "unable to find added domain") - }, retryDuration, tick, "timeout waiting for adding domain") + require.NoError(ttt, err) + assert.Len(ttt, queryRes.Domains, int(tt.want.response.GetPagination().GetTotalResult())) + assert.EqualExportedValues(ttt, tt.want.response.GetPagination(), queryRes.GetPagination()) + assert.ElementsMatch(ttt, tt.want.response.GetDomains(), queryRes.GetDomains()) + }, retryDuration, tick, "timeout waiting for adding domain") + }) } } @@ -1158,6 +1330,7 @@ func TestServer_DeleteOrganizationDomain(t *testing.T) { }{ { name: "delete org domain, happy path", + ctx: CTX, domain: domain, testFunc: func() string { // 1. create organization @@ -1192,6 +1365,7 @@ func TestServer_DeleteOrganizationDomain(t *testing.T) { }, { name: "delete org domain, twice", + ctx: CTX, domain: integration.DomainName(), testFunc: func() string { // 1. create organization @@ -1238,6 +1412,7 @@ func TestServer_DeleteOrganizationDomain(t *testing.T) { }, { name: "delete org domain to non existent org", + ctx: CTX, domain: integration.DomainName(), testFunc: func() string { return "non-existing-org-id" @@ -1245,13 +1420,49 @@ func TestServer_DeleteOrganizationDomain(t *testing.T) { // BUG: err: errors.New("Domain doesn't exist on organization"), }, + { + name: "delete org domain no permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + domain: domain, + testFunc: func() string { + // 1. create organization + orgs, _, _ := createOrgs(CTX, t, Client, 1) + orgId := orgs[0].Id + + // 2. add domain + addOrgDomainRes, err := Client.AddOrganizationDomain(CTX, &v2beta_org.AddOrganizationDomainRequest{ + OrganizationId: orgId, + Domain: domain, + }) + require.NoError(t, err) + // check details + gotCD := addOrgDomainRes.GetCreationDate().AsTime() + now := time.Now() + assert.WithinRange(t, gotCD, now.Add(-time.Minute), now.Add(time.Minute)) + + // check domain added + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + queryRes, err := Client.ListOrganizationDomains(CTX, &v2beta_org.ListOrganizationDomainsRequest{ + OrganizationId: orgId, + }) + require.NoError(ttt, err) + + found := slices.ContainsFunc(queryRes.Domains, func(d *v2beta_org.Domain) bool { return d.GetDomainName() == domain }) + require.True(ttt, found, "unable to find added domain") + }, retryDuration, tick, "timeout waiting for expected organizations being created") + + return orgId + }, + err: errors.New("membership not found"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { orgId := tt.testFunc() - _, err := Client.DeleteOrganizationDomain(CTX, &v2beta_org.DeleteOrganizationDomainRequest{ + _, err := Client.DeleteOrganizationDomain(tt.ctx, &v2beta_org.DeleteOrganizationDomainRequest{ OrganizationId: orgId, Domain: tt.domain, }) @@ -1466,7 +1677,7 @@ func TestServer_ValidateOrganizationDomain(t *testing.T) { err: errors.New("Domain doesn't exist on organization"), }, { - name: "validate org non existnetn domain", + name: "validate org non existent domain", ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), req: &v2beta_org.GenerateOrganizationDomainValidationRequest{ OrganizationId: orgId, @@ -1475,6 +1686,16 @@ func TestServer_ValidateOrganizationDomain(t *testing.T) { }, err: errors.New("Domain doesn't exist on organization"), }, + { + name: "validate without permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + req: &v2beta_org.GenerateOrganizationDomainValidationRequest{ + OrganizationId: orgId, + Domain: domain, + Type: v2beta_org.DomainValidationType_DOMAIN_VALIDATION_TYPE_HTTP, + }, + err: errors.New("membership not found"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1504,6 +1725,14 @@ func TestServer_SetOrganizationMetadata(t *testing.T) { value string err error }{ + { + name: "no permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + orgId: orgId, + key: "key1", + value: "value1", + err: errors.New("membership not found"), + }, { name: "set org metadata", ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), @@ -1611,109 +1840,168 @@ func TestServer_SetOrganizationMetadata(t *testing.T) { func TestServer_ListOrganizationMetadata(t *testing.T) { orgs, _, _ := createOrgs(CTX, t, Client, 1) orgId := orgs[0].Id + setRespoonse, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ + OrganizationId: orgId, + Metadata: []*v2beta_org.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + { + Key: "key2.1", + Value: []byte("value3"), + }, + { + Key: "key2.2", + Value: []byte("value4"), + }, + }, + }) + require.NoError(t, err) + + type args struct { + ctx context.Context + request *v2beta_org.ListOrganizationMetadataRequest + } + type want struct { + response *v2beta_org.ListOrganizationMetadataResponse + err error + } tests := []struct { - name string - ctx context.Context - setupFunc func() - orgId string - keyValuePairs []struct { - key string - value string - } + name string + args args + want want }{ { name: "list org metadata happy path", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ + args: args{ + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), + request: &v2beta_org.ListOrganizationMetadataRequest{ OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 4, + }, + Metadata: []*metadata.Metadata{ { - Key: "key1", - Value: []byte("value1"), + Key: "key1", + Value: []byte("value1"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), + }, + { + Key: "key2", + Value: []byte("value2"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), + }, + { + Key: "key2.1", + Value: []byte("value3"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), + }, + { + Key: "key2.2", + Value: []byte("value4"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), }, }, - }) - require.NoError(t, err) - }, - orgId: orgId, - keyValuePairs: []struct{ key, value string }{ - { - key: "key1", - value: "value1", }, }, }, { - name: "list multiple org metadata happy path", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ + name: "list org metadata filter key", + args: args{ + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), + request: &v2beta_org.ListOrganizationMetadataRequest{ OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ + Pagination: &filter.PaginationRequest{ + Offset: 1, + Limit: 2, + }, + Filter: []*metadata.MetadataQuery{ { - Key: "key2", - Value: []byte("value2"), - }, - { - Key: "key3", - Value: []byte("value3"), - }, - { - Key: "key4", - Value: []byte("value4"), + Query: &metadata.MetadataQuery_KeyQuery{ + KeyQuery: &metadata.MetadataKeyQuery{ + Key: "key2", + Method: v2beta_object.TextQueryMethod_TEXT_QUERY_METHOD_STARTS_WITH, + }, + }, }, }, - }) - require.NoError(t, err) + }, }, - orgId: orgId, - keyValuePairs: []struct{ key, value string }{ - { - key: "key2", - value: "value2", - }, - { - key: "key3", - value: "value3", - }, - { - key: "key4", - value: "value4", + want: want{ + response: &v2beta_org.ListOrganizationMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 3, + AppliedLimit: 2, + }, + Metadata: []*metadata.Metadata{ + { + Key: "key2.1", + Value: []byte("value3"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), + }, + { + Key: "key2.2", + Value: []byte("value4"), + CreationDate: setRespoonse.GetSetDate(), + ChangeDate: setRespoonse.GetSetDate(), + }, + }, }, }, }, { - name: "list org metadata for non existent org", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - orgId: "non existent orgid", - keyValuePairs: []struct{ key, value string }{}, + name: "list org metadata for non existent org", + args: args{ + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), + request: &v2beta_org.ListOrganizationMetadataRequest{ + OrganizationId: "non existent orgid", + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationMetadataResponse{ + Pagination: &filter.PaginationResponse{}, + }, + }, + }, + { + name: "list org metadata without permission (other organization)", + args: args{ + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), + request: &v2beta_org.ListOrganizationMetadataRequest{ + OrganizationId: orgId, + }, + }, + want: want{ + response: &v2beta_org.ListOrganizationMetadataResponse{ + Pagination: &filter.PaginationResponse{}, + }, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.setupFunc != nil { - tt.setupFunc() - } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 1*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, err := Client.ListOrganizationMetadata(tt.ctx, &v2beta_org.ListOrganizationMetadataRequest{ - OrganizationId: tt.orgId, - }) + got, err := Client.ListOrganizationMetadata(tt.args.ctx, tt.args.request) require.NoError(ttt, err) - foundMetadataCount := 0 - for _, kv := range tt.keyValuePairs { - for _, res := range got.Metadata { - if res.Key == kv.key && - string(res.Value) == kv.value { - foundMetadataCount += 1 - } - } - } - require.Len(ttt, tt.keyValuePairs, foundMetadataCount) + assert.EqualExportedValues(ttt, tt.want.response, got) }, retryDuration, tick, "timeout waiting for expected organizations being created") }) } @@ -1723,6 +2011,39 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { orgs, _, _ := createOrgs(CTX, t, Client, 1) orgId := orgs[0].Id + _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ + OrganizationId: orgId, + Metadata: []*v2beta_org.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + { + Key: "key3", + Value: []byte("value3"), + }, { + + Key: "key4", + Value: []byte("value4"), + }, + }, + }) + require.NoError(t, err) + + // check metadata exists + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 1*time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + listOrgMetadataRes, err := Client.ListOrganizationMetadata(CTX, &v2beta_org.ListOrganizationMetadataRequest{ + OrganizationId: orgId, + }) + require.NoError(ttt, err) + require.Len(ttt, listOrgMetadataRes.GetMetadata(), 4) + }, retryDuration, tick, "timeout waiting for expected organizations being created") + tests := []struct { name string ctx context.Context @@ -1732,27 +2053,11 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { key string value string } - metadataToRemain []struct { - key string - value string - } err error }{ { - name: "delete org metadata happy path", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ - OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ - { - Key: "key1", - Value: []byte("value1"), - }, - }, - }) - require.NoError(t, err) - }, + name: "delete org metadata happy path", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), orgId: orgId, metadataToDelete: []struct{ key, value string }{ { @@ -1762,24 +2067,8 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { }, }, { - name: "delete multiple org metadata happy path", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ - OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ - { - Key: "key2", - Value: []byte("value2"), - }, - { - Key: "key3", - Value: []byte("value3"), - }, - }, - }) - require.NoError(t, err) - }, + name: "delete multiple org metadata happy path", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), orgId: orgId, metadataToDelete: []struct{ key, value string }{ { @@ -1793,118 +2082,44 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { }, }, { - name: "delete some org metadata but not all", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ - OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ - { - Key: "key4", - Value: []byte("value4"), - }, - // key5 should not be deleted - { - Key: "key5", - Value: []byte("value5"), - }, - { - Key: "key6", - Value: []byte("value6"), - }, - }, - }) - require.NoError(t, err) + name: "delete org metadata that does not exist", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), + orgId: orgId, + metadataToDelete: []struct{ key, value string }{ + { + key: "key5", + value: "value5", + }, }, + err: errors.New("One or more keys do not exist"), + }, + { + name: "delete org metadata for org that does not exist", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), + orgId: "non existant org id", + metadataToDelete: []struct{ key, value string }{ + { + key: "key4", + value: "value4", + }, + }, + err: errors.New("Organisation not found"), + }, + { + name: "delete org metadata without permission", + ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeOrgOwner), orgId: orgId, metadataToDelete: []struct{ key, value string }{ { key: "key4", value: "value4", }, - { - key: "key6", - value: "value6", - }, }, - metadataToRemain: []struct{ key, value string }{ - { - key: "key5", - value: "value5", - }, - }, - }, - { - name: "delete org metadata that does not exist", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ - OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ - { - Key: "key88", - Value: []byte("value74"), - }, - { - Key: "key5888", - Value: []byte("value8885"), - }, - }, - }) - require.NoError(t, err) - }, - orgId: orgId, - // TODO: this error message needs to be either removed or changed - err: errors.New("Metadata list is empty"), - }, - { - name: "delete org metadata for org that does not exist", - ctx: Instance.WithAuthorizationToken(CTX, integration.UserTypeIAMOwner), - setupFunc: func() { - _, err := Client.SetOrganizationMetadata(CTX, &v2beta_org.SetOrganizationMetadataRequest{ - OrganizationId: orgId, - Metadata: []*v2beta_org.Metadata{ - { - Key: "key88", - Value: []byte("value74"), - }, - { - Key: "key5888", - Value: []byte("value8885"), - }, - }, - }) - require.NoError(t, err) - }, - orgId: "non existant org id", - // TODO: this error message needs to be either removed or changed - err: errors.New("Metadata list is empty"), + err: errors.New("membership not found"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.setupFunc != nil { - tt.setupFunc() - } - - // check metadata exists - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) - require.EventuallyWithT(t, func(ttt *assert.CollectT) { - listOrgMetadataRes, err := Client.ListOrganizationMetadata(tt.ctx, &v2beta_org.ListOrganizationMetadataRequest{ - OrganizationId: tt.orgId, - }) - require.NoError(ttt, err) - foundMetadataCount := 0 - for _, kv := range tt.metadataToDelete { - for _, res := range listOrgMetadataRes.Metadata { - if res.Key == kv.key && - string(res.Value) == kv.value { - foundMetadataCount += 1 - } - } - } - require.Equal(ttt, len(tt.metadataToDelete), foundMetadataCount) - }, retryDuration, tick, "timeout waiting for expected organizations being created") keys := make([]string, len(tt.metadataToDelete)) for i, kvp := range tt.metadataToDelete { @@ -1922,7 +2137,7 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { } require.NoError(t, err) - retryDuration, tick = integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, 10*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { // check metadata was definitely deleted listOrgMetadataRes, err := Client.ListOrganizationMetadata(tt.ctx, &v2beta_org.ListOrganizationMetadataRequest{ @@ -1940,22 +2155,6 @@ func TestServer_DeleteOrganizationMetadata(t *testing.T) { } require.Equal(ttt, foundMetadataCount, 0) }, retryDuration, tick, "timeout waiting for expected organizations being created") - - // check metadata that should not be delted was not deleted - listOrgMetadataRes, err := Client.ListOrganizationMetadata(tt.ctx, &v2beta_org.ListOrganizationMetadataRequest{ - OrganizationId: tt.orgId, - }) - require.NoError(t, err) - foundMetadataCount := 0 - for _, kv := range tt.metadataToRemain { - for _, res := range listOrgMetadataRes.Metadata { - if res.Key == kv.key && - string(res.Value) == kv.value { - foundMetadataCount += 1 - } - } - } - require.Equal(t, len(tt.metadataToRemain), foundMetadataCount) }) } } diff --git a/internal/api/grpc/org/v2beta/org.go b/internal/api/grpc/org/v2beta/org.go index edc667409ae..6668c585d1a 100644 --- a/internal/api/grpc/org/v2beta/org.go +++ b/internal/api/grpc/org/v2beta/org.go @@ -2,7 +2,6 @@ package org import ( "context" - "errors" "connectrpc.com/connect" "google.golang.org/protobuf/types/known/timestamppb" @@ -23,7 +22,7 @@ func (s *Server) CreateOrganization(ctx context.Context, request *connect.Reques if err != nil { return nil, err } - createdOrg, err := s.command.SetUpOrg(ctx, orgSetup, false) + createdOrg, err := s.command.SetUpOrg(ctx, orgSetup, false, s.command.CheckPermissionOrganizationCreate) if err != nil { return nil, err } @@ -31,7 +30,7 @@ func (s *Server) CreateOrganization(ctx context.Context, request *connect.Reques } func (s *Server) UpdateOrganization(ctx context.Context, request *connect.Request[v2beta_org.UpdateOrganizationRequest]) (*connect.Response[v2beta_org.UpdateOrganizationResponse], error) { - org, err := s.command.ChangeOrg(ctx, request.Msg.GetId(), request.Msg.GetName()) + org, err := s.command.ChangeOrg(ctx, request.Msg.GetId(), request.Msg.GetName(), s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -60,12 +59,8 @@ func (s *Server) ListOrganizations(ctx context.Context, request *connect.Request } func (s *Server) DeleteOrganization(ctx context.Context, request *connect.Request[v2beta_org.DeleteOrganizationRequest]) (*connect.Response[v2beta_org.DeleteOrganizationResponse], error) { - details, err := s.command.RemoveOrg(ctx, request.Msg.GetId()) + details, err := s.command.RemoveOrg(ctx, request.Msg.GetId(), s.command.CheckPermissionOrganizationDelete, false) if err != nil { - var notFoundError *zerrors.NotFoundError - if errors.As(err, ¬FoundError) { - return connect.NewResponse(&v2beta_org.DeleteOrganizationResponse{}), nil - } return nil, err } return connect.NewResponse(&v2beta_org.DeleteOrganizationResponse{ @@ -74,7 +69,7 @@ func (s *Server) DeleteOrganization(ctx context.Context, request *connect.Reques } func (s *Server) SetOrganizationMetadata(ctx context.Context, request *connect.Request[v2beta_org.SetOrganizationMetadataRequest]) (*connect.Response[v2beta_org.SetOrganizationMetadataResponse], error) { - result, err := s.command.BulkSetOrgMetadata(ctx, request.Msg.GetOrganizationId(), BulkSetOrgMetadataToDomain(request.Msg)...) + result, err := s.command.BulkSetOrgMetadata(ctx, request.Msg.GetOrganizationId(), s.command.CheckPermissionOrganizationWrite, BulkSetOrgMetadataToDomain(request.Msg)...) if err != nil { return nil, err } @@ -102,7 +97,7 @@ func (s *Server) ListOrganizationMetadata(ctx context.Context, request *connect. } func (s *Server) DeleteOrganizationMetadata(ctx context.Context, request *connect.Request[v2beta_org.DeleteOrganizationMetadataRequest]) (*connect.Response[v2beta_org.DeleteOrganizationMetadataResponse], error) { - result, err := s.command.BulkRemoveOrgMetadata(ctx, request.Msg.GetOrganizationId(), request.Msg.Keys...) + result, err := s.command.BulkRemoveOrgMetadata(ctx, request.Msg.GetOrganizationId(), s.command.CheckPermissionOrganizationWrite, request.Msg.Keys...) if err != nil { return nil, err } @@ -112,7 +107,7 @@ func (s *Server) DeleteOrganizationMetadata(ctx context.Context, request *connec } func (s *Server) DeactivateOrganization(ctx context.Context, request *connect.Request[org.DeactivateOrganizationRequest]) (*connect.Response[org.DeactivateOrganizationResponse], error) { - objectDetails, err := s.command.DeactivateOrg(ctx, request.Msg.GetId()) + objectDetails, err := s.command.DeactivateOrg(ctx, request.Msg.GetId(), s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -122,7 +117,7 @@ func (s *Server) DeactivateOrganization(ctx context.Context, request *connect.Re } func (s *Server) ActivateOrganization(ctx context.Context, request *connect.Request[org.ActivateOrganizationRequest]) (*connect.Response[org.ActivateOrganizationResponse], error) { - objectDetails, err := s.command.ReactivateOrg(ctx, request.Msg.GetId()) + objectDetails, err := s.command.ReactivateOrg(ctx, request.Msg.GetId(), s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -136,7 +131,7 @@ func (s *Server) AddOrganizationDomain(ctx context.Context, request *connect.Req if err != nil { return nil, err } - details, err := s.command.AddOrgDomain(ctx, request.Msg.GetOrganizationId(), request.Msg.GetDomain(), userIDs) + details, err := s.command.AddOrgDomain(ctx, request.Msg.GetOrganizationId(), request.Msg.GetDomain(), userIDs, s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -170,7 +165,7 @@ func (s *Server) ListOrganizationDomains(ctx context.Context, req *connect.Reque } func (s *Server) DeleteOrganizationDomain(ctx context.Context, req *connect.Request[org.DeleteOrganizationDomainRequest]) (*connect.Response[org.DeleteOrganizationDomainResponse], error) { - details, err := s.command.RemoveOrgDomain(ctx, RemoveOrgDomainRequestToDomain(ctx, req.Msg)) + details, err := s.command.RemoveOrgDomain(ctx, RemoveOrgDomainRequestToDomain(ctx, req.Msg), s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -180,7 +175,7 @@ func (s *Server) DeleteOrganizationDomain(ctx context.Context, req *connect.Requ } func (s *Server) GenerateOrganizationDomainValidation(ctx context.Context, req *connect.Request[org.GenerateOrganizationDomainValidationRequest]) (*connect.Response[org.GenerateOrganizationDomainValidationResponse], error) { - token, url, err := s.command.GenerateOrgDomainValidation(ctx, GenerateOrgDomainValidationRequestToDomain(ctx, req.Msg)) + token, url, err := s.command.GenerateOrgDomainValidation(ctx, GenerateOrgDomainValidationRequestToDomain(ctx, req.Msg), s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } @@ -195,7 +190,7 @@ func (s *Server) VerifyOrganizationDomain(ctx context.Context, request *connect. if err != nil { return nil, err } - details, err := s.command.ValidateOrgDomain(ctx, ValidateOrgDomainRequestToDomain(ctx, request.Msg), userIDs) + details, err := s.command.ValidateOrgDomain(ctx, ValidateOrgDomainRequestToDomain(ctx, request.Msg), userIDs, s.command.CheckPermissionOrganizationWrite) if err != nil { return nil, err } diff --git a/internal/api/ui/login/register_org_handler.go b/internal/api/ui/login/register_org_handler.go index 58a49f1d08f..33c56632bc0 100644 --- a/internal/api/ui/login/register_org_handler.go +++ b/internal/api/ui/login/register_org_handler.go @@ -84,7 +84,7 @@ func (l *Login) handleRegisterOrgCheck(w http.ResponseWriter, r *http.Request) { l.renderRegisterOrg(w, r, authRequest, data, err) return } - _, err = l.command.SetUpOrg(ctx, data.toCommandOrg(), true, userIDs...) + _, err = l.command.SetUpOrg(ctx, data.toCommandOrg(), true, nil, userIDs...) if err != nil { l.renderRegisterOrg(w, r, authRequest, data, err) return diff --git a/internal/command/main_test.go b/internal/command/main_test.go index 6effedefa70..8b519c66d46 100644 --- a/internal/command/main_test.go +++ b/internal/command/main_test.go @@ -255,6 +255,17 @@ func newMockProjectPermissionCheckSAMLNotAllowed() domain.ProjectPermissionCheck } } +func newMockOrganizationPermissionCheckNotAllowed() OrganizationPermissionCheck { + return func(ctx context.Context, organizationID string) (err error) { + return zerrors.ThrowPermissionDenied(nil, "", "Errors.PermissionDenied") + } +} +func newMockOrganizationPermissionCheckAllowed() OrganizationPermissionCheck { + return func(ctx context.Context, organizationID string) (err error) { + return nil + } +} + func newMockTokenVerifierValid() func(ctx context.Context, sessionToken, sessionID, tokenID string) (err error) { return func(ctx context.Context, sessionToken, sessionID, tokenID string) (err error) { return nil diff --git a/internal/command/org.go b/internal/command/org.go index 215fe0b5cc2..ef67b7af548 100644 --- a/internal/command/org.go +++ b/internal/command/org.go @@ -181,7 +181,7 @@ func (c *orgSetupCommands) setupOrgAdminMachine(orgAgg *org.Aggregate, machine * func (c *orgSetupCommands) addCustomDomain(domain string, userIDs []string) error { if domain != "" { - c.validations = append(c.validations, c.commands.prepareAddOrgDomain(c.aggregate, domain, userIDs)) + c.validations = append(c.validations, c.commands.prepareAddOrgDomain(c.aggregate, domain, userIDs, nil)) } return nil } @@ -263,7 +263,7 @@ func (c *orgSetupCommands) createdMachineAdmin(admin *OrgSetupAdmin) *CreatedOrg return createdAdmin } -func (c *Commands) SetUpOrg(ctx context.Context, o *OrgSetup, allowInitialMail bool, userIDs ...string) (*CreatedOrg, error) { +func (c *Commands) SetUpOrg(ctx context.Context, o *OrgSetup, allowInitialMail bool, permissionCheck OrganizationPermissionCheck, userIDs ...string) (*CreatedOrg, error) { if err := o.Validate(); err != nil { return nil, err } @@ -276,6 +276,12 @@ func (c *Commands) SetUpOrg(ctx context.Context, o *OrgSetup, allowInitialMail b } } + if permissionCheck != nil { + if err := permissionCheck(ctx, o.OrgID); err != nil { + return nil, err + } + } + // because users can choose their own ID, we must check that an org with the same ID does not already exist existingOrg, err := c.getOrgWriteModelByID(ctx, o.OrgID) if err != nil { @@ -394,13 +400,17 @@ func (c *Commands) addOrgWithIDAndMember(ctx context.Context, name, userID, reso return orgWriteModelToOrg(addedOrg), nil } -func (c *Commands) ChangeOrg(ctx context.Context, orgID, name string) (*domain.ObjectDetails, error) { +func (c *Commands) ChangeOrg(ctx context.Context, organizationID, name string, permissionCheck OrganizationPermissionCheck) (*domain.ObjectDetails, error) { name = strings.TrimSpace(name) - if orgID == "" || name == "" { + if organizationID == "" || name == "" { return nil, zerrors.ThrowInvalidArgument(nil, "EVENT-Mf9sd", "Errors.Org.Invalid") } - - orgWriteModel, err := c.getOrgWriteModelByID(ctx, orgID) + if permissionCheck != nil { + if err := permissionCheck(ctx, organizationID); err != nil { + return nil, err + } + } + orgWriteModel, err := c.getOrgWriteModelByID(ctx, organizationID) if err != nil { return nil, err } @@ -413,7 +423,7 @@ func (c *Commands) ChangeOrg(ctx context.Context, orgID, name string) (*domain.O orgAgg := OrgAggregateFromWriteModel(&orgWriteModel.WriteModel) events := make([]eventstore.Command, 0) events = append(events, org.NewOrgChangedEvent(ctx, orgAgg, orgWriteModel.Name, name)) - changeDomainEvents, err := c.changeDefaultDomain(ctx, orgID, name) + changeDomainEvents, err := c.changeDefaultDomain(ctx, organizationID, name) if err != nil { return nil, err } @@ -431,8 +441,15 @@ func (c *Commands) ChangeOrg(ctx context.Context, orgID, name string) (*domain.O return writeModelToObjectDetails(&orgWriteModel.WriteModel), nil } -func (c *Commands) DeactivateOrg(ctx context.Context, orgID string) (*domain.ObjectDetails, error) { - orgWriteModel, err := c.getOrgWriteModelByID(ctx, orgID) +type OrganizationPermissionCheck func(ctx context.Context, organizationID string) error + +func (c *Commands) DeactivateOrg(ctx context.Context, organizationID string, permissionCheck OrganizationPermissionCheck) (*domain.ObjectDetails, error) { + if permissionCheck != nil { + if err := permissionCheck(ctx, organizationID); err != nil { + return nil, err + } + } + orgWriteModel, err := c.getOrgWriteModelByID(ctx, organizationID) if err != nil { return nil, err } @@ -454,8 +471,13 @@ func (c *Commands) DeactivateOrg(ctx context.Context, orgID string) (*domain.Obj return writeModelToObjectDetails(&orgWriteModel.WriteModel), nil } -func (c *Commands) ReactivateOrg(ctx context.Context, orgID string) (*domain.ObjectDetails, error) { - orgWriteModel, err := c.getOrgWriteModelByID(ctx, orgID) +func (c *Commands) ReactivateOrg(ctx context.Context, organizationID string, permissionCheck OrganizationPermissionCheck) (*domain.ObjectDetails, error) { + if permissionCheck != nil { + if err := permissionCheck(ctx, organizationID); err != nil { + return nil, err + } + } + orgWriteModel, err := c.getOrgWriteModelByID(ctx, organizationID) if err != nil { return nil, err } @@ -477,13 +499,16 @@ func (c *Commands) ReactivateOrg(ctx context.Context, orgID string) (*domain.Obj return writeModelToObjectDetails(&orgWriteModel.WriteModel), nil } -func (c *Commands) RemoveOrg(ctx context.Context, id string) (*domain.ObjectDetails, error) { +func (c *Commands) RemoveOrg(ctx context.Context, id string, permissionCheck OrganizationPermissionCheck, mustExist bool) (*domain.ObjectDetails, error) { orgAgg := org.NewAggregate(id) - cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareRemoveOrg(orgAgg)) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareRemoveOrg(orgAgg, permissionCheck, mustExist)) if err != nil { return nil, err } + if len(cmds) == 0 { + return &domain.ObjectDetails{}, nil + } events, err := c.eventstore.Push(ctx, cmds...) if err != nil { @@ -497,9 +522,14 @@ func (c *Commands) RemoveOrg(ctx context.Context, id string) (*domain.ObjectDeta }, nil } -func (c *Commands) prepareRemoveOrg(a *org.Aggregate) preparation.Validation { +func (c *Commands) prepareRemoveOrg(a *org.Aggregate, permissionCheck OrganizationPermissionCheck, mustExist bool) preparation.Validation { return func() (preparation.CreateCommands, error) { return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { + if permissionCheck != nil { + if err := permissionCheck(ctx, a.ID); err != nil { + return nil, err + } + } instance := authz.GetInstance(ctx) if a.ID == instance.DefaultOrganisationID() { return nil, zerrors.ThrowPreconditionFailed(nil, "COMMA-wG9p1", "Errors.Org.DefaultOrgNotDeletable") @@ -519,7 +549,10 @@ func (c *Commands) prepareRemoveOrg(a *org.Aggregate) preparation.Validation { return nil, zerrors.ThrowPreconditionFailed(err, "COMMA-wG9p1", "Errors.Org.NotFound") } if !isOrgStateExists(writeModel.State) { - return nil, zerrors.ThrowNotFound(nil, "COMMA-aps2n", "Errors.Org.NotFound") + if mustExist { + return nil, zerrors.ThrowNotFound(nil, "COMMA-aps2n", "Errors.Org.NotFound") + } + return nil, nil } domainPolicy, err := c.domainPolicyWriteModel(ctx, a.ID) diff --git a/internal/command/org_domain.go b/internal/command/org_domain.go index 2e132f6c478..81b0e4c0155 100644 --- a/internal/command/org_domain.go +++ b/internal/command/org_domain.go @@ -20,7 +20,7 @@ import ( "github.com/zitadel/zitadel/internal/zerrors" ) -func (c *Commands) prepareAddOrgDomain(a *org.Aggregate, addDomain string, userIDs []string) preparation.Validation { +func (c *Commands) prepareAddOrgDomain(a *org.Aggregate, addDomain string, userIDs []string, permissionCheck OrganizationPermissionCheck) preparation.Validation { return func() (preparation.CreateCommands, error) { if addDomain = strings.TrimSpace(addDomain); addDomain == "" { return nil, zerrors.ThrowInvalidArgument(nil, "ORG-r3h4J", "Errors.Invalid.Argument") @@ -29,6 +29,15 @@ func (c *Commands) prepareAddOrgDomain(a *org.Aggregate, addDomain string, userI ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() + if permissionCheck != nil { + if err := permissionCheck(ctx, a.ID); err != nil { + return nil, err + } + } + if err := c.checkOrgExists(ctx, a.ID); err != nil { + return nil, err + } + existing, err := orgDomain(ctx, filter, a.ID, addDomain) if err != nil && !errors.Is(err, zerrors.ThrowNotFound(nil, "", "")) { return nil, err @@ -123,12 +132,12 @@ func (c *Commands) VerifyOrgDomain(ctx context.Context, orgID, domain string) (_ return pushedEventsToObjectDetails(pushedEvents), nil } -func (c *Commands) AddOrgDomain(ctx context.Context, orgID, domain string, claimedUserIDs []string) (_ *domain.ObjectDetails, err error) { +func (c *Commands) AddOrgDomain(ctx context.Context, orgID, domain string, claimedUserIDs []string, permissionCheck OrganizationPermissionCheck) (_ *domain.ObjectDetails, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() orgAgg := org.NewAggregate(orgID) - cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareAddOrgDomain(orgAgg, domain, claimedUserIDs)) + cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.prepareAddOrgDomain(orgAgg, domain, claimedUserIDs, permissionCheck)) if err != nil { return nil, err } @@ -139,7 +148,7 @@ func (c *Commands) AddOrgDomain(ctx context.Context, orgID, domain string, claim return pushedEventsToObjectDetails(pushedEvents), nil } -func (c *Commands) GenerateOrgDomainValidation(ctx context.Context, orgDomain *domain.OrgDomain) (token, url string, err error) { +func (c *Commands) GenerateOrgDomainValidation(ctx context.Context, orgDomain *domain.OrgDomain, permissionCheck OrganizationPermissionCheck) (token, url string, err error) { if orgDomain == nil || !orgDomain.IsValid() || orgDomain.AggregateID == "" { return "", "", zerrors.ThrowInvalidArgument(nil, "ORG-R24hb", "Errors.Org.InvalidDomain") } @@ -147,6 +156,11 @@ func (c *Commands) GenerateOrgDomainValidation(ctx context.Context, orgDomain *d if !ok { return "", "", zerrors.ThrowInvalidArgument(nil, "ORG-Gsw31", "Errors.Org.DomainVerificationTypeInvalid") } + if permissionCheck != nil { + if err := permissionCheck(ctx, orgDomain.AggregateID); err != nil { + return "", "", err + } + } domainWriteModel, err := c.getOrgDomainWriteModel(ctx, orgDomain.AggregateID, orgDomain.Domain) if err != nil { return "", "", err @@ -177,10 +191,15 @@ func (c *Commands) GenerateOrgDomainValidation(ctx context.Context, orgDomain *d return token, url, nil } -func (c *Commands) ValidateOrgDomain(ctx context.Context, orgDomain *domain.OrgDomain, claimedUserIDs []string) (*domain.ObjectDetails, error) { +func (c *Commands) ValidateOrgDomain(ctx context.Context, orgDomain *domain.OrgDomain, claimedUserIDs []string, permissionCheck OrganizationPermissionCheck) (*domain.ObjectDetails, error) { if orgDomain == nil || !orgDomain.IsValid() || orgDomain.AggregateID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "ORG-R24hb", "Errors.Org.InvalidDomain") } + if permissionCheck != nil { + if err := permissionCheck(ctx, orgDomain.AggregateID); err != nil { + return nil, err + } + } domainWriteModel, err := c.getOrgDomainWriteModel(ctx, orgDomain.AggregateID, orgDomain.Domain) if err != nil { return nil, err @@ -261,10 +280,15 @@ func (c *Commands) SetPrimaryOrgDomain(ctx context.Context, orgDomain *domain.Or return writeModelToObjectDetails(&domainWriteModel.WriteModel), nil } -func (c *Commands) RemoveOrgDomain(ctx context.Context, orgDomain *domain.OrgDomain) (*domain.ObjectDetails, error) { +func (c *Commands) RemoveOrgDomain(ctx context.Context, orgDomain *domain.OrgDomain, permissionCheck OrganizationPermissionCheck) (*domain.ObjectDetails, error) { if orgDomain == nil || !orgDomain.IsValid() || orgDomain.AggregateID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "ORG-SJsK3", "Errors.Org.InvalidDomain") } + if permissionCheck != nil { + if err := permissionCheck(ctx, orgDomain.AggregateID); err != nil { + return nil, err + } + } domainWriteModel, err := c.getOrgDomainWriteModel(ctx, orgDomain.AggregateID, orgDomain.Domain) if err != nil { return nil, err diff --git a/internal/command/org_domain_test.go b/internal/command/org_domain_test.go index df797109558..1ac4425c928 100644 --- a/internal/command/org_domain_test.go +++ b/internal/command/org_domain_test.go @@ -23,11 +23,13 @@ import ( func TestAddDomain(t *testing.T) { type args struct { - a *org.Aggregate - domain string - claimedUserIDs []string - idGenerator id.Generator - filter preparation.FilterToQueryReducer + a *org.Aggregate + domain string + claimedUserIDs []string + idGenerator id.Generator + filter preparation.FilterToQueryReducer + eventstore func(*testing.T) *eventstore.Eventstore + permissionCheck OrganizationPermissionCheck } agg := org.NewAggregate("test") @@ -40,8 +42,9 @@ func TestAddDomain(t *testing.T) { { name: "invalid domain", args: args{ - a: agg, - domain: "", + a: agg, + domain: "", + eventstore: expectEventstore(), }, want: Want{ ValidationErr: zerrors.ThrowInvalidArgument(nil, "ORG-r3h4J", "Errors.Invalid.Argument"), @@ -58,6 +61,14 @@ func TestAddDomain(t *testing.T) { org.NewDomainPolicyAddedEvent(ctx, &agg.Aggregate, true, true, true), }, nil }, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + ), }, want: Want{ Commands: []eventstore.Command{ @@ -99,6 +110,14 @@ func TestAddDomain(t *testing.T) { return []eventstore.Event{org.NewDomainPolicyAddedEvent(ctx, &agg.Aggregate, true, false, false)}, nil } }(), + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + ), }, want: Want{ Commands: []eventstore.Command{ @@ -121,18 +140,67 @@ func TestAddDomain(t *testing.T) { org.NewDomainVerifiedEvent(ctx, &agg.Aggregate, "domain"), }, nil }, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + ), }, want: Want{ CreateErr: zerrors.ThrowAlreadyExists(nil, "", ""), }, }, + { + name: "no permission", + args: args{ + a: agg, + domain: "domain", + claimedUserIDs: []string{"userID1"}, + filter: nil, + eventstore: expectEventstore(), + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + want: Want{ + CreateErr: zerrors.ThrowPermissionDenied(nil, "", "Errors.PermissionDenied"), + }, + }, + { + name: "correct with permission check", + args: args{ + a: agg, + domain: "domain", + claimedUserIDs: []string{"userID1"}, + filter: func(ctx context.Context, queryFactory *eventstore.SearchQueryBuilder) ([]eventstore.Event, error) { + return []eventstore.Event{ + org.NewDomainPolicyAddedEvent(ctx, &agg.Aggregate, true, true, true), + }, nil + }, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + ), + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + want: Want{ + Commands: []eventstore.Command{ + org.NewDomainAddedEvent(context.Background(), &agg.Aggregate, "domain"), + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { AssertValidation( t, http.WithRequestedHost(context.Background(), "domain"), - (&Commands{idGenerator: tt.args.idGenerator}).prepareAddOrgDomain(tt.args.a, tt.args.domain, tt.args.claimedUserIDs), + (&Commands{idGenerator: tt.args.idGenerator, eventstore: tt.args.eventstore(t)}).prepareAddOrgDomain(tt.args.a, tt.args.domain, tt.args.claimedUserIDs, tt.args.permissionCheck), tt.args.filter, tt.want, ) @@ -278,13 +346,14 @@ func TestSetDomainPrimary(t *testing.T) { func TestCommandSide_AddOrgDomain(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type args struct { - ctx context.Context - orgID string - domain string - claimedUserIDs []string + ctx context.Context + orgID string + domain string + claimedUserIDs []string + permissionCheck OrganizationPermissionCheck } type res struct { want *domain.ObjectDetails @@ -299,9 +368,7 @@ func TestCommandSide_AddOrgDomain(t *testing.T) { { name: "invalid domain, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -313,8 +380,13 @@ func TestCommandSide_AddOrgDomain(t *testing.T) { { name: "domain already exists, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -343,8 +415,13 @@ func TestCommandSide_AddOrgDomain(t *testing.T) { { name: "domain add, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -382,13 +459,76 @@ func TestCommandSide_AddOrgDomain(t *testing.T) { }, }, }, + { + name: "domain add (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "name", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + true, + true, + ), + ), + ), + expectPush( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + domain: "domain.ch", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "no permission, error", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + domain: "domain.ch", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } - got, err := r.AddOrgDomain(tt.args.ctx, tt.args.orgID, tt.args.domain, tt.args.claimedUserIDs) + got, err := r.AddOrgDomain(tt.args.ctx, tt.args.orgID, tt.args.domain, tt.args.claimedUserIDs, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -404,12 +544,13 @@ func TestCommandSide_AddOrgDomain(t *testing.T) { func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore secretGenerator crypto.Generator } type args struct { - ctx context.Context - domain *domain.OrgDomain + ctx context.Context + domain *domain.OrgDomain + permissionCheck OrganizationPermissionCheck } type res struct { wantToken string @@ -425,9 +566,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "invalid domain, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -444,9 +583,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "missing aggregateid, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -461,9 +598,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "invalid validation type, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -481,8 +616,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "domain not exists, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -510,8 +644,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "domain already verified, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -551,8 +684,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "add dns validation, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -601,8 +733,7 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { { name: "add http validation, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -648,14 +779,85 @@ func TestCommandSide_GenerateOrgDomainValidation(t *testing.T) { wantURL: "https://domain.ch/.well-known/zitadel-challenge/a.txt", }, }, + { + name: "add validation (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "name", + ), + ), + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + ), + expectPush( + org.NewDomainVerificationAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + domain.OrgDomainValidationTypeHTTP, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + ), + ), + ), + secretGenerator: GetMockSecretGenerator(t), + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + ValidationType: domain.OrgDomainValidationTypeHTTP, + }, + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{ + wantToken: "a", + wantURL: "https://domain.ch/.well-known/zitadel-challenge/a.txt", + }, + }, + { + name: "add validation (no permission), error", + fields: fields{ + eventstore: expectEventstore(), + secretGenerator: GetMockSecretGenerator(t), + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + ValidationType: domain.OrgDomainValidationTypeHTTP, + }, + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), domainVerificationGenerator: tt.fields.secretGenerator, } - token, url, err := r.GenerateOrgDomainValidation(tt.args.ctx, tt.args.domain) + token, url, err := r.GenerateOrgDomainValidation(tt.args.ctx, tt.args.domain, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -679,9 +881,10 @@ func TestCommandSide_ValidateOrgDomain(t *testing.T) { domainValidationFunc func(domain, token, verifier string, checkType http.CheckType) error } type args struct { - ctx context.Context - domain *domain.OrgDomain - claimedUserIDs []string + ctx context.Context + domain *domain.OrgDomain + claimedUserIDs []string + permissionCheck OrganizationPermissionCheck } type res struct { want *domain.ObjectDetails @@ -1084,6 +1287,86 @@ func TestCommandSide_ValidateOrgDomain(t *testing.T) { }, }, }, + { + name: "domain verification (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "name", + ), + ), + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + eventFromEventPusher( + org.NewDomainVerificationAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + domain.OrgDomainValidationTypeDNS, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + ), + ), + ), + expectPush( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + ), + alg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + domainValidationFunc: validDomainVerification, + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + ValidationType: domain.OrgDomainValidationTypeDNS, + }, + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "domain verification (no permission), error", + fields: fields{ + eventstore: expectEventstore(), + alg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + domainValidationFunc: validDomainVerification, + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + ValidationType: domain.OrgDomainValidationTypeDNS, + }, + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1094,7 +1377,7 @@ func TestCommandSide_ValidateOrgDomain(t *testing.T) { domainVerificationValidator: tt.fields.domainValidationFunc, idGenerator: tt.fields.idGenerator, } - got, err := r.ValidateOrgDomain(http.WithRequestedHost(tt.args.ctx, "zitadel.ch"), tt.args.domain, tt.args.claimedUserIDs) + got, err := r.ValidateOrgDomain(http.WithRequestedHost(tt.args.ctx, "zitadel.ch"), tt.args.domain, tt.args.claimedUserIDs, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -1295,11 +1578,12 @@ func TestCommandSide_SetPrimaryDomain(t *testing.T) { func TestCommandSide_RemoveOrgDomain(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type args struct { - ctx context.Context - domain *domain.OrgDomain + ctx context.Context + domain *domain.OrgDomain + permissionCheck OrganizationPermissionCheck } type res struct { want *domain.ObjectDetails @@ -1314,9 +1598,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "invalid domain, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -1333,9 +1615,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "missing aggregateid, error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -1350,8 +1630,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "domain not exists, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -1379,8 +1658,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "remove verified domain, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -1425,8 +1703,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "remove domain, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -1467,8 +1744,7 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { { name: "remove verified domain, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -1512,13 +1788,80 @@ func TestCommandSide_RemoveOrgDomain(t *testing.T) { }, }, }, + { + name: "remove verified domain (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "name", + ), + ), + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", + ), + ), + ), + expectPush( + org.NewDomainRemovedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "domain.ch", true, + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + }, + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "remove verified domain (no permission), error", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + domain: &domain.OrgDomain{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "org1", + }, + Domain: "domain.ch", + }, + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } - got, err := r.RemoveOrgDomain(tt.args.ctx, tt.args.domain) + got, err := r.RemoveOrgDomain(tt.args.ctx, tt.args.domain, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/org_metadata.go b/internal/command/org_metadata.go index e564bb264e6..3f9637b76aa 100644 --- a/internal/command/org_metadata.go +++ b/internal/command/org_metadata.go @@ -32,10 +32,15 @@ func (c *Commands) SetOrgMetadata(ctx context.Context, orgID string, metadata *d return writeModelToOrgMetadata(setMetadata), nil } -func (c *Commands) BulkSetOrgMetadata(ctx context.Context, orgID string, metadatas ...*domain.Metadata) (_ *domain.ObjectDetails, err error) { +func (c *Commands) BulkSetOrgMetadata(ctx context.Context, orgID string, permissionCheck OrganizationPermissionCheck, metadatas ...*domain.Metadata) (_ *domain.ObjectDetails, err error) { if len(metadatas) == 0 { return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mm2d", "Errors.Metadata.NoData") } + if permissionCheck != nil { + if err := permissionCheck(ctx, orgID); err != nil { + return nil, err + } + } err = c.checkOrgExists(ctx, orgID) if err != nil { return nil, err @@ -108,10 +113,15 @@ func (c *Commands) RemoveOrgMetadata(ctx context.Context, orgID, metadataKey str return writeModelToObjectDetails(&removeMetadata.WriteModel), nil } -func (c *Commands) BulkRemoveOrgMetadata(ctx context.Context, orgID string, metadataKeys ...string) (_ *domain.ObjectDetails, err error) { +func (c *Commands) BulkRemoveOrgMetadata(ctx context.Context, orgID string, permissionCheck OrganizationPermissionCheck, metadataKeys ...string) (_ *domain.ObjectDetails, err error) { if len(metadataKeys) == 0 { return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mw2d", "Errors.Metadata.NoData") } + if permissionCheck != nil { + if err := permissionCheck(ctx, orgID); err != nil { + return nil, err + } + } err = c.checkOrgExists(ctx, orgID) if err != nil { return nil, err diff --git a/internal/command/org_metadata_test.go b/internal/command/org_metadata_test.go index 667263007b9..ba1954b8380 100644 --- a/internal/command/org_metadata_test.go +++ b/internal/command/org_metadata_test.go @@ -144,13 +144,14 @@ func TestCommandSide_SetOrgMetadata(t *testing.T) { func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type ( args struct { - ctx context.Context - orgID string - metadataList []*domain.Metadata + ctx context.Context + orgID string + permissionCheck OrganizationPermissionCheck + metadataList []*domain.Metadata } ) type res struct { @@ -166,9 +167,7 @@ func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { { name: "empty meta data list, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -181,8 +180,7 @@ func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { { name: "org not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -201,8 +199,7 @@ func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -228,8 +225,7 @@ func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { { name: "add metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -266,13 +262,72 @@ func TestCommandSide_BulkSetOrgMetadata(t *testing.T) { }, }, }, + { + name: "add metadata (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "ZITADEL", + ), + ), + ), + expectPush( + org.NewMetadataSetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key", + []byte("value"), + ), + org.NewMetadataSetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key1", + []byte("value1"), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + metadataList: []*domain.Metadata{ + {Key: "key", Value: []byte("value")}, + {Key: "key1", Value: []byte("value1")}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "add metadata (no permission), error", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + metadataList: []*domain.Metadata{ + {Key: "key", Value: []byte("value")}, + {Key: "key1", Value: []byte("value1")}, + }, + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } - got, err := r.BulkSetOrgMetadata(tt.args.ctx, tt.args.orgID, tt.args.metadataList...) + got, err := r.BulkSetOrgMetadata(tt.args.ctx, tt.args.orgID, tt.args.permissionCheck, tt.args.metadataList...) if tt.res.err == nil { assert.NoError(t, err) } @@ -428,13 +483,14 @@ func TestCommandSide_OrgRemoveMetadata(t *testing.T) { func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type ( args struct { - ctx context.Context - orgID string - metadataList []string + ctx context.Context + orgID string + permissionCheck OrganizationPermissionCheck + metadataList []string } ) type res struct { @@ -450,9 +506,7 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { { name: "empty meta data list, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -465,8 +519,7 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { { name: "org not existing, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -482,8 +535,7 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { { name: "remove metadata keys not existing, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -515,8 +567,7 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { { name: "invalid metadata, pre condition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -555,8 +606,7 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { { name: "remove metadata, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( org.NewOrgAddedEvent(context.Background(), @@ -604,13 +654,80 @@ func TestCommandSide_BulkRemoveOrgMetadata(t *testing.T) { }, }, }, + { + name: "remove metadata (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "ZITADEL", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewMetadataSetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key", + []byte("value"), + ), + ), + eventFromEventPusher( + org.NewMetadataSetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key1", + []byte("value1"), + ), + ), + ), + expectPush( + org.NewMetadataRemovedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key", + ), + org.NewMetadataRemovedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "key1", + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + metadataList: []string{"key", "key1"}, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "remove metadata (no permission), ok", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + metadataList: []string{"key", "key1"}, + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } - got, err := r.BulkRemoveOrgMetadata(tt.args.ctx, tt.args.orgID, tt.args.metadataList...) + got, err := r.BulkRemoveOrgMetadata(tt.args.ctx, tt.args.orgID, tt.args.permissionCheck, tt.args.metadataList...) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/org_test.go b/internal/command/org_test.go index c07d5f7678a..f77ccc25b50 100644 --- a/internal/command/org_test.go +++ b/internal/command/org_test.go @@ -463,9 +463,10 @@ func TestCommandSide_ChangeOrg(t *testing.T) { eventstore func(t *testing.T) *eventstore.Eventstore } type args struct { - ctx context.Context - orgID string - name string + ctx context.Context + orgID string + name string + permissionCheck OrganizationPermissionCheck } type res struct { err func(error) bool @@ -727,13 +728,76 @@ func TestCommandSide_ChangeOrg(t *testing.T) { }, res: res{}, }, + { + name: "change org name (with permission check), ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org"), + ), + eventFromEventPusher( + org.NewDomainAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.zitadel.ch"), + ), + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.zitadel.ch"), + ), + eventFromEventPusher( + org.NewDomainPrimarySetEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org.zitadel.ch"), + ), + ), + expectPush( + org.NewOrgChangedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, "org", "ORG", + ), + ), + ), + }, + args: args{ + ctx: http_util.WithRequestedHost(context.Background(), "zitadel.ch"), + orgID: "org1", + name: "ORG", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{}, + }, + { + name: "change org name (no permission), error", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: http_util.WithRequestedHost(context.Background(), "zitadel.ch"), + orgID: "org1", + name: "ORG", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - _, err := r.ChangeOrg(tt.args.ctx, tt.args.orgID, tt.args.name) + _, err := r.ChangeOrg(tt.args.ctx, tt.args.orgID, tt.args.name, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -751,8 +815,9 @@ func TestCommandSide_DeactivateOrg(t *testing.T) { iamDomain string } type args struct { - ctx context.Context - orgID string + ctx context.Context + orgID string + permissionCheck OrganizationPermissionCheck } type res struct { want *domain.Org @@ -855,6 +920,45 @@ func TestCommandSide_DeactivateOrg(t *testing.T) { }, res: res{}, }, + { + name: "deactivate org (with permission check)", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + expectPush( + org.NewOrgDeactivatedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{}, + }, + { + name: "deactivate org (no permission)", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -862,7 +966,7 @@ func TestCommandSide_DeactivateOrg(t *testing.T) { eventstore: tt.fields.eventstore(t), idGenerator: tt.fields.idGenerator, } - _, err := r.DeactivateOrg(tt.args.ctx, tt.args.orgID) + _, err := r.DeactivateOrg(tt.args.ctx, tt.args.orgID, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -880,8 +984,9 @@ func TestCommandSide_ReactivateOrg(t *testing.T) { iamDomain string } type args struct { - ctx context.Context - orgID string + ctx context.Context + orgID string + permissionCheck OrganizationPermissionCheck } type res struct { want *domain.Org @@ -989,6 +1094,49 @@ func TestCommandSide_ReactivateOrg(t *testing.T) { }, res: res{}, }, + { + name: "reactivate org (with permission check)", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org"), + ), + eventFromEventPusher( + org.NewOrgDeactivatedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate), + ), + ), + expectPush( + org.NewOrgReactivatedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{}, + }, + { + name: "reactivate org (with permission check)", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -996,7 +1144,7 @@ func TestCommandSide_ReactivateOrg(t *testing.T) { eventstore: tt.fields.eventstore(t), idGenerator: tt.fields.idGenerator, } - _, err := r.ReactivateOrg(tt.args.ctx, tt.args.orgID) + _, err := r.ReactivateOrg(tt.args.ctx, tt.args.orgID, tt.args.permissionCheck) if tt.res.err == nil { assert.NoError(t, err) } @@ -1013,8 +1161,10 @@ func TestCommandSide_RemoveOrg(t *testing.T) { idGenerator id.Generator } type args struct { - ctx context.Context - orgID string + ctx context.Context + orgID string + permissionCheck OrganizationPermissionCheck + mustExist bool } type res struct { err func(error) bool @@ -1064,7 +1214,24 @@ func TestCommandSide_RemoveOrg(t *testing.T) { }, }, { - name: "org not found, error", + name: "org not found, must exist, error", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), // zitadel project check + expectFilter(), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + mustExist: true, + }, + res: res{ + err: zerrors.IsNotFound, + }, + }, + { + name: "org not found, ok", fields: fields{ eventstore: expectEventstore( expectFilter(), // zitadel project check @@ -1075,9 +1242,7 @@ func TestCommandSide_RemoveOrg(t *testing.T) { ctx: context.Background(), orgID: "org1", }, - res: res{ - err: zerrors.IsNotFound, - }, + res: res{}, }, { name: "push failed, error", @@ -1249,6 +1414,62 @@ func TestCommandSide_RemoveOrg(t *testing.T) { }, res: res{}, }, + + { + name: "remove org (with permission check)", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), // zitadel project check + expectFilter( + eventFromEventPusher( + org.NewOrgAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "org"), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + true, + true, + ), + ), + ), + expectFilter(), + expectFilter(), + expectFilter(), + expectFilter(), + expectPush( + org.NewOrgRemovedEvent( + context.Background(), &org.NewAggregate("org1").Aggregate, "org", []string{}, false, []string{}, []*domain.UserIDPLink{}, []string{}, + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckAllowed(), + }, + res: res{}, + }, + + { + name: "remove org (no permission)", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.IsPermissionDenied, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1256,7 +1477,7 @@ func TestCommandSide_RemoveOrg(t *testing.T) { eventstore: tt.fields.eventstore(t), idGenerator: tt.fields.idGenerator, } - _, err := r.RemoveOrg(tt.args.ctx, tt.args.orgID) + _, err := r.RemoveOrg(tt.args.ctx, tt.args.orgID, tt.args.permissionCheck, tt.args.mustExist) if tt.res.err == nil { assert.NoError(t, err) } @@ -1278,6 +1499,7 @@ func TestCommandSide_SetUpOrg(t *testing.T) { ctx context.Context setupOrg *OrgSetup allowInitialMail bool + permissionCheck OrganizationPermissionCheck userIDs []string } type res struct { @@ -1816,6 +2038,41 @@ func TestCommandSide_SetUpOrg(t *testing.T) { }, }, }, + { + name: "no permission, error", + fields: fields{ + eventstore: expectEventstore(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t), + newCode: mockEncryptedCode("userinit", time.Hour), + keyAlgorithm: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + ctx: http_util.WithRequestedHost(context.Background(), "iam-domain"), + setupOrg: &OrgSetup{ + Name: "Org", + Admins: []*OrgSetupAdmin{ + { + Machine: &AddMachine{ + Machine: &Machine{ + Username: "username", + Name: "name", + Description: "description", + AccessTokenType: domain.OIDCTokenTypeBearer, + }, + Pat: &AddPat{ + ExpirationDate: testNow.Add(time.Hour), + Scopes: []string{openid.ScopeOpenID}, + }, + }, + }, + }, + }, + permissionCheck: newMockOrganizationPermissionCheckNotAllowed(), + }, + res: res{ + err: zerrors.ThrowPermissionDenied(nil, "", "Errors.PermissionDenied"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1830,7 +2087,7 @@ func TestCommandSide_SetUpOrg(t *testing.T) { }, }, } - got, err := r.SetUpOrg(tt.args.ctx, tt.args.setupOrg, tt.args.allowInitialMail, tt.args.userIDs...) + got, err := r.SetUpOrg(tt.args.ctx, tt.args.setupOrg, tt.args.allowInitialMail, tt.args.permissionCheck, tt.args.userIDs...) assert.ErrorIs(t, err, tt.res.err) assert.Equal(t, tt.res.createdOrg, got) }) diff --git a/internal/command/permission_checks.go b/internal/command/permission_checks.go index 65ab3fa01a5..0062161a175 100644 --- a/internal/command/permission_checks.go +++ b/internal/command/permission_checks.go @@ -159,3 +159,15 @@ func (c *Commands) NewPermissionCheckUserGrantWrite(ctx context.Context) UserGra func (c *Commands) NewPermissionCheckUserGrantDelete(ctx context.Context) UserGrantPermissionCheck { return c.newUserGrantPermissionCheck(ctx, domain.PermissionUserGrantDelete) } + +func (c *Commands) CheckPermissionOrganizationCreate(ctx context.Context, organizationID string) error { + return c.newPermissionCheck(ctx, domain.PermissionOrganizationWrite, org.AggregateType)(organizationID, organizationID) +} + +func (c *Commands) CheckPermissionOrganizationWrite(ctx context.Context, organizationID string) error { + return c.newPermissionCheck(ctx, domain.PermissionOrganizationWrite, org.AggregateType)(organizationID, organizationID) +} + +func (c *Commands) CheckPermissionOrganizationDelete(ctx context.Context, organizationID string) error { + return c.newPermissionCheck(ctx, domain.PermissionOrganizationDelete, org.AggregateType)(organizationID, organizationID) +} diff --git a/internal/domain/permission.go b/internal/domain/permission.go index 978408a3c40..3b2670265be 100644 --- a/internal/domain/permission.go +++ b/internal/domain/permission.go @@ -37,6 +37,8 @@ const ( PermissionSessionLink = "session.link" PermissionSessionDelete = "session.delete" PermissionOrgRead = "org.read" + PermissionOrganizationWrite = "org.write" + PermissionOrganizationDelete = "org.delete" PermissionIDPRead = "iam.idp.read" PermissionOrgIDPRead = "org.idp.read" PermissionProjectCreate = "project.create" diff --git a/internal/query/org_domain.go b/internal/query/org_domain.go index ed0dba9c173..0b71bd064f4 100644 --- a/internal/query/org_domain.go +++ b/internal/query/org_domain.go @@ -64,6 +64,9 @@ func (q *Queries) SearchOrgDomains(ctx context.Context, queries *OrgDomainSearch if !withOwnerRemoved { eq[OrgDomainOwnerRemovedCol.identifier()] = false } + // We always use the permission v2 check and don't check the feature flag, since it's stable enough to work + // in this case and using the old checks only adds more latency, but no benefit. + query = orgDomainPermissionCheckV2(ctx, query, queries) stmt, args, err := queries.toQuery(query).Where(eq).ToSql() if err != nil { return nil, zerrors.ThrowInvalidArgument(err, "QUERY-ZRfj1", "Errors.Query.SQLStatement") @@ -176,3 +179,13 @@ var ( table: orgDomainsTable, } ) + +func orgDomainPermissionCheckV2(ctx context.Context, query sq.SelectBuilder, queries *OrgDomainSearchQueries) sq.SelectBuilder { + join, args := PermissionClause( + ctx, + OrgDomainOrgIDCol, + domain.PermissionOrgRead, + SingleOrgPermissionOption(queries.Queries), + ) + return query.JoinClause(join, args...) +} diff --git a/internal/query/org_metadata.go b/internal/query/org_metadata.go index e67c7222cd9..b332de9f288 100644 --- a/internal/query/org_metadata.go +++ b/internal/query/org_metadata.go @@ -10,6 +10,7 @@ import ( "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/api/authz" + domain_pkg "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore/handler/v2" "github.com/zitadel/zitadel/internal/query/projection" "github.com/zitadel/zitadel/internal/telemetry/tracing" @@ -131,6 +132,9 @@ func (q *Queries) SearchOrgMetadata(ctx context.Context, shouldTriggerBulk bool, eq[OrgMetadataOwnerRemovedCol.identifier()] = false } query, scan := prepareOrgMetadataListQuery() + // We always use the permission v2 check and don't check the feature flag, since it's stable enough to work + // in this case and using the old checks only adds more latency, but no benefit. + query = orgMetadataPermissionCheckV2(ctx, query, queries) stmt, args, err := queries.toQuery(query).Where(eq).ToSql() if err != nil { return nil, zerrors.ThrowInternal(err, "QUERY-Egbld", "Errors.Query.SQLStatement") @@ -248,3 +252,13 @@ func prepareOrgMetadataListQuery() (sq.SelectBuilder, func(*sql.Rows) (*OrgMetad }, nil } } + +func orgMetadataPermissionCheckV2(ctx context.Context, query sq.SelectBuilder, queries *OrgMetadataSearchQueries) sq.SelectBuilder { + join, args := PermissionClause( + ctx, + OrgMetadataOrgIDCol, + domain_pkg.PermissionOrgRead, + SingleOrgPermissionOption(queries.Queries), + ) + return query.JoinClause(join, args...) +} diff --git a/proto/zitadel/org/v2/org_service.proto b/proto/zitadel/org/v2/org_service.proto index 729350e1f94..9de0aabd109 100644 --- a/proto/zitadel/org/v2/org_service.proto +++ b/proto/zitadel/org/v2/org_service.proto @@ -123,7 +123,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.create" + permission: "authenticated" } http_response: { success_code: 201 diff --git a/proto/zitadel/org/v2beta/org_service.proto b/proto/zitadel/org/v2beta/org_service.proto index 43307214f5e..1e54d179b3a 100644 --- a/proto/zitadel/org/v2beta/org_service.proto +++ b/proto/zitadel/org/v2beta/org_service.proto @@ -125,7 +125,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.create" + permission: "authenticated" } }; @@ -160,7 +160,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -192,7 +192,7 @@ service OrganizationService { // Returns a list of organizations that match the requesting filters. All filters are applied with an AND condition. // // Required permission: - // - `iam.read` + // - `org.read` // // Deprecated: Use [ListOrganizations](/apis/resources/org_service_v2/organization-service-list-organizations.api.mdx) instead to list organizations. rpc ListOrganizations(ListOrganizationsRequest) returns (ListOrganizationsResponse) { @@ -203,7 +203,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "iam.read"; + permission: "authenticated"; } }; @@ -228,7 +228,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.delete"; + permission: "authenticated"; } }; @@ -262,7 +262,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -293,7 +293,7 @@ service OrganizationService { }; option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.read" + permission: "authenticated" } }; @@ -318,7 +318,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -344,7 +344,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -376,7 +376,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.read" + permission: "authenticated" } }; @@ -401,7 +401,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -427,7 +427,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -458,7 +458,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -483,7 +483,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } }; @@ -509,7 +509,7 @@ service OrganizationService { option (zitadel.protoc_gen_zitadel.v2.options) = { auth_option: { - permission: "org.write" + permission: "authenticated" } };