-
Notifications
You must be signed in to change notification settings - Fork 271
fix(python): make token parameter optional only for OAuth flows #11040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
…h flows Co-Authored-By: [email protected] <[email protected]>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
… auth This is a targeted fix that matches TypeScript's behavior: - OAuth flows: token is optional (OAuthTokenProvider needs to create wrapper before token exists) - Plain bearer auth: token is required when isAuthMandatory=true Added _has_oauth() method to detect OAuth authentication and use it to determine if the token parameter should be optional. Co-Authored-By: [email protected] <[email protected]>
Added fix as version 4.42.1 since 4.42.0 and 4.41.11 were already taken in main. Co-Authored-By: [email protected] <[email protected]>
tstanmay13
approved these changes
Dec 8, 2025
devin-ai-integration bot
added a commit
that referenced
this pull request
Dec 8, 2025
This fixture tests OAuth flows where all business endpoints require authentication (isAuthMandatory = true). Unlike existing OAuth fixtures that have endpoints with auth: false (making isAuthMandatory = false), this fixture ensures the token parameter remains optional for OAuth flows even when auth is mandatory. This would have caught the regression fixed in PR #11040 where OAuth-based SDKs broke when all endpoints required authentication. Co-Authored-By: [email protected] <[email protected]>
This was referenced Dec 8, 2025
fern-support
pushed a commit
that referenced
this pull request
Dec 9, 2025
) * test: add oauth-client-credentials-mandatory-auth fixture This fixture tests OAuth flows where all business endpoints require authentication (isAuthMandatory = true). Unlike existing OAuth fixtures that have endpoints with auth: false (making isAuthMandatory = false), this fixture ensures the token parameter remains optional for OAuth flows even when auth is mandatory. This would have caught the regression fixed in PR #11040 where OAuth-based SDKs broke when all endpoints required authentication. Co-Authored-By: [email protected] <[email protected]> * chore(seed): add Swift snapshot for oauth-client-credentials-mandatory-auth fixture Co-Authored-By: [email protected] <[email protected]> * chore(seed): add ir-to-jsonschema snapshot for oauth-client-credentials-mandatory-auth fixture Co-Authored-By: [email protected] <[email protected]> * chore(seed): add oauth-client-credentials-mandatory-auth to ruby-sdk allowedFailures Co-Authored-By: [email protected] <[email protected]> * chore(seed): add oauth-client-credentials-mandatory-auth to csharp-sdk and java-sdk allowedFailures Co-Authored-By: [email protected] <[email protected]> * chore(seed): fix java-sdk allowedFailures entry to include :no-custom-config suffix Co-Authored-By: [email protected] <[email protected]> * chore(seed): fix python-sdk seed.yml merge conflict artifacts Co-Authored-By: [email protected] <[email protected]> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.
Description
Fixes a regression in Python SDKs where
SyncClientWrapperandAsyncClientWrapperrequired atokenparameter whenis_auth_mandatory=True. This broke OAuth flows because theOAuthTokenProviderneeds to create a wrapper without a token to fetch the initial token (onlyclientIdandclientSecretare available at that point).Link to Devin run: https://app.devin.ai/sessions/6d9d04443ed841218846f1fc37826bad
Requested by: [email protected] (@fern-support)
Updates Since Last Revision
Implemented a targeted fix that matches TypeScript's behavior:
isAuthMandatory=true(preserves original behavior)This replaces the previous approach that made token always optional for all bearer auth.
Changes Made
_has_oauth()method to detect OAuth authentication schemesclient_wrapper_generator.pyto useis_token_optional = self._has_oauth() or not self._context.ir.sdk_config.is_auth_mandatoryversions.ymlwith version 4.42.1 changelog entryTesting
pnpm seed:local test --generator python-sdk --fixture oauth-client-credentials- passedpnpm seed:local test --generator python-sdk --fixture bearer-token-environment-variable- passedpoetry run ruff checkandpoetry run pre-commit run --all-files- passedHuman Review Checklist
Optional[...]in the client wrapper (allows OAuthTokenProvider to bootstrap)isAuthMandatory=true: token parameter should be required (seeseed/python-sdk/bearer-token-environment-variable/src/seed/core/client_wrapper.py-api_keyis now required, not optional)_has_oauth()method checksscheme_as_union.type == "oauth"- verify this correctly identifies OAuth schemes in the IR