Skip to content

fix(account): offer only ID token providers on the ID token session - #13759

Merged
ChiragAgg5k merged 2 commits into
mainfrom
fix/id-token-provider-enum
Sep 18, 2026
Merged

ChiragAgg5k merged 2 commits into
mainfrom
fix/id-token-provider-enum

Conversation

@ChiragAgg5k

@ChiragAgg5k ChiragAgg5k commented Sep 18, 2026

Copy link
Copy Markdown
Member

POST /account/sessions/id-token only works with Apple and Google, but the provider parameter whitelisted all 52 OAuth providers and generated the shared OAuthProvider enum. SDKs offered every provider on this method, the docs example was amazon, and sending one passed validation and then failed in the action with PROJECT_PROVIDER_UNSUPPORTED.

The whitelist is now the providers that configure idToken, and the parameter gets its own IdTokenProvider enum.

In the published document, the provider property goes from this:

"provider": {
  "description": "OAuth2 provider that issued the ID token. Currently, supported providers are: apple, google.",
  "type": "string",
  "example": "amazon",
  "title": "OAuthProvider",
  "oneOf": [
    { "type": "string", "enum": ["amazon"], "title": "amazon" },
    { "type": "string", "enum": ["apple"],  "title": "apple" },
    { "type": "string", "enum": ["appwrite"], "title": "appwrite" },
    // … 47 more, through "zoom" and "mock-no-email"
  ]
}

to this:

"provider": {
  "description": "OAuth2 provider that issued the ID token. Currently, supported providers are: apple, google.",
  "type": "string",
  "example": "apple",
  "title": "IdTokenProvider",
  "oneOf": [
    { "type": "string", "enum": ["apple"],  "title": "apple" },
    { "type": "string", "enum": ["google"], "title": "google" }
  ]
}

The example follows the first whitelist entry (OpenAPI3.php, $validator->getList()[0]), so it moves from amazon to apple on its own.

The mock providers stay whitelisted because the e2e account suites sign in with them; the generated enum still excludes them, which is why the branches come out as apple, google. The separate enum name is needed because OAuthProvider is generated elsewhere with the full set.

Found while reviewing appwrite/specs#114, which publishes this route for the first time. The "before" block above is copied from that document.

The route described `apple, google` while whitelisting all 52 OAuth
providers and generating the shared `OAuthProvider` enum, so an SDK
offered every provider on this method and the docs example was `amazon`.
Sending any of them passed validation and then failed in the action with
PROJECT_PROVIDER_UNSUPPORTED.

The whitelist now holds the providers that configure `idToken`, and the
parameter generates its own `IdTokenProvider` enum. The mock providers
stay whitelisted, since the e2e suites sign in with them, and the
generated enum keeps excluding them, so it reads `apple, google`.
@greptile-apps

greptile-apps Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The implementation appears safe to merge, with a non-blocking need for an observable regression test covering request rejection and the generated provider contract.

Fix All in Claude CodeFindings

  1. P2 Missing observable regression coverage
Fix with agent prompt
### Issue 1
src/Appwrite/Platform/Modules/Account/Http/Account/Sessions/IdToken/Create.php:85
This changes both request validation and the generated SDK contract, but the PR contains no regression test. Add an API-level test proving an unsupported provider such as `amazon` is rejected during validation, plus a schema or linter check proving the public `IdTokenProvider` contains only `apple` and `google`. Avoid reconstructing the configuration filter or inspecting action internals in the test, since that would duplicate the implementation instead of protecting observable behavior.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR narrows the native ID-token session provider contract to providers with configured ID-token verification profiles.

  • Uses the same supported-provider set for request validation.
  • Publishes a dedicated IdTokenProvider SDK enum containing Apple and Google.
  • Retains mock providers in runtime validation while excluding them from public documentation and generated SDKs.
  • Needs an observable regression test committed with the behavior change.

Reviews (1) · Last reviewed commit: "test: drop the ID token provider unit te..."

@ChiragAgg5k
ChiragAgg5k merged commit 3390f9d into main Sep 18, 2026
51 of 52 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the fix/id-token-provider-enum branch September 18, 2026 12:12
@github-actions

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → fix/id-token-provider-enum (after).

Metric Before After Change
🚀 Requests/sec 198.9 192.82 -3.1%
⏱️ Latency P50 88.2 ms 89.86 ms +1.9%
⏱️ Latency P95 203.69 ms 212.04 ms +4.1%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 89.86 212.04 12,141 192.82 +8.35
Account 171.67 330.56 639 10.67 +11.45
TablesDB 87.33 172.07 6,603 107.94 +8.62
Storage 81 180.26 3,195 53.83 -5.86
Functions 124.5 254.19 1,704 29.4 +3.12

Top API waits (after)

API request Max wait (ms)
storage.files.create 532.53
account.name.update 511.39
functions.variables.update 494.25
storage.buckets.create 468.29
tablesdb.rows.create 424.8

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants