Conversation
|
View your CI Pipeline Execution ↗ for commit 455570a
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the resource query parameter was being stripped from request.query by the Zod type provider in GET routes. The implementation now extracts the resource parameters directly from the raw request URL using URLSearchParams to ensure they are correctly captured. Corresponding tests have been updated and expanded to verify that both single and multiple resource parameters are correctly parsed even when absent from the validated query object. I have no feedback to provide.
…onsent paths fastify-type-provider-zod@6.1.0 does not write Zod-transformed fields back onto request.query for GET routes, so query.resource was always undefined even when the URL contained resource= params. Both code-creation paths now parse resource indicators directly from request.url via URLSearchParams: • authorize.ts (consent-skip path): parseResourceFromUrl(request.url) • consent.ts GET: parses resource from URL → renders hidden <input> per URI • consent.ts POST: reads body.resource → persists onto the authorization code Tests cover the URL-parse regression, multi-value resource params, and the full consent POST allow/allow_forever paths including resource passthrough. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0249fe5 to
455570a
Compare
resource from request.url (follow-up to #161)
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
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.
Problem
fastify-type-provider-zod@6.1.0does not write Zod-transformed fields back ontorequest.queryfor GET routes. This meantquery.resourcewas alwaysundefinedeven when the URL containedresource=params — so authorization codes were minted with an emptyresourcearray, causing downstream/oauth/tokento produce tokens withaudset to the client UUID instead of the requested resource URI. Theresourceparam IS validated by Zod at schema time (invalid URIs still fail the request), but the parsed value is never surfaced onrequest.query.The same bug affected both code-creation paths:
GET /oauth/authorize.POST /ui/consent. This is the path taken by every new client authorization.Fix
Parse
resourceindicators directly fromrequest.urlviaURLSearchParamsinstead of relying onrequest.query.resource:authorize.ts—parseResourceFromUrl(request.url)on theauthorizationCodes.create()call (consent-skip path)consent.tsGET — parses resource URIs from the URL and renders one<input type="hidden" name="resource" value="..." />per URI so values survive the POSTconsent.tsPOST — readsbody.resourceand passes it toauthorizationCodes.create()Tests
query.resourceabsent but URL carriesresource=→ code gets correct arrayresource=params parsed into arrayallow_foreverpath:resourcefield persisted onto the authorization code🤖 Generated with Claude Code