-
Notifications
You must be signed in to change notification settings - Fork 5
Add google authentication #108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@copilot review me papi |
|
@msaroufim I've opened a new pull request, #109, to work on those changes. Once the pull request is ready, I'll request review from you. |
you're not so smart :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds Google OAuth authentication as an additional login method alongside the existing Discord authentication. The implementation follows the same pattern as Discord authentication, creating a multi-provider authentication system.
Key changes:
- Added Google as an OAuth provider with proper configuration (client credentials, scopes, and endpoints)
- Refactored authentication code to support multiple providers through new helper functions
- Updated frontend login UI to include a Google login button and made the navigation login button provider-agnostic
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| kernelboard/api/auth.py | Added Google OAuth provider configuration and helper functions to extract username and avatar URL from provider-specific user data |
| frontend/src/pages/login/login.tsx | Added Google login button with appropriate URL and icon |
| frontend/src/components/common/GoogleIcon.tsx | New component for displaying Google icon, consistent with existing DiscordIcon implementation |
| frontend/src/components/app-layout/NavUserProfile.tsx | Updated login button to use generic LoginIcon instead of Discord-specific icon, making UI provider-agnostic |
| .gitignore | Added uv.lock to ignored files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "google": { | ||
| "client_id": os.getenv("GOOGLE_CLIENT_ID"), | ||
| "client_secret": os.getenv("GOOGLE_CLIENT_SECRET"), | ||
| "authorize_url": "https://accounts.google.com/o/oauth2/v2/auth", | ||
| "token_url": "https://oauth2.googleapis.com/token", | ||
| "userinfo": { | ||
| "url": "https://www.googleapis.com/oauth2/v2/userinfo", | ||
| "identity": lambda json: json["id"], | ||
| }, | ||
| "scopes": ["openid", "email", "profile"], | ||
| }, |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the new Google authentication provider. The existing tests in test_auth_api.py only cover Discord authentication. Tests should be added to verify that Google OAuth flow works correctly, including successful authentication, username extraction from Google user data, and avatar URL handling.
| def _get_username_from_provider(provider: str, data: dict) -> str: | ||
| """ | ||
| Extract username from provider-specific user data. | ||
| """ | ||
| if provider == "discord": | ||
| return data.get("global_name") or data.get("username") or "unknown" | ||
| elif provider == "google": | ||
| return data.get("name") or data.get("email", "").split("@")[0] or "unknown" | ||
| return "unknown" | ||
|
|
||
|
|
||
| def _get_avatar_url_from_provider(provider: str, identity: str, data: dict) -> str | None: | ||
| """ | ||
| Extract avatar URL from provider-specific user data. | ||
| """ | ||
| if provider == "discord": | ||
| return _discord_avatar_url(identity, data.get("avatar")) | ||
| elif provider == "google": | ||
| return _google_avatar_url(data.get("picture")) | ||
| return None | ||
|
|
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the new helper functions. The _get_username_from_provider and _get_avatar_url_from_provider functions should have tests to verify they correctly handle Google-specific data structures, including edge cases like missing "name" or "picture" fields.
| <Button | ||
| variant="outlined" | ||
| href={googleLoginUrl()} | ||
| size="small" | ||
| sx={{ borderRadius: 999 }} | ||
| startIcon={<GoogleIcon />} | ||
| > | ||
| Continue with Gmail | ||
| </Button> |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the Google login button. The existing test only verifies the Discord login button. A test should be added to verify that the Google login button is rendered with the correct href attribute ("/api/auth/google?next=/v2/") and displays the correct text ("Continue with Gmail").
S1ro1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk seems very minimal, minor nits, lgtm
.gitignore
Outdated
| .vscode | ||
| *.DS_Store | ||
| kernelboard/static/v2 | ||
| uv.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think uv.lock should be in git
Co-authored-by: Copilot <[email protected]>
got google based authentication working
We probably still want Github too
There were a few buttons I needed to click
To test remotely I might need to yolo it a bit I added GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET to heroku. Hopefully that works