feat: setup wizard api + authentication frontend #29371
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds a setup wizard API and authentication frontend for PostHog, enabling temporary authentication for CLI users during project setup.
- Added
SetupWizardViewSetinposthog/api/wizard.pywith endpoints for initializing, retrieving data, and proxying OpenAI queries - Implemented temporary authentication using Redis-cached hashes with 10-minute expiration via
SETUP_WIZARD_CACHE_PREFIXandSETUP_WIZARD_CACHE_TIMEOUT - Created frontend authentication flow in
frontend/src/scenes/wizard/Wizard.tsxwith pending, success, and error states - Added rate limiting via
SetupWizardAuthenticationRateThrottleandSetupWizardQueryRateThrottleclasses to prevent abuse - Integrated new wizard scene into application routing with corresponding URL and scene configuration
14 file(s) reviewed, 10 comment(s)
Edit PR Review Bot Settings | Greptile
| } catch (e: any) { | ||
| actions.setView('invalid') | ||
| return { success: false, errorCode: e.code, errorDetail: e.detail } |
There was a problem hiding this comment.
style: Error handling accesses e.code and e.detail directly, but these properties might not exist on all error objects. Consider using optional chaining (e?.code, e?.detail) or providing fallback values.
| } catch (e: any) { | |
| actions.setView('invalid') | |
| return { success: false, errorCode: e.code, errorDetail: e.detail } | |
| } catch (e: any) { | |
| actions.setView('invalid') | |
| return { success: false, errorCode: e?.code, errorDetail: e?.detail } |
| wizard_data = { | ||
| "project_api_key": request.user.team.api_token, | ||
| "host": get_api_host(), | ||
| "user_distinct_id": request.user.distinct_id, | ||
| } |
There was a problem hiding this comment.
logic: Consider adding a check to verify if request.user.team exists before accessing api_token to prevent potential errors if a user doesn't have a team assigned.
| wizard_data = { | |
| "project_api_key": request.user.team.api_token, | |
| "host": get_api_host(), | |
| "user_distinct_id": request.user.distinct_id, | |
| } | |
| wizard_data = { | |
| "project_api_key": request.user.team.api_token if hasattr(request.user, 'team') and request.user.team else None, | |
| "host": get_api_host(), | |
| "user_distinct_id": request.user.distinct_id, | |
| } |
| def test_data_endpoint_returns_data(self): | ||
| response = self.client.get(self.data_url, HTTP_X_POSTHOG_WIZARD_HASH=self.hash) | ||
| assert response.status_code == status.HTTP_200_OK | ||
| assert response.data["project_api_key"] == "test-key" |
There was a problem hiding this comment.
style: Test should also verify the 'host' field is returned correctly in the response.
| def test_data_endpoint_returns_data(self): | |
| response = self.client.get(self.data_url, HTTP_X_POSTHOG_WIZARD_HASH=self.hash) | |
| assert response.status_code == status.HTTP_200_OK | |
| assert response.data["project_api_key"] == "test-key" | |
| def test_data_endpoint_returns_data(self): | |
| response = self.client.get(self.data_url, HTTP_X_POSTHOG_WIZARD_HASH=self.hash) | |
| assert response.status_code == status.HTTP_200_OK | |
| assert response.data["project_api_key"] == "test-key" | |
| assert response.data["host"] == "http://localhost:8010" |
| @patch("posthog.api.wizard.OpenAI") | ||
| def test_query_endpoint_rate_limit(self, mock_openai): | ||
| mock_openai_instance = mock_openai.return_value | ||
| # Simulate an OpenAI response with JSON {"foo": "bar"} | ||
| mock_openai_instance.beta.chat.completions.parse.return_value = MagicMock( | ||
| choices=[MagicMock(message=MagicMock(content=json.dumps({"foo": "bar"})))] | ||
| ) | ||
|
|
||
| for _ in range(20): | ||
| response = self.client.post( | ||
| self.query_url, | ||
| data=json.dumps( | ||
| {"message": "test", "json_schema": {"type": "object", "properties": {"name": {"type": "string"}}}} | ||
| ), | ||
| content_type="application/json", | ||
| HTTP_X_POSTHOG_WIZARD_HASH=self.hash, | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK | ||
|
|
||
| response = self.client.post( | ||
| self.query_url, | ||
| data=json.dumps( | ||
| {"message": "test", "json_schema": {"type": "object", "properties": {"name": {"type": "string"}}}} | ||
| ), | ||
| content_type="application/json", | ||
| HTTP_X_POSTHOG_WIZARD_HASH=self.hash, | ||
| ) | ||
| assert response.status_code == status.HTTP_429_TOO_MANY_REQUESTS |
There was a problem hiding this comment.
style: The rate limit test is hardcoded to 20 requests, but it should reference the actual rate limit value from the SetupWizardQueryRateThrottle class to ensure the test remains valid if the rate limit changes.
| @patch("posthog.api.wizard.OpenAI") | ||
| def test_query_endpoint(self, mock_openai): | ||
| mock_openai_instance = mock_openai.return_value | ||
| mock_openai_instance.beta.chat.completions.parse.return_value = MagicMock( | ||
| choices=[MagicMock(message=MagicMock(content=json.dumps({"foo": "bar"})))] | ||
| ) | ||
| response = self.client.post( | ||
| self.query_url, | ||
| data=json.dumps( | ||
| {"message": "test", "json_schema": {"type": "object", "properties": {"name": {"type": "number"}}}} | ||
| ), | ||
| content_type="application/json", | ||
| HTTP_X_POSTHOG_WIZARD_HASH=self.hash, | ||
| ) | ||
| assert response.status_code == status.HTTP_200_OK |
There was a problem hiding this comment.
logic: This test doesn't verify the structure of the response data. Add an assertion to check that response.data contains the expected 'data' field with the mocked response.
| hash = request.headers.get("X-PostHog-Wizard-Hash") | ||
| if not hash: | ||
| return None |
There was a problem hiding this comment.
style: If hash is missing, returning None will bypass rate limiting entirely. Consider if this is the intended behavior or if it should throw an authentication error instead.
| return {"hash": instance} | ||
|
|
||
| def create(self) -> dict[str, str]: | ||
| hash = get_random_string(64, allowed_chars="abcdefghijklmnopqrstuvwxyz012345679") |
There was a problem hiding this comment.
syntax: The allowed_chars string is missing the digit '8' in the character set
| hash = get_random_string(64, allowed_chars="abcdefghijklmnopqrstuvwxyz012345679") | |
| hash = get_random_string(64, allowed_chars="abcdefghijklmnopqrstuvwxyz0123456789") |
| hash = get_random_string(64, allowed_chars="abcdefghijklmnopqrstuvwxyz012345679") | ||
| key = f"{SETUP_WIZARD_CACHE_PREFIX}{hash}" | ||
|
|
||
| cache.set(key, {"project_api_key": None, "host": None}, SETUP_WIZARD_CACHE_TIMEOUT) |
There was a problem hiding this comment.
style: Consider refreshing the cache expiration when the data is accessed to prevent expiration during active use
| if wizard_data is None: | ||
| return Response(status=404) |
There was a problem hiding this comment.
style: Return a more descriptive error message when the hash is invalid or expired
|
Size Change: +305 B (0%) Total Size: 9.73 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
Problem
We need an API for our setup wizard CLI and a method to temporarily authenticate users when they are using the CLI to setup their project.
Changes
SetupWizardview setThe query endpoint is strongly rate limited to avoid abuse, and the hashes are authed for 15 minutes only after which they expire in the redis cache.
Does this work well for both Cloud and self-hosted?
It is only available on Cloud
How did you test this code?
Added unit tests for the endpoints, manually tested the frontend wizard page authed and unauthed