feat: oauth2 scopes for authentication#276
Merged
SoulPancake merged 4 commits intomainfrom Apr 17, 2026
Merged
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis change makes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #276 +/- ##
==========================================
+ Coverage 69.85% 69.91% +0.06%
==========================================
Files 140 140
Lines 10748 10764 +16
==========================================
+ Hits 7508 7526 +18
+ Misses 3240 3238 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SoulPancake
changed the title
1 task
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
207-235: Consider documenting list-basedscopesinput as an alternative.At Line 227, the example only shows a space-delimited string. The SDK also accepts
list[str], so adding a short alternative example would reduce ambiguity for users loading scopes from config arrays.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 207 - 235, Update the OAuth2 example to show that the CredentialConfiguration.scopes field can accept a list of strings as well as a space-delimited string: in the README snippet around ClientConfiguration / Credentials / CredentialConfiguration (the block that sets scopes="email profile"), add a brief alternative line or short example demonstrating scopes=["email", "profile"] so users loading scopes from config arrays understand the supported list[str] input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openfga_sdk/credentials.py`:
- Around line 220-222: The error message raised in the credentials validation
uses a literal "{}" placeholder which is never substituted; update the raise in
the ApiValueError so it interpolates the relevant configuration identifier
(e.g., use an f-string or .format to include the variable referenced as
configuration or self.configuration) in the message for the client_credentials
path (the raise in credentials.py where ApiValueError is thrown). Ensure the
message reads something like "configuration '<name>' requires client_id,
client_secret and api_issuer defined for client_credentials method." and keep
ApiValueError and the surrounding validation logic unchanged apart from
formatting the message.
In `@test/sync/oauth2_test.py`:
- Around line 479-481: Remove the stray class-scope call to rest_client.close()
that sits outside any test/method in oauth2_test.py; locate the dangling
"rest_client.close()" (the second occurrence after the method that ends at line
~479) and delete it so no code runs at import time and test collection won't
raise NameError due to undefined rest_client.
---
Nitpick comments:
In `@README.md`:
- Around line 207-235: Update the OAuth2 example to show that the
CredentialConfiguration.scopes field can accept a list of strings as well as a
space-delimited string: in the README snippet around ClientConfiguration /
Credentials / CredentialConfiguration (the block that sets scopes="email
profile"), add a brief alternative line or short example demonstrating
scopes=["email", "profile"] so users loading scopes from config arrays
understand the supported list[str] input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b64a81c5-9de1-4e33-bba3-263f58208531
📒 Files selected for processing (7)
README.mdopenfga_sdk/credentials.pyopenfga_sdk/oauth2.pyopenfga_sdk/sync/oauth2.pytest/credentials_test.pytest/oauth2_test.pytest/sync/oauth2_test.py
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for standard OAuth2 client-credentials flows by making api_audience optional and supporting scope in token requests.
Changes:
- Update OAuth2 token request construction to omit
audiencewhen unset/blank and includescopewhen configured. - Relax credential validation by making
api_audienceoptional and normalizing blankapi_audience/scopesinputs. - Add async/sync test coverage and extend README guidance for standard OAuth2 providers.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/sync/oauth2_test.py | Adds sync tests covering no-audience and scopes-without-audience token requests. |
| test/oauth2_test.py | Adds async tests covering no-audience and scopes-without-audience token requests. |
| test/credentials_test.py | Updates validation expectations (audience optional) and adds normalization tests for blank inputs. |
| openfga_sdk/sync/oauth2.py | Omits audience when unset/blank; builds scope parameter when configured. |
| openfga_sdk/oauth2.py | Same as sync: omit audience when unset/blank; build scope parameter when configured. |
| openfga_sdk/credentials.py | Makes api_audience optional; normalizes blank/whitespace api_audience and scopes. |
| README.md | Documents optional api_audience, scopes usage, and guidance for non-standard token endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
emilic
approved these changes
Apr 17, 2026
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.
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Release Notes
New Features
api_audienceis now optional for OAuth2 client credentials flow.Documentation
api_audienceandapi_issueroptions, including standard OAuth2 examples.Improvements
api_audienceandscopesare now handled appropriately.