feat(oauth): First-party CLI client and per-client scope policy - #8029
Conversation
Create the well-known public flagsmith-cli OAuth application via data migration, pinned to a fixed client_id with RFC 8252 loopback redirect URIs so the CLI binary can hardcode it against any instance. Add a management-api scope alongside mcp, and a scopes backend that issues scopes per client class: the first-party CLI may be issued management-api, DCR-registered third-party clients only mcp. Scopes name API surfaces and act as a ceiling; RBAC still decides what the authenticating user can do. Set REFRESH_TOKEN_GRACE_PERIOD_SECONDS so clients can retry a token refresh after a network failure during rotation. beep boop
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughOAuth2 configuration now supports refresh-token reuse protection, a grace period, Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
Docker builds report
|
github-actions
Bot
added
feature
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 412f7bd5-4636-4276-8f72-0cd5caf5a2ee
📒 Files selected for processing (8)
api/app/settings/common.pyapi/oauth2_metadata/constants.pyapi/oauth2_metadata/migrations/0001_create_flagsmith_cli_application.pyapi/oauth2_metadata/migrations/__init__.pyapi/oauth2_metadata/scopes.pyapi/tests/unit/oauth2_metadata/test_authorize_view.pyapi/tests/unit/oauth2_metadata/test_migrations.pyapi/tests/unit/oauth2_metadata/test_scopes.py
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8029 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 1501 1506 +5
Lines 59474 59572 +98
=======================================
+ Hits 58668 58766 +98
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The forward operation refuses to clobber an existing flagsmith-cli application, but the reverse deleted it unconditionally, cascading to any tokens issued through it. Make the reverse a noop; migration tests now simulate a fresh install explicitly instead of relying on reverse deletion. beep boop
github-actions
Bot
added
feature
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/tests/unit/oauth2_metadata/test_migrations.py (1)
54-63: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCover rollback and issued-token preservation.
This test only applies the migration forwards and checks two application fields. It never verifies that rolling back preserves a pre-existing application and its issued tokens, so a regression to destructive rollback could pass. Add a token and exercise the supported rollback path, asserting both records and the existing fields remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: beb45510-cb80-48a9-9e11-6a3152e95dfa
📒 Files selected for processing (2)
api/oauth2_metadata/migrations/0001_create_flagsmith_cli_application.pyapi/tests/unit/oauth2_metadata/test_migrations.py
Importing runtime constants into the migration meant a future edit would silently change what fresh installs create, and the lockstep test would not detect the drift. Inline the values in the migration and assert literals in its tests; constants.py keeps only the client_id the runtime scope policy needs. beep boop
github-actions
Bot
added
feature
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 90956103-1ae0-4ea8-80ed-95a60ec381eb
📒 Files selected for processing (3)
api/oauth2_metadata/constants.pyapi/oauth2_metadata/migrations/0001_create_flagsmith_cli_application.pyapi/tests/unit/oauth2_metadata/test_migrations.py
💤 Files with no reviewable changes (1)
- api/oauth2_metadata/constants.py
Without it, rotation is only cosmetic: replaying a rotated-away refresh token after the grace period merely fails, and a thief holding the current token can keep rotating indefinitely. With reuse protection, a stale replay revokes the whole token family, cutting off both parties and forcing re-authentication. beep boop
The preservation test only checked name and skip_authorization, and the pre-existing row's other fields matched the migration defaults, so an overwrite of those fields would have passed unnoticed. Seed deliberately different values and assert all of them. beep boop
github-actions
Bot
added
feature
emyller
left a comment
emyller
left a comment
There was a problem hiding this comment.
Non-blocking comments about code comments, and one possibly-blocking discussion about scope naming. Looks great otherwise!
Reversing URL names derives the tested path from the code under test, so an accidental route change would go unnoticed. Assert the public paths directly: /api/v1/oauth/authorize/ and /o/token/. beep boop
github-actions
Bot
added
feature
github-actions
Bot
added
docs
github-actions
Bot
added
feature
3 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to Flagsmith/flagsmith-cli#42
In this PR, we prepare the OAuth implementation for the upcoming CLI with the following additions:
flagsmith-clipublic OAuth app with a constantclient_idfor the CLI.admin-apiscope, to distinguish CLI grants from MCP. A customFlagsmithScopesscopes backend makes sureadmin-apiis only allowed for the CLI.REFRESH_TOKEN_GRACE_PERIOD_SECONDSis set to 120 so a client that loses the response to a refresh-token rotation can retry with the previous token instead of being locked out.REFRESH_TOKEN_REUSE_PROTECTIONis enabled: replaying a rotated-away refresh token after the grace period revokes the whole token family, cutting off a potential token thief.How did you test this code?
Added new tests.