DutchCodingCompany / filament-socialite

Add OAuth login through Laravel Socialite to Filament.
MIT License
141 stars 36 forks source link

SAML2 compatibility #101

Closed juliangums closed 1 month ago

juliangums commented 1 month ago

I tried to implement a simple integration of the SAML2 provider: https://socialiteproviders.com/Saml2/ I can see the button

Screenshot 2024-06-20 at 14 00 03

but it takes me to /admin/oauth/saml2 which results in a 404. I don't even understand where the admin part comes from as that isn't my panel url. Did I miss anything setup-wise? I installed this package and the socialite driver and set up the provider config. I then simply added this to the panel:

            ->plugin(
                FilamentSocialitePlugin::make()
                    ->providers([
                        Provider::make('saml2')
                            ->label('SSO'),
                    ])
                    ->registration(false)
            );

and this to my EventServiceProvider

        SocialiteWasCalled::class => [
            Saml2ExtendSocialite::class.'@handle',
        ],

Are there any routes I need to set up? I was hoping this package handles that.

bert-w commented 1 month ago

Can you do some debugging using php artisan route:list? The package should add the GET /$slug/oauth/{provider} route for you which handles the authentication flow.

juliangums commented 1 month ago

This is the only one I can find: admin/oauth/{provider}

socialite.filament.admin.oauth.redirect › DutchCodingCompany\FilamentSocialite › SocialiteLoginController@redirectToProvider

bert-w commented 1 month ago

Yes,so the route seems to be defined. Please check carefully where the 404 comes from because this route will redirect you to the external provider. (btw the "admin" prefix comes from your Filament Panel ID if im correct; it can be changed with the ->slug() function on the plugin).

juliangums commented 1 month ago

@bert-w, thanks for your help. I changed the slug to "auth" to test it out, but I was still having the same issue.

The route is set up correctly. The package recognises that the saml2 provider is registered, otherwise, it would throw an error indicating it's not configured.

After some debugging, I found that the issue occurs here: https://github.com/DutchCodingCompany/filament-socialite/blob/main/src/Http/Controllers/SocialiteLoginController.php#L36-L42. I commented out ->with() and ->scopes(), so I knew it seems to be the ->redirect() that's causing the 404 error.

To rule out any issues with the driver, I tried a few things and I worked out that it works if I add one of these two routes in my web routes file:

Route::get('/auth/callback', function () {
    $user = Socialite::driver('saml2')->user();
});

Route::post('/auth/callback', function () {
    $user = Socialite::driver('saml2')->user();
});

Now, I'm not getting a 404 anymore, but I'm encountering an incompatible type hint error:

DutchCodingCompany\FilamentSocialite\Http\Controllers\SocialiteLoginController::redirectToProvider(): Return value must be of type Illuminate\Http\RedirectResponse, Symfony\Component\HttpFoundation\RedirectResponse returned

After removing the return type here https://github.com/DutchCodingCompany/filament-socialite/blob/main/src/Http/Controllers/SocialiteLoginController.php#L27 , I finally get redirected to the identity provider. But I still have two questions:

  1. I'm still unsure why I have to define /auth/callback, as I thought this package handles it. Do I need to write my own route and own logic like here: https://laravel.com/docs/11.x/socialite#authentication-and-storage?

  2. Is the type hint wrong in this package or the saml2 socialite provider?

juliangums commented 1 month ago

I guess for the return type as one extends the other and it seems to be working with the wider one, I could submit a PR for that if you like if it isn't breaking anything.

bramr94 commented 1 month ago

Maybe the redirect URL is incorrect, can you verify it is the same as this:

https://example.com/oauth/callback/saml2

Since you are getting a 404 the redirect URL might be incorrect.

juliangums commented 1 month ago

@bramr94 I see. I played around with the routes again. If I register something at /auth/callback as described above like this it works:

Route::get('/auth/callback', function () {
    $user = Socialite::driver('saml2')->user();
});

But if I change it to /oauth/callback/saml2 it does not. Where do I set this up through?

juliangums commented 1 month ago

I can see it's hardcoded in this package so it can't be changed. I'll check if I find something in the saml2 driver package https://github.com/DutchCodingCompany/filament-socialite/blob/main/routes/web.php#L28

juliangums commented 1 month ago

Got it now. I had to define it with the correct sp_acs value

    'saml2' => [
        'metadata' => '...',
        'sp_acs' => '/oauth/callback/saml2',
    ],

The return type is still an issue though. Do you think it should be changed here or on the SAML2 socialite provider?

juliangums commented 1 month ago

I can't see how I can change it in the SAML2 driver repo, but I also think it could be a breaking change. Someone else might be checking for the parent class. So I think it would be best to change it in this package. Are you ok with that? I prepared a pull request for that: https://github.com/DutchCodingCompany/filament-socialite/pull/103