fix(OAuth): flagsmith login never completes for a new signup - #8281
fix(OAuth): flagsmith login never completes for a new signup#8281khvn26 wants to merge 3 commits into
flagsmith login never completes for a new signup#8281Conversation
A client that opens the consent screen while the user is logged out has its request stashed in the redirect cookie. `App.onLogin` restored that cookie only after its "no organisation yet" branch, which returns early to route new users into onboarding - so a signup dropped the request and left the client waiting until it timed out. Onboarding now hands the request on instead of losing it. It provisions the organisation the client needs, then answers the request itself: from the create-organisation page once the organisation exists, and from the single-page flow once its workspace is ready, never on load, since bootstrapping is a chain of creates that must not be interrupted half-done. Two things had to stop moving the browser out from under that. Creating an organisation re-fires `onLogin` while the organisation list is still refreshing, whose onboarding branch would then yank the user off a request they were about to authorise; it now leaves the consent screen alone. And a page load stashed the loaded path over a waiting request, so a reload mid-onboarding - which its own Try again performs - lost it. Alongside, on the same path: identity-provider signups now get the same onboarding entry decision as `register` makes, rather than falling through to the legacy create-organisation page; the redirect is re-encoded when re-embedded, so a redirect carrying its own query is no longer truncated at its first `&`; and the cookie holding it expires in an hour instead of thirty days, so an abandoned login cannot hijack a later one. beep boop
The consent screen described the `mcp` scope from a map hardcoded in the
page, so what a scope grants was written a repository apart from the
policy that decides who may hold it, and `admin-api` - the scope the CLI
asks for - got only the registry's one-line label.
Describe scopes in `oauth2_metadata` beside that policy and return
`{label, grants}` per scope, letting the page render whatever it is
given. A scope with no grants written falls back to its label, so adding
one cannot produce an empty permissions box.
Both lists now state that the token acts with the user's own permissions:
neither scope is enforced per request, so the previous wording implied a
boundary that does not exist.
beep boop
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
front-end
Docker builds report
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe API now returns structured OAuth scope descriptions with labels and grants. The frontend uses this metadata on the consent page. Pending authorisation redirects persist through login, organisation creation, and onboarding. Redirect values are encoded, and redirect cookies expire after one hour. OAuth sign-up handling identifies new users without invitations. Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to This PR routes new OAuth signups through onboarding and resumes authorization after organization setup. It is mergeable with owner awareness that the changed TypeScript signatures still need to be aligned with the repository’s named-union-type requirement. 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 #8281 +/- ##
=======================================
Coverage 98.75% 98.75%
=======================================
Files 1569 1571 +2
Lines 62827 62855 +28
=======================================
+ Hits 62042 62070 +28
Misses 785 785 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: 4f78302d-242d-4790-9a8b-b40d8a88f70d
📒 Files selected for processing (17)
api/oauth2_metadata/constants.pyapi/oauth2_metadata/mappers.pyapi/oauth2_metadata/types.pyapi/oauth2_metadata/views.pyapi/tests/unit/oauth2_metadata/test_authorize_view.pyapi/tests/unit/oauth2_metadata/test_mappers.pyfrontend/common/stores/account-store.jsfrontend/common/types/responses.tsfrontend/common/utils/__tests__/pendingAuthorisation.test.tsfrontend/common/utils/pendingAuthorisation.tsfrontend/web/components/App.jsfrontend/web/components/pages/CreateOrganisationPage.tsxfrontend/web/components/pages/OAuthAuthorizePage.tsxfrontend/web/components/pages/home-page/HomePage.tsxfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsxfrontend/web/main.jsfrontend/web/project/api.ts
✅ private-cloud · depot-ubuntu-latest-16 — run #19298 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-arm-16 — run #19298 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19298 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19298 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Skipped testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss ✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19296 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19296 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19296 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #19296 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
It carries what the two accepted forms mean, which a comment was doing. beep boop
emyller
left a comment
emyller
left a comment
There was a problem hiding this comment.
Backend changes look good, bar one test tweak.
| SCOPE_GRANTS: dict[str, tuple[str, ...]] = { | ||
| SCOPE_MCP: ( | ||
| "Manage feature flags, toggle states, and update values", | ||
| "Create and manage audience targeting segments", | ||
| "View and configure environments", | ||
| "View and update project settings", | ||
| "Create and review change requests", | ||
| "View organisation details, roles, and groups", | ||
| "Act with your own permissions, in every organisation you belong to", | ||
| ), | ||
| SCOPE_ADMIN_API: ( | ||
| "Act with your own permissions, in every organisation you belong to", | ||
| "Manage feature flags, segments, environments and projects you can access", | ||
| "Manage organisation settings, members and roles you can access", | ||
| ), | ||
| } |
There was a problem hiding this comment.
| SCOPE_GRANTS: dict[str, tuple[str, ...]] = { | |
| SCOPE_MCP: ( | |
| "Manage feature flags, toggle states, and update values", | |
| "Create and manage audience targeting segments", | |
| "View and configure environments", | |
| "View and update project settings", | |
| "Create and review change requests", | |
| "View organisation details, roles, and groups", | |
| "Act with your own permissions, in every organisation you belong to", | |
| ), | |
| SCOPE_ADMIN_API: ( | |
| "Act with your own permissions, in every organisation you belong to", | |
| "Manage feature flags, segments, environments and projects you can access", | |
| "Manage organisation settings, members and roles you can access", | |
| ), | |
| } | |
| SCOPE_GRANTS: dict[str, frozenset[str, ...]] = { | |
| SCOPE_MCP: frozenset([ | |
| "Manage feature flags, toggle states, and update values", | |
| "Create and manage audience targeting segments", | |
| "View and configure environments", | |
| "View and update project settings", | |
| "Create and review change requests", | |
| "View organisation details, roles, and groups", | |
| "Act with your own permissions, in every organisation you belong to", | |
| ]), | |
| SCOPE_ADMIN_API: frozenset([ | |
| "Act with your own permissions, in every organisation you belong to", | |
| "Manage feature flags, segments, environments and projects you can access", | |
| "Manage organisation settings, members and roles you can access", | |
| ]), | |
| } |
nit: The immutability of tuples does not justify using them as immutable collections. Please, if you agree, prefer to use tuples when they could be interchanged with their verbose brother, NamedTuple.
There was a problem hiding this comment.
It's hard to justify frozenset here considering we don't need O(1) membership checks here; and using a mutable type like list makes things geniunely worse in my opinion.
I'm sorry, I'd rather you come to terms with the fact that tuple[str, ...] is a legitimate type.
There was a problem hiding this comment.
I'm sorry, I'd rather you come to terms with the fact that
tuple[str, ...]is a legitimate type.
I continue to disagree, and I'd be glad to argument, though this isn't worth it. I will try not to pick these from now on.
emyller
left a comment
emyller
left a comment
There was a problem hiding this comment.
Approved from backend standpoint.
talissoncosta
left a comment
talissoncosta
left a comment
There was a problem hiding this comment.
Looks good on FE side! 💯
4 participants
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8165.
In this PR, we:
How did you test this code?
Added unit tests.
Manually, against a local API and dashboard, with
flagsmith-clias the client andscope=admin-api. Open/oauth/authorize/?client_id=flagsmith-cli&response_type=code&redirect_uri=http%3A%2F%2F127.0.0.1%2Fcallback&code_challenge=…&code_challenge_method=S256&scope=admin-api&state=xwhile logged out, then sign up.Will test the Google path in production.