cdaecke / md_saml

TYPO3 SSO Login with SAML authentication
Other
2 stars 8 forks source link

IdP-initiated logout missing #27

Open jwtue opened 2 months ago

jwtue commented 2 months ago

Hi there, I did some testing around and found out that the extension does not support IdP-initiated logout at the moment.

How it should work

When user initiates logout through another SP, the SP will redirect him to the IdP for processing the SLO. The IdP will then consecutively redirect the user to all other SPs that are logged in, with a SingleLogoutRequest. The SPs will perform a logout within their software, and redirect to the IdP with a SingleLogoutResponse. Once all SPs are logged out, the IdP will redirect the user back to the initiating SP.

An example would be both frontend and backend logged in through the extension with the same IdP. When logging out from either FE or BE, the other should get logged out as well.

What actually happens

Nothing. When the IdP sends the user to Typo3 with the SingleLogoutRequest, it stays at the login page and nothing happens.

How to fix it

I tried to implement the functionality myself, and I think I got it to work this way:

  1. For backend: Add the following snippet into https://github.com/cdaecke/md_saml/blob/0d22915087d5feefc50c7da8d518d91493269ec8/Classes/Middleware/SamlMiddleware.php before the return statement:

        if (
            (int)GeneralUtility::_GP('loginProvider') === 1648123062
            && GeneralUtility::_GP('sls') !== null
        ) {
            $context = GeneralUtility::makeInstance(Context::class);
    
            if (isset($GLOBALS['BE_USER']->user)) {
                $GLOBALS['BE_USER']->logoff();
            }
    
            $extSettings = $this->settingsService->getSettings("BE");
            $auth = new Auth($extSettings['saml'], true);
            $auth->processSLO();
        }
  2. For frontend: Add a new middleware to https://github.com/cdaecke/md_saml/blob/0d22915087d5feefc50c7da8d518d91493269ec8/Configuration/RequestMiddlewares.php, that is called after authentication, e.g.

    return [
    'frontend' => [
        'mdsaml/saml-data' => [
            'target' => AcsSamlMiddleware::class,
            'before' => [
                'typo3/cms-frontend/authentication',
            ],
        ],
        'mdsaml/saml-data-sls-frontend' => [
            'target' => FrontendSlsSamlMiddleware::class,
            'after' => [
                'typo3/cms-frontend/authentication',
            ],
        ],
    ],
    // backend middleware here
    ];
  3. Add the new middleware like Mediadreams\MdSaml\Middleware\FrontendSlsSamlMiddleware:

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    {
        if (
            (int)GeneralUtility::_GP('loginProvider') === 1648123062
            && GeneralUtility::_GP('sls') !== null
         ) {
            $extSettings = $this->settingsService->getSettings('fe');
            $context = GeneralUtility::makeInstance(Context::class);
    
            if ($context->getPropertyFromAspect('frontend.user', 'isLoggedIn')) {
                $feUser = $request->getAttribute('frontend.user');
                $feUser->logoff();
            }
    
            $auth = new Auth($extSettings['saml'], true);
            $auth->processSLO();
        }
        return $handler->handle($request);
    }
  4. Finally, in the https://github.com/cdaecke/md_saml/blob/0d22915087d5feefc50c7da8d518d91493269ec8/Classes/EventListener/SamlAfterUserLoggedOutEventListener.php add the following condition, to prevent the OneLogin-Auth to logout again during the SLS process (which would trigger another redirect that would override the SLS-redirect from the Middleware):

        if (
            (int)GeneralUtility::_GP('loginProvider') === 1648123062
            && GeneralUtility::_GP('sls') === null
         ) {
            $extSettings = $settingsService->getSettings($loginType);
            $auth = new Auth($extSettings['saml']);
            $auth->logout();
        }

Final note: If implemented, you should probably make changes to the setup.typoscript, as it mentions SLS here: https://github.com/cdaecke/md_saml/blob/0d22915087d5feefc50c7da8d518d91493269ec8/Configuration/TypoScript/setup.typoscript#L135 In the above implementation, this URL is hardcoded, so changes to the Typoscript will have no effect. Also, the URL for backend SLS (/typo3/index.php?loginProvider=1648123062&sls) is found neither in documentation nor Typoscript.

Let me know what you think :)

cdaecke commented 2 months ago

I have just added the implementation for the Single Logout Service.

Would you like to test the current master and give some feedback?

Please don't forget to clear the TYPO3 cache after updating the extension.

jwtue commented 2 months ago

Thank you for looking into this.

I haven't tested yet, but I looked at your commit, and as far as I can see, it's not doing what it should. This issue is something different than what I was talking about in #24.

24 was about the user clicking "logout" in the backend, and notifying the IdP about the logout. This is about the logout being requested in another application or the IdP, and notifying Typo3 about the logout.

For that, calling $auth->processSLO() is not enough. It does work in processing the SingleLogoutRequest from the IdP and returning a SingleLogoutResponse, but it's not performing the actual logout.

When a SingleLogoutRequest arrives, we need to terminate the current Typo3 fe/be session, that's why I suggested calling $GLOBALS['BE_USER']->logoff() and $feUser->logoff(), respectively. Those are only available after typo3/cms-backend/authentication, that's why I registered the Middleware as such.

jwtue commented 2 months ago

Tested and as I suspected, the logout is not performed.

Also, there is a minor yet breaking bug (wrong class name) in https://github.com/cdaecke/md_saml/blob/e76f25d487616061102cce41aba32eebfb409478/Classes/Middleware/SlsBackendSamlMiddleware.php#L24

Do you need more information about what I mean, regarding 1c18a7e not doing what I would expect it to do? If you want, I can provide more detailed explanation about this issue.