Repository navigation
fix(keycardai-oauth): preserve authorization endpoint query parameters - #372
Draft
Kayvan-Zahiri wants to merge 1 commit into
Draft
Kayvan-Zahiri wants to merge 1 commit into
Kayvan-Zahiri wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
build_authorize_urlappends?unconditionally, so an authorization endpoint such ashttps://auth.example.com/authorize?tenant=acmebecomes...?tenant=acme?response_type=code&.... The tenant value changes andresponse_typeis missing as a separate query parameter. A trailing empty?similarly produces??response_type=code.Split the endpoint and append encoded OAuth parameters to its existing query component. Preserve existing parameter order, percent-encoding and blank values without decoding/re-encoding them. Endpoints without an existing query retain their usual output. This follows RFC 6749 section 3.1, which requires retaining an authorization endpoint's existing query when adding parameters.
Add three public-builder cases (tenant query, encoded/blank parameters, empty query) and one public
begin_authorizationcase using supplied authorization-server metadata. They verify tenant routing, OAuth fields, repeated resources and PKCE state/challenge. All four fail on the original source and pass after the fix; no browser or server is needed for these new regressions.Validation:
ruff check, OAuth-sourcety check, changed-file Ruff formatting andgit diff --check: pass using the committed lock and declared tools.Other package test suites, the separate FastMCP environment, full seven-package typecheck, hosted authorization and platform/Python matrices were not run. No browser, hosted credentials or production login was used. No dependencies, generated files or lockfiles change. Developed with Codex assistance and reproduced locally.