From f858d89b9b804ea48c09de21061922f5b2ae6ff0 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 19:56:54 +0100 Subject: [PATCH 01/17] docs: specify external identity linking api --- README.md | 2 +- ...ernal-identity-linking-api-architecture.md | 596 ++++++++++++++++++ 2 files changed, 597 insertions(+), 1 deletion(-) create mode 100644 docs/2026-03-12-external-identity-linking-api-architecture.md diff --git a/README.md b/README.md index ce968e8..6781ee2 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The backend inside a workspace is pluggable. OpenClaw is one example runtime. An > Spritz is in active development and should be treated as alpha software. APIs, CRDs, Helm values, and UI details may still change while the deployment model is being hardened. -[Deployment Spec](docs/2026-02-24-simplest-spritz-deployment-spec.md) · [ACP Architecture](docs/2026-03-09-acp-port-and-agent-chat-architecture.md) · [Portable Auth](docs/2026-02-24-portable-authentication-and-account-architecture.md) · [External Provisioners](docs/2026-03-11-external-provisioner-and-service-principal-architecture.md) · [OpenClaw Integration](OPENCLAW.md) +[Deployment Spec](docs/2026-02-24-simplest-spritz-deployment-spec.md) · [ACP Architecture](docs/2026-03-09-acp-port-and-agent-chat-architecture.md) · [Portable Auth](docs/2026-02-24-portable-authentication-and-account-architecture.md) · [External Provisioners](docs/2026-03-11-external-provisioner-and-service-principal-architecture.md) · [External Identity Links](docs/2026-03-12-external-identity-linking-api-architecture.md) · [OpenClaw Integration](OPENCLAW.md) ## Vision diff --git a/docs/2026-03-12-external-identity-linking-api-architecture.md b/docs/2026-03-12-external-identity-linking-api-architecture.md new file mode 100644 index 0000000..11b991a --- /dev/null +++ b/docs/2026-03-12-external-identity-linking-api-architecture.md @@ -0,0 +1,596 @@ +--- +date: 2026-03-12 +author: Spritz Maintainers +title: External Identity Linking API Architecture +tags: [spritz, auth, provisioning, identity, api, architecture] +--- + +## Overview + +This document defines the long-term stable Spritz API for linking external +identities to Spritz owners. + +The goal is to let external systems such as chat bots, workflow engines, or +support tools identify a human user without requiring that user to manually +paste a Spritz-specific owner ID. + +Typical examples include: + +- a Discord bot, +- a Slack app, +- a Microsoft Teams assistant, +- a Google Chat integration, +- a Mattermost automation. + +Spritz must stay backend-agnostic and deployment-agnostic. It must not learn +provider-specific user models from any particular product deployment. + +Instead, Spritz should standardize a generic external identity model and own +the lifecycle for linking those identities to normal Spritz owners. + +This architecture extends, but does not replace, the external provisioner model +defined in [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) +and the portable auth model defined in +[2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md). + +## Goals + +- Remove the need for humans to manually provide Spritz owner IDs to external + systems. +- Keep Spritz core portable and free of provider-specific assumptions. +- Let external provisioners resolve a human owner through a narrow, + action-specific API. +- Require explicit human confirmation before an external identity is bound to a + Spritz owner. +- Keep the create/provision flow stable across Discord, Slack, Teams, and + future providers. +- Align the identity model with widely used federation concepts such as + issuer-plus-subject identifiers. +- Preserve current explicit ownership and lifecycle guarantees. + +## Non-goals + +- Turning Spritz into a general identity provider. +- Replacing OIDC, OAuth2, SAML, or SCIM in external systems. +- Letting service principals search arbitrary users by email, name, or handle. +- Letting an external system claim a user account without a human confirmation + step. +- Embedding Discord-, Slack-, or deployment-specific logic in Spritz core. +- Requiring a provider-specific verification protocol in the core API. + +## Design Principles + +### External identities are references, not sessions + +An external identity is not a login session and not an authorization grant. + +It is a stable reference that can later resolve to a Spritz owner. + +### Spritz owns the binding + +External systems may identify a candidate external subject, but only Spritz may +bind that subject to a human owner after the human authenticates through the +normal Spritz browser path. + +### Stable identity keys use namespace plus subject + +The canonical identity key is: + +- `issuer` +- `provider` +- `subject` + +This follows the same broad model as federated identity systems that treat +issuer-plus-subject as the stable user key. + +### Linking must be explicit + +Provisioning and linking are related, but they are not the same operation. + +The stable API should keep linking explicit so that: + +- create requests stay idempotent, +- unresolved identities are a well-defined precondition failure, +- link creation remains auditable, +- provider-specific UX can evolve without breaking the create contract. + +### Providers stay pluggable + +Spritz should not require Discord-specific or Slack-specific fields in canonical +records. + +Provider-specific metadata should remain optional, opaque, and advisory. + +## Standards Alignment + +Spritz should align with existing standards where practical: + +- Use issuer-plus-subject semantics similar to OIDC federated identities. +- Treat display name, email, and handle as advisory metadata rather than stable + identifiers. +- Treat external identity scoping similarly to SCIM `externalId` semantics: + identifiers are scoped to the external provisioning domain, not globally + portable by assumption. + +Spritz does not need to implement a general OIDC account linking protocol in +core. It needs a portable control-plane API that uses those same identity +principles. + +## Canonical Terms + +### Spritz owner + +A Spritz owner is the normal human principal that owns and later accesses a +workspace. + +This is the same owner model used by existing UI and provisioner flows. + +### External identity + +An external identity is an opaque tuple: + +- `issuer`: the namespace that controls the identity binding authority, +- `provider`: the external system category such as `discord`, `slack`, or + `teams`, +- `subject`: the stable provider-specific user identifier. + +### External identity link + +A durable binding from one external identity to one Spritz owner. + +### External identity link intent + +A short-lived, one-time claim artifact created by a service principal and later +claimed by a human in the browser to create or confirm a durable link. + +### Owner reference + +A stable create-time abstraction that allows Spritz to accept either: + +- a direct owner ID, or +- a linked external identity reference. + +## Identity Namespace Model + +The identity tuple is: + +```json +{ + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" +} +``` + +Rules: + +- `issuer` MUST be a stable namespace controlled by the service principal or + integration owner. +- `provider` MUST be a normalized lower-case token. +- `subject` MUST be treated as an opaque string. +- The tuple `(issuer, provider, subject)` MUST be globally unique within a + Spritz deployment. +- One tuple MUST resolve to at most one active Spritz owner at a time. +- One owner MAY have multiple linked external identities. + +The `issuer` field is critical. It prevents one integration from asserting or +resolving identities in another integration's namespace. + +## Core Resources + +### ExternalIdentityRef + +```json +{ + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" +} +``` + +### OwnerRef + +```json +{ + "type": "owner", + "id": "user-123" +} +``` + +or + +```json +{ + "type": "external", + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" +} +``` + +### ExternalIdentityLink + +Recommended response shape: + +```json +{ + "id": "eil_01j...", + "identity": { + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + }, + "ownerId": "user-123", + "status": "active", + "verifiedAt": "2026-03-12T15:00:00Z", + "verificationMode": "delivery", + "createdAt": "2026-03-12T15:00:00Z", + "metadata": { + "displayName": "onur" + } +} +``` + +### ExternalIdentityLinkIntent + +Recommended response shape: + +```json +{ + "id": "eili_01j...", + "identity": { + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + }, + "status": "pending", + "claimUrl": "https://spritz.example.com/link/eili_01j.../claim", + "userCode": "7J4K-9P", + "expiresAt": "2026-03-12T15:15:00Z", + "createdAt": "2026-03-12T15:00:00Z" +} +``` + +`userCode` is optional but recommended. It gives deployments a stronger defense +against forwarded or copied links without making the core flow provider-specific. + +## Stable API Surface + +### 1. Resolve a linked identity + +`POST /api/external-identity-links/resolve` + +Request: + +```json +{ + "identity": { + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + } +} +``` + +Success when linked: + +```json +{ + "linked": true, + "link": { + "id": "eil_01j...", + "ownerId": "user-123", + "status": "active", + "verifiedAt": "2026-03-12T15:00:00Z" + } +} +``` + +Success when unlinked: + +```json +{ + "linked": false +} +``` + +This endpoint MUST match exact tuples only. It MUST NOT perform fuzzy matching +by email, display name, or handle. + +### 2. Create a link intent + +`POST /api/external-identity-link-intents` + +Request: + +```json +{ + "identity": { + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + }, + "metadata": { + "displayName": "onur" + }, + "ttl": "15m", + "requireUserCode": true, + "clientState": "discord-interaction-123" +} +``` + +Response: + +```json +{ + "id": "eili_01j...", + "status": "pending", + "claimUrl": "https://spritz.example.com/link/eili_01j.../claim", + "userCode": "7J4K-9P", + "expiresAt": "2026-03-12T15:15:00Z" +} +``` + +Properties: + +- Creating an intent MUST be idempotent for the same service principal and + unresolved identity while the active intent remains pending. +- If the identity is already linked, the API SHOULD return the existing active + link rather than creating a new intent. +- Intent creation MUST NOT silently relink an identity that is already linked + to a different owner. + +### 3. Inspect a link intent + +`GET /api/external-identity-link-intents/{intentId}` + +This endpoint exists so an external client can poll intent state without +needing browser callbacks. + +Response status values: + +- `pending` +- `claimed` +- `expired` +- `cancelled` + +### 4. Claim a link intent as a human + +`POST /api/external-identity-link-intents/{intentId}/claim` + +This endpoint requires normal human browser authentication. + +Request: + +```json +{ + "userCode": "7J4K-9P" +} +``` + +Behavior: + +- The authenticated human becomes the target owner. +- If the identity is not yet linked, Spritz creates the durable link. +- If it is already linked to the same owner, the operation is idempotent. +- If it is already linked to a different owner, Spritz returns conflict. +- Successful claim marks the intent as `claimed`. + +### 5. List current human links + +`GET /api/external-identity-links` + +Default behavior for human principals: + +- return only the links owned by the current principal. + +Default behavior for service principals: + +- not allowed unless explicitly granted a list scope. + +### 6. Unlink + +`DELETE /api/external-identity-links/{linkId}` + +Rules: + +- Human principals may unlink their own links. +- Service principals may not unlink by default. +- Admins may unlink as break-glass. +- Unlinking affects future resolution only. It MUST NOT mutate the ownership of + existing workspaces. + +## Provisioning Integration + +The create API should add `ownerRef` while preserving existing `ownerId` +compatibility. + +Recommended request: + +```json +{ + "ownerRef": { + "type": "external", + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + }, + "presetId": "openclaw", + "idempotencyKey": "discord-123" +} +``` + +Rules: + +- `ownerId` and `ownerRef` MUST be mutually exclusive. +- If `ownerRef.type=owner`, Spritz uses the explicit owner ID. +- If `ownerRef.type=external`, Spritz MUST resolve the link before create. +- If the external identity is unlinked, create MUST fail with a typed conflict. +- Create MUST NOT implicitly create link intents in the stable v1 contract. + +Recommended typed error: + +```json +{ + "error": "external_identity_unlinked", + "identity": { + "issuer": "support-bot", + "provider": "discord", + "subject": "123456789012345678" + } +} +``` + +This keeps create semantics clean and preserves the idempotency guarantees from +the current provisioner architecture. + +## Verification Modes + +The core API should define a generic `verificationMode` field on durable links +and intents. + +Stable modes: + +- `delivery`: the external system delivered the claim artifact out-of-band to + the target subject. +- `provider_oauth`: the link was completed after provider-specific external + OAuth verification. +- `attested`: a trusted external bridge supplied an additional signed + verification signal. + +The core contract only requires `delivery`. + +This keeps the API portable while allowing stronger provider-specific +verification in overlays or extensions later. + +## Authorization Model + +Recommended new service scopes: + +- `spritz.external_identities.resolve` +- `spritz.external_identities.link_intents.create` + +Recommended human capabilities: + +- claim own pending intents, +- list own links, +- unlink own links. + +Recommended admin capabilities: + +- inspect links across owners, +- revoke links, +- relink by explicit override. + +Service principals MUST be scoped to their own `issuer` namespace unless +explicitly granted broader authority. + +## State Model + +### Link intent states + +- `pending` +- `claimed` +- `expired` +- `cancelled` + +### Durable link states + +- `active` +- `revoked` + +State transition rules: + +- `pending -> claimed` when a human successfully completes the claim. +- `pending -> expired` when TTL elapses. +- `active -> revoked` when owner or admin unlinks. +- `revoked` links MUST NOT resolve for future creates. + +## Storage and Implementation Strategy + +The API contract should not depend on a specific storage backend. + +Spritz should define two internal store interfaces: + +- `ExternalIdentityLinkStore` +- `ExternalIdentityLinkIntentStore` + +Recommended initial backend for Kubernetes-native deployments: + +- store records in the control namespace, +- derive object names from a hash of `(issuer, provider, subject)`, +- keep raw `subject` out of object names, +- store advisory metadata separately from canonical key fields, +- garbage-collect expired intents opportunistically or through a small cleanup + loop. + +For deployments without a dedicated database, a control-namespace store is +acceptable for v1. The store interface keeps open the option of a SQL or KV +backend later without changing the public API. + +## Security Requirements + +- Never use email, display name, or handle as the canonical identity key. +- Never allow fuzzy resolution. +- Require short TTLs on link intents. +- Treat claim URLs as one-time artifacts. +- Support optional `userCode` confirmation for higher assurance. +- Keep issuer namespace boundaries strict. +- Record actor ID, owner ID, identity tuple, verification mode, and timestamps + in audit logs. + +## Backward Compatibility + +The stable migration path is: + +1. Keep `ownerId` as-is. +2. Add `ownerRef`. +3. Prefer `ownerRef` for new integrations. +4. Keep direct owner-ID creates for existing clients. + +This avoids a breaking change to current provisioner clients while giving new +integrations a portable path that does not leak internal user identifiers. + +## Validation + +The architecture is complete when Spritz can demonstrate all of these flows: + +1. A service principal creates a link intent for an unresolved external + identity. +2. A human logs in and claims the link. +3. A service principal resolves the identity and receives the correct owner. +4. A service principal provisions a workspace using `ownerRef.type=external`. +5. An unlinked identity fails create with a typed conflict. +6. A revoked link no longer resolves. +7. Cross-issuer lookup is denied. +8. Duplicate claim attempts remain idempotent for the same owner and conflict + for different owners. + +## Recommended Sequencing + +### Phase 1 - Core resource model + +- Define `ExternalIdentityRef`, durable link records, and link intent records. +- Implement store interfaces. +- Implement resolve, create-intent, and claim flows. + +### Phase 2 - Provisioning integration + +- Add `ownerRef` to create requests. +- Keep `ownerId` compatibility. +- Add typed unresolved-link errors. + +### Phase 3 - Human management + +- Add list-self-links and unlink flows. +- Add audit-friendly UI surfaces. + +### Phase 4 - Optional stronger verification + +- Add provider-specific verification overlays where deployments need them. +- Keep the stable core contract unchanged. + +## References + +- [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) +- [2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md) From c5339b897a564ea75668742f674d4d5cb0a3490b Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 20:05:33 +0100 Subject: [PATCH 02/17] docs: make msteams the canonical example --- ...ernal-identity-linking-api-architecture.md | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/docs/2026-03-12-external-identity-linking-api-architecture.md b/docs/2026-03-12-external-identity-linking-api-architecture.md index 11b991a..db1cdbd 100644 --- a/docs/2026-03-12-external-identity-linking-api-architecture.md +++ b/docs/2026-03-12-external-identity-linking-api-architecture.md @@ -16,9 +16,9 @@ paste a Spritz-specific owner ID. Typical examples include: -- a Discord bot, -- a Slack app, - a Microsoft Teams assistant, +- a Slack app, +- a Discord bot, - a Google Chat integration, - a Mattermost automation. @@ -42,8 +42,8 @@ and the portable auth model defined in action-specific API. - Require explicit human confirmation before an external identity is bound to a Spritz owner. -- Keep the create/provision flow stable across Discord, Slack, Teams, and - future providers. +- Keep the create/provision flow stable across Microsoft Teams, Slack, + Discord, and future providers. - Align the identity model with widely used federation concepts such as issuer-plus-subject identifiers. - Preserve current explicit ownership and lifecycle guarantees. @@ -55,7 +55,8 @@ and the portable auth model defined in - Letting service principals search arbitrary users by email, name, or handle. - Letting an external system claim a user account without a human confirmation step. -- Embedding Discord-, Slack-, or deployment-specific logic in Spritz core. +- Embedding Microsoft Teams-, Slack-, or deployment-specific logic in Spritz + core. - Requiring a provider-specific verification protocol in the core API. ## Design Principles @@ -96,8 +97,8 @@ The stable API should keep linking explicit so that: ### Providers stay pluggable -Spritz should not require Discord-specific or Slack-specific fields in canonical -records. +Spritz should not require Microsoft Teams-specific or Slack-specific fields in +canonical records. Provider-specific metadata should remain optional, opaque, and advisory. @@ -130,8 +131,8 @@ This is the same owner model used by existing UI and provisioner flows. An external identity is an opaque tuple: - `issuer`: the namespace that controls the identity binding authority, -- `provider`: the external system category such as `discord`, `slack`, or - `teams`, +- `provider`: the external system category such as `msteams`, `slack`, or + `discord`, - `subject`: the stable provider-specific user identifier. ### External identity link @@ -157,8 +158,8 @@ The identity tuple is: ```json { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" } ``` @@ -183,8 +184,8 @@ resolving identities in another integration's namespace. ```json { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" } ``` @@ -203,8 +204,8 @@ or { "type": "external", "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" } ``` @@ -217,8 +218,8 @@ Recommended response shape: "id": "eil_01j...", "identity": { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" }, "ownerId": "user-123", "status": "active", @@ -240,8 +241,8 @@ Recommended response shape: "id": "eili_01j...", "identity": { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" }, "status": "pending", "claimUrl": "https://spritz.example.com/link/eili_01j.../claim", @@ -266,8 +267,8 @@ Request: { "identity": { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" } } ``` @@ -307,15 +308,15 @@ Request: { "identity": { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" }, "metadata": { "displayName": "onur" }, "ttl": "15m", "requireUserCode": true, - "clientState": "discord-interaction-123" + "clientState": "msteams-turn-context-123" } ``` @@ -412,11 +413,11 @@ Recommended request: "ownerRef": { "type": "external", "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" }, "presetId": "openclaw", - "idempotencyKey": "discord-123" + "idempotencyKey": "msteams-123" } ``` @@ -435,8 +436,8 @@ Recommended typed error: "error": "external_identity_unlinked", "identity": { "issuer": "support-bot", - "provider": "discord", - "subject": "123456789012345678" + "provider": "msteams", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" } } ``` From 8de9126d312d62b12f6396f0f952475603720ed4 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 20:29:47 +0100 Subject: [PATCH 03/17] docs: reframe identity architecture around resolution --- README.md | 2 +- ...ernal-identity-linking-api-architecture.md | 597 ------------------ ...al-identity-resolution-api-architecture.md | 421 ++++++++++++ 3 files changed, 422 insertions(+), 598 deletions(-) delete mode 100644 docs/2026-03-12-external-identity-linking-api-architecture.md create mode 100644 docs/2026-03-12-external-identity-resolution-api-architecture.md diff --git a/README.md b/README.md index 6781ee2..d259b9d 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ The backend inside a workspace is pluggable. OpenClaw is one example runtime. An > Spritz is in active development and should be treated as alpha software. APIs, CRDs, Helm values, and UI details may still change while the deployment model is being hardened. -[Deployment Spec](docs/2026-02-24-simplest-spritz-deployment-spec.md) · [ACP Architecture](docs/2026-03-09-acp-port-and-agent-chat-architecture.md) · [Portable Auth](docs/2026-02-24-portable-authentication-and-account-architecture.md) · [External Provisioners](docs/2026-03-11-external-provisioner-and-service-principal-architecture.md) · [External Identity Links](docs/2026-03-12-external-identity-linking-api-architecture.md) · [OpenClaw Integration](OPENCLAW.md) +[Deployment Spec](docs/2026-02-24-simplest-spritz-deployment-spec.md) · [ACP Architecture](docs/2026-03-09-acp-port-and-agent-chat-architecture.md) · [Portable Auth](docs/2026-02-24-portable-authentication-and-account-architecture.md) · [External Provisioners](docs/2026-03-11-external-provisioner-and-service-principal-architecture.md) · [External Identity Resolution](docs/2026-03-12-external-identity-resolution-api-architecture.md) · [OpenClaw Integration](OPENCLAW.md) ## Vision diff --git a/docs/2026-03-12-external-identity-linking-api-architecture.md b/docs/2026-03-12-external-identity-linking-api-architecture.md deleted file mode 100644 index db1cdbd..0000000 --- a/docs/2026-03-12-external-identity-linking-api-architecture.md +++ /dev/null @@ -1,597 +0,0 @@ ---- -date: 2026-03-12 -author: Spritz Maintainers -title: External Identity Linking API Architecture -tags: [spritz, auth, provisioning, identity, api, architecture] ---- - -## Overview - -This document defines the long-term stable Spritz API for linking external -identities to Spritz owners. - -The goal is to let external systems such as chat bots, workflow engines, or -support tools identify a human user without requiring that user to manually -paste a Spritz-specific owner ID. - -Typical examples include: - -- a Microsoft Teams assistant, -- a Slack app, -- a Discord bot, -- a Google Chat integration, -- a Mattermost automation. - -Spritz must stay backend-agnostic and deployment-agnostic. It must not learn -provider-specific user models from any particular product deployment. - -Instead, Spritz should standardize a generic external identity model and own -the lifecycle for linking those identities to normal Spritz owners. - -This architecture extends, but does not replace, the external provisioner model -defined in [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) -and the portable auth model defined in -[2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md). - -## Goals - -- Remove the need for humans to manually provide Spritz owner IDs to external - systems. -- Keep Spritz core portable and free of provider-specific assumptions. -- Let external provisioners resolve a human owner through a narrow, - action-specific API. -- Require explicit human confirmation before an external identity is bound to a - Spritz owner. -- Keep the create/provision flow stable across Microsoft Teams, Slack, - Discord, and future providers. -- Align the identity model with widely used federation concepts such as - issuer-plus-subject identifiers. -- Preserve current explicit ownership and lifecycle guarantees. - -## Non-goals - -- Turning Spritz into a general identity provider. -- Replacing OIDC, OAuth2, SAML, or SCIM in external systems. -- Letting service principals search arbitrary users by email, name, or handle. -- Letting an external system claim a user account without a human confirmation - step. -- Embedding Microsoft Teams-, Slack-, or deployment-specific logic in Spritz - core. -- Requiring a provider-specific verification protocol in the core API. - -## Design Principles - -### External identities are references, not sessions - -An external identity is not a login session and not an authorization grant. - -It is a stable reference that can later resolve to a Spritz owner. - -### Spritz owns the binding - -External systems may identify a candidate external subject, but only Spritz may -bind that subject to a human owner after the human authenticates through the -normal Spritz browser path. - -### Stable identity keys use namespace plus subject - -The canonical identity key is: - -- `issuer` -- `provider` -- `subject` - -This follows the same broad model as federated identity systems that treat -issuer-plus-subject as the stable user key. - -### Linking must be explicit - -Provisioning and linking are related, but they are not the same operation. - -The stable API should keep linking explicit so that: - -- create requests stay idempotent, -- unresolved identities are a well-defined precondition failure, -- link creation remains auditable, -- provider-specific UX can evolve without breaking the create contract. - -### Providers stay pluggable - -Spritz should not require Microsoft Teams-specific or Slack-specific fields in -canonical records. - -Provider-specific metadata should remain optional, opaque, and advisory. - -## Standards Alignment - -Spritz should align with existing standards where practical: - -- Use issuer-plus-subject semantics similar to OIDC federated identities. -- Treat display name, email, and handle as advisory metadata rather than stable - identifiers. -- Treat external identity scoping similarly to SCIM `externalId` semantics: - identifiers are scoped to the external provisioning domain, not globally - portable by assumption. - -Spritz does not need to implement a general OIDC account linking protocol in -core. It needs a portable control-plane API that uses those same identity -principles. - -## Canonical Terms - -### Spritz owner - -A Spritz owner is the normal human principal that owns and later accesses a -workspace. - -This is the same owner model used by existing UI and provisioner flows. - -### External identity - -An external identity is an opaque tuple: - -- `issuer`: the namespace that controls the identity binding authority, -- `provider`: the external system category such as `msteams`, `slack`, or - `discord`, -- `subject`: the stable provider-specific user identifier. - -### External identity link - -A durable binding from one external identity to one Spritz owner. - -### External identity link intent - -A short-lived, one-time claim artifact created by a service principal and later -claimed by a human in the browser to create or confirm a durable link. - -### Owner reference - -A stable create-time abstraction that allows Spritz to accept either: - -- a direct owner ID, or -- a linked external identity reference. - -## Identity Namespace Model - -The identity tuple is: - -```json -{ - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" -} -``` - -Rules: - -- `issuer` MUST be a stable namespace controlled by the service principal or - integration owner. -- `provider` MUST be a normalized lower-case token. -- `subject` MUST be treated as an opaque string. -- The tuple `(issuer, provider, subject)` MUST be globally unique within a - Spritz deployment. -- One tuple MUST resolve to at most one active Spritz owner at a time. -- One owner MAY have multiple linked external identities. - -The `issuer` field is critical. It prevents one integration from asserting or -resolving identities in another integration's namespace. - -## Core Resources - -### ExternalIdentityRef - -```json -{ - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" -} -``` - -### OwnerRef - -```json -{ - "type": "owner", - "id": "user-123" -} -``` - -or - -```json -{ - "type": "external", - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" -} -``` - -### ExternalIdentityLink - -Recommended response shape: - -```json -{ - "id": "eil_01j...", - "identity": { - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - }, - "ownerId": "user-123", - "status": "active", - "verifiedAt": "2026-03-12T15:00:00Z", - "verificationMode": "delivery", - "createdAt": "2026-03-12T15:00:00Z", - "metadata": { - "displayName": "onur" - } -} -``` - -### ExternalIdentityLinkIntent - -Recommended response shape: - -```json -{ - "id": "eili_01j...", - "identity": { - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - }, - "status": "pending", - "claimUrl": "https://spritz.example.com/link/eili_01j.../claim", - "userCode": "7J4K-9P", - "expiresAt": "2026-03-12T15:15:00Z", - "createdAt": "2026-03-12T15:00:00Z" -} -``` - -`userCode` is optional but recommended. It gives deployments a stronger defense -against forwarded or copied links without making the core flow provider-specific. - -## Stable API Surface - -### 1. Resolve a linked identity - -`POST /api/external-identity-links/resolve` - -Request: - -```json -{ - "identity": { - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - } -} -``` - -Success when linked: - -```json -{ - "linked": true, - "link": { - "id": "eil_01j...", - "ownerId": "user-123", - "status": "active", - "verifiedAt": "2026-03-12T15:00:00Z" - } -} -``` - -Success when unlinked: - -```json -{ - "linked": false -} -``` - -This endpoint MUST match exact tuples only. It MUST NOT perform fuzzy matching -by email, display name, or handle. - -### 2. Create a link intent - -`POST /api/external-identity-link-intents` - -Request: - -```json -{ - "identity": { - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - }, - "metadata": { - "displayName": "onur" - }, - "ttl": "15m", - "requireUserCode": true, - "clientState": "msteams-turn-context-123" -} -``` - -Response: - -```json -{ - "id": "eili_01j...", - "status": "pending", - "claimUrl": "https://spritz.example.com/link/eili_01j.../claim", - "userCode": "7J4K-9P", - "expiresAt": "2026-03-12T15:15:00Z" -} -``` - -Properties: - -- Creating an intent MUST be idempotent for the same service principal and - unresolved identity while the active intent remains pending. -- If the identity is already linked, the API SHOULD return the existing active - link rather than creating a new intent. -- Intent creation MUST NOT silently relink an identity that is already linked - to a different owner. - -### 3. Inspect a link intent - -`GET /api/external-identity-link-intents/{intentId}` - -This endpoint exists so an external client can poll intent state without -needing browser callbacks. - -Response status values: - -- `pending` -- `claimed` -- `expired` -- `cancelled` - -### 4. Claim a link intent as a human - -`POST /api/external-identity-link-intents/{intentId}/claim` - -This endpoint requires normal human browser authentication. - -Request: - -```json -{ - "userCode": "7J4K-9P" -} -``` - -Behavior: - -- The authenticated human becomes the target owner. -- If the identity is not yet linked, Spritz creates the durable link. -- If it is already linked to the same owner, the operation is idempotent. -- If it is already linked to a different owner, Spritz returns conflict. -- Successful claim marks the intent as `claimed`. - -### 5. List current human links - -`GET /api/external-identity-links` - -Default behavior for human principals: - -- return only the links owned by the current principal. - -Default behavior for service principals: - -- not allowed unless explicitly granted a list scope. - -### 6. Unlink - -`DELETE /api/external-identity-links/{linkId}` - -Rules: - -- Human principals may unlink their own links. -- Service principals may not unlink by default. -- Admins may unlink as break-glass. -- Unlinking affects future resolution only. It MUST NOT mutate the ownership of - existing workspaces. - -## Provisioning Integration - -The create API should add `ownerRef` while preserving existing `ownerId` -compatibility. - -Recommended request: - -```json -{ - "ownerRef": { - "type": "external", - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - }, - "presetId": "openclaw", - "idempotencyKey": "msteams-123" -} -``` - -Rules: - -- `ownerId` and `ownerRef` MUST be mutually exclusive. -- If `ownerRef.type=owner`, Spritz uses the explicit owner ID. -- If `ownerRef.type=external`, Spritz MUST resolve the link before create. -- If the external identity is unlinked, create MUST fail with a typed conflict. -- Create MUST NOT implicitly create link intents in the stable v1 contract. - -Recommended typed error: - -```json -{ - "error": "external_identity_unlinked", - "identity": { - "issuer": "support-bot", - "provider": "msteams", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" - } -} -``` - -This keeps create semantics clean and preserves the idempotency guarantees from -the current provisioner architecture. - -## Verification Modes - -The core API should define a generic `verificationMode` field on durable links -and intents. - -Stable modes: - -- `delivery`: the external system delivered the claim artifact out-of-band to - the target subject. -- `provider_oauth`: the link was completed after provider-specific external - OAuth verification. -- `attested`: a trusted external bridge supplied an additional signed - verification signal. - -The core contract only requires `delivery`. - -This keeps the API portable while allowing stronger provider-specific -verification in overlays or extensions later. - -## Authorization Model - -Recommended new service scopes: - -- `spritz.external_identities.resolve` -- `spritz.external_identities.link_intents.create` - -Recommended human capabilities: - -- claim own pending intents, -- list own links, -- unlink own links. - -Recommended admin capabilities: - -- inspect links across owners, -- revoke links, -- relink by explicit override. - -Service principals MUST be scoped to their own `issuer` namespace unless -explicitly granted broader authority. - -## State Model - -### Link intent states - -- `pending` -- `claimed` -- `expired` -- `cancelled` - -### Durable link states - -- `active` -- `revoked` - -State transition rules: - -- `pending -> claimed` when a human successfully completes the claim. -- `pending -> expired` when TTL elapses. -- `active -> revoked` when owner or admin unlinks. -- `revoked` links MUST NOT resolve for future creates. - -## Storage and Implementation Strategy - -The API contract should not depend on a specific storage backend. - -Spritz should define two internal store interfaces: - -- `ExternalIdentityLinkStore` -- `ExternalIdentityLinkIntentStore` - -Recommended initial backend for Kubernetes-native deployments: - -- store records in the control namespace, -- derive object names from a hash of `(issuer, provider, subject)`, -- keep raw `subject` out of object names, -- store advisory metadata separately from canonical key fields, -- garbage-collect expired intents opportunistically or through a small cleanup - loop. - -For deployments without a dedicated database, a control-namespace store is -acceptable for v1. The store interface keeps open the option of a SQL or KV -backend later without changing the public API. - -## Security Requirements - -- Never use email, display name, or handle as the canonical identity key. -- Never allow fuzzy resolution. -- Require short TTLs on link intents. -- Treat claim URLs as one-time artifacts. -- Support optional `userCode` confirmation for higher assurance. -- Keep issuer namespace boundaries strict. -- Record actor ID, owner ID, identity tuple, verification mode, and timestamps - in audit logs. - -## Backward Compatibility - -The stable migration path is: - -1. Keep `ownerId` as-is. -2. Add `ownerRef`. -3. Prefer `ownerRef` for new integrations. -4. Keep direct owner-ID creates for existing clients. - -This avoids a breaking change to current provisioner clients while giving new -integrations a portable path that does not leak internal user identifiers. - -## Validation - -The architecture is complete when Spritz can demonstrate all of these flows: - -1. A service principal creates a link intent for an unresolved external - identity. -2. A human logs in and claims the link. -3. A service principal resolves the identity and receives the correct owner. -4. A service principal provisions a workspace using `ownerRef.type=external`. -5. An unlinked identity fails create with a typed conflict. -6. A revoked link no longer resolves. -7. Cross-issuer lookup is denied. -8. Duplicate claim attempts remain idempotent for the same owner and conflict - for different owners. - -## Recommended Sequencing - -### Phase 1 - Core resource model - -- Define `ExternalIdentityRef`, durable link records, and link intent records. -- Implement store interfaces. -- Implement resolve, create-intent, and claim flows. - -### Phase 2 - Provisioning integration - -- Add `ownerRef` to create requests. -- Keep `ownerId` compatibility. -- Add typed unresolved-link errors. - -### Phase 3 - Human management - -- Add list-self-links and unlink flows. -- Add audit-friendly UI surfaces. - -### Phase 4 - Optional stronger verification - -- Add provider-specific verification overlays where deployments need them. -- Keep the stable core contract unchanged. - -## References - -- [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) -- [2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md new file mode 100644 index 0000000..3d72265 --- /dev/null +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -0,0 +1,421 @@ +--- +date: 2026-03-12 +author: Spritz Maintainers +title: External Identity Resolution API Architecture +tags: [spritz, auth, provisioning, identity, api, architecture] +--- + +## Overview + +This document defines the long-term stable Spritz architecture for resolving an +external identity to a Spritz owner during provisioning. + +In this model, Spritz does not own account linking. Spritz consumes an external +identity resolver that is already authoritative for mappings such as: + +- Microsoft Teams user -> product user +- Slack user -> product user +- Discord user -> product user + +Spritz stays provider-agnostic and product-agnostic. It accepts a normalized +external owner reference, derives the caller namespace from authentication, and +asks a configured resolver which Spritz owner should own the workspace. + +This architecture extends the external provisioner model defined in +[2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) +and the portable auth model defined in +[2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md). + +## Goals + +- Let external systems provision for a human without exposing manual owner-ID + lookup UX. +- Keep Spritz core portable and free of product-specific account-linking logic. +- Make the public create API stable across Microsoft Teams, Slack, Discord, and + future providers. +- Keep ownership resolution narrow, explicit, auditable, and deterministic. +- Avoid leaking internal owner IDs to external bots when they do not need them. +- Support enterprise providers where identity is tenant-scoped. + +## Non-goals + +- Turning Spritz into a general identity provider. +- Making Spritz the source of truth for account linking. +- Adding browser claim flows, link intents, or user-entered verification codes + to Spritz core. +- Letting service principals search users by email, display name, or handle. +- Embedding Microsoft Teams-specific or deployment-specific mapping logic in + Spritz core. + +## Design Principles + +### Spritz resolves, but does not link + +Spritz is a consumer of authoritative identity mappings, not the owner of those +mappings. + +If a deployment already knows which external user belongs to which product +account, Spritz should ask that system directly instead of recreating a second +linking database. + +### Caller namespace comes from authentication + +The integration namespace is derived from the authenticated service principal. + +The caller must not be allowed to spoof namespace selection by sending an +arbitrary `issuer` value in the request body. + +### External identity is opaque and provider-scoped + +Spritz treats provider user IDs as opaque strings. + +Canonicalization belongs to the provider adapter or external resolver, not to +Spritz core. + +### Enterprise providers need tenant scope + +For enterprise messaging systems, a provider user ID is often not globally +meaningful without tenant context. + +The public API must support an explicit tenant or realm field. + +### Create is the main contract + +The stable public contract should center on create-time owner resolution. + +Spritz may expose operational debug endpoints later, but the main integration +surface is still `POST /spritzes`. + +## Canonical Terms + +### Spritz owner + +The human principal that owns and later accesses a workspace. + +### External owner reference + +A provider-scoped, product-agnostic reference to a human in an external +messaging or workflow system. + +### Resolver namespace + +The namespace bound to the authenticated service principal. It tells Spritz +which external resolver configuration to use. + +### External identity resolver + +A deployment-owned authoritative system that maps an external owner reference +to a Spritz owner. + +## Core Model + +The public owner reference should support either a direct owner ID or an +external identity: + +```json +{ + "type": "owner", + "id": "user-123" +} +``` + +or + +```json +{ + "type": "external", + "provider": "msteams", + "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" +} +``` + +Rules: + +- `type` MUST be either `owner` or `external`. +- `provider` MUST be a normalized lower-case token. +- `subject` MUST be treated as an opaque string. +- `tenant` MAY be omitted only for providers where the subject is globally + stable without tenant scope. +- For `msteams`, `tenant` SHOULD be required by policy. +- The effective identity key inside Spritz is: + `resolverNamespace + provider + tenant + subject` +- `resolverNamespace` is derived from the authenticated service principal, not + from request payload. + +## High-Level Architecture + +Components: + +- external bot or automation +- Spritz API +- resolver configuration bound to the service principal +- deployment-owned external identity resolver + +Flow: + +1. The bot calls `POST /spritzes` with `ownerRef.type=external`. +2. Spritz authenticates the service principal. +3. Spritz derives the resolver namespace from that principal. +4. Spritz validates that the provider and tenant are allowed for that namespace. +5. Spritz calls the configured external resolver. +6. The resolver returns either `resolved`, `unresolved`, `forbidden`, or + `ambiguous`. +7. Spritz either provisions for the resolved owner or returns a typed error. + +## Stable Public API + +The create API should add `ownerRef` while preserving existing `ownerId` +compatibility. + +Recommended request: + +```json +{ + "ownerRef": { + "type": "external", + "provider": "msteams", + "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + }, + "presetId": "openclaw", + "idempotencyKey": "msteams-123" +} +``` + +Rules: + +- `ownerId` and `ownerRef` MUST be mutually exclusive. +- If `ownerRef.type=owner`, Spritz uses the explicit owner ID. +- If `ownerRef.type=external`, Spritz MUST resolve the owner through the + configured resolver before create. +- Create MUST NOT require the caller to know the internal owner ID. +- The caller MUST NOT provide `resolverNamespace` or `issuer` in the request + body. +- Spritz MUST use the resolver namespace bound to the authenticated principal. + +## External Resolver Contract + +Spritz core should define an internal resolver interface: + +- `ResolveExternalOwner(ctx, namespace, identity) -> result` + +Recommended canonical result states: + +- `resolved` +- `unresolved` +- `forbidden` +- `ambiguous` +- `unavailable` + +For HTTP-backed deployments, a portable resolver API can look like this: + +`POST /v1/external-owners/resolve` + +Request: + +```json +{ + "namespace": "support-bot", + "identity": { + "provider": "msteams", + "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + }, + "requestId": "req_01j..." +} +``` + +Response when resolved: + +```json +{ + "status": "resolved", + "ownerId": "user-123" +} +``` + +Response when unresolved: + +```json +{ + "status": "unresolved" +} +``` + +Response when forbidden: + +```json +{ + "status": "forbidden" +} +``` + +Response when ambiguous: + +```json +{ + "status": "ambiguous" +} +``` + +Properties: + +- The resolver MUST return at most one owner. +- The resolver MUST NOT perform fuzzy matching by email, name, or handle. +- The resolver is the source of truth for external-to-owner mappings. +- Spritz MAY cache results briefly, but the resolver remains authoritative. + +## Public Error Model + +When create is called with `ownerRef.type=external`, Spritz should return typed +errors rather than leaking resolver internals. + +Recommended errors: + +- `external_identity_unresolved` +- `external_identity_forbidden` +- `external_identity_ambiguous` +- `external_identity_provider_unsupported` +- `external_identity_resolution_unavailable` + +Recommended unresolved example: + +```json +{ + "error": "external_identity_unresolved", + "identity": { + "provider": "msteams", + "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", + "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + } +} +``` + +Recommended status semantics: + +- `external_identity_unresolved`: `409` +- `external_identity_forbidden`: `403` +- `external_identity_ambiguous`: `409` +- `external_identity_provider_unsupported`: `400` +- `external_identity_resolution_unavailable`: `503` + +## Authorization Model + +Recommended service capabilities: + +- `spritz.external_identities.resolve_via_create` + +Optional future capability: + +- `spritz.external_identities.check` + +Rules: + +- Resolver namespace MUST be bound to the authenticated service principal. +- A service principal MUST be allowed to resolve only within its own namespace + unless explicitly granted broader authority. +- Provider allowlists and tenant allowlists SHOULD be policy-controlled per + namespace. + +## Caching and Consistency + +Spritz should treat the external resolver as the source of truth. + +Safe default behavior: + +- no durable copy of identity mappings in Spritz core +- optional short positive cache +- optional shorter negative cache +- cache key includes namespace, provider, tenant, and subject + +If a mapping changes in the external system, future creates should follow the +new resolver answer after cache expiry. + +Existing workspaces keep their existing owner. Resolution affects only future +create operations. + +## Microsoft Teams Guidance + +The first concrete provider should be `msteams`. + +Provider guidance: + +- `provider` value: `msteams` +- `tenant` SHOULD be the Microsoft tenant identifier and SHOULD be required by + policy +- `subject` MUST be the stable Teams user identifier chosen by the deployment's + Teams integration + +Spritz core should not attempt to normalize Teams identifiers itself. The Teams +adapter or authoritative resolver must choose one canonical subject format and +keep it consistent. + +## Optional Future Extension + +If a deployment later wants lower latency or fewer synchronous dependencies, it +may add a signed owner assertion flow on top of this architecture. + +That would allow a trusted external resolver to mint a short-lived signed +assertion that Spritz can verify locally during create. + +This is an extension, not part of the stable v1 contract. The stable core +contract should remain synchronous external resolution through a configured +resolver. + +## Backward Compatibility + +The stable migration path is: + +1. Keep `ownerId` as-is. +2. Add `ownerRef`. +3. Add resolver-backed handling for `ownerRef.type=external`. +4. Keep direct owner-ID creates for existing clients. + +This avoids a breaking change to current provisioner clients while giving new +integrations a portable path that does not require manual owner-ID exchange. + +## Validation + +The architecture is complete when Spritz can demonstrate all of these flows: + +1. A service principal provisions successfully with `ownerRef.type=external` + for a resolved Microsoft Teams user. +2. An unresolved external identity fails with `external_identity_unresolved`. +3. A forbidden namespace or provider fails with `external_identity_forbidden`. +4. An ambiguous resolver answer fails with `external_identity_ambiguous`. +5. A resolver outage fails with `external_identity_resolution_unavailable`. +6. A positive cache hit still provisions for the same owner. +7. A mapping change in the external resolver affects future creates after cache + expiry. + +## Recommended Sequencing + +### Phase 1 - Public API and internal abstraction + +- Add `ownerRef` to create requests. +- Derive resolver namespace from authenticated service principal. +- Define the internal resolver interface and typed result states. + +### Phase 2 - Resolver-backed provisioning + +- Implement the HTTP resolver adapter. +- Add provider and tenant policy validation. +- Add typed create-time error mapping. + +### Phase 3 - Reliability and observability + +- Add short TTL caching. +- Add metrics and structured audit logs for resolution attempts. +- Add timeout, retry, and circuit-breaker policy. + +### Phase 4 - Optional assertion-based optimization + +- Add signed owner assertions only if synchronous resolution becomes a proven + bottleneck. +- Keep the public `ownerRef` contract unchanged. + +## References + +- [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) +- [2026-02-24-portable-authentication-and-account-architecture.md](2026-02-24-portable-authentication-and-account-architecture.md) From 95a9907f65badbff916c5138406e2af33fea104d Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 20:44:09 +0100 Subject: [PATCH 04/17] docs: clarify bot-only external identity flow --- ...al-identity-resolution-api-architecture.md | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md index 3d72265..df67149 100644 --- a/docs/2026-03-12-external-identity-resolution-api-architecture.md +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -21,6 +21,10 @@ Spritz stays provider-agnostic and product-agnostic. It accepts a normalized external owner reference, derives the caller namespace from authentication, and asks a configured resolver which Spritz owner should own the workspace. +The external caller sends only the external platform identity it already has, +such as a Microsoft Teams user ID. Internal Spritz owner identifiers remain a +backend concern and do not need to cross the bot boundary. + This architecture extends the external provisioner model defined in [2026-03-11-external-provisioner-and-service-principal-architecture.md](2026-03-11-external-provisioner-and-service-principal-architecture.md) and the portable auth model defined in @@ -34,7 +38,7 @@ and the portable auth model defined in - Make the public create API stable across Microsoft Teams, Slack, Discord, and future providers. - Keep ownership resolution narrow, explicit, auditable, and deterministic. -- Avoid leaking internal owner IDs to external bots when they do not need them. +- Ensure external bots never need to send or learn internal owner IDs. - Support enterprise providers where identity is tenant-scoped. ## Non-goals @@ -72,6 +76,14 @@ Spritz treats provider user IDs as opaque strings. Canonicalization belongs to the provider adapter or external resolver, not to Spritz core. +### Bots speak only in external IDs + +The bot should send only the provider identity it already knows, such as a +Teams or Discord user ID plus tenant when required. + +It should not collect, store, or forward internal Spritz owner IDs as part of +the normal provisioning path. + ### Enterprise providers need tenant scope For enterprise messaging systems, a provider user ID is often not globally @@ -154,7 +166,8 @@ Components: Flow: -1. The bot calls `POST /spritzes` with `ownerRef.type=external`. +1. The bot calls `POST /spritzes` with `ownerRef.type=external` and the normal + external user identity it already has. 2. Spritz authenticates the service principal. 3. Spritz derives the resolver namespace from that principal. 4. Spritz validates that the provider and tenant are allowed for that namespace. @@ -163,6 +176,9 @@ Flow: `ambiguous`. 7. Spritz either provisions for the resolved owner or returns a typed error. +The bot never handles the internal owner ID directly. That translation happens +inside the trusted backend path between Spritz and the resolver. + ## Stable Public API The create API should add `ownerRef` while preserving existing `ownerId` @@ -189,10 +205,13 @@ Rules: - If `ownerRef.type=owner`, Spritz uses the explicit owner ID. - If `ownerRef.type=external`, Spritz MUST resolve the owner through the configured resolver before create. +- New bot and automation integrations SHOULD use `ownerRef.type=external`. - Create MUST NOT require the caller to know the internal owner ID. - The caller MUST NOT provide `resolverNamespace` or `issuer` in the request body. - Spritz MUST use the resolver namespace bound to the authenticated principal. +- Spritz MUST keep the resolved internal owner ID on the backend side of the + create flow. ## External Resolver Contract @@ -265,6 +284,8 @@ Properties: - The resolver MUST NOT perform fuzzy matching by email, name, or handle. - The resolver is the source of truth for external-to-owner mappings. - Spritz MAY cache results briefly, but the resolver remains authoritative. +- The resolved owner ID is an internal backend value for Spritz and does not + need to be exposed to the bot. ## Public Error Model @@ -370,7 +391,8 @@ The stable migration path is: 1. Keep `ownerId` as-is. 2. Add `ownerRef`. 3. Add resolver-backed handling for `ownerRef.type=external`. -4. Keep direct owner-ID creates for existing clients. +4. Keep direct owner-ID creates only for existing clients that already use them. +5. Move new bot integrations to external-ID-only create requests. This avoids a breaking change to current provisioner clients while giving new integrations a portable path that does not require manual owner-ID exchange. @@ -388,6 +410,7 @@ The architecture is complete when Spritz can demonstrate all of these flows: 6. A positive cache hit still provisions for the same owner. 7. A mapping change in the external resolver affects future creates after cache expiry. +8. The bot never sends an internal owner ID during the normal provisioning path. ## Recommended Sequencing @@ -396,12 +419,14 @@ The architecture is complete when Spritz can demonstrate all of these flows: - Add `ownerRef` to create requests. - Derive resolver namespace from authenticated service principal. - Define the internal resolver interface and typed result states. +- Make external-ID-only bot calls the preferred integration path. ### Phase 2 - Resolver-backed provisioning - Implement the HTTP resolver adapter. - Add provider and tenant policy validation. - Add typed create-time error mapping. +- Keep resolved owner IDs internal to Spritz after resolver lookup. ### Phase 3 - Reliability and observability From 43c083e6d9cc1933f8e229384a23a2777df68ead Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 20:59:06 +0100 Subject: [PATCH 05/17] docs: move nonessential resolver features to future work --- ...al-identity-resolution-api-architecture.md | 56 ++++++------------- 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md index df67149..dc78932 100644 --- a/docs/2026-03-12-external-identity-resolution-api-architecture.md +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -283,7 +283,6 @@ Properties: - The resolver MUST return at most one owner. - The resolver MUST NOT perform fuzzy matching by email, name, or handle. - The resolver is the source of truth for external-to-owner mappings. -- Spritz MAY cache results briefly, but the resolver remains authoritative. - The resolved owner ID is an internal backend value for Spritz and does not need to be exposed to the bot. @@ -327,10 +326,6 @@ Recommended service capabilities: - `spritz.external_identities.resolve_via_create` -Optional future capability: - -- `spritz.external_identities.check` - Rules: - Resolver namespace MUST be bound to the authenticated service principal. @@ -339,19 +334,18 @@ Rules: - Provider allowlists and tenant allowlists SHOULD be policy-controlled per namespace. -## Caching and Consistency +## V1 Consistency Model Spritz should treat the external resolver as the source of truth. -Safe default behavior: +Safe default behavior for v1: - no durable copy of identity mappings in Spritz core -- optional short positive cache -- optional shorter negative cache -- cache key includes namespace, provider, tenant, and subject +- no cache required +- synchronous resolution during create If a mapping changes in the external system, future creates should follow the -new resolver answer after cache expiry. +current resolver answer. Existing workspaces keep their existing owner. Resolution affects only future create operations. @@ -372,18 +366,6 @@ Spritz core should not attempt to normalize Teams identifiers itself. The Teams adapter or authoritative resolver must choose one canonical subject format and keep it consistent. -## Optional Future Extension - -If a deployment later wants lower latency or fewer synchronous dependencies, it -may add a signed owner assertion flow on top of this architecture. - -That would allow a trusted external resolver to mint a short-lived signed -assertion that Spritz can verify locally during create. - -This is an extension, not part of the stable v1 contract. The stable core -contract should remain synchronous external resolution through a configured -resolver. - ## Backward Compatibility The stable migration path is: @@ -407,10 +389,7 @@ The architecture is complete when Spritz can demonstrate all of these flows: 3. A forbidden namespace or provider fails with `external_identity_forbidden`. 4. An ambiguous resolver answer fails with `external_identity_ambiguous`. 5. A resolver outage fails with `external_identity_resolution_unavailable`. -6. A positive cache hit still provisions for the same owner. -7. A mapping change in the external resolver affects future creates after cache - expiry. -8. The bot never sends an internal owner ID during the normal provisioning path. +6. The bot never sends an internal owner ID during the normal provisioning path. ## Recommended Sequencing @@ -428,17 +407,18 @@ The architecture is complete when Spritz can demonstrate all of these flows: - Add typed create-time error mapping. - Keep resolved owner IDs internal to Spritz after resolver lookup. -### Phase 3 - Reliability and observability - -- Add short TTL caching. -- Add metrics and structured audit logs for resolution attempts. -- Add timeout, retry, and circuit-breaker policy. - -### Phase 4 - Optional assertion-based optimization - -- Add signed owner assertions only if synchronous resolution becomes a proven - bottleneck. -- Keep the public `ownerRef` contract unchanged. +## Potential Future Features + +- A separate `spritz.external_identities.check` capability or debug endpoint if + deployments later need preflight inspection outside create. +- A short positive cache if synchronous resolver lookups become a proven + performance bottleneck. Avoid negative caching unless there is a demonstrated + need. +- Additional reliability controls such as timeout tuning, retries, + circuit-breakers, and richer resolution metrics. +- A signed owner assertion flow if a deployment later wants Spritz to verify a + short-lived resolver-signed identity result locally instead of performing a + live resolver call during every create. ## References From 6bb31224ba73038cd37fa5db18c938b61d4c098b Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 21:22:52 +0100 Subject: [PATCH 06/17] docs: add spz owner input modes --- ...al-identity-resolution-api-architecture.md | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md index dc78932..3e97d93 100644 --- a/docs/2026-03-12-external-identity-resolution-api-architecture.md +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -213,6 +213,40 @@ Rules: - Spritz MUST keep the resolved internal owner ID on the backend side of the create flow. +## CLI Shape + +`spz` should support both direct-owner and external-identity create flows. + +Direct owner form: + +```bash +spz create --owner-id user-123 --preset openclaw +``` + +External identity form: + +```bash +spz create --owner-provider discord --owner-subject 123456789012345678 --preset openclaw +``` + +Tenant-scoped external identity form: + +```bash +spz create --owner-provider msteams \ + --owner-tenant 72f988bf-86f1-41af-91ab-2d7cd011db47 \ + --owner-subject 29:1A2BcD3EfG4HiJ5KlM6NoP \ + --preset openclaw +``` + +CLI rules: + +- `--owner-id` and external owner flags MUST be mutually exclusive. +- `spz` should build `ownerRef.type=external` when external owner flags are + used. +- `spz` should require `--owner-tenant` for providers that are tenant-scoped by + policy. +- `spz` should keep supporting `--owner-id` for direct internal and admin use. + ## External Resolver Contract Spritz core should define an internal resolver interface: @@ -390,6 +424,8 @@ The architecture is complete when Spritz can demonstrate all of these flows: 4. An ambiguous resolver answer fails with `external_identity_ambiguous`. 5. A resolver outage fails with `external_identity_resolution_unavailable`. 6. The bot never sends an internal owner ID during the normal provisioning path. +7. `spz` accepts either direct owner ID input or external identity input, but + rejects requests that try to send both. ## Recommended Sequencing @@ -399,6 +435,8 @@ The architecture is complete when Spritz can demonstrate all of these flows: - Derive resolver namespace from authenticated service principal. - Define the internal resolver interface and typed result states. - Make external-ID-only bot calls the preferred integration path. +- Add `spz` flags for external owner input while keeping direct `--owner-id` + support. ### Phase 2 - Resolver-backed provisioning From 49cf988d8b3e930c36b892e7cbb010f49b98ac8c Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 21:29:15 +0100 Subject: [PATCH 07/17] docs: specify production resolver design --- ...al-identity-resolution-api-architecture.md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md index 3e97d93..bfb11e7 100644 --- a/docs/2026-03-12-external-identity-resolution-api-architecture.md +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -261,6 +261,62 @@ Recommended canonical result states: - `ambiguous` - `unavailable` +Recommended internal shape: + +```go +type ExternalOwnerResolver interface { + ResolveExternalOwner(ctx context.Context, principal principal, ref ExternalOwnerRef) (ExternalOwnerResolution, error) +} +``` + +The resolver is an internal Spritz dependency. The bot does not call it +directly. + +### Resolver policy binding + +Spritz should not let the caller choose which resolver to trust. + +Instead, each authenticated service principal should be bound to resolver +policy that defines: + +- resolver endpoint or adapter reference +- resolver authentication reference +- allowed providers +- allowed tenants +- resolver timeout + +That policy should be selected from the authenticated service principal +identity, not from request payload. + +### Resolver transport + +The recommended production default is one HTTP adapter inside Spritz that calls +the deployment's authoritative identity service. + +Why this is the right default: + +- it keeps the public Spritz API small +- it avoids duplicating identity state inside Spritz +- it lets deployments use an existing product API instead of building a second + control plane +- it preserves a strict backend-only trust boundary + +### Resolver authentication + +Spritz should authenticate to the resolver with its own backend credential. + +Recommended order of preference: + +1. workload identity or mTLS if the deployment already has it +2. a dedicated Spritz-to-resolver bearer token + +Rules: + +- Spritz MUST NOT forward the bot's bearer token to the resolver as the primary + trust mechanism. +- Resolver authentication is between Spritz and the resolver. +- The external bot authenticates only to Spritz. + For HTTP-backed deployments, a portable resolver API can look like this: `POST /v1/external-owners/resolve` @@ -320,6 +376,33 @@ Properties: - The resolved owner ID is an internal backend value for Spritz and does not need to be exposed to the bot. +### Create-time resolver behavior + +Resolution should happen during create request normalization, before any create +attempt reaches the normal provisioning transaction. + +Rules: + +- If resolution returns `resolved`, Spritz substitutes the resolved internal + owner ID and continues through the normal create path. +- If resolution returns `unresolved`, `forbidden`, `ambiguous`, or + `unavailable`, Spritz MUST fail the create before creating any resource. +- Service-principal create responses SHOULD omit `ownerId` once the owner was + resolved from an external identity. + +### Idempotency rule + +Resolver lookup should happen before a create is finalized, but successful +creates must still preserve normal Spritz idempotency guarantees. + +Rules: + +- Before the first successful create, Spritz may resolve again on retried + requests with the same idempotency key. +- After a successful create, retries with the same idempotency key MUST replay + the same created workspace even if the external mapping later changes. +- Resolver failure or timeout MUST leave no partially created workspace behind. + ## Public Error Model When create is called with `ownerRef.type=external`, Spritz should return typed @@ -367,6 +450,8 @@ Rules: unless explicitly granted broader authority. - Provider allowlists and tenant allowlists SHOULD be policy-controlled per namespace. +- Resolver selection and resolver credentials SHOULD be policy-controlled per + service principal. ## V1 Consistency Model @@ -426,6 +511,10 @@ The architecture is complete when Spritz can demonstrate all of these flows: 6. The bot never sends an internal owner ID during the normal provisioning path. 7. `spz` accepts either direct owner ID input or external identity input, but rejects requests that try to send both. +8. Service-principal create responses omit `ownerId` when the request used + `ownerRef.type=external`. +9. Retrying a successful create with the same idempotency key replays the same + workspace even if the resolver mapping later changes. ## Recommended Sequencing @@ -437,10 +526,12 @@ The architecture is complete when Spritz can demonstrate all of these flows: - Make external-ID-only bot calls the preferred integration path. - Add `spz` flags for external owner input while keeping direct `--owner-id` support. +- Add resolver policy binding to service-principal configuration. ### Phase 2 - Resolver-backed provisioning - Implement the HTTP resolver adapter. +- Add backend-only resolver authentication. - Add provider and tenant policy validation. - Add typed create-time error mapping. - Keep resolved owner IDs internal to Spritz after resolver lookup. From f30f37a841c98c0bb159f83a8c0a73f378a60b81 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 21:34:53 +0100 Subject: [PATCH 08/17] docs: lock resolver contract details --- ...al-identity-resolution-api-architecture.md | 104 +++++++++++++----- 1 file changed, 78 insertions(+), 26 deletions(-) diff --git a/docs/2026-03-12-external-identity-resolution-api-architecture.md b/docs/2026-03-12-external-identity-resolution-api-architecture.md index bfb11e7..7988541 100644 --- a/docs/2026-03-12-external-identity-resolution-api-architecture.md +++ b/docs/2026-03-12-external-identity-resolution-api-architecture.md @@ -114,6 +114,9 @@ messaging or workflow system. The namespace bound to the authenticated service principal. It tells Spritz which external resolver configuration to use. +In request and policy examples, this should be called `issuer` rather than +plain `namespace` to avoid confusion with Kubernetes namespaces. + ### External identity resolver A deployment-owned authoritative system that maps an external owner reference @@ -138,7 +141,7 @@ or "type": "external", "provider": "msteams", "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + "subject": "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f" } ``` @@ -149,11 +152,11 @@ Rules: - `subject` MUST be treated as an opaque string. - `tenant` MAY be omitted only for providers where the subject is globally stable without tenant scope. -- For `msteams`, `tenant` SHOULD be required by policy. +- For `msteams`, `tenant` MUST be required. - The effective identity key inside Spritz is: - `resolverNamespace + provider + tenant + subject` -- `resolverNamespace` is derived from the authenticated service principal, not - from request payload. + `issuer + provider + tenant + subject` +- `issuer` is derived from the authenticated service principal, not from + request payload. ## High-Level Architecture @@ -169,8 +172,8 @@ Flow: 1. The bot calls `POST /spritzes` with `ownerRef.type=external` and the normal external user identity it already has. 2. Spritz authenticates the service principal. -3. Spritz derives the resolver namespace from that principal. -4. Spritz validates that the provider and tenant are allowed for that namespace. +3. Spritz derives the resolver issuer from that principal. +4. Spritz validates that the provider and tenant are allowed for that issuer. 5. Spritz calls the configured external resolver. 6. The resolver returns either `resolved`, `unresolved`, `forbidden`, or `ambiguous`. @@ -192,7 +195,7 @@ Recommended request: "type": "external", "provider": "msteams", "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + "subject": "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f" }, "presetId": "openclaw", "idempotencyKey": "msteams-123" @@ -207,11 +210,12 @@ Rules: configured resolver before create. - New bot and automation integrations SHOULD use `ownerRef.type=external`. - Create MUST NOT require the caller to know the internal owner ID. -- The caller MUST NOT provide `resolverNamespace` or `issuer` in the request - body. -- Spritz MUST use the resolver namespace bound to the authenticated principal. +- The caller MUST NOT provide `issuer` in the request body. +- Spritz MUST use the resolver issuer bound to the authenticated principal. - Spritz MUST keep the resolved internal owner ID on the backend side of the create flow. +- `ownerRef.type=external` SHOULD be available only to service principals by + default. ## CLI Shape @@ -234,7 +238,7 @@ Tenant-scoped external identity form: ```bash spz create --owner-provider msteams \ --owner-tenant 72f988bf-86f1-41af-91ab-2d7cd011db47 \ - --owner-subject 29:1A2BcD3EfG4HiJ5KlM6NoP \ + --owner-subject 6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f \ --preset openclaw ``` @@ -251,7 +255,7 @@ CLI rules: Spritz core should define an internal resolver interface: -- `ResolveExternalOwner(ctx, namespace, identity) -> result` +- `ResolveExternalOwner(ctx, issuer, identity) -> result` Recommended canonical result states: @@ -284,6 +288,7 @@ policy that defines: - allowed providers - allowed tenants - resolver timeout +- issuer identifier for resolver calls That policy should be selected from the authenticated service principal identity, not from request payload. @@ -316,6 +321,8 @@ Rules: trust mechanism. - Resolver authentication is between Spritz and the resolver. - The external bot authenticates only to Spritz. +- The default resolver timeout SHOULD be 5 seconds. +- Spritz SHOULD perform no automatic retries in v1. For HTTP-backed deployments, a portable resolver API can look like this: @@ -325,11 +332,11 @@ Request: ```json { - "namespace": "support-bot", + "issuer": "support-bot", "identity": { "provider": "msteams", "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + "subject": "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f" }, "requestId": "req_01j..." } @@ -376,6 +383,30 @@ Properties: - The resolved owner ID is an internal backend value for Spritz and does not need to be exposed to the bot. +### Audit and persistence + +When Spritz creates a workspace from `ownerRef.type=external`, it should record +enough information for audit without leaking raw external identifiers in common +resource metadata. + +Recommended stored fields: + +- issuer +- provider +- tenant +- `subjectHash` +- actor principal ID +- resolution timestamp + +Rules: + +- Spritz SHOULD NOT store raw external `subject` values in Kubernetes labels. +- Spritz SHOULD NOT store raw external `subject` values in normal annotations. +- `subjectHash` SHOULD be an HMAC-SHA-256 derived from the external subject and + a deployment secret. +- If raw external subject values are needed for incident response, they should + appear only in secured audit logs. + ### Create-time resolver behavior Resolution should happen during create request normalization, before any create @@ -390,6 +421,19 @@ Rules: - Service-principal create responses SHOULD omit `ownerId` once the owner was resolved from an external identity. +Recommended service-principal response shape for `ownerRef.type=external`: + +- `spritz` +- `accessUrl` +- `chatUrl` +- `workspaceUrl` +- Kubernetes namespace for the workspace +- `presetId` +- `idempotencyKey` +- `replayed` + +The response SHOULD NOT include the resolved internal owner ID. + ### Idempotency rule Resolver lookup should happen before a create is finalized, but successful @@ -397,8 +441,13 @@ creates must still preserve normal Spritz idempotency guarantees. Rules: +- The idempotency fingerprint for external-owner create requests SHOULD include + `issuer + provider + tenant + subject` plus the rest of the normalized + create request. - Before the first successful create, Spritz may resolve again on retried requests with the same idempotency key. +- Once a request has resolved successfully, Spritz SHOULD persist the resolved + owner in the idempotency reservation payload used for that create attempt. - After a successful create, retries with the same idempotency key MUST replay the same created workspace even if the external mapping later changes. - Resolver failure or timeout MUST leave no partially created workspace behind. @@ -424,7 +473,7 @@ Recommended unresolved example: "identity": { "provider": "msteams", "tenant": "72f988bf-86f1-41af-91ab-2d7cd011db47", - "subject": "29:1A2BcD3EfG4HiJ5KlM6NoP" + "subject": "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f" } } ``` @@ -445,11 +494,11 @@ Recommended service capabilities: Rules: -- Resolver namespace MUST be bound to the authenticated service principal. -- A service principal MUST be allowed to resolve only within its own namespace +- Resolver issuer MUST be bound to the authenticated service principal. +- A service principal MUST be allowed to resolve only within its own issuer unless explicitly granted broader authority. - Provider allowlists and tenant allowlists SHOULD be policy-controlled per - namespace. + issuer. - Resolver selection and resolver credentials SHOULD be policy-controlled per service principal. @@ -476,14 +525,15 @@ The first concrete provider should be `msteams`. Provider guidance: - `provider` value: `msteams` -- `tenant` SHOULD be the Microsoft tenant identifier and SHOULD be required by - policy -- `subject` MUST be the stable Teams user identifier chosen by the deployment's - Teams integration +- `tenant` MUST be the Microsoft Entra tenant ID +- `subject` MUST be the Microsoft Entra user object ID +- If the bot receives a Teams chat-surface identifier such as a `29:` ID, the + integration must translate it to the canonical Entra user object ID before + calling Spritz Spritz core should not attempt to normalize Teams identifiers itself. The Teams -adapter or authoritative resolver must choose one canonical subject format and -keep it consistent. +adapter or authoritative resolver must keep that canonical subject format +consistent. ## Backward Compatibility @@ -515,13 +565,15 @@ The architecture is complete when Spritz can demonstrate all of these flows: `ownerRef.type=external`. 9. Retrying a successful create with the same idempotency key replays the same workspace even if the resolver mapping later changes. +10. Microsoft Teams requests use Entra tenant ID plus Entra user object ID as + the canonical external identity. ## Recommended Sequencing ### Phase 1 - Public API and internal abstraction - Add `ownerRef` to create requests. -- Derive resolver namespace from authenticated service principal. +- Derive resolver issuer from authenticated service principal. - Define the internal resolver interface and typed result states. - Make external-ID-only bot calls the preferred integration path. - Add `spz` flags for external owner input while keeping direct `--owner-id` From 7c071af87e75a2ebb3954eea480834dc13085900 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:02:21 +0100 Subject: [PATCH 09/17] feat(api): resolve external owners during create --- api/create_request_normalization.go | 77 ++-- api/external_owner_resolution.go | 473 +++++++++++++++++++++++++ api/jsend.go | 7 + api/main.go | 21 ++ api/main_create_external_owner_test.go | 246 +++++++++++++ api/provisioning.go | 130 ++++++- cli/src/index.ts | 33 +- cli/test/provisioner-create.test.ts | 126 +++++++ 8 files changed, 1076 insertions(+), 37 deletions(-) create mode 100644 api/external_owner_resolution.go create mode 100644 api/main_create_external_owner_test.go diff --git a/api/create_request_normalization.go b/api/create_request_normalization.go index 3f3bd37..737d2bc 100644 --- a/api/create_request_normalization.go +++ b/api/create_request_normalization.go @@ -15,6 +15,7 @@ import ( type createRequestError struct { status int message string + data any err error } @@ -34,30 +35,43 @@ func newCreateRequestError(status int, err error) error { } } +func newCreateRequestErrorWithData(status int, message string, data any, err error) error { + return &createRequestError{ + status: status, + message: message, + data: data, + err: err, + } +} + func writeCreateRequestError(c echo.Context, err error) error { var requestErr *createRequestError if errors.As(err, &requestErr) { + if requestErr.data != nil { + return writeJSendFailData(c, requestErr.status, requestErr.data) + } return writeError(c, requestErr.status, requestErr.message) } return writeError(c, http.StatusInternalServerError, err.Error()) } type normalizedCreateRequest struct { - body createRequest - fingerprintRequest createRequest - namespace string - owner spritzv1.SpritzOwner - userConfigKeys map[string]json.RawMessage - userConfigPayload userConfigPayload - normalizedUserConfig json.RawMessage - requestedImage bool - requestedRepo bool - requestedNamespace bool - nameProvided bool - requestedNamePrefix string + body createRequest + fingerprintRequest createRequest + namespace string + owner spritzv1.SpritzOwner + resolvedExternalOwner *externalOwnerResolution + userConfigKeys map[string]json.RawMessage + userConfigPayload userConfigPayload + normalizedUserConfig json.RawMessage + requestedImage bool + requestedRepo bool + requestedNamespace bool + nameProvided bool + requestedNamePrefix string } -func (s *server) normalizeCreateRequest(_ context.Context, principal principal, body createRequest) (*normalizedCreateRequest, error) { +func (s *server) normalizeCreateRequest(ctx context.Context, principal principal, body createRequest) (*normalizedCreateRequest, error) { body.Name = strings.TrimSpace(body.Name) body.NamePrefix = strings.TrimSpace(body.NamePrefix) applyTopLevelCreateFields(&body) @@ -73,12 +87,22 @@ func (s *server) normalizeCreateRequest(_ context.Context, principal principal, } requestedNamespace := s.namespaceOverrideRequested(body.Namespace, namespace) - owner, err := normalizeCreateOwner(&body, principal, s.auth.enabled()) + owner, resolvedExternalOwner, err := s.resolveCreateOwner(ctx, &body, principal) if err != nil { + var resolutionErr externalOwnerResolutionError + if errors.As(err, &resolutionErr) { + return nil, newCreateRequestErrorWithData(resolutionErr.status, resolutionErr.message, resolutionErr.responseData(), err) + } + if errors.Is(err, errForbidden) { + return nil, newCreateRequestError(http.StatusForbidden, err) + } return nil, newCreateRequestError(http.StatusBadRequest, err) } body.Spec.Owner = owner fingerprintRequest := body + if resolvedExternalOwner != nil { + fingerprintRequest.Spec.Owner = spritzv1.SpritzOwner{} + } requestedImage := strings.TrimSpace(body.Spec.Image) != "" requestedRepo := body.Spec.Repo != nil || len(body.Spec.Repos) > 0 @@ -121,18 +145,19 @@ func (s *server) normalizeCreateRequest(_ context.Context, principal principal, } return &normalizedCreateRequest{ - body: body, - fingerprintRequest: fingerprintRequest, - namespace: namespace, - owner: owner, - userConfigKeys: userConfigKeys, - userConfigPayload: userConfigPayload, - normalizedUserConfig: normalizedUserConfig, - requestedImage: requestedImage, - requestedRepo: requestedRepo, - requestedNamespace: requestedNamespace, - nameProvided: body.Name != "", - requestedNamePrefix: strings.TrimSpace(fingerprintRequest.NamePrefix), + body: body, + fingerprintRequest: fingerprintRequest, + namespace: namespace, + owner: owner, + resolvedExternalOwner: resolvedExternalOwner, + userConfigKeys: userConfigKeys, + userConfigPayload: userConfigPayload, + normalizedUserConfig: normalizedUserConfig, + requestedImage: requestedImage, + requestedRepo: requestedRepo, + requestedNamespace: requestedNamespace, + nameProvided: body.Name != "", + requestedNamePrefix: strings.TrimSpace(fingerprintRequest.NamePrefix), }, nil } diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go new file mode 100644 index 0000000..8907814 --- /dev/null +++ b/api/external_owner_resolution.go @@ -0,0 +1,473 @@ +package main + +import ( + "bytes" + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "regexp" + "strings" + "time" + + "github.com/google/uuid" +) + +const ( + externalOwnerIssuerAnnotationKey = "spritz.sh/external-owner.issuer" + externalOwnerProviderAnnotationKey = "spritz.sh/external-owner.provider" + externalOwnerTenantAnnotationKey = "spritz.sh/external-owner.tenant" + externalOwnerSubjectHashAnnotationKey = "spritz.sh/external-owner.subject-hash" + externalOwnerResolvedAtAnnotationKey = "spritz.sh/external-owner.resolved-at" + + defaultExternalOwnerResolverTimeout = 5 * time.Second +) + +var externalOwnerProviderTokenPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]*$`) + +type ownerRef struct { + Type string `json:"type,omitempty"` + ID string `json:"id,omitempty"` + Provider string `json:"provider,omitempty"` + Tenant string `json:"tenant,omitempty"` + Subject string `json:"subject,omitempty"` +} + +type externalOwnerResolutionStatus string + +const ( + externalOwnerResolved externalOwnerResolutionStatus = "resolved" + externalOwnerUnresolved externalOwnerResolutionStatus = "unresolved" + externalOwnerForbidden externalOwnerResolutionStatus = "forbidden" + externalOwnerAmbiguous externalOwnerResolutionStatus = "ambiguous" + externalOwnerUnavailable externalOwnerResolutionStatus = "unavailable" +) + +type externalOwnerPolicy struct { + PrincipalID string + Issuer string + URL string + AuthHeader string + AllowedProviders map[string]struct{} + AllowedTenants map[string]struct{} + Timeout time.Duration +} + +type externalOwnerPolicyInput struct { + PrincipalID string `json:"principalId"` + Issuer string `json:"issuer,omitempty"` + URL string `json:"url"` + AuthHeader string `json:"authHeader,omitempty"` + AllowedProviders []string `json:"allowedProviders,omitempty"` + AllowedTenants []string `json:"allowedTenants,omitempty"` + Timeout string `json:"timeout,omitempty"` +} + +type externalOwnerResolution struct { + Status externalOwnerResolutionStatus `json:"status,omitempty"` + OwnerID string `json:"ownerId,omitempty"` + Issuer string + Provider string + Tenant string + SubjectHash string + ResolvedAt time.Time +} + +type externalOwnerResolver interface { + ResolveExternalOwner(ctx context.Context, policy externalOwnerPolicy, principal principal, ref ownerRef, requestID string) (externalOwnerResolution, error) +} + +type httpExternalOwnerResolver struct{} + +type externalOwnerConfig struct { + subjectHashKey []byte + policies map[string]externalOwnerPolicy + resolver externalOwnerResolver +} + +type externalOwnerResolutionError struct { + status int + code string + message string + provider string + tenant string + subject string +} + +type externalOwnerResolverRequest struct { + Issuer string `json:"issuer"` + Identity struct { + Provider string `json:"provider"` + Tenant string `json:"tenant,omitempty"` + Subject string `json:"subject"` + } `json:"identity"` + RequestID string `json:"requestId,omitempty"` +} + +type externalOwnerResolverResponse struct { + Status externalOwnerResolutionStatus `json:"status,omitempty"` + OwnerID string `json:"ownerId,omitempty"` +} + +func newExternalOwnerConfig() (externalOwnerConfig, error) { + raw := strings.TrimSpace(os.Getenv("SPRITZ_EXTERNAL_OWNER_POLICIES_JSON")) + if raw == "" { + return externalOwnerConfig{}, nil + } + + var inputs []externalOwnerPolicyInput + if err := json.Unmarshal([]byte(raw), &inputs); err != nil { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: %w", err) + } + if len(inputs) == 0 { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: at least one policy is required") + } + + hashKey := strings.TrimSpace(os.Getenv("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY")) + if hashKey == "" { + return externalOwnerConfig{}, fmt.Errorf("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY is required when external owner policies are configured") + } + + policies := make(map[string]externalOwnerPolicy, len(inputs)) + for index, input := range inputs { + principalID := strings.TrimSpace(input.PrincipalID) + if principalID == "" { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].principalId is required", index) + } + if _, exists := policies[principalID]; exists { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: duplicate principalId %q", principalID) + } + urlValue := strings.TrimSpace(input.URL) + if urlValue == "" { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].url is required", index) + } + allowedProviders := normalizeTokenSet(input.AllowedProviders) + if len(allowedProviders) == 0 { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].allowedProviders is required", index) + } + timeout := defaultExternalOwnerResolverTimeout + if strings.TrimSpace(input.Timeout) != "" { + parsed, err := time.ParseDuration(strings.TrimSpace(input.Timeout)) + if err != nil { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].timeout is invalid", index) + } + if parsed <= 0 { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].timeout must be greater than zero", index) + } + timeout = parsed + } + policies[principalID] = externalOwnerPolicy{ + PrincipalID: principalID, + Issuer: strings.TrimSpace(input.Issuer), + URL: urlValue, + AuthHeader: strings.TrimSpace(input.AuthHeader), + AllowedProviders: allowedProviders, + AllowedTenants: normalizeStringSet(input.AllowedTenants), + Timeout: timeout, + } + } + + return externalOwnerConfig{ + subjectHashKey: []byte(hashKey), + policies: policies, + resolver: httpExternalOwnerResolver{}, + }, nil +} + +func normalizeTokenSet(values []string) map[string]struct{} { + if len(values) == 0 { + return nil + } + out := make(map[string]struct{}, len(values)) + for _, value := range values { + token := strings.ToLower(strings.TrimSpace(value)) + if token == "" { + continue + } + out[token] = struct{}{} + } + return out +} + +func normalizeStringSet(values []string) map[string]struct{} { + if len(values) == 0 { + return nil + } + out := make(map[string]struct{}, len(values)) + for _, value := range values { + token := strings.TrimSpace(value) + if token == "" { + continue + } + out[token] = struct{}{} + } + return out +} + +func (c externalOwnerConfig) enabled() bool { + return c.resolver != nil && len(c.policies) > 0 +} + +func (c externalOwnerConfig) policyForPrincipal(principal principal) (externalOwnerPolicy, bool) { + if len(c.policies) == 0 { + return externalOwnerPolicy{}, false + } + policy, ok := c.policies[strings.TrimSpace(principal.ID)] + return policy, ok +} + +func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, ref ownerRef, requestID string) (externalOwnerResolution, error) { + policy, ok := c.policyForPrincipal(principal) + if !ok { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "external identity resolution is not allowed for this principal", + provider: strings.TrimSpace(ref.Provider), + tenant: strings.TrimSpace(ref.Tenant), + subject: strings.TrimSpace(ref.Subject), + } + } + normalized, err := normalizeExternalOwnerRef(ref) + if err != nil { + return externalOwnerResolution{}, err + } + if _, ok := policy.AllowedProviders[normalized.Provider]; !ok { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "provider is not allowed for this principal", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } + if len(policy.AllowedTenants) > 0 && normalized.Tenant != "" { + if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "tenant is not allowed for this principal", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } + } + + resolution, err := c.resolver.ResolveExternalOwner(ctx, policy, principal, normalized, requestID) + if err != nil { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusServiceUnavailable, + code: "external_identity_resolution_unavailable", + message: "external identity resolution is unavailable", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } + + resolution.Issuer = policy.issuer() + resolution.Provider = normalized.Provider + resolution.Tenant = normalized.Tenant + resolution.SubjectHash = c.subjectHash(normalized.Provider, normalized.Tenant, normalized.Subject) + if resolution.ResolvedAt.IsZero() { + resolution.ResolvedAt = time.Now().UTC() + } + switch resolution.Status { + case externalOwnerResolved: + if strings.TrimSpace(resolution.OwnerID) == "" { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusServiceUnavailable, + code: "external_identity_resolution_unavailable", + message: "external identity resolution returned an invalid owner", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } + return resolution, nil + case externalOwnerUnresolved: + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusConflict, + code: "external_identity_unresolved", + message: "external identity is unresolved", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + case externalOwnerForbidden: + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "external identity resolution is forbidden", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + case externalOwnerAmbiguous: + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusConflict, + code: "external_identity_ambiguous", + message: "external identity is ambiguous", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + default: + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusServiceUnavailable, + code: "external_identity_resolution_unavailable", + message: "external identity resolution is unavailable", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } +} + +func (p externalOwnerPolicy) issuer() string { + if issuer := strings.TrimSpace(p.Issuer); issuer != "" { + return issuer + } + return strings.TrimSpace(p.PrincipalID) +} + +func normalizeExternalOwnerRef(ref ownerRef) (ownerRef, error) { + normalized := ownerRef{ + Type: strings.ToLower(strings.TrimSpace(ref.Type)), + ID: strings.TrimSpace(ref.ID), + Provider: strings.ToLower(strings.TrimSpace(ref.Provider)), + Tenant: strings.TrimSpace(ref.Tenant), + Subject: strings.TrimSpace(ref.Subject), + } + if normalized.Type != "external" { + return normalized, fmt.Errorf("ownerRef.type must be external") + } + if normalized.Provider == "" { + return normalized, fmt.Errorf("ownerRef.provider is required") + } + if !externalOwnerProviderTokenPattern.MatchString(normalized.Provider) { + return normalized, fmt.Errorf("ownerRef.provider must be a normalized lower-case token") + } + if normalized.Subject == "" { + return normalized, fmt.Errorf("ownerRef.subject is required") + } + + switch normalized.Provider { + case "msteams": + if normalized.Tenant == "" { + return normalized, fmt.Errorf("ownerRef.tenant is required for msteams") + } + tenantID, err := uuid.Parse(normalized.Tenant) + if err != nil { + return normalized, fmt.Errorf("ownerRef.tenant must be a valid UUID for msteams") + } + subjectID, err := uuid.Parse(normalized.Subject) + if err != nil { + return normalized, fmt.Errorf("ownerRef.subject must be a valid UUID for msteams") + } + normalized.Tenant = tenantID.String() + normalized.Subject = subjectID.String() + } + + return normalized, nil +} + +func (r httpExternalOwnerResolver) ResolveExternalOwner(ctx context.Context, policy externalOwnerPolicy, _ principal, ref ownerRef, requestID string) (externalOwnerResolution, error) { + timeout := policy.Timeout + if timeout <= 0 { + timeout = defaultExternalOwnerResolverTimeout + } + requestCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + requestBody := externalOwnerResolverRequest{ + Issuer: policy.issuer(), + RequestID: strings.TrimSpace(requestID), + } + requestBody.Identity.Provider = ref.Provider + requestBody.Identity.Tenant = ref.Tenant + requestBody.Identity.Subject = ref.Subject + + encoded, err := json.Marshal(requestBody) + if err != nil { + return externalOwnerResolution{}, err + } + + req, err := http.NewRequestWithContext(requestCtx, http.MethodPost, policy.URL, bytes.NewReader(encoded)) + if err != nil { + return externalOwnerResolution{}, err + } + req.Header.Set("Content-Type", "application/json") + if authHeader := strings.TrimSpace(policy.AuthHeader); authHeader != "" { + req.Header.Set("Authorization", authHeader) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return externalOwnerResolution{}, err + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusForbidden: + return externalOwnerResolution{Status: externalOwnerForbidden}, nil + case http.StatusNotFound: + return externalOwnerResolution{Status: externalOwnerUnresolved}, nil + case http.StatusConflict: + return externalOwnerResolution{Status: externalOwnerAmbiguous}, nil + case http.StatusServiceUnavailable, http.StatusBadGateway, http.StatusGatewayTimeout: + return externalOwnerResolution{Status: externalOwnerUnavailable}, nil + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return externalOwnerResolution{}, fmt.Errorf("resolver returned status %d", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return externalOwnerResolution{}, err + } + payload := externalOwnerResolverResponse{} + if err := json.Unmarshal(body, &payload); err != nil { + return externalOwnerResolution{}, err + } + return externalOwnerResolution{ + Status: payload.Status, + OwnerID: strings.TrimSpace(payload.OwnerID), + }, nil +} + +func (c externalOwnerConfig) subjectHash(provider, tenant, subject string) string { + mac := hmac.New(sha256.New, c.subjectHashKey) + _, _ = mac.Write([]byte(strings.ToLower(strings.TrimSpace(provider)))) + _, _ = mac.Write([]byte{0}) + _, _ = mac.Write([]byte(strings.TrimSpace(tenant))) + _, _ = mac.Write([]byte{0}) + _, _ = mac.Write([]byte(strings.TrimSpace(subject))) + return hex.EncodeToString(mac.Sum(nil)) +} + +func (e externalOwnerResolutionError) Error() string { + return e.message +} + +func (e externalOwnerResolutionError) responseData() map[string]any { + data := map[string]any{ + "message": e.message, + "error": e.code, + "identity": map[string]string{ + "provider": e.provider, + "subject": e.subject, + }, + } + if strings.TrimSpace(e.tenant) != "" { + data["identity"].(map[string]string)["tenant"] = e.tenant + } + return data +} diff --git a/api/jsend.go b/api/jsend.go index 77304c4..c6f83ad 100644 --- a/api/jsend.go +++ b/api/jsend.go @@ -25,6 +25,13 @@ func writeJSendFail(c echo.Context, status int, message string) error { }) } +func writeJSendFailData(c echo.Context, status int, payload any) error { + return c.JSON(status, jsendResponse{ + Status: "fail", + Data: payload, + }) +} + func writeError(c echo.Context, status int, message string) error { if status >= 500 { return c.JSON(status, jsendResponse{ diff --git a/api/main.go b/api/main.go index b448a62..7e9bc3f 100644 --- a/api/main.go +++ b/api/main.go @@ -45,6 +45,7 @@ type server struct { acp acpConfig presets presetCatalog provisioners provisionerPolicy + externalOwners externalOwnerConfig defaultMetadata map[string]string sharedMounts sharedMountsConfig sharedMountsStore *sharedMountsStore @@ -106,6 +107,11 @@ func main() { os.Exit(1) } provisioners := newProvisionerPolicy() + externalOwners, err := newExternalOwnerConfig() + if err != nil { + fmt.Fprintf(os.Stderr, "invalid external owner config: %v\n", err) + os.Exit(1) + } sshDefaults := newSSHDefaults() sshGateway, err := newSSHGatewayConfig() if err != nil { @@ -155,6 +161,7 @@ func main() { acp: acp, presets: presets, provisioners: provisioners, + externalOwners: externalOwners, defaultMetadata: defaultAnnotations, sharedMounts: sharedMounts, sharedMountsStore: sharedStore, @@ -249,6 +256,7 @@ type createRequest struct { Namespace string `json:"namespace,omitempty"` PresetID string `json:"presetId,omitempty"` OwnerID string `json:"ownerId,omitempty"` + OwnerRef *ownerRef `json:"ownerRef,omitempty"` IdleTTL string `json:"idleTtl,omitempty"` TTL string `json:"ttl,omitempty"` IdempotencyKey string `json:"idempotencyKey,omitempty"` @@ -360,6 +368,7 @@ func (s *server) createSpritz(c echo.Context) error { body = normalized.body namespace := normalized.namespace owner := normalized.owner + resolvedExternalOwner := normalized.resolvedExternalOwner userConfigKeys := normalized.userConfigKeys userConfigPayload := normalized.userConfigPayload nameProvided := normalized.nameProvided @@ -464,6 +473,18 @@ func (s *server) createSpritz(c echo.Context) error { presetIDAnnotationKey: body.PresetID, }) } + if resolvedExternalOwner != nil { + externalOwnerAnnotations := map[string]string{ + externalOwnerIssuerAnnotationKey: resolvedExternalOwner.Issuer, + externalOwnerProviderAnnotationKey: resolvedExternalOwner.Provider, + externalOwnerSubjectHashAnnotationKey: resolvedExternalOwner.SubjectHash, + externalOwnerResolvedAtAnnotationKey: resolvedExternalOwner.ResolvedAt.Format(time.RFC3339), + } + if strings.TrimSpace(resolvedExternalOwner.Tenant) != "" { + externalOwnerAnnotations[externalOwnerTenantAnnotationKey] = resolvedExternalOwner.Tenant + } + annotations = mergeStringMap(annotations, externalOwnerAnnotations) + } applySSHDefaults(&body.Spec, s.sshDefaults, namespace) baseSpec := body.Spec diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go new file mode 100644 index 0000000..4fc9ab6 --- /dev/null +++ b/api/main_create_external_owner_test.go @@ -0,0 +1,246 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/labstack/echo/v4" + "sigs.k8s.io/controller-runtime/pkg/client" + + spritzv1 "spritz.sh/operator/api/v1" +) + +type fakeExternalOwnerResolver struct { + resolve func(context.Context, externalOwnerPolicy, principal, ownerRef, string) (externalOwnerResolution, error) +} + +func (f fakeExternalOwnerResolver) ResolveExternalOwner(ctx context.Context, policy externalOwnerPolicy, principal principal, ref ownerRef, requestID string) (externalOwnerResolution, error) { + return f.resolve(ctx, policy, principal, ref, requestID) +} + +func configureExternalOwnerTestServer(s *server, resolver externalOwnerResolver) { + s.externalOwners = externalOwnerConfig{ + subjectHashKey: []byte("test-external-owner-secret"), + policies: map[string]externalOwnerPolicy{ + "zenobot": { + PrincipalID: "zenobot", + Issuer: "zenobot", + URL: "http://resolver.example.com/v1/external-owners/resolve", + AllowedProviders: map[string]struct{}{ + "msteams": {}, + }, + AllowedTenants: map[string]struct{}{ + "72f988bf-86f1-41af-91ab-2d7cd011db47": {}, + }, + Timeout: defaultExternalOwnerResolverTimeout, + }, + }, + resolver: resolver, + } +} + +func newCreateSpritzAPI(t *testing.T, s *server) *echo.Echo { + t.Helper() + e := echo.New() + secured := e.Group("", s.authMiddleware()) + secured.POST("/api/spritzes", s.createSpritz) + return e +} + +func newServiceCreateRequest(body []byte) (*http.Request, *httptest.ResponseRecorder) { + req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + req.Header.Set("X-Spritz-User-Id", "zenobot") + req.Header.Set("X-Spritz-Principal-Type", "service") + req.Header.Set("X-Spritz-Principal-Scopes", strings.Join([]string{ + scopeInstancesCreate, + scopeInstancesAssignOwner, + scopeExternalResolveViaCreate, + }, ",")) + return req, httptest.NewRecorder() +} + +func TestCreateSpritzResolvesExternalOwnerForProvisioner(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, policy externalOwnerPolicy, principal principal, ref ownerRef, requestID string) (externalOwnerResolution, error) { + if policy.issuer() != "zenobot" { + t.Fatalf("expected issuer zenobot, got %q", policy.issuer()) + } + if principal.ID != "zenobot" { + t.Fatalf("expected principal zenobot, got %q", principal.ID) + } + if requestID != "interaction-1" { + t.Fatalf("expected requestID interaction-1, got %q", requestID) + } + if ref.Provider != "msteams" { + t.Fatalf("expected provider msteams, got %q", ref.Provider) + } + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: "user-123", + }, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-1","requestId":"interaction-1"}`) + req, rec := newServiceCreateRequest(body) + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusCreated { + t.Fatalf("expected status 201, got %d: %s", rec.Code, rec.Body.String()) + } + if strings.Contains(rec.Body.String(), "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f") { + t.Fatalf("expected raw external subject to be absent from response, got %s", rec.Body.String()) + } + + var payload map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode response json: %v", err) + } + data := payload["data"].(map[string]any) + if _, exists := data["ownerId"]; exists { + t.Fatalf("expected ownerId to be omitted for external owner create, got %#v", data["ownerId"]) + } + spritzData := data["spritz"].(map[string]any) + spec := spritzData["spec"].(map[string]any) + owner := spec["owner"].(map[string]any) + if owner["id"] != "" { + t.Fatalf("expected nested owner.id to be redacted, got %#v", owner["id"]) + } + annotations := spritzData["metadata"].(map[string]any)["annotations"].(map[string]any) + if annotations[externalOwnerIssuerAnnotationKey] != "zenobot" { + t.Fatalf("expected external issuer annotation, got %#v", annotations[externalOwnerIssuerAnnotationKey]) + } + if annotations[externalOwnerProviderAnnotationKey] != "msteams" { + t.Fatalf("expected external provider annotation, got %#v", annotations[externalOwnerProviderAnnotationKey]) + } + if annotations[externalOwnerTenantAnnotationKey] != "72f988bf-86f1-41af-91ab-2d7cd011db47" { + t.Fatalf("expected external tenant annotation, got %#v", annotations[externalOwnerTenantAnnotationKey]) + } + if strings.TrimSpace(annotations[externalOwnerSubjectHashAnnotationKey].(string)) == "" { + t.Fatalf("expected external subject hash annotation, got %#v", annotations[externalOwnerSubjectHashAnnotationKey]) + } +} + +func TestCreateSpritzReturnsTypedFailureWhenExternalOwnerIsUnresolved(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + return externalOwnerResolution{Status: externalOwnerUnresolved}, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-unresolved"}`) + req, rec := newServiceCreateRequest(body) + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusConflict { + t.Fatalf("expected status 409, got %d: %s", rec.Code, rec.Body.String()) + } + + var payload map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("failed to decode response json: %v", err) + } + if payload["status"] != "fail" { + t.Fatalf("expected jsend fail status, got %#v", payload["status"]) + } + data := payload["data"].(map[string]any) + if data["error"] != "external_identity_unresolved" { + t.Fatalf("expected unresolved error code, got %#v", data["error"]) + } + identity := data["identity"].(map[string]any) + if identity["provider"] != "msteams" { + t.Fatalf("expected provider msteams, got %#v", identity["provider"]) + } + + list := &spritzv1.SpritzList{} + if err := s.client.List(context.Background(), list, client.InNamespace("spritz-test")); err != nil { + t.Fatalf("failed to list spritz resources: %v", err) + } + if len(list.Items) != 0 { + t.Fatalf("expected no spritz resources after unresolved owner, got %d", len(list.Items)) + } +} + +func TestCreateSpritzReplaysExternalOwnerProvisioningAfterResolverMappingChanges(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + resolveOwnerID := "user-123" + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: resolveOwnerID, + }, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-replay"}`) + + req1, rec1 := newServiceCreateRequest(body) + e.ServeHTTP(rec1, req1) + if rec1.Code != http.StatusCreated { + t.Fatalf("expected first create status 201, got %d: %s", rec1.Code, rec1.Body.String()) + } + + resolveOwnerID = "user-999" + + req2, rec2 := newServiceCreateRequest(body) + e.ServeHTTP(rec2, req2) + if rec2.Code != http.StatusOK { + t.Fatalf("expected replay status 200, got %d: %s", rec2.Code, rec2.Body.String()) + } + + var firstPayload map[string]any + if err := json.Unmarshal(rec1.Body.Bytes(), &firstPayload); err != nil { + t.Fatalf("failed to decode first response: %v", err) + } + var replayPayload map[string]any + if err := json.Unmarshal(rec2.Body.Bytes(), &replayPayload); err != nil { + t.Fatalf("failed to decode replay response: %v", err) + } + firstName := firstPayload["data"].(map[string]any)["spritz"].(map[string]any)["metadata"].(map[string]any)["name"] + replayedName := replayPayload["data"].(map[string]any)["spritz"].(map[string]any)["metadata"].(map[string]any)["name"] + if firstName != replayedName { + t.Fatalf("expected replayed workspace name to match, got first=%#v replay=%#v", firstName, replayedName) + } + if _, exists := replayPayload["data"].(map[string]any)["ownerId"]; exists { + t.Fatalf("expected replayed external-owner response to omit ownerId") + } + + stored := &spritzv1.Spritz{} + if err := s.client.Get(context.Background(), client.ObjectKey{Namespace: "spritz-test", Name: firstName.(string)}, stored); err != nil { + t.Fatalf("failed to load stored spritz: %v", err) + } + if stored.Spec.Owner.ID != "user-123" { + t.Fatalf("expected stored owner to remain the original resolved owner, got %q", stored.Spec.Owner.ID) + } +} + +func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) { + body := &createRequest{ + OwnerRef: &ownerRef{Type: "owner", ID: "user-123"}, + } + owner, err := normalizeCreateOwner(body, principal{ID: "zenobot", Type: principalTypeService}, true) + if err != nil { + t.Fatalf("normalizeCreateOwner failed: %v", err) + } + if owner.ID != "user-123" { + t.Fatalf("expected owner id user-123, got %q", owner.ID) + } + if body.OwnerID != "user-123" { + t.Fatalf("expected body ownerId to be populated from ownerRef, got %q", body.OwnerID) + } +} diff --git a/api/provisioning.go b/api/provisioning.go index ab3b551..3c3a6c8 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "os" "reflect" "sort" @@ -21,10 +22,11 @@ import ( ) const ( - scopeInstancesCreate = "spritz.instances.create" - scopeInstancesAssignOwner = "spritz.instances.assign_owner" - scopePresetsRead = "spritz.presets.read" - scopeInstancesSuggestName = "spritz.instances.suggest_name" + scopeInstancesCreate = "spritz.instances.create" + scopeInstancesAssignOwner = "spritz.instances.assign_owner" + scopePresetsRead = "spritz.presets.read" + scopeInstancesSuggestName = "spritz.instances.suggest_name" + scopeExternalResolveViaCreate = "spritz.external_identities.resolve_via_create" actorIDAnnotationKey = "spritz.sh/actor.id" actorTypeAnnotationKey = "spritz.sh/actor.type" @@ -203,6 +205,31 @@ func applyTopLevelCreateFields(body *createRequest) { func normalizeCreateOwner(body *createRequest, principal principal, authEnabled bool) (spritzv1.SpritzOwner, error) { owner := body.Spec.Owner + if body.OwnerRef != nil { + ref := *body.OwnerRef + ref.Type = strings.ToLower(strings.TrimSpace(ref.Type)) + ref.ID = strings.TrimSpace(ref.ID) + switch ref.Type { + case "": + case "owner": + if ref.ID == "" { + return owner, fmt.Errorf("ownerRef.id is required when ownerRef.type=owner") + } + if explicitOwner := strings.TrimSpace(body.OwnerID); explicitOwner != "" && explicitOwner != ref.ID { + return owner, fmt.Errorf("ownerId conflicts with ownerRef.id") + } + if strings.TrimSpace(owner.ID) != "" && strings.TrimSpace(owner.ID) != ref.ID { + return owner, fmt.Errorf("ownerRef.id conflicts with spec.owner.id") + } + body.OwnerID = ref.ID + owner.ID = ref.ID + body.Spec.Owner = owner + case "external": + return owner, fmt.Errorf("ownerRef.type=external requires external resolution") + default: + return owner, fmt.Errorf("ownerRef.type must be owner or external") + } + } if explicitOwner := strings.TrimSpace(body.OwnerID); explicitOwner != "" && strings.TrimSpace(owner.ID) != "" && explicitOwner != strings.TrimSpace(owner.ID) { return owner, fmt.Errorf("ownerId conflicts with spec.owner.id") } @@ -219,6 +246,44 @@ func normalizeCreateOwner(body *createRequest, principal principal, authEnabled return owner, nil } +func (s *server) resolveCreateOwner(ctx context.Context, body *createRequest, principal principal) (spritzv1.SpritzOwner, *externalOwnerResolution, error) { + if body != nil && body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") { + if strings.TrimSpace(body.OwnerID) != "" { + return spritzv1.SpritzOwner{}, nil, fmt.Errorf("ownerId conflicts with ownerRef") + } + if strings.TrimSpace(body.Spec.Owner.ID) != "" { + return spritzv1.SpritzOwner{}, nil, fmt.Errorf("spec.owner.id conflicts with ownerRef") + } + if !principalCanUseProvisionerFlow(principal) { + return spritzv1.SpritzOwner{}, nil, errForbidden + } + if err := authorizeServiceAction(principal, scopeExternalResolveViaCreate, s.auth.enabled()); err != nil { + return spritzv1.SpritzOwner{}, nil, errForbidden + } + if !s.externalOwners.enabled() { + return spritzv1.SpritzOwner{}, nil, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "external identity resolution is not configured", + provider: strings.TrimSpace(body.OwnerRef.Provider), + tenant: strings.TrimSpace(body.OwnerRef.Tenant), + subject: strings.TrimSpace(body.OwnerRef.Subject), + } + } + resolution, err := s.externalOwners.resolve(ctx, principal, *body.OwnerRef, body.RequestID) + if err != nil { + return spritzv1.SpritzOwner{}, nil, err + } + owner := body.Spec.Owner + owner.ID = resolution.OwnerID + body.Spec.Owner = owner + return owner, &resolution, nil + } + + owner, err := normalizeCreateOwner(body, principal, s.auth.enabled()) + return owner, nil, err +} + func validateProvisionerRequestSurface(body *createRequest) error { if body == nil { return nil @@ -398,6 +463,8 @@ func (s *server) enforceProvisionerQuotas(ctx context.Context, namespace string, func createRequestFingerprint(body createRequest, namespace, name, namePrefix string, userConfig json.RawMessage) (string, error) { return createFingerprint( + body.OwnerID, + body.OwnerRef, body.Spec.Owner.ID, sanitizeSpritzNameToken(body.PresetID), strings.TrimSpace(name), @@ -575,6 +642,8 @@ func (s *server) resolvedCreateFingerprint(body createRequest, namespace, explic namePrefix = s.resolvedCreateNamePrefix(body, explicitNamePrefix) } return createFingerprint( + body.OwnerID, + body.OwnerRef, body.Spec.Owner.ID, sanitizeSpritzNameToken(body.PresetID), strings.TrimSpace(body.Name), @@ -724,12 +793,21 @@ func hashLabelValue(prefix, value string) string { return fmt.Sprintf("%s-%x", prefix, sum[:12]) } -func createFingerprint(ownerID, presetID, name, namePrefix, namespace, source string, spec spritzv1.SpritzSpec, userConfig json.RawMessage) (string, error) { +func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, presetID, name, namePrefix, namespace, source string, spec spritzv1.SpritzSpec, userConfig json.RawMessage) (string, error) { specCopy := spec specCopy.Annotations = nil specCopy.Labels = nil + var ownerPayload any + switch { + case ref != nil: + ownerPayload = canonicalOwnerRefPayload(*ref) + case strings.TrimSpace(ownerID) != "": + ownerPayload = map[string]string{"ownerId": strings.TrimSpace(ownerID)} + default: + ownerPayload = map[string]string{"ownerId": strings.TrimSpace(resolvedOwnerID)} + } payload := struct { - OwnerID string `json:"ownerId"` + Owner any `json:"owner"` PresetID string `json:"presetId,omitempty"` Name string `json:"name,omitempty"` NamePrefix string `json:"namePrefix,omitempty"` @@ -738,7 +816,7 @@ func createFingerprint(ownerID, presetID, name, namePrefix, namespace, source st Spec spritzv1.SpritzSpec `json:"spec"` UserConfig json.RawMessage `json:"userConfig,omitempty"` }{ - OwnerID: ownerID, + Owner: ownerPayload, PresetID: presetID, Name: name, NamePrefix: strings.TrimSpace(namePrefix), @@ -755,6 +833,25 @@ func createFingerprint(ownerID, presetID, name, namePrefix, namespace, source st return fmt.Sprintf("%x", sum[:]), nil } +func canonicalOwnerRefPayload(ref ownerRef) map[string]string { + payload := map[string]string{ + "type": strings.ToLower(strings.TrimSpace(ref.Type)), + } + if id := strings.TrimSpace(ref.ID); id != "" { + payload["id"] = id + } + if provider := strings.ToLower(strings.TrimSpace(ref.Provider)); provider != "" { + payload["provider"] = provider + } + if tenant := strings.TrimSpace(ref.Tenant); tenant != "" { + payload["tenant"] = tenant + } + if subject := strings.TrimSpace(ref.Subject); subject != "" { + payload["subject"] = subject + } + return payload +} + func idempotencyReservationName(actorID, key string) string { sum := sha256.Sum256([]byte(strings.TrimSpace(actorID) + ":" + strings.TrimSpace(key))) return fmt.Sprintf("%s%x", idempotencyReservationPrefix, sum[:16]) @@ -952,6 +1049,12 @@ func matchesIdempotentReplayTarget(spritz *spritzv1.Spritz, principal principal, func summarizeCreateResponse(spritz *spritzv1.Spritz, principal principal, presetID, source, idempotencyKey string, replayed bool) createSpritzResponse { annotations := spritz.GetAnnotations() + responseSpritz := spritz.DeepCopy() + ownerID := spritz.Spec.Owner.ID + if principal.isService() && hasExternalOwnerAnnotations(annotations) { + ownerID = "" + responseSpritz.Spec.Owner.ID = "" + } if principal.isService() { if storedPresetID := strings.TrimSpace(annotations[presetIDAnnotationKey]); storedPresetID != "" { presetID = storedPresetID @@ -968,12 +1071,12 @@ func summarizeCreateResponse(spritz *spritzv1.Spritz, principal principal, prese workspaceURL := spritzv1.WorkspaceURLForSpritz(spritz) chatURL := spritzv1.ChatURLForSpritz(spritz) return createSpritzResponse{ - Spritz: spritz, + Spritz: responseSpritz, AccessURL: spritzv1.AccessURLForSpritz(spritz), ChatURL: chatURL, WorkspaceURL: workspaceURL, Namespace: spritz.Namespace, - OwnerID: spritz.Spec.Owner.ID, + OwnerID: ownerID, ActorID: principal.ID, ActorType: string(principal.Type), PresetID: presetID, @@ -989,6 +1092,15 @@ func summarizeCreateResponse(spritz *spritzv1.Spritz, principal principal, prese } } +func hasExternalOwnerAnnotations(annotations map[string]string) bool { + if len(annotations) == 0 { + return false + } + return strings.TrimSpace(annotations[externalOwnerIssuerAnnotationKey]) != "" && + strings.TrimSpace(annotations[externalOwnerProviderAnnotationKey]) != "" && + strings.TrimSpace(annotations[externalOwnerSubjectHashAnnotationKey]) != "" +} + func lifecycleExpiryTimes(spritz *spritzv1.Spritz, _ time.Time) (*metav1.Time, *metav1.Time, *metav1.Time) { idleExpiresAt, maxExpiresAt, effectiveExpiresAt, _, err := spritzv1.LifecycleExpiryTimes(spritz) if err != nil { diff --git a/cli/src/index.ts b/cli/src/index.ts index 3e4293e..da59bb5 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -375,7 +375,7 @@ function usage() { Usage: spritz list [--namespace ] - spritz create [name] [--preset ] [--image ] [--repo ] [--branch ] [--owner-id ] [--idle-ttl ] [--ttl ] [--idempotency-key ] [--source ] [--request-id ] [--name-prefix ] [--namespace ] + spritz create [name] [--preset ] [--image ] [--repo ] [--branch ] [--owner-id | --owner-provider --owner-subject [--owner-tenant ]] [--idle-ttl ] [--ttl ] [--idempotency-key ] [--source ] [--request-id ] [--name-prefix ] [--namespace ] spritz suggest-name [--preset ] [--image ] [--name-prefix ] [--namespace ] spritz delete [--namespace ] spritz open [--namespace ] @@ -1110,7 +1110,28 @@ async function main() { const repo = argValue('--repo'); const branch = argValue('--branch'); const token = argValue('--token') || process.env.SPRITZ_BEARER_TOKEN; - const ownerId = argValue('--owner-id') || (token?.trim() ? process.env.SPRITZ_OWNER_ID : await resolveDefaultOwnerId()); + const ownerProvider = argValue('--owner-provider')?.trim().toLowerCase(); + const ownerTenant = argValue('--owner-tenant')?.trim(); + const ownerSubject = argValue('--owner-subject')?.trim(); + const explicitOwnerId = argValue('--owner-id')?.trim(); + const usingExternalOwner = Boolean(ownerProvider || ownerTenant || ownerSubject); + if (explicitOwnerId && usingExternalOwner) { + throw new Error('--owner-id and external owner flags are mutually exclusive'); + } + if (usingExternalOwner) { + if (!ownerProvider) { + throw new Error('--owner-provider is required when using external owner flags'); + } + if (!ownerSubject) { + throw new Error('--owner-subject is required when using external owner flags'); + } + if (ownerProvider === 'msteams' && !ownerTenant) { + throw new Error('--owner-tenant is required for msteams'); + } + } + const ownerId = usingExternalOwner + ? undefined + : explicitOwnerId || (token?.trim() ? process.env.SPRITZ_OWNER_ID : await resolveDefaultOwnerId()); const idleTtl = argValue('--idle-ttl'); const ttl = argValue('--ttl'); const idempotencyKey = argValue('--idempotency-key'); @@ -1126,6 +1147,14 @@ async function main() { if (namePrefix) body.namePrefix = namePrefix; if (presetId) body.presetId = presetId; if (ownerId) body.ownerId = ownerId; + if (usingExternalOwner) { + body.ownerRef = { + type: 'external', + provider: ownerProvider, + subject: ownerSubject, + }; + if (ownerTenant) body.ownerRef.tenant = ownerTenant; + } if (idleTtl) body.idleTtl = idleTtl; if (ttl) body.ttl = ttl; if (idempotencyKey) body.idempotencyKey = idempotencyKey; diff --git a/cli/test/provisioner-create.test.ts b/cli/test/provisioner-create.test.ts index 4f061f6..695c7f0 100644 --- a/cli/test/provisioner-create.test.ts +++ b/cli/test/provisioner-create.test.ts @@ -88,6 +88,132 @@ test('create uses bearer auth and provisioner fields for preset-based creation', assert.equal(payload.presetId, 'openclaw'); }); +test('create sends external owner identity when requested', async (t) => { + let requestBody: any = null; + let requestHeaders: http.IncomingHttpHeaders | null = null; + + const server = http.createServer((req, res) => { + requestHeaders = req.headers; + const chunks: Buffer[] = []; + req.on('data', (chunk) => chunks.push(Buffer.from(chunk))); + req.on('end', () => { + requestBody = JSON.parse(Buffer.concat(chunks).toString('utf8')); + res.writeHead(201, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ + status: 'success', + data: { + accessUrl: 'https://console.example.com/#chat/openclaw-tide-wind', + chatUrl: 'https://console.example.com/#chat/openclaw-tide-wind', + workspaceUrl: 'https://console.example.com/w/openclaw-tide-wind/', + presetId: 'openclaw', + }, + })); + }); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + t.after(() => { + server.close(); + }); + const address = server.address(); + assert.ok(address && typeof address === 'object'); + + const child = spawn( + process.execPath, + [ + '--import', + 'tsx', + cliPath, + 'create', + '--preset', + 'openclaw', + '--owner-provider', + 'msteams', + '--owner-tenant', + '72f988bf-86f1-41af-91ab-2d7cd011db47', + '--owner-subject', + '6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f', + '--idempotency-key', + 'teams-123', + ], + { + env: { + ...process.env, + SPRITZ_API_URL: `http://127.0.0.1:${address.port}/api`, + SPRITZ_BEARER_TOKEN: 'service-token', + SPRITZ_CONFIG_DIR: mkdtempSync(path.join(os.tmpdir(), 'spz-config-')), + }, + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exitCode = await new Promise((resolve) => child.on('exit', resolve)); + assert.equal(exitCode, 0, `spz create should succeed: ${stderr}`); + + assert.equal(requestHeaders?.authorization, 'Bearer service-token'); + assert.deepEqual(requestBody, { + presetId: 'openclaw', + ownerRef: { + type: 'external', + provider: 'msteams', + tenant: '72f988bf-86f1-41af-91ab-2d7cd011db47', + subject: '6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f', + }, + idempotencyKey: 'teams-123', + spec: {}, + }); + + const payload = JSON.parse(stdout); + assert.equal(payload.presetId, 'openclaw'); + assert.equal(payload.ownerId, undefined); +}); + +test('create rejects mixed owner-id and external owner flags', async () => { + const child = spawn( + process.execPath, + [ + '--import', + 'tsx', + cliPath, + 'create', + '--preset', + 'openclaw', + '--owner-id', + 'user-123', + '--owner-provider', + 'discord', + '--owner-subject', + '123456789012345678', + ], + { + env: { + ...process.env, + SPRITZ_API_URL: 'http://127.0.0.1:9/api', + SPRITZ_BEARER_TOKEN: 'service-token', + SPRITZ_CONFIG_DIR: mkdtempSync(path.join(os.tmpdir(), 'spz-config-')), + }, + stdio: ['ignore', 'pipe', 'pipe'], + }, + ); + + let stderr = ''; + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exitCode = await new Promise((resolve) => child.on('exit', resolve)); + assert.notEqual(exitCode, 0, 'spz create should fail for conflicting owner inputs'); + assert.match(stderr, /mutually exclusive/); +}); + test('create falls back to local owner identity without bearer auth', async (t) => { let requestBody: any = null; let requestHeaders: http.IncomingHttpHeaders | null = null; From dd44a4c32a3eaf5e196a0dda74005d7cc3bae1be Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:12:09 +0100 Subject: [PATCH 10/17] fix(api): harden external owner idempotency --- api/create_request_normalization.go | 69 +++++------ api/main.go | 3 +- api/main_create_external_owner_test.go | 138 ++++++++++++++++++++- api/main_create_owner_test.go | 6 +- api/provisioning.go | 161 ++++++++++++++++++++----- api/provisioning_transaction.go | 31 ++++- 6 files changed, 334 insertions(+), 74 deletions(-) diff --git a/api/create_request_normalization.go b/api/create_request_normalization.go index 737d2bc..56bd5e0 100644 --- a/api/create_request_normalization.go +++ b/api/create_request_normalization.go @@ -56,22 +56,21 @@ func writeCreateRequestError(c echo.Context, err error) error { } type normalizedCreateRequest struct { - body createRequest - fingerprintRequest createRequest - namespace string - owner spritzv1.SpritzOwner - resolvedExternalOwner *externalOwnerResolution - userConfigKeys map[string]json.RawMessage - userConfigPayload userConfigPayload - normalizedUserConfig json.RawMessage - requestedImage bool - requestedRepo bool - requestedNamespace bool - nameProvided bool - requestedNamePrefix string + body createRequest + fingerprintRequest createRequest + namespace string + owner spritzv1.SpritzOwner + userConfigKeys map[string]json.RawMessage + userConfigPayload userConfigPayload + normalizedUserConfig json.RawMessage + requestedImage bool + requestedRepo bool + requestedNamespace bool + nameProvided bool + requestedNamePrefix string } -func (s *server) normalizeCreateRequest(ctx context.Context, principal principal, body createRequest) (*normalizedCreateRequest, error) { +func (s *server) normalizeCreateRequest(_ context.Context, principal principal, body createRequest) (*normalizedCreateRequest, error) { body.Name = strings.TrimSpace(body.Name) body.NamePrefix = strings.TrimSpace(body.NamePrefix) applyTopLevelCreateFields(&body) @@ -87,22 +86,21 @@ func (s *server) normalizeCreateRequest(ctx context.Context, principal principal } requestedNamespace := s.namespaceOverrideRequested(body.Namespace, namespace) - owner, resolvedExternalOwner, err := s.resolveCreateOwner(ctx, &body, principal) + owner, err := normalizeCreateOwnerRequest(&body, principal, s.auth.enabled()) if err != nil { - var resolutionErr externalOwnerResolutionError - if errors.As(err, &resolutionErr) { - return nil, newCreateRequestErrorWithData(resolutionErr.status, resolutionErr.message, resolutionErr.responseData(), err) - } if errors.Is(err, errForbidden) { return nil, newCreateRequestError(http.StatusForbidden, err) } return nil, newCreateRequestError(http.StatusBadRequest, err) } - body.Spec.Owner = owner - fingerprintRequest := body - if resolvedExternalOwner != nil { - fingerprintRequest.Spec.Owner = spritzv1.SpritzOwner{} + if body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") { + if !principalCanUseProvisionerFlow(principal) { + return nil, newCreateRequestError(http.StatusForbidden, errForbidden) + } + } else { + body.Spec.Owner = owner } + fingerprintRequest := body requestedImage := strings.TrimSpace(body.Spec.Image) != "" requestedRepo := body.Spec.Repo != nil || len(body.Spec.Repos) > 0 @@ -145,19 +143,18 @@ func (s *server) normalizeCreateRequest(ctx context.Context, principal principal } return &normalizedCreateRequest{ - body: body, - fingerprintRequest: fingerprintRequest, - namespace: namespace, - owner: owner, - resolvedExternalOwner: resolvedExternalOwner, - userConfigKeys: userConfigKeys, - userConfigPayload: userConfigPayload, - normalizedUserConfig: normalizedUserConfig, - requestedImage: requestedImage, - requestedRepo: requestedRepo, - requestedNamespace: requestedNamespace, - nameProvided: body.Name != "", - requestedNamePrefix: strings.TrimSpace(fingerprintRequest.NamePrefix), + body: body, + fingerprintRequest: fingerprintRequest, + namespace: namespace, + owner: owner, + userConfigKeys: userConfigKeys, + userConfigPayload: userConfigPayload, + normalizedUserConfig: normalizedUserConfig, + requestedImage: requestedImage, + requestedRepo: requestedRepo, + requestedNamespace: requestedNamespace, + nameProvided: body.Name != "", + requestedNamePrefix: strings.TrimSpace(fingerprintRequest.NamePrefix), }, nil } diff --git a/api/main.go b/api/main.go index 7e9bc3f..c0ea20c 100644 --- a/api/main.go +++ b/api/main.go @@ -368,7 +368,7 @@ func (s *server) createSpritz(c echo.Context) error { body = normalized.body namespace := normalized.namespace owner := normalized.owner - resolvedExternalOwner := normalized.resolvedExternalOwner + var resolvedExternalOwner *externalOwnerResolution userConfigKeys := normalized.userConfigKeys userConfigPayload := normalized.userConfigPayload nameProvided := normalized.nameProvided @@ -417,6 +417,7 @@ func (s *server) createSpritz(c echo.Context) error { return writeProvisionerCreateError(c, err) } owner = body.Spec.Owner + resolvedExternalOwner = provisionerTx.resolvedExternalOwner provisionerFingerprint = provisionerTx.provisionerFingerprint if !nameProvided { if err := buildNameGenerator(body); err != nil { diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index 4fc9ab6..25f7b80 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -53,15 +53,19 @@ func newCreateSpritzAPI(t *testing.T, s *server) *echo.Echo { } func newServiceCreateRequest(body []byte) (*http.Request, *httptest.ResponseRecorder) { + return newServiceCreateRequestWithScopes(body, + scopeInstancesCreate, + scopeInstancesAssignOwner, + scopeExternalResolveViaCreate, + ) +} + +func newServiceCreateRequestWithScopes(body []byte, scopes ...string) (*http.Request, *httptest.ResponseRecorder) { req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body)) req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) req.Header.Set("X-Spritz-User-Id", "zenobot") req.Header.Set("X-Spritz-Principal-Type", "service") - req.Header.Set("X-Spritz-Principal-Scopes", strings.Join([]string{ - scopeInstancesCreate, - scopeInstancesAssignOwner, - scopeExternalResolveViaCreate, - }, ",")) + req.Header.Set("X-Spritz-Principal-Scopes", strings.Join(scopes, ",")) return req, httptest.NewRecorder() } @@ -229,6 +233,130 @@ func TestCreateSpritzReplaysExternalOwnerProvisioningAfterResolverMappingChanges } } +func TestCreateSpritzReplaysExternalOwnerProvisioningWhenResolverBecomesUnavailable(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + resolverCalls := 0 + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + resolverCalls++ + if resolverCalls == 1 { + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: "user-123", + }, nil + } + return externalOwnerResolution{}, context.DeadlineExceeded + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-replay-unavailable"}`) + + req1, rec1 := newServiceCreateRequest(body) + e.ServeHTTP(rec1, req1) + if rec1.Code != http.StatusCreated { + t.Fatalf("expected first create status 201, got %d: %s", rec1.Code, rec1.Body.String()) + } + + req2, rec2 := newServiceCreateRequest(body) + e.ServeHTTP(rec2, req2) + if rec2.Code != http.StatusOK { + t.Fatalf("expected replay status 200, got %d: %s", rec2.Code, rec2.Body.String()) + } + if resolverCalls != 1 { + t.Fatalf("expected replay to avoid a second resolver call, got %d calls", resolverCalls) + } +} + +func TestCreateSpritzRejectsExternalOwnerResolutionWithoutCreateScopes(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + resolverCalls := 0 + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + resolverCalls++ + return externalOwnerResolution{Status: externalOwnerUnresolved}, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-no-create-scope"}`) + + req, rec := newServiceCreateRequestWithScopes(body, scopeExternalResolveViaCreate) + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if resolverCalls != 0 { + t.Fatalf("expected resolver to be skipped when create scopes are missing, got %d calls", resolverCalls) + } +} + +func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { + directFingerprint, err := createRequestFingerprint(createRequest{ + OwnerID: "user-123", + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprint failed for direct owner: %v", err) + } + + ownerRefFingerprint, err := createRequestFingerprint(createRequest{ + OwnerRef: &ownerRef{ + Type: "owner", + ID: "user-123", + }, + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprint failed for ownerRef owner: %v", err) + } + + if directFingerprint != ownerRefFingerprint { + t.Fatalf("expected equivalent direct and ownerRef owner inputs to share a fingerprint") + } + + lowerFingerprint, err := createRequestFingerprint(createRequest{ + OwnerRef: &ownerRef{ + Type: "external", + Provider: "msteams", + Tenant: "72f988bf-86f1-41af-91ab-2d7cd011db47", + Subject: "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f", + }, + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprint failed for lowercase msteams identity: %v", err) + } + + upperFingerprint, err := createRequestFingerprint(createRequest{ + OwnerRef: &ownerRef{ + Type: "external", + Provider: "msteams", + Tenant: "72F988BF-86F1-41AF-91AB-2D7CD011DB47", + Subject: "6F0F9D4F-9B0E-4D52-8C3A-EF0FD64B9B9F", + }, + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprint failed for uppercase msteams identity: %v", err) + } + + if lowerFingerprint != upperFingerprint { + t.Fatalf("expected equivalent msteams UUID casing to share a fingerprint") + } +} + func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) { body := &createRequest{ OwnerRef: &ownerRef{Type: "owner", ID: "user-123"}, diff --git a/api/main_create_owner_test.go b/api/main_create_owner_test.go index dd23fbd..123a178 100644 --- a/api/main_create_owner_test.go +++ b/api/main_create_owner_test.go @@ -1094,7 +1094,7 @@ func TestCreateSpritzUsesPendingReservationPayloadAfterDefaultPresetChanges(t *t if err != nil { t.Fatalf("createRequestFingerprint failed: %v", err) } - resolvedPayload, err := createResolvedProvisionerPayload(requestBody, s.resolvedCreateNamePrefix(requestBody, requestFingerprintBody.NamePrefix)) + resolvedPayload, err := createResolvedProvisionerPayload(requestBody, s.resolvedCreateNamePrefix(requestBody, requestFingerprintBody.NamePrefix), nil) if err != nil { t.Fatalf("createResolvedProvisionerPayload failed: %v", err) } @@ -1547,7 +1547,7 @@ func TestCreateSpritzRetriesPendingIdempotencyReservationWithConflictingOccupant if err != nil { t.Fatalf("createRequestFingerprint failed: %v", err) } - resolvedPayload, err := createResolvedProvisionerPayload(body, s.resolvedCreateNamePrefix(body, requestFingerprintBody.NamePrefix)) + resolvedPayload, err := createResolvedProvisionerPayload(body, s.resolvedCreateNamePrefix(body, requestFingerprintBody.NamePrefix), nil) if err != nil { t.Fatalf("createResolvedProvisionerPayload failed: %v", err) } @@ -1641,7 +1641,7 @@ func TestCreateSpritzReplaysPendingIdempotentCreateBeforeQuotaCheck(t *testing.T if err != nil { t.Fatalf("createRequestFingerprint failed: %v", err) } - resolvedPayload, err := createResolvedProvisionerPayload(body, s.resolvedCreateNamePrefix(body, requestFingerprintBody.NamePrefix)) + resolvedPayload, err := createResolvedProvisionerPayload(body, s.resolvedCreateNamePrefix(body, requestFingerprintBody.NamePrefix), nil) if err != nil { t.Fatalf("createResolvedProvisionerPayload failed: %v", err) } diff --git a/api/provisioning.go b/api/provisioning.go index 3c3a6c8..ae0c68d 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -128,11 +128,12 @@ type suggestNameMetadata struct { } type idempotentCreatePayload struct { - PresetID string `json:"presetId,omitempty"` - NamePrefix string `json:"namePrefix,omitempty"` - Source string `json:"source,omitempty"` - RequestID string `json:"requestId,omitempty"` - Spec spritzv1.SpritzSpec `json:"spec"` + PresetID string `json:"presetId,omitempty"` + NamePrefix string `json:"namePrefix,omitempty"` + Source string `json:"source,omitempty"` + RequestID string `json:"requestId,omitempty"` + Spec spritzv1.SpritzSpec `json:"spec"` + ExternalOwner *idempotentExternalOwnerPayload `json:"externalOwner,omitempty"` } type provisionerIdempotencyState struct { @@ -140,6 +141,14 @@ type provisionerIdempotencyState struct { resolvedPayload string } +type idempotentExternalOwnerPayload struct { + Issuer string `json:"issuer,omitempty"` + Provider string `json:"provider,omitempty"` + Tenant string `json:"tenant,omitempty"` + SubjectHash string `json:"subjectHash,omitempty"` + ResolvedAt string `json:"resolvedAt,omitempty"` +} + func (s *server) applyCreatePreset(body *createRequest) (*runtimePreset, error) { body.PresetID = sanitizeSpritzNameToken(body.PresetID) if body.PresetID == "" { @@ -203,7 +212,7 @@ func applyTopLevelCreateFields(body *createRequest) { body.IdempotencyKey = strings.TrimSpace(body.IdempotencyKey) } -func normalizeCreateOwner(body *createRequest, principal principal, authEnabled bool) (spritzv1.SpritzOwner, error) { +func normalizeCreateOwnerRequest(body *createRequest, principal principal, authEnabled bool) (spritzv1.SpritzOwner, error) { owner := body.Spec.Owner if body.OwnerRef != nil { ref := *body.OwnerRef @@ -222,10 +231,22 @@ func normalizeCreateOwner(body *createRequest, principal principal, authEnabled return owner, fmt.Errorf("ownerRef.id conflicts with spec.owner.id") } body.OwnerID = ref.ID + body.OwnerRef = &ownerRef{Type: "owner", ID: ref.ID} owner.ID = ref.ID body.Spec.Owner = owner case "external": - return owner, fmt.Errorf("ownerRef.type=external requires external resolution") + normalized, err := normalizeExternalOwnerRef(ref) + if err != nil { + return owner, err + } + if strings.TrimSpace(body.OwnerID) != "" { + return owner, fmt.Errorf("ownerId conflicts with ownerRef") + } + if strings.TrimSpace(owner.ID) != "" { + return owner, fmt.Errorf("spec.owner.id conflicts with ownerRef") + } + body.OwnerRef = &normalized + return owner, nil default: return owner, fmt.Errorf("ownerRef.type must be owner or external") } @@ -246,8 +267,24 @@ func normalizeCreateOwner(body *createRequest, principal principal, authEnabled return owner, nil } +func normalizeCreateOwner(body *createRequest, principal principal, authEnabled bool) (spritzv1.SpritzOwner, error) { + owner, err := normalizeCreateOwnerRequest(body, principal, authEnabled) + if err != nil { + return owner, err + } + if body != nil && body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") { + return owner, fmt.Errorf("ownerRef.type=external requires external resolution") + } + return owner, nil +} + func (s *server) resolveCreateOwner(ctx context.Context, body *createRequest, principal principal) (spritzv1.SpritzOwner, *externalOwnerResolution, error) { if body != nil && body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") { + normalizedRef, err := normalizeExternalOwnerRef(*body.OwnerRef) + if err != nil { + return spritzv1.SpritzOwner{}, nil, err + } + body.OwnerRef = &normalizedRef if strings.TrimSpace(body.OwnerID) != "" { return spritzv1.SpritzOwner{}, nil, fmt.Errorf("ownerId conflicts with ownerRef") } @@ -257,7 +294,13 @@ func (s *server) resolveCreateOwner(ctx context.Context, body *createRequest, pr if !principalCanUseProvisionerFlow(principal) { return spritzv1.SpritzOwner{}, nil, errForbidden } - if err := authorizeServiceAction(principal, scopeExternalResolveViaCreate, s.auth.enabled()); err != nil { + if err := authorizeServiceAction(principal, scopeInstancesCreate, true); err != nil { + return spritzv1.SpritzOwner{}, nil, errForbidden + } + if err := authorizeServiceAction(principal, scopeInstancesAssignOwner, true); err != nil { + return spritzv1.SpritzOwner{}, nil, errForbidden + } + if err := authorizeServiceAction(principal, scopeExternalResolveViaCreate, true); err != nil { return spritzv1.SpritzOwner{}, nil, errForbidden } if !s.externalOwners.enabled() { @@ -265,12 +308,12 @@ func (s *server) resolveCreateOwner(ctx context.Context, body *createRequest, pr status: http.StatusForbidden, code: "external_identity_forbidden", message: "external identity resolution is not configured", - provider: strings.TrimSpace(body.OwnerRef.Provider), - tenant: strings.TrimSpace(body.OwnerRef.Tenant), - subject: strings.TrimSpace(body.OwnerRef.Subject), + provider: normalizedRef.Provider, + tenant: normalizedRef.Tenant, + subject: normalizedRef.Subject, } } - resolution, err := s.externalOwners.resolve(ctx, principal, *body.OwnerRef, body.RequestID) + resolution, err := s.externalOwners.resolve(ctx, principal, normalizedRef, body.RequestID) if err != nil { return spritzv1.SpritzOwner{}, nil, err } @@ -476,13 +519,14 @@ func createRequestFingerprint(body createRequest, namespace, name, namePrefix st ) } -func createResolvedProvisionerPayload(body createRequest, resolvedNamePrefix string) (string, error) { +func createResolvedProvisionerPayload(body createRequest, resolvedNamePrefix string, resolvedExternalOwner *externalOwnerResolution) (string, error) { payload := idempotentCreatePayload{ - PresetID: sanitizeSpritzNameToken(body.PresetID), - NamePrefix: sanitizeSpritzNameToken(resolvedNamePrefix), - Source: provisionerSource(&body), - RequestID: strings.TrimSpace(body.RequestID), - Spec: body.Spec, + PresetID: sanitizeSpritzNameToken(body.PresetID), + NamePrefix: sanitizeSpritzNameToken(resolvedNamePrefix), + Source: provisionerSource(&body), + RequestID: strings.TrimSpace(body.RequestID), + Spec: body.Spec, + ExternalOwner: newIdempotentExternalOwnerPayload(resolvedExternalOwner), } encoded, err := json.Marshal(payload) if err != nil { @@ -655,7 +699,7 @@ func (s *server) resolvedCreateFingerprint(body createRequest, namespace, explic ) } -func (s *server) provisionerIdempotencyFingerprints(requestBody, resolvedBody createRequest, namespace string, userConfig json.RawMessage) (provisionerIdempotencyState, error) { +func (s *server) provisionerIdempotencyFingerprints(requestBody, resolvedBody createRequest, resolvedExternalOwner *externalOwnerResolution, namespace string, userConfig json.RawMessage) (provisionerIdempotencyState, error) { canonicalName := strings.TrimSpace(requestBody.Name) canonicalNamePrefix := "" if canonicalName == "" { @@ -665,7 +709,7 @@ func (s *server) provisionerIdempotencyFingerprints(requestBody, resolvedBody cr if err != nil { return provisionerIdempotencyState{}, err } - resolvedPayload, err := createResolvedProvisionerPayload(resolvedBody, s.resolvedCreateNamePrefix(resolvedBody, requestBody.NamePrefix)) + resolvedPayload, err := createResolvedProvisionerPayload(resolvedBody, s.resolvedCreateNamePrefix(resolvedBody, requestBody.NamePrefix), resolvedExternalOwner) if err != nil { return provisionerIdempotencyState{}, err } @@ -797,14 +841,9 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, presetID, specCopy := spec specCopy.Annotations = nil specCopy.Labels = nil - var ownerPayload any - switch { - case ref != nil: - ownerPayload = canonicalOwnerRefPayload(*ref) - case strings.TrimSpace(ownerID) != "": - ownerPayload = map[string]string{"ownerId": strings.TrimSpace(ownerID)} - default: - ownerPayload = map[string]string{"ownerId": strings.TrimSpace(resolvedOwnerID)} + ownerPayload, err := canonicalOwnerFingerprintPayload(ownerID, ref, resolvedOwnerID) + if err != nil { + return "", err } payload := struct { Owner any `json:"owner"` @@ -833,6 +872,37 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, presetID, return fmt.Sprintf("%x", sum[:]), nil } +func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwnerID string) (any, error) { + switch { + case ref != nil: + normalizedType := strings.ToLower(strings.TrimSpace(ref.Type)) + switch normalizedType { + case "owner": + if id := strings.TrimSpace(ref.ID); id != "" { + return map[string]string{"ownerId": id}, nil + } + case "external": + normalized, err := normalizeExternalOwnerRef(*ref) + if err != nil { + return nil, err + } + return canonicalOwnerRefPayload(normalized), nil + case "": + // Fall through to the direct owner path below so empty ownerRef does + // not create a distinct fingerprint representation. + default: + return nil, fmt.Errorf("ownerRef.type must be owner or external") + } + case strings.TrimSpace(ownerID) != "": + return map[string]string{"ownerId": strings.TrimSpace(ownerID)}, nil + } + + if strings.TrimSpace(ownerID) != "" { + return map[string]string{"ownerId": strings.TrimSpace(ownerID)}, nil + } + return map[string]string{"ownerId": strings.TrimSpace(resolvedOwnerID)}, nil +} + func canonicalOwnerRefPayload(ref ownerRef) map[string]string { payload := map[string]string{ "type": strings.ToLower(strings.TrimSpace(ref.Type)), @@ -852,6 +922,41 @@ func canonicalOwnerRefPayload(ref ownerRef) map[string]string { return payload } +func newIdempotentExternalOwnerPayload(resolution *externalOwnerResolution) *idempotentExternalOwnerPayload { + if resolution == nil { + return nil + } + payload := &idempotentExternalOwnerPayload{ + Issuer: strings.TrimSpace(resolution.Issuer), + Provider: strings.TrimSpace(resolution.Provider), + Tenant: strings.TrimSpace(resolution.Tenant), + SubjectHash: strings.TrimSpace(resolution.SubjectHash), + } + if !resolution.ResolvedAt.IsZero() { + payload.ResolvedAt = resolution.ResolvedAt.UTC().Format(time.RFC3339) + } + return payload +} + +func (p *idempotentExternalOwnerPayload) resolution() *externalOwnerResolution { + if p == nil { + return nil + } + resolution := &externalOwnerResolution{ + Issuer: strings.TrimSpace(p.Issuer), + Provider: strings.TrimSpace(p.Provider), + Tenant: strings.TrimSpace(p.Tenant), + SubjectHash: strings.TrimSpace(p.SubjectHash), + } + if strings.TrimSpace(p.ResolvedAt) != "" { + parsed, err := time.Parse(time.RFC3339, p.ResolvedAt) + if err == nil { + resolution.ResolvedAt = parsed.UTC() + } + } + return resolution +} + func idempotencyReservationName(actorID, key string) string { sum := sha256.Sum256([]byte(strings.TrimSpace(actorID) + ":" + strings.TrimSpace(key))) return fmt.Sprintf("%s%x", idempotencyReservationPrefix, sum[:16]) diff --git a/api/provisioning_transaction.go b/api/provisioning_transaction.go index e2006e7..5bd1105 100644 --- a/api/provisioning_transaction.go +++ b/api/provisioning_transaction.go @@ -14,6 +14,7 @@ import ( type provisionerCreateError struct { status int message string + data any err error } @@ -33,6 +34,15 @@ func newProvisionerCreateError(status int, err error) error { } } +func newProvisionerCreateErrorWithData(status int, message string, data any, err error) error { + return &provisionerCreateError{ + status: status, + message: message, + data: data, + err: err, + } +} + func newProvisionerForbiddenError() error { return &provisionerCreateError{ status: http.StatusForbidden, @@ -44,6 +54,9 @@ func newProvisionerForbiddenError() error { func writeProvisionerCreateError(c echo.Context, err error) error { var provisionerErr *provisionerCreateError if errors.As(err, &provisionerErr) { + if provisionerErr.data != nil { + return writeJSendFailData(c, provisionerErr.status, provisionerErr.data) + } return writeError(c, provisionerErr.status, provisionerErr.message) } return writeError(c, http.StatusInternalServerError, err.Error()) @@ -65,6 +78,7 @@ type provisionerCreateTransaction struct { provisionerFingerprint string idempotencyState provisionerIdempotencyState resolvedFromReservation bool + resolvedExternalOwner *externalOwnerResolution completed bool } @@ -138,6 +152,20 @@ func (tx *provisionerCreateTransaction) prepare() error { return nil } + owner, resolvedExternalOwner, err := tx.server.resolveCreateOwner(tx.ctx, tx.body, tx.principal) + if err != nil { + var resolutionErr externalOwnerResolutionError + if errors.As(err, &resolutionErr) { + return newProvisionerCreateErrorWithData(resolutionErr.status, resolutionErr.message, resolutionErr.responseData(), err) + } + if errors.Is(err, errForbidden) { + return newProvisionerForbiddenError() + } + return newProvisionerCreateError(http.StatusBadRequest, err) + } + tx.body.Spec.Owner = owner + tx.resolvedExternalOwner = resolvedExternalOwner + if err := tx.server.validateProvisionerCreate(tx.ctx, tx.principal, tx.namespace, tx.body, tx.requestedImage, tx.requestedRepo, tx.requestedNamespace); err != nil { if errors.Is(err, errForbidden) { return newProvisionerForbiddenError() @@ -147,7 +175,7 @@ func (tx *provisionerCreateTransaction) prepare() error { if err := resolveCreateLifetimes(&tx.body.Spec, tx.server.provisioners, true); err != nil { return newProvisionerCreateError(http.StatusBadRequest, err) } - tx.idempotencyState, err = tx.server.provisionerIdempotencyFingerprints(tx.fingerprintRequest, *tx.body, tx.namespace, tx.normalizedUserConfig) + tx.idempotencyState, err = tx.server.provisionerIdempotencyFingerprints(tx.fingerprintRequest, *tx.body, tx.resolvedExternalOwner, tx.namespace, tx.normalizedUserConfig) if err != nil { return newProvisionerCreateError(http.StatusInternalServerError, err) } @@ -181,6 +209,7 @@ func (tx *provisionerCreateTransaction) restoreStoredPayload(raw string) error { tx.body.Source = payload.Source tx.body.RequestID = payload.RequestID tx.body.Spec = payload.Spec + tx.resolvedExternalOwner = payload.ExternalOwner.resolution() return nil } From ac5e1716eec8d3e96aa41559cf7223651d47b747 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:18:55 +0100 Subject: [PATCH 11/17] fix(api): reject invalid external owner inputs --- api/create_request_normalization.go | 2 +- api/external_owner_resolution.go | 12 +++- api/main_create_external_owner_test.go | 87 ++++++++++++++++++++++++++ api/provisioning.go | 4 +- 4 files changed, 101 insertions(+), 4 deletions(-) diff --git a/api/create_request_normalization.go b/api/create_request_normalization.go index 56bd5e0..8181b4c 100644 --- a/api/create_request_normalization.go +++ b/api/create_request_normalization.go @@ -94,7 +94,7 @@ func (s *server) normalizeCreateRequest(_ context.Context, principal principal, return nil, newCreateRequestError(http.StatusBadRequest, err) } if body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") { - if !principalCanUseProvisionerFlow(principal) { + if !principal.isService() { return nil, newCreateRequestError(http.StatusForbidden, errForbidden) } } else { diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index 8907814..c92fc2e 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -247,7 +247,17 @@ func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, r subject: normalized.Subject, } } - if len(policy.AllowedTenants) > 0 && normalized.Tenant != "" { + if len(policy.AllowedTenants) > 0 { + if normalized.Tenant == "" { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "tenant is required for this principal", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok { return externalOwnerResolution{}, externalOwnerResolutionError{ status: http.StatusForbidden, diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index 25f7b80..f372489 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -294,6 +295,79 @@ func TestCreateSpritzRejectsExternalOwnerResolutionWithoutCreateScopes(t *testin } } +func TestCreateSpritzRejectsExternalOwnerForAdminCallers(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + resolverCalls := 0 + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + resolverCalls++ + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: "user-123", + }, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-admin"}`) + req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + req.Header.Set("X-Spritz-User-Id", "admin-1") + req.Header.Set("X-Spritz-Principal-Type", "admin") + rec := httptest.NewRecorder() + + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String()) + } + if resolverCalls != 0 { + t.Fatalf("expected resolver to be skipped for admin callers, got %d calls", resolverCalls) + } +} + +func TestExternalOwnerResolveRequiresTenantWhenTenantAllowlistIsConfigured(t *testing.T) { + config := externalOwnerConfig{ + subjectHashKey: []byte("test-external-owner-secret"), + policies: map[string]externalOwnerPolicy{ + "zenobot": { + PrincipalID: "zenobot", + Issuer: "zenobot", + URL: "http://resolver.example.com/v1/external-owners/resolve", + AllowedProviders: map[string]struct{}{ + "slack": {}, + }, + AllowedTenants: map[string]struct{}{ + "enterprise-1": {}, + }, + }, + }, + resolver: fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + t.Fatal("expected resolver call to be blocked when tenant is missing") + return externalOwnerResolution{}, nil + }, + }, + } + + _, err := config.resolve(context.Background(), principal{ID: "zenobot", Type: principalTypeService}, ownerRef{ + Type: "external", + Provider: "slack", + Subject: "U123456", + }, "") + if err == nil { + t.Fatal("expected resolve to fail when tenant allowlist is configured but tenant is missing") + } + var resolutionErr externalOwnerResolutionError + if !errors.As(err, &resolutionErr) { + t.Fatalf("expected externalOwnerResolutionError, got %T", err) + } + if resolutionErr.code != "external_identity_forbidden" { + t.Fatalf("expected external_identity_forbidden, got %q", resolutionErr.code) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123", @@ -372,3 +446,16 @@ func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) { t.Fatalf("expected body ownerId to be populated from ownerRef, got %q", body.OwnerID) } } + +func TestNormalizeCreateOwnerRejectsOwnerRefWithoutType(t *testing.T) { + body := &createRequest{ + OwnerRef: &ownerRef{ID: "user-123"}, + } + _, err := normalizeCreateOwnerRequest(body, principal{ID: "user-123", Type: principalTypeHuman}, true) + if err == nil { + t.Fatal("expected normalizeCreateOwnerRequest to reject ownerRef without type") + } + if !strings.Contains(err.Error(), "ownerRef.type is required") { + t.Fatalf("expected ownerRef.type validation error, got %v", err) + } +} diff --git a/api/provisioning.go b/api/provisioning.go index ae0c68d..3188922 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -220,6 +220,7 @@ func normalizeCreateOwnerRequest(body *createRequest, principal principal, authE ref.ID = strings.TrimSpace(ref.ID) switch ref.Type { case "": + return owner, fmt.Errorf("ownerRef.type is required") case "owner": if ref.ID == "" { return owner, fmt.Errorf("ownerRef.id is required when ownerRef.type=owner") @@ -888,8 +889,7 @@ func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwn } return canonicalOwnerRefPayload(normalized), nil case "": - // Fall through to the direct owner path below so empty ownerRef does - // not create a distinct fingerprint representation. + return nil, fmt.Errorf("ownerRef.type is required") default: return nil, fmt.Errorf("ownerRef.type must be owner or external") } From 963c0ee7a75b8d35a4f30f7ebf0a0db8105d2fa9 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:26:28 +0100 Subject: [PATCH 12/17] fix(api): enforce external owner replay scopes --- api/external_owner_resolution.go | 29 ++++++++-- api/main_create_external_owner_test.go | 74 ++++++++++++++++++++++++++ api/provisioning_transaction.go | 5 ++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index c92fc2e..9af6674 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -55,6 +55,7 @@ type externalOwnerPolicy struct { AuthHeader string AllowedProviders map[string]struct{} AllowedTenants map[string]struct{} + TenantRequired map[string]struct{} Timeout time.Duration } @@ -65,6 +66,7 @@ type externalOwnerPolicyInput struct { AuthHeader string `json:"authHeader,omitempty"` AllowedProviders []string `json:"allowedProviders,omitempty"` AllowedTenants []string `json:"allowedTenants,omitempty"` + TenantRequired []string `json:"tenantRequired,omitempty"` Timeout string `json:"timeout,omitempty"` } @@ -168,6 +170,7 @@ func newExternalOwnerConfig() (externalOwnerConfig, error) { AuthHeader: strings.TrimSpace(input.AuthHeader), AllowedProviders: allowedProviders, AllowedTenants: normalizeStringSet(input.AllowedTenants), + TenantRequired: normalizeTokenSet(input.TenantRequired), Timeout: timeout, } } @@ -248,7 +251,8 @@ func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, r } } if len(policy.AllowedTenants) > 0 { - if normalized.Tenant == "" { + tenantRequired := policy.requiresTenant(normalized.Provider) + if normalized.Tenant == "" && tenantRequired { return externalOwnerResolution{}, externalOwnerResolutionError{ status: http.StatusForbidden, code: "external_identity_forbidden", @@ -258,11 +262,22 @@ func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, r subject: normalized.Subject, } } - if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok { + if normalized.Tenant != "" { + if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "tenant is not allowed for this principal", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, + } + } + } else if tenantRequired { return externalOwnerResolution{}, externalOwnerResolutionError{ status: http.StatusForbidden, code: "external_identity_forbidden", - message: "tenant is not allowed for this principal", + message: "tenant is required for this principal", provider: normalized.Provider, tenant: normalized.Tenant, subject: normalized.Subject, @@ -348,6 +363,14 @@ func (p externalOwnerPolicy) issuer() string { return strings.TrimSpace(p.PrincipalID) } +func (p externalOwnerPolicy) requiresTenant(provider string) bool { + if len(p.TenantRequired) == 0 { + return false + } + _, ok := p.TenantRequired[strings.ToLower(strings.TrimSpace(provider))] + return ok +} + func normalizeExternalOwnerRef(ref ownerRef) (ownerRef, error) { normalized := ownerRef{ Type: strings.ToLower(strings.TrimSpace(ref.Type)), diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index f372489..584c040 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -327,6 +327,34 @@ func TestCreateSpritzRejectsExternalOwnerForAdminCallers(t *testing.T) { } } +func TestCreateSpritzReplayRequiresExternalResolveScope(t *testing.T) { + s := newCreateSpritzTestServer(t) + configureProvisionerTestServer(s) + configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: "user-123", + }, nil + }, + }) + e := newCreateSpritzAPI(t, s) + + body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-scope-bypass"}`) + + req1, rec1 := newServiceCreateRequest(body) + e.ServeHTTP(rec1, req1) + if rec1.Code != http.StatusCreated { + t.Fatalf("expected first create status 201, got %d: %s", rec1.Code, rec1.Body.String()) + } + + req2, rec2 := newServiceCreateRequestWithScopes(body, scopeInstancesCreate, scopeInstancesAssignOwner) + e.ServeHTTP(rec2, req2) + if rec2.Code != http.StatusForbidden { + t.Fatalf("expected status 403, got %d: %s", rec2.Code, rec2.Body.String()) + } +} + func TestExternalOwnerResolveRequiresTenantWhenTenantAllowlistIsConfigured(t *testing.T) { config := externalOwnerConfig{ subjectHashKey: []byte("test-external-owner-secret"), @@ -341,6 +369,9 @@ func TestExternalOwnerResolveRequiresTenantWhenTenantAllowlistIsConfigured(t *te AllowedTenants: map[string]struct{}{ "enterprise-1": {}, }, + TenantRequired: map[string]struct{}{ + "slack": {}, + }, }, }, resolver: fakeExternalOwnerResolver{ @@ -368,6 +399,49 @@ func TestExternalOwnerResolveRequiresTenantWhenTenantAllowlistIsConfigured(t *te } } +func TestExternalOwnerResolveAllowsTenantlessProviderWhenTenantIsNotRequired(t *testing.T) { + config := externalOwnerConfig{ + subjectHashKey: []byte("test-external-owner-secret"), + policies: map[string]externalOwnerPolicy{ + "zenobot": { + PrincipalID: "zenobot", + Issuer: "zenobot", + URL: "http://resolver.example.com/v1/external-owners/resolve", + AllowedProviders: map[string]struct{}{ + "slack": {}, + "msteams": {}, + }, + AllowedTenants: map[string]struct{}{ + "72f988bf-86f1-41af-91ab-2d7cd011db47": {}, + }, + }, + }, + resolver: fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, ref ownerRef, _ string) (externalOwnerResolution, error) { + if ref.Provider != "slack" { + t.Fatalf("expected provider slack, got %q", ref.Provider) + } + return externalOwnerResolution{ + Status: externalOwnerResolved, + OwnerID: "user-123", + }, nil + }, + }, + } + + resolution, err := config.resolve(context.Background(), principal{ID: "zenobot", Type: principalTypeService}, ownerRef{ + Type: "external", + Provider: "slack", + Subject: "U123456", + }, "") + if err != nil { + t.Fatalf("expected tenantless slack identity to resolve when tenant is not required, got %v", err) + } + if resolution.OwnerID != "user-123" { + t.Fatalf("expected ownerId user-123, got %q", resolution.OwnerID) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123", diff --git a/api/provisioning_transaction.go b/api/provisioning_transaction.go index 5bd1105..d20031b 100644 --- a/api/provisioning_transaction.go +++ b/api/provisioning_transaction.go @@ -115,6 +115,11 @@ func (tx *provisionerCreateTransaction) prepare() error { if err := authorizeServiceAction(tx.principal, scopeInstancesAssignOwner, true); err != nil { return newProvisionerForbiddenError() } + if tx.fingerprintRequest.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(tx.fingerprintRequest.OwnerRef.Type), "external") { + if err := authorizeServiceAction(tx.principal, scopeExternalResolveViaCreate, true); err != nil { + return newProvisionerForbiddenError() + } + } if strings.TrimSpace(tx.body.IdempotencyKey) == "" { return newProvisionerCreateError(http.StatusBadRequest, errors.New("idempotencyKey is required")) } From 99aa3fe425901179fc33a9a609620424716320d9 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:33:06 +0100 Subject: [PATCH 13/17] fix(api): redact external owner responses --- api/external_owner_resolution.go | 22 ++++++------ api/main_create_external_owner_test.go | 49 ++++++++++++++++++++++++++ api/provisioning.go | 3 ++ 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index 9af6674..716a76a 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -250,18 +250,18 @@ func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, r subject: normalized.Subject, } } - if len(policy.AllowedTenants) > 0 { - tenantRequired := policy.requiresTenant(normalized.Provider) - if normalized.Tenant == "" && tenantRequired { - return externalOwnerResolution{}, externalOwnerResolutionError{ - status: http.StatusForbidden, - code: "external_identity_forbidden", - message: "tenant is required for this principal", - provider: normalized.Provider, - tenant: normalized.Tenant, - subject: normalized.Subject, - } + tenantRequired := policy.requiresTenant(normalized.Provider) + if normalized.Tenant == "" && tenantRequired { + return externalOwnerResolution{}, externalOwnerResolutionError{ + status: http.StatusForbidden, + code: "external_identity_forbidden", + message: "tenant is required for this principal", + provider: normalized.Provider, + tenant: normalized.Tenant, + subject: normalized.Subject, } + } + if len(policy.AllowedTenants) > 0 { if normalized.Tenant != "" { if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok { return externalOwnerResolution{}, externalOwnerResolutionError{ diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index 584c040..3a4d2ff 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -120,6 +120,10 @@ func TestCreateSpritzResolvesExternalOwnerForProvisioner(t *testing.T) { if owner["id"] != "" { t.Fatalf("expected nested owner.id to be redacted, got %#v", owner["id"]) } + labels := spritzData["metadata"].(map[string]any)["labels"].(map[string]any) + if _, exists := labels[ownerLabelKey]; exists { + t.Fatalf("expected owner label to be omitted for external owner create response") + } annotations := spritzData["metadata"].(map[string]any)["annotations"].(map[string]any) if annotations[externalOwnerIssuerAnnotationKey] != "zenobot" { t.Fatalf("expected external issuer annotation, got %#v", annotations[externalOwnerIssuerAnnotationKey]) @@ -224,6 +228,10 @@ func TestCreateSpritzReplaysExternalOwnerProvisioningAfterResolverMappingChanges if _, exists := replayPayload["data"].(map[string]any)["ownerId"]; exists { t.Fatalf("expected replayed external-owner response to omit ownerId") } + replayedLabels := replayPayload["data"].(map[string]any)["spritz"].(map[string]any)["metadata"].(map[string]any)["labels"].(map[string]any) + if _, exists := replayedLabels[ownerLabelKey]; exists { + t.Fatalf("expected replayed external-owner response to omit owner label") + } stored := &spritzv1.Spritz{} if err := s.client.Get(context.Background(), client.ObjectKey{Namespace: "spritz-test", Name: firstName.(string)}, stored); err != nil { @@ -442,6 +450,47 @@ func TestExternalOwnerResolveAllowsTenantlessProviderWhenTenantIsNotRequired(t * } } +func TestExternalOwnerResolveRequiresTenantWithoutAllowlistWhenConfigured(t *testing.T) { + config := externalOwnerConfig{ + subjectHashKey: []byte("test-external-owner-secret"), + policies: map[string]externalOwnerPolicy{ + "zenobot": { + PrincipalID: "zenobot", + Issuer: "zenobot", + URL: "http://resolver.example.com/v1/external-owners/resolve", + AllowedProviders: map[string]struct{}{ + "slack": {}, + }, + TenantRequired: map[string]struct{}{ + "slack": {}, + }, + }, + }, + resolver: fakeExternalOwnerResolver{ + resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) { + t.Fatal("expected resolver call to be blocked when tenant is required but missing") + return externalOwnerResolution{}, nil + }, + }, + } + + _, err := config.resolve(context.Background(), principal{ID: "zenobot", Type: principalTypeService}, ownerRef{ + Type: "external", + Provider: "slack", + Subject: "U123456", + }, "") + if err == nil { + t.Fatal("expected resolve to fail when tenant is required but missing") + } + var resolutionErr externalOwnerResolutionError + if !errors.As(err, &resolutionErr) { + t.Fatalf("expected externalOwnerResolutionError, got %T", err) + } + if resolutionErr.code != "external_identity_forbidden" { + t.Fatalf("expected external_identity_forbidden, got %q", resolutionErr.code) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123", diff --git a/api/provisioning.go b/api/provisioning.go index 3188922..44b791a 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -1159,6 +1159,9 @@ func summarizeCreateResponse(spritz *spritzv1.Spritz, principal principal, prese if principal.isService() && hasExternalOwnerAnnotations(annotations) { ownerID = "" responseSpritz.Spec.Owner.ID = "" + if responseSpritz.Labels != nil { + delete(responseSpritz.Labels, ownerLabelKey) + } } if principal.isService() { if storedPresetID := strings.TrimSpace(annotations[presetIDAnnotationKey]); storedPresetID != "" { From 67d62dde5454e94f617d75b1832b61cb5e881a5b Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:41:05 +0100 Subject: [PATCH 14/17] fix(api): key external owner replay by issuer --- api/external_owner_resolution.go | 20 +++++++++- api/main_create_external_owner_test.go | 54 ++++++++++++++++++++++++++ api/provisioning.go | 29 +++++++++++--- api/provisioning_transaction.go | 5 ++- 4 files changed, 99 insertions(+), 9 deletions(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index 716a76a..a00b29e 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -169,7 +169,7 @@ func newExternalOwnerConfig() (externalOwnerConfig, error) { URL: urlValue, AuthHeader: strings.TrimSpace(input.AuthHeader), AllowedProviders: allowedProviders, - AllowedTenants: normalizeStringSet(input.AllowedTenants), + AllowedTenants: normalizeTenantSet(input.AllowedTenants), TenantRequired: normalizeTokenSet(input.TenantRequired), Timeout: timeout, } @@ -212,6 +212,24 @@ func normalizeStringSet(values []string) map[string]struct{} { return out } +func normalizeTenantSet(values []string) map[string]struct{} { + if len(values) == 0 { + return nil + } + out := make(map[string]struct{}, len(values)) + for _, value := range values { + token := strings.TrimSpace(value) + if token == "" { + continue + } + if parsed, err := uuid.Parse(token); err == nil { + token = parsed.String() + } + out[token] = struct{}{} + } + return out +} + func (c externalOwnerConfig) enabled() bool { return c.resolver != nil && len(c.policies) > 0 } diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index 3a4d2ff..dbcae7f 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -491,6 +491,24 @@ func TestExternalOwnerResolveRequiresTenantWithoutAllowlistWhenConfigured(t *tes } } +func TestNewExternalOwnerConfigNormalizesTenantAllowlistUUIDs(t *testing.T) { + t.Setenv("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY", "test-external-owner-secret") + t.Setenv("SPRITZ_EXTERNAL_OWNER_POLICIES_JSON", `[{"principalId":"zenobot","url":"http://resolver.example.com/v1/external-owners/resolve","allowedProviders":["msteams"],"allowedTenants":["72F988BF-86F1-41AF-91AB-2D7CD011DB47"]}]`) + + config, err := newExternalOwnerConfig() + if err != nil { + t.Fatalf("newExternalOwnerConfig failed: %v", err) + } + + policy, ok := config.policyForPrincipal(principal{ID: "zenobot", Type: principalTypeService}) + if !ok { + t.Fatal("expected policy for principal zenobot") + } + if _, ok := policy.AllowedTenants["72f988bf-86f1-41af-91ab-2d7cd011db47"]; !ok { + t.Fatalf("expected canonical lowercase tenant UUID in allowlist, got %#v", policy.AllowedTenants) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123", @@ -554,6 +572,42 @@ func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T } } +func TestCreateRequestFingerprintIncludesExternalIssuer(t *testing.T) { + firstFingerprint, err := createRequestFingerprintWithIssuer(createRequest{ + OwnerRef: &ownerRef{ + Type: "external", + Provider: "msteams", + Tenant: "72f988bf-86f1-41af-91ab-2d7cd011db47", + Subject: "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f", + }, + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "issuer-a", "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprintWithIssuer failed for issuer-a: %v", err) + } + + secondFingerprint, err := createRequestFingerprintWithIssuer(createRequest{ + OwnerRef: &ownerRef{ + Type: "external", + Provider: "msteams", + Tenant: "72f988bf-86f1-41af-91ab-2d7cd011db47", + Subject: "6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f", + }, + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + }, "issuer-b", "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprintWithIssuer failed for issuer-b: %v", err) + } + + if firstFingerprint == secondFingerprint { + t.Fatalf("expected issuer to affect external owner fingerprint") + } +} + func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) { body := &createRequest{ OwnerRef: &ownerRef{Type: "owner", ID: "user-123"}, diff --git a/api/provisioning.go b/api/provisioning.go index 44b791a..8218762 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -506,10 +506,15 @@ func (s *server) enforceProvisionerQuotas(ctx context.Context, namespace string, } func createRequestFingerprint(body createRequest, namespace, name, namePrefix string, userConfig json.RawMessage) (string, error) { + return createRequestFingerprintWithIssuer(body, "", namespace, name, namePrefix, userConfig) +} + +func createRequestFingerprintWithIssuer(body createRequest, externalIssuer, namespace, name, namePrefix string, userConfig json.RawMessage) (string, error) { return createFingerprint( body.OwnerID, body.OwnerRef, body.Spec.Owner.ID, + externalIssuer, sanitizeSpritzNameToken(body.PresetID), strings.TrimSpace(name), sanitizeSpritzNameToken(namePrefix), @@ -690,6 +695,7 @@ func (s *server) resolvedCreateFingerprint(body createRequest, namespace, explic body.OwnerID, body.OwnerRef, body.Spec.Owner.ID, + "", sanitizeSpritzNameToken(body.PresetID), strings.TrimSpace(body.Name), sanitizeSpritzNameToken(namePrefix), @@ -700,13 +706,13 @@ func (s *server) resolvedCreateFingerprint(body createRequest, namespace, explic ) } -func (s *server) provisionerIdempotencyFingerprints(requestBody, resolvedBody createRequest, resolvedExternalOwner *externalOwnerResolution, namespace string, userConfig json.RawMessage) (provisionerIdempotencyState, error) { +func (s *server) provisionerIdempotencyFingerprints(requestBody, resolvedBody createRequest, resolvedExternalOwner *externalOwnerResolution, externalIssuer, namespace string, userConfig json.RawMessage) (provisionerIdempotencyState, error) { canonicalName := strings.TrimSpace(requestBody.Name) canonicalNamePrefix := "" if canonicalName == "" { canonicalNamePrefix = strings.TrimSpace(requestBody.NamePrefix) } - canonicalFingerprint, err := createRequestFingerprint(requestBody, namespace, canonicalName, canonicalNamePrefix, userConfig) + canonicalFingerprint, err := createRequestFingerprintWithIssuer(requestBody, externalIssuer, namespace, canonicalName, canonicalNamePrefix, userConfig) if err != nil { return provisionerIdempotencyState{}, err } @@ -838,11 +844,11 @@ func hashLabelValue(prefix, value string) string { return fmt.Sprintf("%s-%x", prefix, sum[:12]) } -func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, presetID, name, namePrefix, namespace, source string, spec spritzv1.SpritzSpec, userConfig json.RawMessage) (string, error) { +func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, externalIssuer, presetID, name, namePrefix, namespace, source string, spec spritzv1.SpritzSpec, userConfig json.RawMessage) (string, error) { specCopy := spec specCopy.Annotations = nil specCopy.Labels = nil - ownerPayload, err := canonicalOwnerFingerprintPayload(ownerID, ref, resolvedOwnerID) + ownerPayload, err := canonicalOwnerFingerprintPayload(ownerID, ref, resolvedOwnerID, externalIssuer) if err != nil { return "", err } @@ -873,7 +879,7 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, presetID, return fmt.Sprintf("%x", sum[:]), nil } -func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwnerID string) (any, error) { +func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwnerID, externalIssuer string) (any, error) { switch { case ref != nil: normalizedType := strings.ToLower(strings.TrimSpace(ref.Type)) @@ -887,7 +893,11 @@ func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwn if err != nil { return nil, err } - return canonicalOwnerRefPayload(normalized), nil + payload := canonicalOwnerRefPayload(normalized) + if issuer := strings.TrimSpace(externalIssuer); issuer != "" { + payload["issuer"] = issuer + } + return payload, nil case "": return nil, fmt.Errorf("ownerRef.type is required") default: @@ -903,6 +913,13 @@ func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwn return map[string]string{"ownerId": strings.TrimSpace(resolvedOwnerID)}, nil } +func (s *server) externalOwnerIssuerForPrincipal(principal principal) string { + if policy, ok := s.externalOwners.policyForPrincipal(principal); ok { + return policy.issuer() + } + return strings.TrimSpace(principal.ID) +} + func canonicalOwnerRefPayload(ref ownerRef) map[string]string { payload := map[string]string{ "type": strings.ToLower(strings.TrimSpace(ref.Type)), diff --git a/api/provisioning_transaction.go b/api/provisioning_transaction.go index d20031b..83b30b3 100644 --- a/api/provisioning_transaction.go +++ b/api/provisioning_transaction.go @@ -123,12 +123,13 @@ func (tx *provisionerCreateTransaction) prepare() error { if strings.TrimSpace(tx.body.IdempotencyKey) == "" { return newProvisionerCreateError(http.StatusBadRequest, errors.New("idempotencyKey is required")) } + externalIssuer := tx.server.externalOwnerIssuerForPrincipal(tx.principal) canonicalName := strings.TrimSpace(tx.fingerprintRequest.Name) canonicalNamePrefix := "" if canonicalName == "" { canonicalNamePrefix = strings.TrimSpace(tx.fingerprintRequest.NamePrefix) } - fingerprint, err := createRequestFingerprint(tx.fingerprintRequest, tx.namespace, canonicalName, canonicalNamePrefix, tx.normalizedUserConfig) + fingerprint, err := createRequestFingerprintWithIssuer(tx.fingerprintRequest, externalIssuer, tx.namespace, canonicalName, canonicalNamePrefix, tx.normalizedUserConfig) if err != nil { return err } @@ -180,7 +181,7 @@ func (tx *provisionerCreateTransaction) prepare() error { if err := resolveCreateLifetimes(&tx.body.Spec, tx.server.provisioners, true); err != nil { return newProvisionerCreateError(http.StatusBadRequest, err) } - tx.idempotencyState, err = tx.server.provisionerIdempotencyFingerprints(tx.fingerprintRequest, *tx.body, tx.resolvedExternalOwner, tx.namespace, tx.normalizedUserConfig) + tx.idempotencyState, err = tx.server.provisionerIdempotencyFingerprints(tx.fingerprintRequest, *tx.body, tx.resolvedExternalOwner, externalIssuer, tx.namespace, tx.normalizedUserConfig) if err != nil { return newProvisionerCreateError(http.StatusInternalServerError, err) } From 4dc4ab6730bc28894250bf663dc814ece5dbfea2 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 22:49:07 +0100 Subject: [PATCH 15/17] fix(api): keep legacy create fingerprints stable --- api/external_owner_resolution.go | 9 ++-- api/main_create_external_owner_test.go | 61 ++++++++++++++++++++++++++ api/provisioning.go | 24 +++++----- 3 files changed, 80 insertions(+), 14 deletions(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index a00b29e..705b3f0 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -409,21 +409,24 @@ func normalizeExternalOwnerRef(ref ownerRef) (ownerRef, error) { if normalized.Subject == "" { return normalized, fmt.Errorf("ownerRef.subject is required") } + if normalized.Tenant != "" { + if tenantID, err := uuid.Parse(normalized.Tenant); err == nil { + normalized.Tenant = tenantID.String() + } + } switch normalized.Provider { case "msteams": if normalized.Tenant == "" { return normalized, fmt.Errorf("ownerRef.tenant is required for msteams") } - tenantID, err := uuid.Parse(normalized.Tenant) - if err != nil { + if _, err := uuid.Parse(normalized.Tenant); err != nil { return normalized, fmt.Errorf("ownerRef.tenant must be a valid UUID for msteams") } subjectID, err := uuid.Parse(normalized.Subject) if err != nil { return normalized, fmt.Errorf("ownerRef.subject must be a valid UUID for msteams") } - normalized.Tenant = tenantID.String() normalized.Subject = subjectID.String() } diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index dbcae7f..b41ca18 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -3,8 +3,10 @@ package main import ( "bytes" "context" + "crypto/sha256" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "strings" @@ -608,6 +610,65 @@ func TestCreateRequestFingerprintIncludesExternalIssuer(t *testing.T) { } } +func TestCreateRequestFingerprintPreservesLegacyDirectOwnerShape(t *testing.T) { + body := createRequest{ + OwnerID: "user-123", + PresetID: "openclaw", + Spec: spritzv1.SpritzSpec{ + Image: "example.com/spritz-openclaw:latest", + }, + } + + got, err := createRequestFingerprint(body, "spritz-test", "", "", nil) + if err != nil { + t.Fatalf("createRequestFingerprint failed: %v", err) + } + + specCopy := body.Spec + specCopy.Annotations = nil + specCopy.Labels = nil + legacyPayload := struct { + OwnerID string `json:"ownerId"` + PresetID string `json:"presetId,omitempty"` + Name string `json:"name,omitempty"` + NamePrefix string `json:"namePrefix,omitempty"` + Namespace string `json:"namespace,omitempty"` + Source string `json:"source,omitempty"` + Spec spritzv1.SpritzSpec `json:"spec"` + UserConfig json.RawMessage `json:"userConfig,omitempty"` + }{ + OwnerID: "user-123", + PresetID: "openclaw", + Namespace: "spritz-test", + Source: provisionerSource(&body), + Spec: specCopy, + } + encoded, err := json.Marshal(legacyPayload) + if err != nil { + t.Fatalf("failed to marshal legacy fingerprint payload: %v", err) + } + sum := sha256.Sum256(encoded) + want := fmt.Sprintf("%x", sum[:]) + if got != want { + t.Fatalf("expected legacy direct-owner fingerprint %q, got %q", want, got) + } +} + +func TestNormalizeExternalOwnerRefCanonicalizesUUIDTenantForAllProviders(t *testing.T) { + normalized, err := normalizeExternalOwnerRef(ownerRef{ + Type: "external", + Provider: "slack", + Tenant: "72F988BF-86F1-41AF-91AB-2D7CD011DB47", + Subject: "U123456", + }) + if err != nil { + t.Fatalf("normalizeExternalOwnerRef failed: %v", err) + } + if normalized.Tenant != "72f988bf-86f1-41af-91ab-2d7cd011db47" { + t.Fatalf("expected canonical tenant UUID, got %q", normalized.Tenant) + } +} + func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) { body := &createRequest{ OwnerRef: &ownerRef{Type: "owner", ID: "user-123"}, diff --git a/api/provisioning.go b/api/provisioning.go index 8218762..9b5ba6e 100644 --- a/api/provisioning.go +++ b/api/provisioning.go @@ -848,12 +848,13 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, externalI specCopy := spec specCopy.Annotations = nil specCopy.Labels = nil - ownerPayload, err := canonicalOwnerFingerprintPayload(ownerID, ref, resolvedOwnerID, externalIssuer) + canonicalOwnerID, ownerPayload, err := canonicalOwnerFingerprintPayload(ownerID, ref, resolvedOwnerID, externalIssuer) if err != nil { return "", err } payload := struct { - Owner any `json:"owner"` + OwnerID string `json:"ownerId,omitempty"` + Owner any `json:"owner,omitempty"` PresetID string `json:"presetId,omitempty"` Name string `json:"name,omitempty"` NamePrefix string `json:"namePrefix,omitempty"` @@ -862,6 +863,7 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, externalI Spec spritzv1.SpritzSpec `json:"spec"` UserConfig json.RawMessage `json:"userConfig,omitempty"` }{ + OwnerID: canonicalOwnerID, Owner: ownerPayload, PresetID: presetID, Name: name, @@ -879,38 +881,38 @@ func createFingerprint(ownerID string, ref *ownerRef, resolvedOwnerID, externalI return fmt.Sprintf("%x", sum[:]), nil } -func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwnerID, externalIssuer string) (any, error) { +func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwnerID, externalIssuer string) (string, any, error) { switch { case ref != nil: normalizedType := strings.ToLower(strings.TrimSpace(ref.Type)) switch normalizedType { case "owner": if id := strings.TrimSpace(ref.ID); id != "" { - return map[string]string{"ownerId": id}, nil + return id, nil, nil } case "external": normalized, err := normalizeExternalOwnerRef(*ref) if err != nil { - return nil, err + return "", nil, err } payload := canonicalOwnerRefPayload(normalized) if issuer := strings.TrimSpace(externalIssuer); issuer != "" { payload["issuer"] = issuer } - return payload, nil + return "", payload, nil case "": - return nil, fmt.Errorf("ownerRef.type is required") + return "", nil, fmt.Errorf("ownerRef.type is required") default: - return nil, fmt.Errorf("ownerRef.type must be owner or external") + return "", nil, fmt.Errorf("ownerRef.type must be owner or external") } case strings.TrimSpace(ownerID) != "": - return map[string]string{"ownerId": strings.TrimSpace(ownerID)}, nil + return strings.TrimSpace(ownerID), nil, nil } if strings.TrimSpace(ownerID) != "" { - return map[string]string{"ownerId": strings.TrimSpace(ownerID)}, nil + return strings.TrimSpace(ownerID), nil, nil } - return map[string]string{"ownerId": strings.TrimSpace(resolvedOwnerID)}, nil + return strings.TrimSpace(resolvedOwnerID), nil, nil } func (s *server) externalOwnerIssuerForPrincipal(principal principal) string { From 84d340b5361274b0fea3092a6696dd49e707ecfc Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 23:03:34 +0100 Subject: [PATCH 16/17] feat(api): support env-backed resolver auth --- api/external_owner_resolution.go | 26 ++++++++++++++++++++- api/main_create_external_owner_test.go | 32 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index 705b3f0..425a860 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -64,6 +64,7 @@ type externalOwnerPolicyInput struct { Issuer string `json:"issuer,omitempty"` URL string `json:"url"` AuthHeader string `json:"authHeader,omitempty"` + AuthHeaderEnv string `json:"authHeaderEnv,omitempty"` AllowedProviders []string `json:"allowedProviders,omitempty"` AllowedTenants []string `json:"allowedTenants,omitempty"` TenantRequired []string `json:"tenantRequired,omitempty"` @@ -152,6 +153,10 @@ func newExternalOwnerConfig() (externalOwnerConfig, error) { if len(allowedProviders) == 0 { return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].allowedProviders is required", index) } + authHeader, err := resolveExternalOwnerAuthHeader(input) + if err != nil { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].%v", index, err) + } timeout := defaultExternalOwnerResolverTimeout if strings.TrimSpace(input.Timeout) != "" { parsed, err := time.ParseDuration(strings.TrimSpace(input.Timeout)) @@ -167,7 +172,7 @@ func newExternalOwnerConfig() (externalOwnerConfig, error) { PrincipalID: principalID, Issuer: strings.TrimSpace(input.Issuer), URL: urlValue, - AuthHeader: strings.TrimSpace(input.AuthHeader), + AuthHeader: authHeader, AllowedProviders: allowedProviders, AllowedTenants: normalizeTenantSet(input.AllowedTenants), TenantRequired: normalizeTokenSet(input.TenantRequired), @@ -197,6 +202,25 @@ func normalizeTokenSet(values []string) map[string]struct{} { return out } +func resolveExternalOwnerAuthHeader(input externalOwnerPolicyInput) (string, error) { + literal := strings.TrimSpace(input.AuthHeader) + envName := strings.TrimSpace(input.AuthHeaderEnv) + if literal != "" && envName != "" { + return "", fmt.Errorf("only one of authHeader or authHeaderEnv may be set") + } + if envName == "" { + return literal, nil + } + value := strings.TrimSpace(os.Getenv(envName)) + if value == "" { + return "", fmt.Errorf("authHeaderEnv %q is empty", envName) + } + if strings.ContainsAny(value, " \t") { + return value, nil + } + return "Bearer " + value, nil +} + func normalizeStringSet(values []string) map[string]struct{} { if len(values) == 0 { return nil diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index b41ca18..db187c2 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -511,6 +511,38 @@ func TestNewExternalOwnerConfigNormalizesTenantAllowlistUUIDs(t *testing.T) { } } +func TestNewExternalOwnerConfigResolvesAuthHeaderFromEnv(t *testing.T) { + t.Setenv("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY", "test-external-owner-secret") + t.Setenv("SPRITZ_INTERNAL_TOKEN", "spritz-internal-token") + t.Setenv("SPRITZ_EXTERNAL_OWNER_POLICIES_JSON", `[{"principalId":"zenobot","url":"http://resolver.example.com/v1/external-owners/resolve","authHeaderEnv":"SPRITZ_INTERNAL_TOKEN","allowedProviders":["discord"]}]`) + + config, err := newExternalOwnerConfig() + if err != nil { + t.Fatalf("newExternalOwnerConfig failed: %v", err) + } + + policy, ok := config.policyForPrincipal(principal{ID: "zenobot", Type: principalTypeService}) + if !ok { + t.Fatal("expected policy for principal zenobot") + } + if policy.AuthHeader != "Bearer spritz-internal-token" { + t.Fatalf("expected bearer auth header from env token, got %q", policy.AuthHeader) + } +} + +func TestNewExternalOwnerConfigRejectsEmptyAuthHeaderEnv(t *testing.T) { + t.Setenv("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY", "test-external-owner-secret") + t.Setenv("SPRITZ_EXTERNAL_OWNER_POLICIES_JSON", `[{"principalId":"zenobot","url":"http://resolver.example.com/v1/external-owners/resolve","authHeaderEnv":"SPRITZ_INTERNAL_TOKEN","allowedProviders":["discord"]}]`) + + _, err := newExternalOwnerConfig() + if err == nil { + t.Fatal("expected newExternalOwnerConfig to reject empty authHeaderEnv") + } + if !strings.Contains(err.Error(), `authHeaderEnv "SPRITZ_INTERNAL_TOKEN" is empty`) { + t.Fatalf("expected authHeaderEnv validation error, got %v", err) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123", From 333e7ab60cda1f9a9711a90b0b33182f19ceec76 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 12 Mar 2026 23:12:03 +0100 Subject: [PATCH 17/17] fix(api): tighten external owner resolver errors --- api/external_owner_resolution.go | 18 +++++++++++++++ api/jsend.go | 31 +++++++++++++++++++++++++- api/jsend_test.go | 29 ++++++++++++++++++++++++ api/main_create_external_owner_test.go | 13 +++++++++++ 4 files changed, 90 insertions(+), 1 deletion(-) diff --git a/api/external_owner_resolution.go b/api/external_owner_resolution.go index 425a860..331b9ca 100644 --- a/api/external_owner_resolution.go +++ b/api/external_owner_resolution.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "regexp" "strings" @@ -149,6 +150,9 @@ func newExternalOwnerConfig() (externalOwnerConfig, error) { if urlValue == "" { return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].url is required", index) } + if _, err := validateExternalOwnerResolverURL(urlValue); err != nil { + return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].url %v", index, err) + } allowedProviders := normalizeTokenSet(input.AllowedProviders) if len(allowedProviders) == 0 { return externalOwnerConfig{}, fmt.Errorf("invalid SPRITZ_EXTERNAL_OWNER_POLICIES_JSON: policies[%d].allowedProviders is required", index) @@ -221,6 +225,20 @@ func resolveExternalOwnerAuthHeader(input externalOwnerPolicyInput) (string, err return "Bearer " + value, nil } +func validateExternalOwnerResolverURL(raw string) (*url.URL, error) { + parsed, err := url.Parse(raw) + if err != nil { + return nil, fmt.Errorf("is invalid: %w", err) + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return nil, fmt.Errorf("must use http or https") + } + if strings.TrimSpace(parsed.Host) == "" { + return nil, fmt.Errorf("must include a host") + } + return parsed, nil +} + func normalizeStringSet(values []string) map[string]struct{} { if len(values) == 0 { return nil diff --git a/api/jsend.go b/api/jsend.go index c6f83ad..02bdeff 100644 --- a/api/jsend.go +++ b/api/jsend.go @@ -1,6 +1,10 @@ package main -import "github.com/labstack/echo/v4" +import ( + "net/http" + + "github.com/labstack/echo/v4" +) type jsendResponse struct { Status string `json:"status"` @@ -26,6 +30,14 @@ func writeJSendFail(c echo.Context, status int, message string) error { } func writeJSendFailData(c echo.Context, status int, payload any) error { + if status >= 500 { + return c.JSON(status, jsendResponse{ + Status: "error", + Message: jsendErrorMessage(payload, status), + Code: status, + Data: payload, + }) + } return c.JSON(status, jsendResponse{ Status: "fail", Data: payload, @@ -42,3 +54,20 @@ func writeError(c echo.Context, status int, message string) error { } return writeJSendFail(c, status, message) } + +func jsendErrorMessage(payload any, status int) string { + if data, ok := payload.(map[string]any); ok { + if message, ok := data["message"].(string); ok && message != "" { + return message + } + } + if data, ok := payload.(map[string]string); ok { + if message, ok := data["message"]; ok && message != "" { + return message + } + } + if message := http.StatusText(status); message != "" { + return message + } + return "internal server error" +} diff --git a/api/jsend_test.go b/api/jsend_test.go index 08991d1..b94bbee 100644 --- a/api/jsend_test.go +++ b/api/jsend_test.go @@ -86,3 +86,32 @@ func TestWriteErrorUsesErrorForServerErrors(t *testing.T) { t.Fatalf("expected code %d, got %d", http.StatusInternalServerError, resp.Code) } } + +func TestWriteJSendFailDataUsesErrorForServerErrors(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + payload := map[string]any{ + "message": "resolver unavailable", + "error": "external_identity_resolution_unavailable", + } + if err := writeJSendFailData(c, http.StatusServiceUnavailable, payload); err != nil { + t.Fatalf("writeJSendFailData failed: %v", err) + } + + var resp jsendResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if resp.Status != "error" { + t.Fatalf("expected status error, got %q", resp.Status) + } + if resp.Message != "resolver unavailable" { + t.Fatalf("expected message to be propagated, got %q", resp.Message) + } + if resp.Code != http.StatusServiceUnavailable { + t.Fatalf("expected code %d, got %d", http.StatusServiceUnavailable, resp.Code) + } +} diff --git a/api/main_create_external_owner_test.go b/api/main_create_external_owner_test.go index db187c2..574c558 100644 --- a/api/main_create_external_owner_test.go +++ b/api/main_create_external_owner_test.go @@ -543,6 +543,19 @@ func TestNewExternalOwnerConfigRejectsEmptyAuthHeaderEnv(t *testing.T) { } } +func TestNewExternalOwnerConfigRejectsMalformedResolverURL(t *testing.T) { + t.Setenv("SPRITZ_EXTERNAL_OWNER_SUBJECT_HASH_KEY", "test-external-owner-secret") + t.Setenv("SPRITZ_EXTERNAL_OWNER_POLICIES_JSON", `[{"principalId":"zenobot","url":"resolver.example.com/v1/external-owners/resolve","allowedProviders":["discord"]}]`) + + _, err := newExternalOwnerConfig() + if err == nil { + t.Fatal("expected newExternalOwnerConfig to reject malformed resolver url") + } + if !strings.Contains(err.Error(), "url must use http or https") { + t.Fatalf("expected resolver url validation error, got %v", err) + } +} + func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) { directFingerprint, err := createRequestFingerprint(createRequest{ OwnerID: "user-123",