Add PocketID OAuth Provider Plugin #75
Conversation
… Provider-Klasse für OAuth-Integration hinzu
…te Provider-Klasse
📝 WalkthroughWalkthroughAdds a new PocketID OAuth provider plugin (manifest, README, Filament plugin class, service provider, OAuth schema) and updates the GitHub Actions PHPStan workflow to require the Socialite PocketID package. Changes
Sequence Diagram(s)%%{init: {"themeVariables": {"primaryColor":"#7AA2F7","secondaryColor":"#CFE9FF","tertiaryColor":"#E6F9E6"}}}%%
sequenceDiagram
participant Panel as Filament Panel
participant Plugin as PocketIDProviderPlugin
participant Provider as ServiceProvider
participant OAuthSvc as OAuthService
participant Schema as PocketIDSchema
rect rgb(235,245,255)
Note over Panel,Plugin: Plugin is discovered/registered
Panel->>Plugin: register(panel)
Plugin-->>Panel: (no-op register/boot hooks)
end
rect rgb(240,255,235)
Note over Provider,OAuthSvc: Service provider boots and registers schema
Provider->>OAuthSvc: resolve OAuthService
Provider->>Schema: new PocketIDSchema()
Provider->>OAuthSvc: register(schema)
OAuthSvc->>Schema: getId(), getServiceConfig(), getSetupSteps(), getSettingsForm()
Schema-->>OAuthSvc: metadata & config
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
pocketid-provider/src/PocketIDProviderPlugin.php (2)
15-20: Consider whether resource discovery is needed.The plugin registers resource discovery for Filament resources, but no resources directory or resource files are included in this PR. If resources are planned for the future, this is fine; otherwise, consider removing the discovery logic to keep the implementation minimal.
22-22: Remove unused parameter or method.The
boot()method is empty and its$panelparameter is unused. If no boot logic is needed, the method can be removed entirely, or at least remove the unused parameter.🔎 Proposed fix
Option 1: Remove the method entirely if not needed:
- - public function boot(Panel $panel): void {}Option 2: Remove the unused parameter:
- public function boot(Panel $panel): void {} + public function boot(): void {}pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (4)
40-47: Consider extracting the Blade template to a separate view file.The inline Blade rendering works but makes the code harder to maintain. Consider extracting this to a dedicated view file (e.g.,
resources/views/setup-instructions.blade.php).🔎 Refactoring suggestion
Create a view file
resources/views/pocketid-setup-instructions.blade.php:<ol class="list-decimal list-inside space-y-1"> <li>Log in to your Pocket ID instance</li> <li>Navigate to your application or create a new OAuth application</li> <li>Copy the <strong>Client ID</strong> and <strong>Client Secret</strong> from your Pocket ID application</li> <li>Configure the redirect URL shown below in your Pocket ID application settings</li> </ol>Then update the code:
- ->state(new HtmlString(Blade::render(' - <ol class="list-decimal list-inside space-y-1"> - <li>Log in to your Pocket ID instance</li> - <li>Navigate to your application or create a new OAuth application</li> - <li>Copy the <strong>Client ID</strong> and <strong>Client Secret</strong> from your Pocket ID application</li> - <li>Configure the redirect URL shown below in your Pocket ID application settings</li> - </ol> - '))), + ->state(new HtmlString(view('pocketid-setup-instructions')->render())),
73-77: Pure black default may not work well with all UI themes.The default color '#000000' (pure black) might not provide good contrast or fit with all UI themes. Consider using a more neutral default like '#374151' (gray-700) or making it match the application's primary color scheme.
68-72: Add maxLength constraint to display name field for UI consistency.The display name field should have a maximum length constraint to prevent excessively long provider names from causing layout issues in the UI. Add
->maxLength(50)or an appropriate limit based on UI layout requirements.Regarding XSS escaping: Filament Forms automatically escapes TextInput values when rendering in the admin interface. The display name is admin-configured via environment variable and not directly user-controlled, which mitigates XSS risk. The actual rendering of this value to end users occurs in the parent application code, which is outside this plugin's scope.
48-52: Consider simplifying the field configuration for the read-only callback display.The field is marked as both
dehydrated()anddisabled(), which is unusual for a display-only setup wizard field. Sincedisabled()already prevents user interaction,dehydrated()appears unnecessary—verify whether it's intentionally included for form submission processing in the parent schema's setup wizard logic.Additionally, the hard-coded callback path would benefit from using Laravel's
route()helper instead ofurl('/auth/oauth/callback/pocketid')for better maintainability if a named route exists in the application's routing configuration.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/lint.ymlpocketid-provider/README.mdpocketid-provider/plugin.jsonpocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.phppocketid-provider/src/PocketIDProviderPlugin.phppocketid-provider/src/Providers/PocketIDProviderPluginProvider.php
🧰 Additional context used
🧬 Code graph analysis (3)
pocketid-provider/src/PocketIDProviderPlugin.php (2)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
getId(16-19)pocketid-provider/src/Providers/PocketIDProviderPluginProvider.php (1)
boot(11-15)
pocketid-provider/src/Providers/PocketIDProviderPluginProvider.php (2)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
PocketIDSchema(14-95)pocketid-provider/src/PocketIDProviderPlugin.php (2)
boot(22-22)register(15-20)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
pocketid-provider/src/PocketIDProviderPlugin.php (1)
getId(10-13)
🪛 markdownlint-cli2 (0.18.1)
pocketid-provider/README.md
12-12: Bare URL used
(MD034, no-bare-urls)
🪛 PHPMD (2.15.0)
pocketid-provider/src/PocketIDProviderPlugin.php
22-22: Avoid unused parameters such as '$panel'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (7)
.github/workflows/lint.yml (1)
59-59: LGTM!The addition of the PocketID provider dependency ensures it's available during PHPStan analysis. The version constraint
^5.0correctly matches the requirement specified inplugin.json.pocketid-provider/src/Providers/PocketIDProviderPluginProvider.php (1)
11-15: LGTM!The service provider correctly registers the PocketID OAuth schema with the
OAuthServiceduring the boot phase, following standard Laravel service provider conventions.pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (5)
1-13: LGTM!The namespace and imports are well-organized and appropriate for a Filament-based OAuth provider plugin.
16-24: LGTM!The identifier and Socialite provider configuration are correctly implemented.
60-67: LGTM! Good security practices on the Base URL field.The URL validation, required constraint, and disabled autocomplete are appropriate security measures for an OAuth configuration field.
86-89: LGTM! Appropriate icon choice.The
heroicon-o-identificationicon is a good fit for an identity/authentication provider.
1-95: The code is compatible withsocialiteproviders/pocketidv5.0. TheProviderclass is correctly imported from theSocialiteProviders\PocketIDnamespace, and the usage pattern of returningProvider::classis standard for registering provider classes with the OAuth service. No breaking changes were found affecting this implementation.
correct the url to the plugin folder Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pocketid-provider/README.mdpocketid-provider/plugin.json
🚧 Files skipped from review as they are similar to previous changes (1)
- pocketid-provider/README.md
🔇 Additional comments (2)
pocketid-provider/plugin.json (2)
1-5: LGTM!The plugin metadata structure is well-formed with valid JSON syntax and appropriate initial fields.
15-15: Thesocialiteproviders/pocketidpackage exists on Packagist and version ^5.0 is available. No known security vulnerabilities are recorded for this package. The dependency declaration is valid.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @Ebnater. * #75 (comment) The following files were modified: * `pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php` * `pocketid-provider/src/PocketIDProviderPlugin.php` * `pocketid-provider/src/Providers/PocketIDProviderPluginProvider.php`
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
48-52: Optional:dehydrated()is redundant on disabled fields.The
dehydrated()call on Line 50 has no effect since disabled fields are not included in form submissions by default. You can safely remove it.🔎 Proposed cleanup
TextInput::make('_noenv_callback') ->label('Callback URL') - ->dehydrated() ->disabled() ->default(fn () => url('/auth/oauth/callback/pocketid')),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdpocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.phppocketid-provider/src/PocketIDProviderPlugin.php
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-04T13:12:10.333Z
Learnt from: Ebnater
Repo: pelican-dev/plugins PR: 75
File: pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php:26-31
Timestamp: 2026-01-04T13:12:10.333Z
Learning: In all OAuth provider schema implementations (e.g., PocketIDSchema.php and other OAuth schema files), read credentials and configuration strictly from environment variables using env(), not from configuration files or database storage. Apply this pattern consistently across all plugins implementing OAuth schemas to avoid hard-coded secrets and to centralize configuration in the environment. If a credential must be sourced differently, document and justify the exception, but prefer env() as the standard for secrets and provider URLs, and ensure tests reflect environment-based configuration.
Applied to files:
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php
🧬 Code graph analysis (2)
pocketid-provider/src/PocketIDProviderPlugin.php (2)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
getId(16-19)pocketid-provider/src/Providers/PocketIDProviderPluginProvider.php (1)
boot(11-15)
pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (1)
pocketid-provider/src/PocketIDProviderPlugin.php (1)
getId(10-13)
🪛 PHPMD (2.15.0)
pocketid-provider/src/PocketIDProviderPlugin.php
15-15: Avoid unused parameters such as '$panel'. (undefined)
(UnusedFormalParameter)
17-17: Avoid unused parameters such as '$panel'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (8)
pocketid-provider/src/PocketIDProviderPlugin.php (1)
8-18: LGTM! Minimal plugin implementation follows Filament conventions.The empty
register()andboot()methods are intentional—this plugin serves as a Filament discovery mechanism. The actual OAuth schema registration is correctly delegated toPocketIDProviderPluginProvider. The PHPMD warnings about unused$panelparameters are false positives, as these parameters are required by thePlugininterface contract.pocketid-provider/src/Extensions/OAuth/Schemas/PocketIDSchema.php (7)
16-19: LGTM! OAuth provider ID is correctly defined.The ID 'pocketid' correctly matches the OAuth callback URL pattern and is appropriately distinct from the plugin ID.
21-24: LGTM! Socialite provider reference is correct.The method correctly returns the PocketID provider class from the socialiteproviders/pocketid package.
26-31: LGTM! Configuration approach follows established OAuth provider pattern.The use of
env()for runtime configuration is consistent with the Pelican panel's OAuth provider architecture. Based on learnings, this is the standard pattern for all OAuth schema implementations in this codebase.
57-79: LGTM! Settings form is well-structured with appropriate validation.The form includes proper validation (URL format, hex color) and follows the established env()-based configuration pattern. The field organization and defaults are sensible.
81-84: LGTM! Display name retrieval follows the established pattern.
86-89: LGTM! Tabler icon addresses previous feedback.The use of 'tabler-id' appropriately addresses Boy132's request to use a Tabler icon.
91-94: LGTM! Color retrieval is consistent with the settings form.
Boy132
left a comment
Boy132
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
For future reference, please create a separate branch for pull requests and don't use main. :)
2 participants
I added a PocketIDSchema to OAuth Providers. The initial code is copied from this repo.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.