fix(oauth): don't authorize the wrong project while a team_id hint is pending#66438
Merged
lucasheriques merged 2 commits intoJun 26, 2026
Merged
Conversation
… pending The consent screen reads a `team_id` hint from the authorize URL (the wizard's `--project-id`) and pre-selects that project + org once teams load. But `setRequiredAccessLevel` eagerly pre-selected the user's *current* org/team, so on a fast CTA click — or whenever the hinted team lives in a different org — `scoped_teams` briefly pointed at the current project before the async team load resolved the hint. Authorizing in that window granted the wrong project. While a hint is pending, skip the eager current-org selection and leave the project empty so the form blocks submit until the hint fills it in. Fall back to the current org/team only if the hint can't be honored (inaccessible team). Generated-By: PostHog Code Task-Id: 5548224f-4527-4b9d-b320-aaf433240340
Contributor
|
Hey @edwinyjlim! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
Contributor
|
Reviews (1): Last reviewed commit: "fix(oauth): don't authorize the wrong pr..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
196
to
240
| }) | ||
|
|
||
| const HINT_TEAMS = [ | ||
| { id: 11, organization: 'org-a', name: 'A project' }, | ||
| { id: 22, organization: 'org-b', name: 'B project' }, | ||
| ] as any | ||
|
|
||
| it('pre-selects the hinted project and its org from the team_id param', () => { | ||
| logic.actions.setTeamHint(22) | ||
| logic.actions.setRequiredAccessLevel('team') | ||
| logic.actions.loadAllTeamsSuccess(HINT_TEAMS) | ||
| expect(logic.values.selectedOrganization).toBe('org-b') | ||
| expect(logic.values.oauthAuthorization.scoped_teams).toEqual([22]) | ||
| expect(logic.values.teamHint).toBeNull() | ||
| }) | ||
|
|
||
| it('does not pre-select the current project while a team_id hint is pending', () => { | ||
| // The eager current-org selection is what authorized the wrong project on a | ||
| // fast CTA click; with a hint pending it must stay empty until teams load. | ||
| logic.actions.setTeamHint(22) | ||
| logic.actions.setRequiredAccessLevel('team') | ||
| expect(logic.values.selectedOrganization).toBeNull() | ||
| expect(logic.values.oauthAuthorization.scoped_teams).toEqual([]) | ||
| }) | ||
|
|
||
| it('falls back to the current org/team when the hinted team is inaccessible', () => { | ||
| const currentTeam = { | ||
| id: MOCK_DEFAULT_USER.team?.id, | ||
| organization: MOCK_DEFAULT_USER.organization?.id, | ||
| name: 'Current project', | ||
| } | ||
| logic.actions.setTeamHint(999) | ||
| logic.actions.setRequiredAccessLevel('team') | ||
| logic.actions.loadAllTeamsSuccess([...HINT_TEAMS, currentTeam] as any) | ||
| expect(logic.values.teamHint).toBeNull() | ||
| expect(logic.values.selectedOrganization).toBe(MOCK_DEFAULT_USER.organization?.id) | ||
| expect(logic.values.oauthAuthorization.scoped_teams).toEqual([MOCK_DEFAULT_USER.team?.id]) | ||
| }) | ||
|
|
||
| it('pre-selects the current org/team when no team_id hint is given', () => { | ||
| logic.actions.setRequiredAccessLevel('team') | ||
| expect(logic.values.selectedOrganization).toBe(MOCK_DEFAULT_USER.organization?.id) | ||
| expect(logic.values.oauthAuthorization.scoped_teams).toEqual([MOCK_DEFAULT_USER.team?.id]) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
The four new cases split cleanly into two structurally identical groups. Tests 1 and 3 (hint present + loadAllTeamsSuccess) follow the exact same steps — setTeamHint → setRequiredAccessLevel → loadAllTeamsSuccess → assert — with only the hint value, teams array, and expected results varying. Tests 2 and 4 (no loadAllTeamsSuccess) are likewise parallel: conditional setTeamHint → setRequiredAccessLevel → assert. Per the codebase convention (see the effectiveScopesCases table in this same file) these pairs should each be a single it.each table, which would also make it easy to add a fifth case (e.g. hint with an org-level grant) later.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
|
Size Change: -357 kB (-0.55%) Total Size: 64.3 MB 📦 View Changed
ℹ️ View Unchanged
|
assign-reviewers-posthog
Bot
requested review from
a team,
MattBro,
fercgomes and
rafaeelaudibert
and removed request for
a team
lucasheriques
approved these changes
Jun 26, 2026
Contributor
|
Reviews (2): Last reviewed commit: "fix(oauth): don't authorize the wrong pr..." | Re-trigger Greptile |
rafaeelaudibert
approved these changes
Jun 26, 2026
rafaeelaudibert
left a comment
rafaeelaudibert
left a comment
Member
There was a problem hiding this comment.
Code is a bit too complicated now, probably deserves a full refactor at some point
Collapse the four new hint cases into two it.each tables (resolution after teams load, and eager-selection before), matching the effectiveScopesCases convention already used in this file. No behavior change. Generated-By: PostHog Code Task-Id: 5548224f-4527-4b9d-b320-aaf433240340
Contributor
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
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
3 participants
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.
Problem
The OAuth consent screen pre-selects a project + org from a
team_idhint in the authorize URL (the wizard's--project-id, PostHog/wizard#743), so a CTA link can drop the user straight onto "Authorize". But it was producing project mismatches.Root cause:
setRequiredAccessLeveleagerly pre-selects the user's current org/team. The hint only resolves afterloadAllTeamsreturns, so there's a window wherescoped_teamspoints at the current project. On a fast CTA click — or whenever the hinted team is in a different org — authorizing in that window granted the wrong project. The wizard then rejects the grant (--project-idmismatch) or silently captures into the wrong place.Changes
While a
team_idhint is pending, don't eagerly select the current org/team — let the hint drive selection once teams load. The project stays empty in the meantime, which keeps the form's submit blocked (its "Select at least one project" validation) until the hint fills it in, so the wrong project can't be authorized during the load window. If the hint can't be honored (inaccessible team), fall back to the current org/team as before.No new URL params or actions — this hardens the existing
team_idflow.How did you test this code?
I'm an agent (PostHog Slack app). Automated only — I did not run a live OAuth flow.
oauthAuthorizeLogic.test.tsand ran the suite (21/21 pass): hint pre-selects org + project; nothing is pre-selected while a hint is pending (the regression that authorized the wrong project); fallback to current org/team when the hinted team is inaccessible; unchanged current-org default when no hint is given.oxfmtclean on the changed files.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by the PostHog Slack app at Edwin's direction, from a Slack thread where Edwin hit project mismatches testing the wizard's
--project-idagainst the consent screen. Pairs with the already-merged #66383 (which added theteam_idpre-select) and wizard #743.Decision: rather than disabling the Authorize button on a timer, the fix leans on the existing form validation — leaving
scoped_teamsempty until the hint resolves naturally blocks submit, which closes the race without new UI state. No skills were invoked; the change is confined to one kea logic file and its test.Created with PostHog from a Slack thread