Optimize tokenRequired middleware performance#467
Merged
dahlia merged 1 commit intofedify-dev:mainfrom May 5, 2026
Merged
Conversation
Split the heavy multi-table JOIN in tokenRequired into two separate queries: a lightweight single-table lookup on access_tokens (always), and a new withAccountOwner middleware that fetches account_owners + accounts + successor only when the route actually needs it. This means requests that fail scope validation never touch the accounts table at all, and the per-request query plan is significantly simpler. Changes: - tokenRequired now issues a single SELECT on access_tokens (no JOINs) - New withAccountOwner middleware fetches account owner on demand; returns 422 if the token has no associated user (client credentials) - Variables type split into Variables (token only) and AccountOwnerVariables (token + accountOwner) for better type safety - All API routes that need the account owner now chain withAccountOwner after scopeRequired, so scope failures short-circuit before the DB hit - GET /api/v1/apps/verify_credentials fetches the application inline - GET /api/v1/tags/:id handles client credentials tokens correctly (returns following: false) instead of rejecting with 422 - Fixed a pre-existing bug in POST /api/v1/tags/:id/follow and unfollow where the UPDATE had no WHERE clause and would have updated every account owner row - OIDC /oauth/userinfo keeps its 401 (not 422) for client credentials tokens, consistent with RFC 6750 Closes fedify-dev#127 Assisted-by: Claude Code:claude-sonnet-4-6 Assisted-by: Codex:gpt-5.5
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every authenticated API request used to trigger a four-table JOIN: access_tokens joined to account_owners, then accounts, then accounts again for the successor relation, plus applications. Most handlers only need to know whether the token is valid and which scopes it has.
The middleware now does two separate jobs. src/oauth/middleware.ts exports a
tokenRequiredthat reads only fromaccess_tokens, and a newwithAccountOwnerthat fetchesaccount_owners,accounts, and the successor account only when a route needs them. Routes that need account data chainwithAccountOwnerafterscopeRequired, so invalid scopes fail before the account query runs.The context type is split the same way:
Variablescontains only the token, andAccountOwnerVariablesadds the account owner. That makes accidentalaccountOwneraccess harder in handlers that did not includewithAccountOwner.The refactor also exposed a few unrelated bugs:
GET /api/v1/apps/verify_credentialsnow queries the application directly byapplicationIdinstead of relying on the eager-loaded relation.GET /api/v1/tags/:idno longer blocks client-credentials tokens. It now returnsfollowing: false, matching Mastodon's "OAuth: Public, or User token" behavior. Thefollowandunfollowhandlers in src/api/v1/tags.ts also had noWHEREclause, so they could update every account owner row.401for client-credentials tokens instead of inheriting the422used bywithAccountOwnerfor Mastodon API routes.Closes #127.