Skip to content

fix(tenant_consent): retry Graph calls that race directory replication (PPT-2000) - #440

Open
camreeves wants to merge 6 commits into
masterfrom
PPT-2000-graph-replication-retry
Open

fix(tenant_consent): retry Graph calls that race directory replication (PPT-2000)#440
camreeves wants to merge 6 commits into
masterfrom
PPT-2000-graph-replication-retry

Conversation

@camreeves

Copy link
Copy Markdown
Contributor

Problem

The Azure 1-click admin-consent callback dies with a 500 when Microsoft Graph's directory replication is slower than the request sequence. The service principal created for the just-registered application is not yet visible to the appRoleAssignments POST issued ~1 second later:

03:22:32 POST /v1.0/applications                    -> app created
03:22:33 POST /v1.0/servicePrincipals               -> SP 634b1fd1 created
03:22:34 POST /v1.0/servicePrincipals/634b1fd1/appRoleAssignments
         -> 404 {"error":{"code":"Request_ResourceNotFound", ...}}

Reproduced live on placeos-dev 2026-08-04, twice, identical failure point — this is why PPT-2000 could never complete an end-to-end run. It leaves partial state behind: an orphaned app registration in the customer tenant, no auth strategy, and a raw 500 shown to the consenting admin. (The 2025 test runs evidently won this race; current Graph replication in our sandbox tenant loses it deterministically.)

Fix

GraphReplicationRetry.run — retries exactly the replication-lag case (HTTP 404 + Graph error code Request_ResourceNotFound) with a 1/2/4/8/8s backoff (~23s budget). Every other error propagates untouched, so genuine failures (permissions, bad requests) are not retried or masked.

Applied to every Graph call in the consent callback that references an object created moments earlier: the app-role assignments, the oauth2 permission grant, addPassword, and the get/update calls against the freshly created delegated app.

Tests

spec/graph_replication_retry_spec.cr — 5 unit specs: returns block value, retries lag until success (attempt-counted), exhausts backoff and re-raises, does not retry non-404 Graph errors, does not retry 404s that are not replication lag.

Verification

Being verified end-to-end on placeos-dev now (host-built image PPT-2000-retry); will comment with the result.

Related findings (out of scope here, for the ticket)

  • Both admin_consent endpoints skip authentication and the callback trusts caller-supplied tenant/state — needs hardening before production use.
  • The flow is non-idempotent: each run registers fresh Azure apps and a fresh auth strategy.
  • add_outlook_plugin_auth/create_outlook_config use the home-tenant Graph client for an app that lives in the customer tenant — works same-tenant, breaks cross-tenant.
  • PLACE_APP_* env vars are not plumbed into k8s-helm or PlaceOS/local, and the flow is undocumented.

🤖 Generated with Claude Code

…n (PPT-2000)

Microsoft Graph is eventually consistent: the service principal created for
a just-registered application is not always visible to the appRoleAssignments
POST issued one second later. Graph returns 404 Request_ResourceNotFound and
the admin-consent callback dies with a 500, leaving partial state (orphaned
app registration, no auth strategy).

Observed live on placeos-dev 2026-08-04 - two consecutive runs failed at the
identical point:

  POST /v1.0/servicePrincipals                     -> 634b1fd1 created
  POST /v1.0/servicePrincipals/634b1fd1/appRoleAssignments
       -> Request_ResourceNotFound

Add GraphReplicationRetry: retries exactly this case (404 + code
Request_ResourceNotFound) with 1/2/4/8/8s backoff, everything else
propagates untouched. Applied to every Graph call in the consent callback
that references an object created moments earlier.
@github-actions github-actions Bot added the type: bug something isn't working label Aug 4, 2026
The lag has (at least) two presentations depending on which side of the
race loses:
- 404 Request_ResourceNotFound: a just-created service principal is not
  visible to the appRoleAssignments POST (observed when calls are slow)
- 400 Request_BadRequest / NoBackingApplicationObject: the service
  principal POST cannot see the application object registered moments
  earlier (observed on a fast box - the flow raced further ahead)

Both observed live on placeos-dev 2026-08-04 within the same hour,
selected purely by host load. Match both.
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Aug 4, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

Verification on placeos-dev surfaced a second presentation of the same replication lag, now covered by 30579aa:

With the first fix deployed, a faster host raced further ahead and failed one step earlier — the service-principal POST could not see the application object registered milliseconds before:

400 Request_BadRequest
The appId 'fdd80418-...' of the service principal does not reference a valid application object.
details[].code = NoBackingApplicationObject

So the lag presents as 404 Request_ResourceNotFound when the flow loses the later race (SP → role assignment) and 400 Request_BadRequest/NoBackingApplicationObject when it loses the earlier one (app → SP creation) — both observed live within the same hour, selected purely by host speed. replication_lag? now matches both; spec added for the 400 flavor.

Live verification hit a third presentation: 404 Directory_ObjectNotFound
("Unable to read the company information from the directory") thrown by
the SECOND create_application call while the tenant was mid-replication -
a documented-transient directory read failure. Also observed a single
object taking 25s+ to replicate, exceeding the previous 23s budget.

- treat Directory_ObjectNotFound as replication lag
- extend backoff to 1/2/4/8/8/12 (~35s)
- wrap create_application + the management-app read as well
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Aug 4, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

Live verification round 2 (with both fixes deployed): the retries worked — 5 logged backoff attempts absorbed a 25s+ replication delay and the flow progressed further than ever (first app + SP + role assignments + repository row all created) — then died on a third face, thrown by the second create_application call:

404 Directory_ObjectNotFound
"Unable to read the company information from the directory."

This is Microsoft's documented-transient directory read failure under replication load. c8d1e2e adds it to the matcher, extends the backoff to ~35s (a single object was observed taking 25s+ to materialise in the sandbox tenant this afternoon), and wraps the create_application / management-app reads as well.

Worth stating plainly for review: this sandbox tenant is having an exceptionally bad replication day, which has been a gift — three distinct lag signatures surfaced in three consecutive runs. A production tenant will rarely need more than one retry.

The two sequential PATCHes in add_outlook_plugin_auth race each other:
the second (preAuthorizedApplications) validates against a replica that
has not yet seen the scope added by the first, failing with
400 InvalidValue on api.preAuthorizedApplications.delegatedPermissionIds.

Matcher is deliberately narrow (that exact target only) so genuine
InvalidValue validation errors are never retried; spec pins both sides.
Every run now dies one step later than the previous - this was the last
Graph write in the flow.
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Aug 4, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

✅ Verified end-to-end on placeos-dev with 4754675 deployed.

Full admin-consent flow (real Microsoft consent → callback): HTTP 303 in 47.6s, absorbing 11 replication-lag retries across the call chain, zero errors. All artifacts created correctly: both app registrations + service principal + role assignments in the customer tenant, delegated-app secret minted, oauth_strat row populated with client_id/secret, authority login_url/logout_url rewritten, Outlook repository row created. The Backoffice authentication tab shows the new Microsoft AD strategy.

Final tally of replication-lag faces encountered live today (all matched by GraphReplicationRetry, each with a unit spec):

  1. 404 Request_ResourceNotFound — role assignment can't see the just-created service principal
  2. 400 NoBackingApplicationObject — SP creation can't see the just-registered application
  3. 404 Directory_ObjectNotFound — transient directory read while the tenant replicates
  4. 400 InvalidValue on api.preAuthorizedApplications.delegatedPermissionIds — the second PATCH validates against a replica without the first PATCH's scope

Which face you hit depends purely on host speed vs the tenant's replication latency that day — which is why this feature passed testing in 2025 and never completed a run since. 47s is a bad-weather worst case (the sandbox tenant was exceptionally slow today); a healthy tenant should need at most one retry.

…ch run

The ensure block deleted the management app's redirect URI whenever a
consent run completed, which set a trap for the NEXT run: the index
endpoint re-adds the URI seconds before consent, and
login.microsoftonline.com's app-metadata cache does not pick the change
up in time - the admin clicks Accept and gets AADSTS500113 'No reply
address is registered for the application'. Observed live minutes after
the first fully successful run (whose cleanup removed the URI that had
been registered for hours).

The churn defeats itself in the other direction too: the add path's
already-present check can read a stale replica that still lists the
just-removed URI and skip the re-add entirely. And two concurrent
consent flows would have the first finisher delete the URI mid-flight
for the second.

A PlaceOS host's callback URI is stable - register it and leave it.
The index endpoint still adds it when missing (first run on a new
host), it just never gets torn down.
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Aug 4, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

Face five, and this one's a design flaw rather than a missing retrycc9cb3b.

The callback's ensure update_app_redirect_uri(false) deleted the management app's redirect URI after every completed run. That set a trap for the next run: index re-adds the URI seconds before consent, login.microsoftonline.com's app-metadata cache lags the change, and the admin's Accept dies with AADSTS500113: No reply address is registered for the application — on Microsoft's page, where no server-side retry can reach. Observed live ~15 minutes after the first fully successful run (whose cleanup removed a URI that had been registered for hours — which is exactly why every earlier run got past consent fine).

Two adjacent defects from the same churn:

  • the add path's already-present check can read a stale replica that still lists the just-removed URI and skip the re-add entirely;
  • two concurrent consent flows would have the first finisher delete the URI mid-flight for the second.

Fix: register-and-keep. The index endpoint still adds the URI when missing (first run on a new host); it just never tears it down. A host's callback URI accumulating on the management app is correct state, not litter.

…stale read

Observed: SP created successfully -> role assignment fails on lag ->
block retries -> the existence check reads a replica that does not yet
list the SP created one second earlier -> create runs again -> 409
Request_MultipleObjectsWithSameKeyValue (the uniqueness constraint sees
the truth even when reads do not).

Retrying converges: the next read eventually sees the object and the
get-or-create skips creation, proceeding to the assignment.
@github-actions github-actions Bot added type: bug something isn't working and removed type: bug something isn't working labels Aug 4, 2026
@camreeves

Copy link
Copy Markdown
Contributor Author

✅ Verified with the FULL browser flow on placeos-dev (1eb8ef8 deployed): Backoffice button → Microsoft account picker → real consent page → Accept → redirect back into Backoffice. 303 in 54.6s, 10 replication retries absorbed, zero errors. New auth strategy created with populated client_id/secret and the authority's login_url updated. Earlier verifications used a direct callback invocation which bypassed Microsoft's consent page — faces 5 (AADSTS500113) and 6 (409 double-create) were both found by real browser runs, so browser-flow verification is the standard for this endpoint from now on.

One cosmetic bug left as a note: the post-consent redirect lands on /backoffice/#/domains/-/about instead of the domain's authentication tab — the double-slash in redirect_back (/domains/ + /{id}/authentication) gets normalised oddly by the Angular router. One-line fix, happy to append or leave for the UX pass.

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

Labels

type: bug something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants