diff --git a/charts/identity/Chart.yaml b/charts/identity/Chart.yaml index e831581..e21d779 100644 --- a/charts/identity/Chart.yaml +++ b/charts/identity/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: identity description: Helm chart for deploying the IdentityService gRPC server. type: application -version: 0.1.0 -appVersion: 0.1.0 +version: 0.2.0 +appVersion: 0.2.0 home: https://github.com/agynio/identity sources: - https://github.com/agynio/identity diff --git a/charts/identity/templates/authorization-policy.yaml b/charts/identity/templates/authorization-policy.yaml new file mode 100644 index 0000000..eba7fd9 --- /dev/null +++ b/charts/identity/templates/authorization-policy.yaml @@ -0,0 +1,31 @@ +{{- if .Values.internalAuthorizationPolicy.enabled }} +{{- $namespace := .Release.Namespace }} +# DeleteOrganizationResources is one step of the organization teardown. It has +# no caller identity to authorize, so who may reach it at all is settled here. +apiVersion: security.istio.io/v1beta1 +kind: AuthorizationPolicy +metadata: + name: {{ .Release.Name }}-internal-identity + labels: + app.kubernetes.io/name: {{ include "service-base.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} +spec: + selector: + matchLabels: + app.kubernetes.io/name: {{ include "service-base.name" . }} + app.kubernetes.io/instance: {{ .Release.Name }} + action: DENY + rules: + - from: + - source: + notPrincipals: +{{- range .Values.internalAuthorizationPolicy.serviceAccounts }} + - "cluster.local/ns/{{ .namespace | default $namespace }}/sa/{{ .name }}" +{{- end }} + to: + - operation: + methods: + - POST + paths: + - /agynio.api.identity.v1.IdentityService/DeleteOrganizationResources +{{- end }} diff --git a/charts/identity/values.yaml b/charts/identity/values.yaml index 4532d40..fc53557 100644 --- a/charts/identity/values.yaml +++ b/charts/identity/values.yaml @@ -159,3 +159,9 @@ metrics: path: /metrics interval: "" scrapeTimeout: "" + +internalAuthorizationPolicy: + enabled: true + serviceAccounts: + - name: organizations + namespace: "" diff --git a/internal/server/nicknames.go b/internal/server/nicknames.go index 3e4d726..76da732 100644 --- a/internal/server/nicknames.go +++ b/internal/server/nicknames.go @@ -377,3 +377,22 @@ func (s *Server) authorizeNicknameRead(ctx context.Context, callerID uuid.UUID, } return nil } + +// DeleteOrganizationResources removes the organization's nicknames. It is +// internal: Istio settles who may call it, so there is no permission check and +// no caller identity to check against. Step 8 of the organization teardown, +// last because the identities that held these nicknames are deleted by the +// services that registered them, in the steps above. +// +// Idempotent: the delete is unconditional, so a retried step finds nothing to +// remove and still succeeds. +func (s *Server) DeleteOrganizationResources(ctx context.Context, req *identityv1.DeleteOrganizationResourcesRequest) (*identityv1.DeleteOrganizationResourcesResponse, error) { + organizationID, err := parseUUID(req.GetOrganizationId()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "organization_id: %v", err) + } + if err := s.store.RemoveOrganizationNicknames(ctx, organizationID); err != nil { + return nil, status.Errorf(codes.Internal, "remove organization nicknames: %v", err) + } + return &identityv1.DeleteOrganizationResourcesResponse{}, nil +} diff --git a/internal/server/nicknames_test.go b/internal/server/nicknames_test.go index ba6127a..8128597 100644 --- a/internal/server/nicknames_test.go +++ b/internal/server/nicknames_test.go @@ -50,6 +50,8 @@ type fakeStore struct { lastInstanceSuffix *string resolution store.NicknameResolution registerErr error + removedOrgs []uuid.UUID + removeOrgErr error } func (f *fakeStore) RegisterIdentity(context.Context, uuid.UUID, int16) error { @@ -93,6 +95,14 @@ func (f *fakeStore) ResolveNickname(_ context.Context, _ uuid.UUID, nickname str return f.resolution, nil } +func (f *fakeStore) RemoveOrganizationNicknames(_ context.Context, organizationID uuid.UUID) error { + if f.removeOrgErr != nil { + return f.removeOrgErr + } + f.removedOrgs = append(f.removedOrgs, organizationID) + return nil +} + func (f *fakeStore) BatchGetNicknames(_ context.Context, organizationID uuid.UUID, identityIDs []uuid.UUID) (map[uuid.UUID]store.NicknameEntry, error) { f.lastOrganizationID = organizationID f.lastIdentityIDs = identityIDs @@ -306,3 +316,29 @@ func TestBatchGetNicknamesReturnsInstanceSuffix(t *testing.T) { require.Equal(t, "bob", response.Entries[0].GetNickname()) require.Equal(t, instanceSuffix, response.Entries[0].GetInstanceSuffix()) } + +func TestDeleteOrganizationResourcesRemovesNicknames(t *testing.T) { + organizationID := uuid.New() + fake := &fakeStore{} + server := New(fake, &fakeAuthClient{}) + + req := &identityv1.DeleteOrganizationResourcesRequest{OrganizationId: organizationID.String()} + // Internal RPC: no identity in the context, and none required. + _, err := server.DeleteOrganizationResources(context.Background(), req) + require.NoError(t, err) + + // The cascade retries a step it is unsure finished, so a second call has to + // succeed on the now-empty organization rather than report NotFound the way + // RemoveNickname would. + _, err = server.DeleteOrganizationResources(context.Background(), req) + require.NoError(t, err) + require.Equal(t, []uuid.UUID{organizationID, organizationID}, fake.removedOrgs) +} + +func TestDeleteOrganizationResourcesRejectsInvalidOrganizationID(t *testing.T) { + server := New(&fakeStore{}, &fakeAuthClient{}) + _, err := server.DeleteOrganizationResources(context.Background(), &identityv1.DeleteOrganizationResourcesRequest{ + OrganizationId: "not-a-uuid", + }) + require.Equal(t, codes.InvalidArgument, status.Code(err)) +} diff --git a/internal/server/server.go b/internal/server/server.go index 507b07b..ddc9675 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -22,6 +22,7 @@ type identityStore interface { RemoveNickname(context.Context, uuid.UUID, uuid.UUID, *uuid.UUID) error ResolveNickname(context.Context, uuid.UUID, string, *string) (store.NicknameResolution, error) BatchGetNicknames(context.Context, uuid.UUID, []uuid.UUID) (map[uuid.UUID]store.NicknameEntry, error) + RemoveOrganizationNicknames(context.Context, uuid.UUID) error } type authorizationChecker interface { diff --git a/internal/store/nicknames.go b/internal/store/nicknames.go index 42008cb..2025216 100644 --- a/internal/store/nicknames.go +++ b/internal/store/nicknames.go @@ -63,6 +63,14 @@ func (s *Store) RemoveNickname(ctx context.Context, organizationID uuid.UUID, id return nil } +// RemoveOrganizationNicknames drops every nickname held in the organization. +// Unlike RemoveNickname it does not report an empty result as NotFound: the +// teardown step it serves is retried, and finding nothing left is success. +func (s *Store) RemoveOrganizationNicknames(ctx context.Context, organizationID uuid.UUID) error { + _, err := s.pool.Exec(ctx, `DELETE FROM org_nicknames WHERE organization_id = $1`, organizationID) + return err +} + func (s *Store) ResolveNickname(ctx context.Context, organizationID uuid.UUID, nickname string, instanceSuffix *string) (NicknameResolution, error) { var resolution NicknameResolution var installationID pgtype.UUID