-
Notifications
You must be signed in to change notification settings - Fork 92
Description
Okay, so I fully recognize this is probably a rare scenario but I thought I should at least post an issue because it was a huge pain to diagnose.
If:
- You use Laravel Passport to provide OAuth2 login from other applications
- You use this package to provide SAML login from other applications
- Your OAuth2 request to Laravel Passport uses the 'prompt=login'
- The user is already logged in when they hit the /oauth/authorize route
The user will be redirected to the home route instead of being sent back to /oauth/authorize like they should be.
The cause:
- When the user hits the /oauth/authorize route, Laravel Passport calls
$this->guard->logout()which fires anIlluminate\Auth\Events\Logoutevent - With the default configuration, laravel-samlidp registers
CodeGreenCreative\SamlIdp\Listeners\SamlLogoutas a listener for that event - The
SamlLogoutlistener callsabort(redirect('saml/logout'), 200)as long as the guard the user logged out of is listed inconfig('samlidp.guards')andsession('saml.slo')isnull
The workaround:
This happens regardless of whether you're actually using SLO - in my case I don't rely on it, so I can just comment out this line in the config:
'Illuminate\Auth\Events\Logout' => ['CodeGreenCreative\SamlIdp\Listeners\SamlLogout'],
If you are using SLO, fixing this likely requires an actual code change. My first thought was that the logout logic should just run in the SamlLogout listener, but since logging the user out requires redirecting them to the SP there's no way around the redirect.
A few ideas:
- Add a special case for Laravel Passport (maybe the
SamlLogoutlistener should do nothing if the current URI is /oauth/authorize and thepromptquery parameter is set tologin). This could even be gated behind a 'passport_compatibility_mode' flag in the config or something to make it opt-in - Before redirecting to /saml/logout, check to see if any SPs actually have the 'logout' property - if they don't, there's no need to redirect
- Implement SLO via back-channel binding (server-to-server requests) instead of redirecting the user - this could be done synchronously (slows down logouts) or offloaded to a queue job. Since not all SPs will support this, it's probably not the best option.
I don't see any particularly elegant solutions here, and running both OAuth2 and SAML in the same codebase is definitely a bit weird, so honestly I think it'd be pretty reasonable to add a quick caveat in the docs and close this. I'm totally new to SAML, though, so maybe someone else will think of a great way to handle this scenario.