-
Notifications
You must be signed in to change notification settings - Fork 5
Add support for Google Auth #32
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request integrates Google OAuth authentication into the application. It adds configuration for Google credentials, a new social login controller to handle OAuth flow, database schema updates to store google_id and avatar_url, controller modifications to expose OAuth status, frontend login and registration page updates, routing changes, a new dependency (Laravel Socialite), and developer documentation for Claude Code. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Http/Controllers/Auth/AuthenticatedSessionController.php (1)
1-3: Missingdeclare(strict_types=1);statement.Per coding guidelines, all PHP files must start with
declare(strict_types=1);.<?php + +declare(strict_types=1); namespace App\Http\Controllers\Auth;app/Http/Controllers/Auth/RegisteredUserController.php (1)
1-3: Missingdeclare(strict_types=1);statement.Per coding guidelines, all PHP files must start with
declare(strict_types=1);.<?php + +declare(strict_types=1); namespace App\Http\Controllers\Auth;
♻️ Duplicate comments (1)
app/Http/Controllers/Auth/RegisteredUserController.php (1)
29-36: Duplicate method already flagged.This is the duplicate of
isGoogleAuthEnabled()mentioned inAuthenticatedSessionController.php. Extract to a shared trait or service as suggested there.
🧹 Nitpick comments (5)
.env.example (1)
61-67: Google OAuth envs line up with config; consider fixing minor lint warningThe
GOOGLE_*vars andOPENAI_MODELentries match the usage inconfig/services.php, and the default callback URL is consistent with the route. You may also want to add a trailing newline to satisfy dotenv linters, but that's optional.resources/js/Components/GoogleIcon.tsx (1)
1-21: Component is fine; consider accessibility attributes on the SVGThe component and typing look good. Since this icon is likely decorative inside a button, consider marking the SVG as hidden from assistive tech:
- <svg className={className} viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> + <svg + className={className} + viewBox="0 0 24 24" + xmlns="http://www.w3.org/2000/svg" + aria-hidden="true" + focusable="false" + >database/migrations/2025_12_10_004748_add_google_oauth_columns_to_users_table.php (1)
1-32: Migration matches model needs; be aware of rollback constraintsThe new columns and nullable password change align with the OAuth flow and
Usermodel. Note that if you ever roll this migration back after creating users withpassword = null, thenullable(false)->change()indown()may fail until those rows are cleaned up or given a password. Otherwise, this looks solid.app/Http/Controllers/Auth/AuthenticatedSessionController.php (1)
29-36: Duplicate method: extract to a shared trait or service.The
isGoogleAuthEnabled()method is identical in bothAuthenticatedSessionControllerandRegisteredUserController(seeapp/Http/Controllers/Auth/RegisteredUserController.php, lines 31-35). Consider extracting this to a trait (e.g.,ChecksGoogleAuth) or a dedicated service class to avoid duplication.Example trait:
// app/Http/Controllers/Auth/Traits/ChecksGoogleAuth.php trait ChecksGoogleAuth { private function isGoogleAuthEnabled(): bool { return !empty(config('services.google.client_id')) && !empty(config('services.google.client_secret')); } }app/Http/Controllers/Auth/SocialLoginController.php (1)
29-33: Consider catching a more specific exception.Catching the generic
\Exceptionworks but is broad. Socialite typically throws\Laravel\Socialite\Two\InvalidStateExceptionfor CSRF/state mismatches and\GuzzleHttp\Exception\ClientExceptionfor API errors. Consider catching these specifically for better error handling and logging.- } catch (\Exception $e) { + } catch (\Laravel\Socialite\Two\InvalidStateException $e) { + return redirect()->route('login')->with('error', 'Authentication session expired. Please try again.'); + } catch (\Exception $e) { + report($e); // Log unexpected errors return redirect()->route('login')->with('error', 'Unable to authenticate with Google. Please try again.'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.env.example(1 hunks)CLAUDE.md(1 hunks)app/Http/Controllers/Auth/AuthenticatedSessionController.php(1 hunks)app/Http/Controllers/Auth/RegisteredUserController.php(1 hunks)app/Http/Controllers/Auth/SocialLoginController.php(1 hunks)app/Models/User.php(3 hunks)composer.json(1 hunks)config/services.php(1 hunks)config/ziggy.php(1 hunks)database/migrations/2025_12_10_004748_add_google_oauth_columns_to_users_table.php(1 hunks)resources/js/Components/GoogleIcon.tsx(1 hunks)resources/js/Pages/Auth/Login.tsx(3 hunks)resources/js/Pages/Auth/Register.tsx(2 hunks)routes/auth.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-conventions.mdc)
**/*.php: PHP 8.3+ required. Use latest language features: typed properties, enums, readonly classes.
Always start files withdeclare(strict_types=1);.
Follow PSR-12 code style.
**/*.php: Always start files withdeclare(strict_types=1);.
PHP 8.3+ features must be leveraged (readonly, typed properties, enums, etc.).
Follow PSR-12 strictly.
Avoid magic methods (__get,__set,__call) unless absolutely necessary.
All methods, functions, and properties must be fully typed.
Usevoidorneverreturn types where appropriate.
Always prefer explicit type declarations over inferred types.
Use dependency injection rather than static access (new Something()inside methods is discouraged).
Use named constructors for complex object creation.
Use docblocks only for revealing array contents (@var list<string>), describing complex structures (generics, traversables), or explaining why something is done.
Use fully qualified class names in docblocks.
Prefer string interpolation with{}braces over concatenation.
Usesprintfwhen formatting multiple variables.
Always throw typed, domain-specific exceptions.
Avoid genericExceptionwherever possible.
Typecast using native casting, not functions (e.g.,$id = (int) $id;).
Preferisset()andarray_key_exists()checks for large arrays.
Always use PHP 8.1+ nativeenumfor fixed value sets.
Use PascalCase for enum cases.
Prioritize regex readability and performance.
Use named capture groups and reuse patterns properly in regular expressions.
Always validate and sanitize external input.
Prevent SQL injection by whitelisting columns.
Escape or purify user-supplied HTML before rendering.
Files:
app/Models/User.phpapp/Http/Controllers/Auth/RegisteredUserController.phpapp/Http/Controllers/Auth/AuthenticatedSessionController.phpapp/Http/Controllers/Auth/SocialLoginController.phproutes/auth.phpconfig/services.phpdatabase/migrations/2025_12_10_004748_add_google_oauth_columns_to_users_table.phpconfig/ziggy.php
app/Models/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-conventions.mdc)
app/Models/*.php: Models must be named in singular form (e.g.,User).
Use custom EloquentBuilder classes for models with 3+ scopes.
Use explicit fill/assign properties instead of mass assignment.
Always use$fillableand whitelist validated data for mass assignment.
Files:
app/Models/User.php
{app,config,routes}/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-conventions.mdc)
Environment variables must only be accessed via
config()calls inside application code.
Files:
app/Models/User.phpapp/Http/Controllers/Auth/RegisteredUserController.phpapp/Http/Controllers/Auth/AuthenticatedSessionController.phpapp/Http/Controllers/Auth/SocialLoginController.phproutes/auth.phpconfig/services.phpconfig/ziggy.php
{app,routes,database}/**/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-conventions.mdc)
{app,routes,database}/**/*.php: Always useModel::query()instead of direct static methods.
Eager load relations to avoid N+1 queries.
Avoid magic methods likewhereName(), prefer explicitwhere().
Never allow dynamic column names from user input to prevent SQL Injection.
Cache heavy queries using Laravel Cache.
Usedispatch()helper, not static::dispatch()or Bus Facade.
Use chunking for large datasets (e.g.,chunk(100)).
Use eager loading and indexed columns for performance.
Files:
app/Models/User.phpapp/Http/Controllers/Auth/RegisteredUserController.phpapp/Http/Controllers/Auth/AuthenticatedSessionController.phpapp/Http/Controllers/Auth/SocialLoginController.phproutes/auth.phpdatabase/migrations/2025_12_10_004748_add_google_oauth_columns_to_users_table.php
resources/js/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/tail-react-components.mdc)
resources/js/**/*.tsx: Always use TailReact components if an equivalent exists instead of building custom implementations.
Do not duplicate functionality that is already available in TailReact.
Maintain consistent styling by using the design system provided by TailReact.
Extend TailReact components via props or composition if needed — do not override styles manually unless necessary.
Always use provided TailReact component props correctly and completely to ensure type safety.
If a TailReact component is missing required behavior, extend it properly or suggest improvements rather than reimplementing.
Only build new components if absolutely necessary and after confirming TailReact doesn't offer it.
Prefer composition (wrapping existing components) over reimplementation when extending TailReact components.
Files:
resources/js/Components/GoogleIcon.tsxresources/js/Pages/Auth/Login.tsxresources/js/Pages/Auth/Register.tsx
routes/*.php
📄 CodeRabbit inference engine (.cursor/rules/laravel-conventions.mdc)
routes/*.php: Route parameters should use camelCase (e.g.,{userId}).
Do not useRoute::resource(). Define routes manually.
Prefer tuple notation for controller bindings in routes.
Always name routes.
Use kebab-case URLs and camelCase route parameters.
Files:
routes/auth.php
{composer.json,composer.lock}
📄 CodeRabbit inference engine (.cursor/rules/php-conventions.mdc)
Use Composer. Lock dependencies via
composer.lock.
Files:
composer.json
composer.json
📄 CodeRabbit inference engine (.cursor/rules/php-conventions.mdc)
Keep
composer.jsonnormalized and audited.
Files:
composer.json
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/php-conventions.mdc:0-0
Timestamp: 2025-08-06T18:17:04.908Z
Learning: Applies to **/*.php : Always prefer explicit type declarations over inferred types.
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/php-conventions.mdc:0-0
Timestamp: 2025-08-06T18:17:04.908Z
Learning: Applies to **/*.php : Prefer string interpolation with `{}` braces over concatenation.
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/php-conventions.mdc:0-0
Timestamp: 2025-08-06T18:17:04.908Z
Learning: Applies to **/*.php : Prefer `isset()` and `array_key_exists()` checks for large arrays.
📚 Learning: 2025-08-06T18:16:46.878Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/laravel-conventions.mdc:0-0
Timestamp: 2025-08-06T18:16:46.878Z
Learning: Applies to app/Models/*.php : Use explicit fill/assign properties instead of mass assignment.
Applied to files:
app/Models/User.php
📚 Learning: 2025-08-06T18:16:46.878Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/laravel-conventions.mdc:0-0
Timestamp: 2025-08-06T18:16:46.878Z
Learning: Applies to app/Models/*.php : Always use `$fillable` and whitelist validated data for mass assignment.
Applied to files:
app/Models/User.php
📚 Learning: 2025-08-06T18:17:15.032Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/tail-react-components.mdc:0-0
Timestamp: 2025-08-06T18:17:15.032Z
Learning: Applies to resources/js/**/*.tsx : Extend TailReact components via props or composition if needed — do not override styles manually unless necessary.
Applied to files:
resources/js/Components/GoogleIcon.tsx
📚 Learning: 2025-08-06T18:17:15.032Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/tail-react-components.mdc:0-0
Timestamp: 2025-08-06T18:17:15.032Z
Learning: Applies to resources/js/**/*.tsx : If a TailReact component is missing required behavior, extend it properly or suggest improvements rather than reimplementing.
Applied to files:
resources/js/Components/GoogleIcon.tsx
📚 Learning: 2025-08-06T18:17:15.032Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/tail-react-components.mdc:0-0
Timestamp: 2025-08-06T18:17:15.032Z
Learning: Applies to resources/js/**/*.tsx : Maintain consistent styling by using the design system provided by TailReact.
Applied to files:
resources/js/Components/GoogleIcon.tsx
📚 Learning: 2025-08-06T18:16:46.878Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/laravel-conventions.mdc:0-0
Timestamp: 2025-08-06T18:16:46.878Z
Learning: Applies to app/Http/Controllers/*Controller.php : Prefer invokable Single Action Controllers.
Applied to files:
routes/auth.php
📚 Learning: 2025-08-06T18:16:46.878Z
Learnt from: CR
Repo: weDevsOfficial/ideabox PR: 0
File: .cursor/rules/laravel-conventions.mdc:0-0
Timestamp: 2025-08-06T18:16:46.878Z
Learning: Applies to routes/*.php : Do not use `Route::resource()`. Define routes manually.
Applied to files:
routes/auth.php
🧬 Code graph analysis (5)
app/Http/Controllers/Auth/RegisteredUserController.php (1)
app/Http/Controllers/Auth/AuthenticatedSessionController.php (1)
isGoogleAuthEnabled(32-36)
app/Http/Controllers/Auth/AuthenticatedSessionController.php (1)
app/Http/Controllers/Auth/RegisteredUserController.php (1)
isGoogleAuthEnabled(32-36)
resources/js/Pages/Auth/Login.tsx (1)
resources/js/Components/GoogleIcon.tsx (1)
GoogleIcon(1-22)
routes/auth.php (1)
app/Http/Controllers/Auth/SocialLoginController.php (1)
SocialLoginController(14-82)
resources/js/Pages/Auth/Register.tsx (1)
resources/js/Components/GoogleIcon.tsx (1)
GoogleIcon(1-22)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 67-67: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (17)
composer.json (1)
7-18: Add Socialite dependency looks correct; ensure lockfile & audit are updatedThe
laravel/socialiterequirement and package ordering look fine; just make surecomposer.lockis updated and you run a dependency audit as part of this change.CLAUDE.md (1)
1-81: Documentation is clear and aligns with project conventionsThe guidance here matches the PHP/Laravel and React/TailReact conventions in the repo and will be useful for future automation. No changes needed.
config/services.php (1)
34-43: Google service configuration is consistent; verify redirect matches provider settingsThe
GOOGLE_REDIRECT_URI(orAPP_URL+ default path) exactly matches the URL configured in the Google Cloud console to avoid callback mismatch errors.app/Models/User.php (2)
26-48: Google fields on User look reasonable; ensure mass-assignment remains constrainedAdding
google_idandavatar_urlto$fillableand hidinggoogle_idfrom serialization is consistent with the new OAuth flow. Just make sure any mass-assignment paths only use validated data from Socialite/your own mapping logic so these fields can’t be spoofed by arbitrary request payloads.
84-96: Avatar accessor behavior change is sensiblePreferring
avatar_urlwhen present and falling back to Gravatar keeps existing behavior while enabling provider avatars. This looks good; no changes needed.config/ziggy.php (1)
3-9: Ziggy route group update correctly exposes Google auth routesIncluding
auth.googleandauth.google.callbackin theauthgroup is consistent with the new OAuth endpoints and will make them available to the frontend via Ziggy. Looks good.routes/auth.php (1)
38-43: LGTM! Google OAuth routes are correctly configured.The routes are properly placed within the guest middleware group, use named routes, and follow the array notation pattern consistent with the existing routes in this file.
resources/js/Pages/Auth/Register.tsx (2)
10-14: Props type definition looks good.The
googleAuthEnabledprop is correctly typed as a boolean.
124-143: Google Sign Up UI implementation is correct.Good implementation:
- Correctly uses an anchor tag (
<a>) instead of Inertia's<Link>for OAuth redirect, which requires a full page navigation.- Proper conditional rendering based on
googleAuthEnabled.- Consistent dark mode styling.
app/Http/Controllers/Auth/SocialLoginController.php (4)
1-13: Well-structured controller with proper declarations.Good adherence to coding guidelines:
declare(strict_types=1)is present, proper namespacing, and explicit return types on methods.
44-46: "Remember me" is always enabled.
Auth::login($user, true)sets remember-me totrueunconditionally. This may be intentional for OAuth flows, but consider whether users should have control over session persistence, especially on shared devices.
49-63: Verify the email-based account linking security model.When a user with an existing email logs in via Google, their account is automatically linked. This is a common pattern but has security implications:
- If an attacker knows a user's email and can create a Google account with that email (unlikely but possible in some scenarios), they could gain access.
- The original user may not expect their account to be linked to Google without explicit consent.
Consider whether you want to prompt the user to confirm linking or require password verification for existing password-based accounts.
65-74: New user creation is correctly implemented.
- Email is marked as verified (appropriate since Google verified it).
- Uses
User::query()->create()per guidelines.- Sets default role appropriately.
- Fires
Registeredevent for any listeners (e.g., welcome emails).resources/js/Pages/Auth/Login.tsx (4)
4-4: LocalGoogleIconimport keeps concerns nicely modularUsing a dedicated
GoogleIconcomponent instead of inlining SVG here keeps the page lean and makes reuse straightforward. No issues from a typing or composition perspective.
80-100: Remember‑me row flex layout and link styling look goodSwitching this row to
flex items-center justify-betweenreads well and should behave better responsively. The simplified Tailwind classes on the “Forgot your password?” link are consistent with the rest of the palette and keep the focus/hover behavior intact.
102-104: Using TailReactButtonfor the primary CTA aligns with the design systemReplacing the old submit control with
Buttonfrom@wedevs/tail-reactkeeps the form aligned with the shared design system. The component properly forwardstype="submit",className="w-full mt-6", anddisabled={processing}to the underlying button element, consistent with established usage patterns throughout the codebase.
107-126: Google auth block implementation is appropriateThe custom divider and Google sign-in button are well-structured with proper focus/hover states and semantic markup. TailReact (@wedevs/tail-react) does not provide divider or social login button components, so the current custom implementation is the correct approach here.
| export default function Login({ | ||
| status, | ||
| canResetPassword, | ||
| googleAuthEnabled, | ||
| }: { | ||
| status?: string; | ||
| canResetPassword: boolean; | ||
| googleAuthEnabled: boolean; | ||
| }) { |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find server-side code that renders the Login Inertia page
rg -n "Auth/Login" resources -S --type php --type blade
rg -n "Auth/Login" app -S --type php
rg -n "googleAuthEnabled" --type php --type blade -B 2 -A 2Repository: weDevsOfficial/ideabox
Length of output: 253
🏁 Script executed:
#!/bin/bash
# Read the AuthenticatedSessionController to see what props are passed
cat -n app/Http/Controllers/Auth/AuthenticatedSessionController.phpRepository: weDevsOfficial/ideabox
Length of output: 1697
🏁 Script executed:
#!/bin/bash
# Search for all Inertia::render calls to Auth/Login
rg -n "Auth/Login" app -S
# Search for googleAuthEnabled in PHP code
rg -n "googleAuthEnabled" app -SRepository: weDevsOfficial/ideabox
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Read the Login component to see how googleAuthEnabled is used
cat -n resources/js/Pages/Auth/Login.tsx | head -80Repository: weDevsOfficial/ideabox
Length of output: 2761
🏁 Script executed:
#!/bin/bash
# Search for googleAuthEnabled usage in the Login component
rg -n "googleAuthEnabled" resources/js/Pages/Auth/Login.tsx -A 3 -B 3Repository: weDevsOfficial/ideabox
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Get full context of the Login component around prop usage
cat -n resources/js/Pages/Auth/Login.tsxRepository: weDevsOfficial/ideabox
Length of output: 3916
Server-side code does not provide googleAuthEnabled prop to match the updated component signature
The AuthenticatedSessionController.create() method (line 22-25) returns Inertia::render('Auth/Login', [...]) with only canResetPassword and status props. It does not provide googleAuthEnabled, which you've added as a required boolean in the component signature. Update the server-side render call to include googleAuthEnabled as a boolean value so the prop type matches at runtime.
🤖 Prompt for AI Agents
In resources/js/Pages/Auth/Login.tsx around lines 11-19 the component signature
requires a boolean googleAuthEnabled prop, but the server render call in
AuthenticatedSessionController.create() (around lines 22-25) does not supply it;
update that controller render to include googleAuthEnabled: <boolean> (e.g.,
from your config or feature-flag: config('services.google.enabled') or a
dedicated feature check) so the Inertia::render('Auth/Login', [...]) payload
contains googleAuthEnabled and matches the component signature at runtime.
This adds support for Google Auth. Also slightly improves the login and signup page forms.
FYI @tareq1988
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.