diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index 59d2ee588..c0b477bca 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -235,6 +235,19 @@ func namespaceAccessPermissionFromString(in string) identityv1.NamespaceAccess_P return identityv1.NamespaceAccess_Permission(rv) } +// nextLowerNamespacePermission returns the next lower permission level in the hierarchy. +// Admin → Write, Write → Read, Read → Unspecified (meaning remove access entirely). +func nextLowerNamespacePermission(current identityv1.NamespaceAccess_Permission) identityv1.NamespaceAccess_Permission { + switch current { + case identityv1.NamespaceAccess_PERMISSION_ADMIN: + return identityv1.NamespaceAccess_PERMISSION_WRITE + case identityv1.NamespaceAccess_PERMISSION_WRITE: + return identityv1.NamespaceAccess_PERMISSION_READ + default: + return identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED + } +} + func AccountAccessRoleFromID(in string, accountID string) identityv1.AccountAccess_Role { if strings.HasSuffix(in, accountID) { // handle legacy admin role ID return identityv1.AccountAccess_ROLE_ADMIN diff --git a/pkg/connector/helpers_test.go b/pkg/connector/helpers_test.go index 56c03133f..e8c09d06b 100644 --- a/pkg/connector/helpers_test.go +++ b/pkg/connector/helpers_test.go @@ -69,6 +69,103 @@ func TestAccountAccessRoleFromID(t *testing.T) { } } +func TestNextLowerNamespacePermission(t *testing.T) { + t.Parallel() + tt := []struct { + Name string + Input identityv1.NamespaceAccess_Permission + Expected identityv1.NamespaceAccess_Permission + }{ + { + Name: "admin downgrades to write", + Input: identityv1.NamespaceAccess_PERMISSION_ADMIN, + Expected: identityv1.NamespaceAccess_PERMISSION_WRITE, + }, + { + Name: "write downgrades to read", + Input: identityv1.NamespaceAccess_PERMISSION_WRITE, + Expected: identityv1.NamespaceAccess_PERMISSION_READ, + }, + { + Name: "read downgrades to unspecified (remove access)", + Input: identityv1.NamespaceAccess_PERMISSION_READ, + Expected: identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED, + }, + { + Name: "unspecified stays unspecified", + Input: identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED, + Expected: identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED, + }, + } + + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + var actual identityv1.NamespaceAccess_Permission + require.NotPanics(t, func() { + actual = nextLowerNamespacePermission(tc.Input) + }) + assert.Equal(t, tc.Expected, actual) + }) + } +} + +func TestNamespaceAccessPermissionFromString(t *testing.T) { + t.Parallel() + tt := []struct { + Name string + Input string + Expected identityv1.NamespaceAccess_Permission + }{ + { + Name: "admin permission", + Input: "admin", + Expected: identityv1.NamespaceAccess_PERMISSION_ADMIN, + }, + { + Name: "write permission", + Input: "write", + Expected: identityv1.NamespaceAccess_PERMISSION_WRITE, + }, + { + Name: "read permission", + Input: "read", + Expected: identityv1.NamespaceAccess_PERMISSION_READ, + }, + { + Name: "uppercase ADMIN", + Input: "ADMIN", + Expected: identityv1.NamespaceAccess_PERMISSION_ADMIN, + }, + { + Name: "mixed case Admin", + Input: "Admin", + Expected: identityv1.NamespaceAccess_PERMISSION_ADMIN, + }, + { + Name: "invalid permission returns unspecified", + Input: "invalid", + Expected: identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED, + }, + { + Name: "empty string returns unspecified", + Input: "", + Expected: identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED, + }, + } + + for _, tc := range tt { + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + var actual identityv1.NamespaceAccess_Permission + require.NotPanics(t, func() { + actual = namespaceAccessPermissionFromString(tc.Input) + }) + assert.Equal(t, tc.Expected, actual) + }) + } +} + func TestAccountAccessRoleFromString(t *testing.T) { t.Parallel() tt := []struct { diff --git a/pkg/connector/namespaces.go b/pkg/connector/namespaces.go index 113fa8ac8..d9f50529c 100644 --- a/pkg/connector/namespaces.go +++ b/pkg/connector/namespaces.go @@ -216,19 +216,47 @@ func (o *namespaceBuilder) Revoke(ctx context.Context, g *v2.Grant) (annotations namespaceID := namespace.GetId().GetResource() namespaceType := namespace.GetId().GetResourceType() + // Parse the permission level being revoked from the entitlement ID + enIDParts := strings.Split(entitlementID, ":") + if len(enIDParts) != 3 { + return nil, fmt.Errorf("baton-temporalcloud: invalid entitlement ID %s", entitlementID) + } + revokedPermStr := enIDParts[2] + revokedPerm := namespaceAccessPermissionFromString(revokedPermStr) + if revokedPerm == identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED { + return nil, fmt.Errorf("baton-temporalcloud: invalid namespace permission %s", revokedPermStr) + } + userResp, err := o.client.GetUser(ctx, &cloudservicev1.GetUserRequest{UserId: userID}) if err != nil { return nil, fmt.Errorf("baton-temporalcloud: couldn't retrieve user: %w", err) } user := userResp.GetUser() spec := user.GetSpec() - _, ok := spec.GetAccess().GetNamespaceAccesses()[namespaceID] + currentAccess, ok := spec.GetAccess().GetNamespaceAccesses()[namespaceID] if !ok { annos := annotations.New(&v2.GrantAlreadyRevoked{}) return annos, fmt.Errorf("baton-temporalcloud: grant does not exist for user") } - delete(spec.Access.NamespaceAccesses, namespaceID) + // Only revoke if the user actually has the permission being revoked. + // If they have a different permission level, this grant doesn't exist. + if currentAccess.GetPermission() != revokedPerm { + annos := annotations.New(&v2.GrantAlreadyRevoked{}) + return annos, fmt.Errorf("baton-temporalcloud: user does not have %s permission on namespace", revokedPermStr) + } + + // Determine the next lower permission level (hierarchical downgrade). + // Admin → Write, Write → Read, Read → remove entirely. + nextPerm := nextLowerNamespacePermission(revokedPerm) + if nextPerm == identityv1.NamespaceAccess_PERMISSION_UNSPECIFIED { + // Read permission: remove access entirely + delete(spec.Access.NamespaceAccesses, namespaceID) + } else { + // Downgrade to the next lower permission level + spec.Access.NamespaceAccesses[namespaceID] = &identityv1.NamespaceAccess{Permission: nextPerm} + } + req := &cloudservicev1.UpdateUserRequest{UserId: userID, Spec: spec, ResourceVersion: user.GetResourceVersion()} resp, err := o.client.UpdateUser(ctx, req) if err != nil { @@ -249,7 +277,7 @@ func (o *namespaceBuilder) Revoke(ctx context.Context, g *v2.Grant) (annotations defer cancel() err = awaitAsyncOperation(waitCtx, l, o.client, requestID, retryDelay) if err != nil { - return nil, fmt.Errorf("baton-temporalcloud: namespace assignment deletion failed: %w", err) + return nil, fmt.Errorf("baton-temporalcloud: namespace assignment update failed: %w", err) } annos := annotations.New()