feat: email-based account linking for existing users in agentic provisioning#53348
feat: email-based account linking for existing users in agentic provisioning#53348
Conversation
|
| non_demo_teams = list(Team.objects.filter(organization_id__in=org_ids, is_demo=False)) | ||
|
|
||
| if len(non_demo_teams) == 1: | ||
| return non_demo_teams[0] | ||
|
|
||
| organization = memberships[0].organization | ||
| if not non_demo_teams: | ||
| return Team.objects.create_with_data(initiating_user=user, organization=organization) | ||
|
|
||
| return Team.objects.create_with_data(initiating_user=user, organization=organization) |
There was a problem hiding this comment.
Duplicate logic — OnceAndOnlyOnce violation
After the len(non_demo_teams) == 1 early return, both remaining branches (if not non_demo_teams and the final return) call Team.objects.create_with_data(initiating_user=user, organization=organization) with identical arguments. The if not non_demo_teams guard is dead code because whether there are 0 or ≥2 teams, the result is the same. The whole block can be simplified to:
| non_demo_teams = list(Team.objects.filter(organization_id__in=org_ids, is_demo=False)) | |
| if len(non_demo_teams) == 1: | |
| return non_demo_teams[0] | |
| organization = memberships[0].organization | |
| if not non_demo_teams: | |
| return Team.objects.create_with_data(initiating_user=user, organization=organization) | |
| return Team.objects.create_with_data(initiating_user=user, organization=organization) | |
| organization = memberships[0].organization | |
| return Team.objects.create_with_data(initiating_user=user, organization=organization) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/api/agentic_provisioning/views.py
Line: 320-329
Comment:
**Duplicate logic — OnceAndOnlyOnce violation**
After the `len(non_demo_teams) == 1` early return, both remaining branches (`if not non_demo_teams` and the final `return`) call `Team.objects.create_with_data(initiating_user=user, organization=organization)` with identical arguments. The `if not non_demo_teams` guard is dead code because whether there are 0 or ≥2 teams, the result is the same. The whole block can be simplified to:
```suggestion
organization = memberships[0].organization
return Team.objects.create_with_data(initiating_user=user, organization=organization)
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Simplified - removed the dead branch.
ee/api/agentic_provisioning/views.py
Outdated
| requested_team_id = configuration.get("team_id") | ||
| if requested_team_id is not None: | ||
| try: | ||
| requested_team_id = int(requested_team_id) | ||
| except (ValueError, TypeError): | ||
| requested_team_id = None |
There was a problem hiding this comment.
Silent coercion of invalid
team_id bypasses explicit team selection
When configuration.team_id is present but cannot be cast to int (e.g. "abc"), requested_team_id is silently set to None and _resolve_team_for_existing_user auto-selects a team instead of returning an error. A caller that passes an explicitly wrong team_id value presumably does not expect silent auto-selection; returning a 400 (as done for a non-existent integer id) would be more consistent and easier to debug.
There is currently no test covering a non-numeric team_id value — test_existing_user_with_invalid_team_id_returns_400 only tests an integer that does not exist in the database.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ee/api/agentic_provisioning/views.py
Line: 241-246
Comment:
**Silent coercion of invalid `team_id` bypasses explicit team selection**
When `configuration.team_id` is present but cannot be cast to `int` (e.g. `"abc"`), `requested_team_id` is silently set to `None` and `_resolve_team_for_existing_user` auto-selects a team instead of returning an error. A caller that passes an explicitly wrong `team_id` value presumably does not expect silent auto-selection; returning a 400 (as done for a non-existent integer id) would be more consistent and easier to debug.
There is currently no test covering a non-numeric `team_id` value — `test_existing_user_with_invalid_team_id_returns_400` only tests an integer that does not exist in the database.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Returns 400 now for non-numeric team_id. Added test.
…sioning Instead of redirecting existing users to a browser auth page, issue OAuth tokens directly based on the Stripe-verified email. This eliminates the interactive flow that was causing developer friction. Also returns available teams in the token exchange response so orchestrators can specify which team to use at resource creation time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The OAuthApplication model validates RSA keys are available when using RS256 algorithm. This was failing in sandbox environments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
78f3bb8 to
02e606d
Compare
…ted OAuth apps - Remove empty `next_cursor` from services response (protocol schema requires min length 1 or omit) - Fall back to matching OAuth app by name when STRIPE_POSTHOG_OAUTH_CLIENT_ID is not configured Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Return 400 for non-numeric team_id instead of silently falling back to auto-selection - Remove dead code branch in _resolve_team_for_existing_user (both paths did the same thing) - Add test for non-numeric team_id Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CodeQL flagged `secret` + SHA256 as insecure password hashing. This is HMAC signature verification for Stripe webhooks, not password hashing. Renaming the parameter to `signing_key` resolves the false positive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem
Existing PostHog users hitting the Stripe agentic provisioning flow get redirected to a browser auth page (
requires_authresponse), which Stripe says causes developer friction. Stripe's V2 requirements (deadline 4/17) need us to eliminate the interactive flow for existing users.Since account_requests are HMAC-signed by Stripe, we can trust the email identity and issue OAuth tokens directly.
Changes
_handle_existing_userto issue an auth code directly instead of returningrequires_auth_resolve_team_for_existing_userhelper: single team auto-selects, only demo teams creates new project, multiple teams creates new project (or usesconfiguration.team_idif specified)_get_available_teams_for_userto return team list in token exchange response (account.available_teams)next_cursor(protocol schema requires min length 1 or omit)STRIPE_POSTHOG_OAUTH_CLIENT_IDis not configured (falls back to matching OAuth app by name)How did you test this code?
Automated tests: 132 pass, 1 pre-existing failure (
test_full_a1_flow_with_token_exchange- OIDC key issue on master)Manual testing with Stripe spec toolkit (posthog-spec-toolkit orchestrator against local dev server):
health- 200 OKservices-list- 200, returns analytics serviceaccount-request --email test-stripe@posthog.com- 200, existing user gets credentials directly withavailable_teamsprovision-resource --service-id analytics- 200, returns api_key + personal_api_keyrotate-credentials- 200, returns new credentialsdeep-link --purpose dashboard- 200, returns login URL with team_idPublish to changelog?
No
Docs update
N/A - internal Stripe integration, no public-facing docs