Auto create missing users when using oauth #1573
Conversation
|
Maybe the option could just be called "Perform automated provisioning" or something similar? The current name seems kind of oververbose. |
Maybe it could be named “Allow user creation”? |
WalkthroughA new configuration option for OAuth was introduced to control whether missing users should be created automatically during OAuth authentication. This involved interface and method additions, logic updates in the OAuth controller, UI and translation updates, and refactoring of username generation logic to centralize it within the user creation service. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OAuthProvider
participant OAuthController
participant OAuthSchema
participant UserCreationService
User->>OAuthProvider: Initiate OAuth login
OAuthProvider->>OAuthController: Redirect with callback data
OAuthController->>OAuthSchema: shouldCreateMissingUsers()
OAuthController->>OAuthController: Find user by OAuth ID
alt User exists
OAuthController->>User: Log in user
else User does not exist
alt Auto-create enabled
OAuthController->>UserCreationService: Create user (with generated username if needed)
UserCreationService-->>OAuthController: User created
OAuthController->>User: Log in user
else Auto-create disabled
OAuthController->>User: Redirect to login with notification
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/Extensions/OAuth/OAuthSchemaInterface.php (1)
36-36: Method naming consideration.While a previous reviewer suggested using singular form
shouldCreateMissingUser(), the plural formshouldCreateMissingUsers()is actually more appropriate as this setting controls whether the system should create users (in general) when they're missing, not just a single user.
🧹 Nitpick comments (2)
lang/en/admin/setting.php (1)
100-100: Consider using a more concise label based on PR feedback.The PR comments suggest alternative names like "Allow user creation" which is more concise and directly descriptive of the functionality.
- 'create_missing_users' => 'Auto Create Missing Users?', + 'create_missing_users' => 'Allow user creation',app/Http/Controllers/Auth/OAuthController.php (1)
82-86: Consider providing more specific error messages.Both error scenarios show "No linked User found" but they represent different situations:
- Auto-creation is disabled
- Email is missing from OAuth provider
// No user found and auto creation is disabled - redirect to normal login if (!$driver->shouldCreateMissingUsers()) { Notification::make() - ->title('No linked User found') + ->title('No account found') + ->body('Please create an account first or contact an administrator.') ->danger() ->persistent() ->send(); return redirect()->route('auth.login'); } $username = $oauthUser->getNickname(); $email = $oauthUser->getEmail(); // Incomplete data, can't create user - redirect to normal login if (!$email) { Notification::make() - ->title('No linked User found') + ->title('Unable to create account') + ->body('Email address is required but was not provided by the OAuth provider.') ->danger() ->persistent() ->send();Also applies to: 96-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/Extensions/OAuth/OAuthSchemaInterface.php(1 hunks)app/Extensions/OAuth/Schemas/OAuthSchema.php(3 hunks)app/Http/Controllers/Auth/OAuthController.php(4 hunks)app/Services/Subusers/SubuserCreationService.php(0 hunks)app/Services/Users/UserCreationService.php(2 hunks)lang/en/admin/setting.php(1 hunks)
💤 Files with no reviewable changes (1)
- app/Services/Subusers/SubuserCreationService.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/Extensions/OAuth/OAuthSchemaInterface.php (1)
app/Extensions/OAuth/Schemas/OAuthSchema.php (1)
shouldCreateMissingUsers(113-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: MariaDB (8.4, mariadb:11.4)
- GitHub Check: MariaDB (8.3, mariadb:11.4)
- GitHub Check: MariaDB (8.4, mariadb:10.11)
- GitHub Check: MariaDB (8.4, mariadb:10.6)
- GitHub Check: MariaDB (8.3, mariadb:10.11)
- GitHub Check: MariaDB (8.2, mariadb:11.4)
- GitHub Check: MariaDB (8.2, mariadb:10.11)
- GitHub Check: PostgreSQL (8.3, postgres:14)
- GitHub Check: MariaDB (8.3, mariadb:10.6)
- GitHub Check: MariaDB (8.2, mariadb:10.6)
- GitHub Check: PostgreSQL (8.4, postgres:14)
- GitHub Check: PostgreSQL (8.2, postgres:14)
- GitHub Check: MySQL (8.4, mysql:8)
- GitHub Check: MySQL (8.2, mysql:8)
- GitHub Check: MySQL (8.3, mysql:8)
- GitHub Check: SQLite (8.3)
- GitHub Check: SQLite (8.4)
- GitHub Check: SQLite (8.2)
🔇 Additional comments (4)
app/Services/Users/UserCreationService.php (2)
46-54: Good implementation of username generation and sanitization.The code correctly generates a username from email when missing and applies sanitization to all usernames (both provided and generated), addressing previous review feedback.
50-54: SFTP compatibility confirmed with sanitized usernamesAll SFTP entry points consistently use the stored $user->username (property that’s already stripped of dots and hyphens) when building the login string:
• tests/Integration/Api/Remote/SftpAuthenticationControllerTest.php’s getUsername()
• Remote\SftpAuthenticationController parses and validates the same sanitized username
• Filament Server Settings URL/username fields use auth()->user()->usernameNo further action is required—removing “.” and “-” during user creation does not break SFTP authentication.
app/Extensions/OAuth/Schemas/OAuthSchema.php (1)
58-68: Well-implemented OAuth configuration toggle and method.The Toggle field configuration is comprehensive with proper state handling, UI customization, and the
shouldCreateMissingUsers()method follows the established pattern used byisEnabled().Also applies to: 113-118
app/Http/Controllers/Auth/OAuthController.php (1)
44-49: Excellent refactoring of OAuth callback logic.The code properly delegates username generation to UserCreationService (addressing previous feedback), handles missing email validation, and cleanly separates the logic for existing vs new users.
Also applies to: 65-114
rmartinoscar
left a comment
rmartinoscar
left a comment
There was a problem hiding this comment.
Im unable to checkout atm but code looks good
Adds a new option to all providers to automatically create (& link) missing users. This is turned off by default.
These users will still recieve a "Account created" mail with a password reset link, if they wish to also setup a panel password.