🏃 fix: Improve OpenID Lookup Planning#13229
Conversation
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR adjusts OpenID user/admin lookup queries to avoid query shapes ($or + findOne/limit: 1, including nested disjunctions) that can trigger planner regressions in MongoDB-compatible backends (e.g., Cosmos DB vCore). It does so by replacing the single $or query with an ordered sequence of simple filters, while preserving legacy “issuer-less” migration behavior.
Changes:
- Refactored OpenID user resolution to try issuer-exact and legacy issuer-less filters sequentially (instead of a single
$or), preserving migration semantics. - Applied the same “ordered simple filters” approach to the admin refresh fallback lookup path.
- Updated/added tests to assert the new query shapes and added MongoMemoryServer coverage that validates index-backed plans for the issuer-exact lookup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/auth/refresh.ts | Replaces $or-based admin user lookup with sequential simple filters, with optional tenant scoping. |
| packages/api/src/auth/refresh.spec.ts | Updates assertions to expect the new non-$or filter shape for issuer-bound refresh lookups. |
| packages/api/src/auth/openid.ts | Refactors OpenID lookup condition generation and performs sequential findUser attempts instead of a single $or query. |
| packages/api/src/auth/openid.spec.ts | Updates existing unit tests for sequential lookups and adds MongoMemoryServer tests to validate emitted filters and index-backed execution. |
| api/strategies/openIdJwtStrategy.spec.js | Updates strategy tests to expect the new issuer-bound filter shape rather than $or conditions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
|
@codex review |
danny-avila
force-pushed
the
danny-avila/fix-openid-cosmos-lookup
branch
from
3ef0ed2 to
bde5c9e
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bde5c9e0e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GitNexus: 🚀 deployedThe |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
danny-avila
force-pushed
the
danny-avila/fix-openid-cosmos-lookup
branch
from
5203788 to
f3fd114
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
danny-avila
changed the title
* fix: improve OpenID lookup planning * fix: add issuer-bound source id index * fix: align OpenID source id index * fix: preserve admin refresh recency
Summary
I fixed the OpenID user lookup path to avoid a Cosmos vCore planner regression caused by nested
$orfilters combined withfindOne/limit: 1.openidIdandidOnTheSource, preserving legacy issuer-less migration without nested disjunctions.$or + limitshapes as well.updatedAtmatch across legacy issuer fallback filters before minting a token.idOnTheSource/openidIssuer/tenantIduser index to match the new source-ID fallback lookup while avoiding partial-index requirements for FerretDB compatibility.IXSCANandtotalDocsExamined <= 1.Change Type
Testing
npm run build:data-provider && npm run build:data-schemas && npm run build:apinpx jest --config jest.config.mjs --runTestsByPath src/auth/openid.spec.ts src/auth/refresh.spec.ts --runInBand --coverage=falsenpx jest --runTestsByPath strategies/openIdJwtStrategy.spec.js --runInBandnpx jest --runTestsByPath src/methods/user.methods.spec.ts --runInBand --coverage=falsenpm run build:data-schemas && npm run build:apigit diff --checkTest Configuration:
mongodb-memory-server.Checklist