-
-
Notifications
You must be signed in to change notification settings - Fork 254
feat: add generic oidc schema #2012
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
|
All contributors have signed the CLA ✍️ ✅ |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds OpenID Connect (OIDC) support by introducing a new Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-08-12T08:29:30.131ZApplied to files:
📚 Learning: 2025-10-15T11:55:53.461ZApplied to files:
🧬 Code graph analysis (1)app/Providers/Extensions/OAuthServiceProvider.php (2)
🔇 Additional comments (8)
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
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
app/Extensions/OAuth/Schemas/GenericOidcSchema.php(1 hunks)app/Providers/Extensions/OAuthServiceProvider.php(2 hunks)composer.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T08:29:30.131Z
Learnt from: Boy132
Repo: pelican-dev/panel PR: 1599
File: app/Http/Controllers/Auth/OAuthController.php:35-35
Timestamp: 2025-08-12T08:29:30.131Z
Learning: GitLab is a built-in OAuth provider in Laravel Socialite and does not require custom provider registration via getSocialiteProvider() method override in GitlabSchema.
Applied to files:
app/Providers/Extensions/OAuthServiceProvider.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Extensions/OAuth/Schemas/GenericOidcSchema.php
🧬 Code graph analysis (2)
app/Providers/Extensions/OAuthServiceProvider.php (2)
app/Extensions/OAuth/Schemas/GenericOidcSchema.php (1)
GenericOidcSchema(13-107)app/Extensions/OAuth/OAuthService.php (1)
register(34-47)
app/Extensions/OAuth/Schemas/GenericOidcSchema.php (1)
app/Extensions/OAuth/Schemas/OAuthSchema.php (1)
OAuthSchema(13-137)
🔇 Additional comments (6)
app/Providers/Extensions/OAuthServiceProvider.php (1)
10-10: LGTM!The import and registration follow the established patterns in this file. The descriptive comment appropriately categorizes this as a distinct provider type.
Also applies to: 42-44
app/Extensions/OAuth/Schemas/GenericOidcSchema.php (4)
13-23: LGTM!The class structure correctly extends
OAuthSchemaand properly overridesgetSocialiteProvider()to return the OIDC provider class. Thefinalmodifier is appropriate for this concrete implementation.
25-37: LGTM!The service configuration correctly merges the parent config (providing
client_idandclient_secret) with OIDC-specific settings. The conditional realm handling is clean.
60-91: LGTM!The settings form correctly extends the parent form with OIDC-specific fields. The URL validation on the base URL and the hex validation on the color picker are appropriate safeguards.
93-106: LGTM!The display methods provide sensible defaults and align with the pattern established by other OAuth schemas.
composer.json (1)
35-35: Package verified on Packagist with version 0.5.0 available.The
kovah/laravel-socialite-oidcpackage exists and version 0.5.0 is the latest stable release available. The version constraint^0.5is valid.
| TextInput::make('_noenv_callback') | ||
| ->label('Callback URL') | ||
| ->dehydrated() | ||
| ->disabled() | ||
| ->hintCopy() | ||
| ->default(fn () => url('/auth/oauth/callback/oidc')), |
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.
Use dehydrated(false) for display-only callback URL field.
The _noenv_callback field is disabled and used only for display purposes. Using ->dehydrated() (which defaults to true) means it may still be included in form submissions. For a read-only display field, use ->dehydrated(false) to prevent it from being processed.
TextInput::make('_noenv_callback')
->label('Callback URL')
- ->dehydrated()
+ ->dehydrated(false)
->disabled()
->hintCopy()
->default(fn () => url('/auth/oauth/callback/oidc')),🤖 Prompt for AI Agents
In app/Extensions/OAuth/Schemas/GenericOidcSchema.php around lines 47 to 52, the
TextInput for '_noenv_callback' is currently using ->dehydrated() which leaves
it included in form submissions; change this to ->dehydrated(false) so the
disabled/read-only callback URL is not dehydrated or processed on submit,
keeping the field display-only.
|
I have read the CLA Document and I hereby sign the CLA |
|
Recheck |
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 PR adds generic OpenID Connect (OIDC) provider support to enable integration with Keycloak, Auth0, Okta, and other OIDC-compatible identity providers. This is the author's first PHP contribution to the project.
Key changes:
- Added
kovah/laravel-socialite-oidcpackage dependency - Created
GenericOidcSchemaclass implementing OIDC authentication flow - Registered the new provider in
OAuthServiceProvider
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Added kovah/laravel-socialite-oidc package dependency |
| composer.lock | Updated dependencies including the new OIDC package and several other packages |
| yarn.lock | Complete rewrite from Yarn v1 to Yarn v2+ format (berry) |
| app/Extensions/OAuth/Schemas/GenericOidcSchema.php | New schema class implementing generic OIDC provider support with configuration UI |
| app/Providers/Extensions/OAuthServiceProvider.php | Registered the new GenericOidcSchema provider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ->schema([ | ||
| TextEntry::make('setup_instructions') | ||
| ->hiddenLabel() | ||
| ->state(new HtmlString(Blade::render('<p>Configure your OIDC provider (e.g., Keycloak, Auth0, Okta) with the following settings:</p><ul><li>Create an OAuth2/OpenID Connect application</li><li>Set the <b>Redirect URI</b> to the value below</li><li>Copy the <b>Client ID</b> and <b>Client Secret</b> for use in the configuration step</li></ul>'))), |
Copilot
AI
Dec 17, 2025
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.
The setup instructions HTML string is too long and hard to maintain inline. Consider extracting this to a Blade component or view file for better readability and maintainability.
| ->state(new HtmlString('<p><b>For Keycloak:</b> The Base URL should point to your realm (e.g., <code>https://keycloak.example.com/realms/my-realm</code>). Optionally, you can specify the realm name separately.</p>')), | ||
| ]), | ||
| ], parent::getSetupSteps()); | ||
| } | ||
|
|
Copilot
AI
Dec 17, 2025
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.
The Keycloak note HTML string is also quite long. Consider extracting this to a separate view or using a more structured approach with Blade components for better maintainability.
| ->state(new HtmlString('<p><b>For Keycloak:</b> The Base URL should point to your realm (e.g., <code>https://keycloak.example.com/realms/my-realm</code>). Optionally, you can specify the realm name separately.</p>')), | |
| ]), | |
| ], parent::getSetupSteps()); | |
| } | |
| ->state($this->getKeycloakNoteHtml()), | |
| ]), | |
| ], parent::getSetupSteps()); | |
| } | |
| private function getKeycloakNoteHtml(): HtmlString | |
| { | |
| return new HtmlString(<<<'HTML' | |
| <p><b>For Keycloak:</b> The Base URL should point to your realm (e.g., <code>https://keycloak.example.com/realms/my-realm</code>). Optionally, you can specify the realm name separately.</p> | |
| HTML | |
| ); | |
| } |
|
We have a plugin for this already. https://github.com/pelican-dev/plugins/tree/main/generic-oidc-providers Note: this was discussed here aswell #2010 |
6746ecc to
b9bf8f3
Compare
b9bf8f3 to
af1fb4f
Compare
Add Generic OIDC Provider Support
Adds support for generic OpenID Connect (OIDC) providers, enabling integration with Keycloak, Auth0, Okta, and other OIDC-compatible identity providers.
Changes:
kovah/laravel-socialite-oidcpackageGenericOidcSchemaclassOAuthServiceProviderNote: This is my first PHP contribution. Feedback and suggestions are welcome!