Skip to content

feat: store app-only Graph credentials in the staff-api tenant (PPT-2000) - #442

Open
camreeves wants to merge 3 commits into
PPT-2032-consent-progressfrom
PPT-2000-calendar-tenant-wiring
Open

feat: store app-only Graph credentials in the staff-api tenant (PPT-2000)#442
camreeves wants to merge 3 commits into
PPT-2032-consent-progressfrom
PPT-2000-calendar-tenant-wiring

Conversation

@camreeves

Copy link
Copy Markdown
Contributor

Stacked on #441 (which stacks on #440). Closes the gap where the 1-click flow registered the app-only "PlaceOS Bookings Visualiser" application — with admin-consented Calendars.ReadWrite / Group.Read.All / User.Read.All application permissions — but never minted a secret for it and discarded its client id, so the consented app-only Graph access was unusable and calendar credentials still had to be typed into Backoffice → Admin → Staff API by hand.

What changes

  • create_app now mints a client secret for the visualiser app (same addPassword call the auth app already used, wrapped in the replication retry).
  • New flow step "Connect room calendar access": writes {tenant, client_id, client_secret} into the staff-api Tenant for the domain — created if the domain has none, updated otherwise. The model encrypts credentials at rest (Level::NeverDisplay) and validates them as Office365Config.
  • Existing delegated tenants are switched to app-only (delegated: false). Rationale: the flow already owns the domain's Microsoft configuration (login_url, outlook_config) and the visualiser app exists precisely to provide app-only access. Delegated mode remains one toggle away in Backoffice.
  • Ordering fix: the tenant row is now guaranteed to exist before create_outlook_config, which previously silently no-oped on a fresh domain (Tenant.find_by? → log + return).
  • Progress page picks the new step up automatically (step list is data-driven).
  • Two ameba nits from feat(tenant_consent): live progress page for the admin-consent flow (PPT-2032) #441 fixed (rescue flow_error naming, verbose block).

Verification

The triggers service (GraphSecretExpiryFinder) will start probing the stored credential for secret-expiry metadata — that's existing behaviour for any non-delegated office365 tenant.

🤖 Generated with Claude Code

The admin-consent flow registered the Bookings Visualiser application
with app-only Graph permissions (Calendars.ReadWrite, Group.Read.All,
User.Read.All) but never minted a secret for it and discarded its
client id - the consented access was unusable, and calendar credentials
still had to be entered by hand in Backoffice.

The visualiser registration now mints a client secret and the flow
writes {tenant, client_id, client_secret} into the staff-api tenant for
the domain (created when missing, updated otherwise - the flow owns the
domain's Microsoft configuration, as it already does for login_url and
outlook_config). Existing delegated tenants are switched to app-only;
delegated mode can be re-enabled in Backoffice. Creating the tenant
before the outlook step also fixes the silent no-op where outlook_config
had nothing to attach to on a fresh domain.

Credentials are encrypted at rest by the model (Level::NeverDisplay)
and surface on the progress page as a new 'Connect room calendar
access' step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the type: enhancement new feature or request label Aug 5, 2026
The flow looked up the staff-api tenant and built the Outlook add-in URLs
from the host in the request. Backoffice can drive the integration for any
domain, so integrating one authority while browsing another wrote the new
Microsoft configuration onto the wrong tenant - observed on dev, where
integrating the demo authority (azure-demo.placeos-dev.aca.im) overwrote
the dev domain's own tenant (placeos-dev.aca.im), switching it off
delegated access and replacing its calendar credentials.

The authority record already supplied the domain used for the auth app's
redirect URI; use it for the tenant lookup, the Outlook identifier URIs
and the add-in URLs too. Only the consent callback URL still derives from
the request host, where it has to - it must match the redirect URI
registered on the management application.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added type: enhancement new feature or request and removed type: enhancement new feature or request labels Aug 5, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

Bug found by the first dev e2e run — fixed in a2acb90

The first full browser run (Integrate Azure → real MS consent → Accept) completed cleanly end to end, but it configured the wrong tenant, and that exposed a defect this PR would otherwise have shipped.

What happened: the flow looked up the staff-api tenant (and built the Outlook add-in URLs) from request.hostname. Backoffice can drive the integration for any authority, so integrating authority-AzureDemo2000 (domain azure-demo.placeos-dev.aca.im) while browsing Backoffice on placeos-dev.aca.im wrote the demo's Microsoft configuration onto the dev domain's own tenant row — flipping it from delegated to app-only and replacing its calendar credentials.

This half is pre-existing, not new: create_outlook_config has always keyed off the request host, which is why the dev tenant's outlook_config.app_id has been silently rewritten by every test run. This PR's credential write inherited the same lookup and widened the blast radius from one Outlook field to the calendar credentials themselves — which is what made it visible.

Fix: the authority record already supplied the domain used for the auth app's redirect URI; it is now used for the tenant lookup, the Outlook identifier URIs and the add-in URLs as well. Only the consent callback URL still derives from the request host, where it must (it has to match the redirect URI registered on the management app).

New spec pins the behaviour: a bystander tenant on a different domain must be byte-identical after a flow targeting another authority's domain.

Dev state: the affected row was restored from a pre-run backup (delegated flag and original credentials verified back in place). Its original outlook_config.app_id is unrecoverable — prior runs had already overwritten it before any backup existed; calendar access, the part that matters, is fully restored. All test app registrations have been deleted from the sandbox tenant (back to its 5 permanent apps).

Re-verification with the fixed image is running; will post the result.

@camreeves

Copy link
Copy Markdown
Contributor Author

Re-verified end to end on a2acb90 — passing, with the negative case proven

Full browser run on dev (Integrate Azure → Microsoft's real consent page → Accept):

Positive: a new tenant row was created for the authority's own domain — azure-demo.placeos-dev.aca.im, platform: office365, delegated: false, credentials populated, Outlook config pointing at the auth app. Progress page showed all 5 steps with live replication narration, then redirected to the Authentication tab.

Negative (the point of the fix): every other tenant row is byte-identical to the pre-run baseline — md5 of credentials unchanged and updated_at untouched across all 8 pre-existing rows, including the dev domain's own row which the previous build clobbered.

App-only Graph access actually works

Using the provisioned "PlaceOS Bookings Visualiser" app: service principal present with 3 admin-consented Graph app-role grants; an app-only client-credentials token minted successfully; GET /users OK, GET /groups OK, and GET /users/{id}/calendar returned a real mailbox's calendar — calendar data with no signed-in user, which is the capability this PR exists to enable. (A temporary secret was minted for that test and removed; only the flow's own secret remains.)

Honest boundary: I could not independently decrypt the stored credential to compare it byte-for-byte with what Graph issued — my own reimplementation of PlaceOS::Encryption fails on known-good pre-existing rows too, so it proves nothing either way. What is proven server-side is that the model's before_save validation decrypts and parses Office365Config (tenant/client_id/client_secret all required) or the save raises — and the save succeeded.

Two findings for follow-up (not blocking this PR)

  1. places/microsoft.graph.room returns 403. The flow grants Calendars.ReadWrite, Group.Read.All and User.Read.All, but not Place.Read.All. Room mailbox access works (the booking path), but anything using the Graph places API for room discovery will fail. Worth deciding whether create_app should request it.
  2. GraphSecretExpiryFinder (triggers) can't monitor these tenants. It calls GET /applications(...), which needs Application.Read.All — not granted. Flow-provisioned tenants will log an error each cycle and never populate secret_expiry, so expiry monitoring is blind for exactly the tenants this feature creates. The secret it mints expires 2028-08-05.

Sandbox tenant is back to 5 permanent apps plus the one live demo pair; all other test registrations deleted.

The visualiser application was granted Calendars.ReadWrite, Group.Read.All
and User.Read.All. Room mailboxes were therefore readable, but the Graph
places API - which room discovery uses to enumerate rooms and room lists -
returned 403, verified against the sandbox tenant with a provisioned app.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added type: enhancement new feature or request and removed type: enhancement new feature or request labels Aug 5, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

Place.Read.All added (2d64a87) and verified on dev

Role ID taken from Graph rather than memory — GET /servicePrincipals?$filter=appId eq '00000003-0000-0000-c000-000000000000'appRoles, giving Place.Read.All = 913b9306-0ce1-42b8-9137-6a7df690a760. The same response confirmed the three IDs already in the code are correct.

Verified with a full browser run on the deployed image: the new visualiser app requests 4 application roles and its service principal carries 4 admin-consented grants. Using an app-only token from that app:

  • places/microsoft.graph.room5 rooms returned (was 403)
  • places/microsoft.graph.roomlist → 0 (no room lists defined in the sandbox, not an error)
  • users, groups → still OK

Tenant isolation still holds: the demo domain's row was updated and all eight other tenant rows are byte-identical to the pre-run baseline.

Secret-expiry monitoring is deliberately left as-is for now — it needs Application.Read.All, and the minted secret doesn't expire until 2028-08-05.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement new feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant