nextcloud / user_saml

:lock: App for authenticating Nextcloud users using SAML https://apps.nextcloud.com/apps/user_saml
https://portal.nextcloud.com/article/configuring-single-sign-on-10.html
GNU Affero General Public License v3.0
95 stars 74 forks source link

SAML Logout is not working after upgrading to Nextcloud 12 #112

Closed staeglis closed 7 years ago

staeglis commented 7 years ago

Steps to reproduce

  1. Connect Nextcloud 12 to Univention Corporate Server 4.1 (not tested with other IdP!)
  2. Login with a User
  3. Logout

Expected behaviour

The user should be logged out

Actual behaviour

The user is still be logged in

Server configuration

Operating system: Linux

Web server: nginx (maybe only as proxy)

Database: MySQL

PHP version: 7.0.19

Nextcloud version: 12.0

Where did you install Nextcloud from: tar.gz

List of activated apps: Standard Apps + user_saml

If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your Nextcloud installation folder

Nextcloud configuration:

If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder

or

Insert your config.php content here
Make sure to remove all sensitive content such as passwords. (e.g. database password, passwordsalt, secret, smtp password, …)

<?php $CONFIG = array ( 'instanceid' => '', 'passwordsalt' => '', 'secret' => '', 'trusted_domains' => array ( 0 => 'nextcloud.xyz.de', ), 'datadirectory' => '/var/www/vhosts/xyz.de/nextcloud.xyz.de/nextcloud/data', 'overwrite.cli.url' => 'https://nextcloud.xyz.de/nextcloud', 'dbtype' => 'mysql', 'version' => '12.0.0.29', 'dbname' => 'nextcloud', 'dbhost' => 'localhost', 'dbtableprefix' => 'oc_', 'dbuser' => 'nextcloud', 'dbpassword' => '(', 'logtimezone' => 'UTC', 'installed' => true, 'mail_from_address' => 'webmaster', 'mail_smtpmode' => 'php', 'mail_domain' => 'xyz.de', 'maintenance' => false, 'updater.release.channel' => 'production', 'theme' => '', 'loglevel' => 2, 'updater.secret' => '', );

Client configuration

Browser: Firefox, Chromium

Operating system: Ubuntu 16.04

Logs

Nextcloud log (data/owncloud.log)

Insert your Nextcloud log here

3,"time":"2017-05-23T07:39:52+00:00","remoteAddr":"1.2.3.4","user":"--","app":"PHP","method":"POST","url":"\/nextcloud\/index.php\/apps\/user_saml\/saml\/acs","message":"Undefined offset: 1 at \/var\/www\/vhosts\/xyz.de\/nextcloud.xyz.de\/nextcloud\/apps\/user_saml\/lib\/userbackend.php#416","userAgent":"Mozilla\/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko\/20100101 Firefox\/53.0","version":"12.0.0.29"} {"reqId":"WSPnSH8AAQEAADV6AYgAAAAH","level":3,"time":"2017-05-23T07:39:52+00:00","remoteAddr":"1.2.3.4","user":"--","app":"PHP","method":"POST","url":"\/nextcloud\/index.php\/apps\/user_saml\/saml\/acs","message":"Invalid argument supplied for foreach() at \/var\/www\/vhosts\/xyz.de\/nextcloud.xyz.de\/nextcloud\/apps\/user_saml\/lib\/userbackend.php#394","userAgent":"Mozilla\/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko\/20100101 Firefox\/53.0","version":"12.0.0.29"}

Browser log

Insert your browser log here, this could for example include:

a) The javascript console log
b) The network log
c) ...
staeglis commented 7 years ago

Update to server configuration: nginx is only reverse proxy, php is running apache (mod_php)

Additional steps for different behavior:

  1. Login in UMC (UCS interface) with SAML
  2. Logout in UMC. Redirects to nextcloud with CSRF failure
  3. Logout in nextcloud: The user is now be logged out
staeglis commented 7 years ago

The different behavior is IMO the same as in this issue

ebogaard commented 7 years ago

You probably mean issue #45

staeglis commented 7 years ago

I can see the error messages also if the user is logging in. Maybe this has something to do with the new attribute assignment functionality? I tried this out but it isn't working too.

staeglis commented 7 years ago

Attribute assignment is now working, but the messages and the problem aren't gone.

tbrandstetter commented 7 years ago

I have the same issue and at least implemented a workaround for this error. I added the @NoCSRFRequired attribute to the singleLogoutService in apps/user_saml/lib/Controller/SAMLController.php. Like this:

/**
 * @NoAdminRequired
 * @NoCSRFRequired
 */
public function singleLogoutService() {
    $auth = new \OneLogin_Saml2_Auth($this->SAMLSettings->getOneLoginSettingsArray());
    $returnTo = null;
    $parameters = array();
    $nameId = $this->session->get('user_saml.samlNameId');
    $sessionIndex = $this->session->get('user_saml.samlSessionIndex');
    $this->userSession->logout();
    $auth->logout($returnTo, $parameters, $nameId, $sessionIndex);
}

I think a CSRF check is not needed for this type of functionality - the user just gets logged out. Are there any security concerns or is this the proper way to do it?

staeglis commented 7 years ago

Thank you for the hint. The logout initiated by an other SP is now working. But I hope in the end there will be a less general solution. But at the moment I can live with this.

Unluckily the logout directly from Nextcloud is still not working. @tbrandstetter Do you have also this problem or is the SAML Logout working in your Nextcloud installation.

tbrandstetter commented 7 years ago

@staeglis I'm still using Nextcloud 11 where the logout from Nextcloud is still working. I'm going to install Nextcloud 12 next week and take a look at it.

KamilWo commented 7 years ago

How about changing logout URL to be /index.php/apps/user_saml/saml/sls when user_saml is installed? If you use the URL in the browser, SAML logout is initiated properly. At the moment Nextcloud 12.0.0 uses /index.php/logout?requesttoken=....

staeglis commented 7 years ago

@KamilWo Interesting point. How was the logout URL in Nextcloud 11? I don't remember

NeilWJames commented 7 years ago

Excuse me for the interruption - but I have the same issue - and as you asked about 11.

I can confirm that Nextcloud 11.0.3 uses the logout url /index.php/apps/user_saml/saml/sls?requesttoken=... and succeeds in logging out, but Nextcloud 12.0.0.29 has /logout?requesttoken=... (i.e. no index.php in the url).

I can also confirm that using the 11.0.3 location in 12.0.0.29 successfully logs out the session/

staeglis commented 7 years ago

As workaround I've created an external site link to /index.php/apps/user_saml/saml/sls

KamilWo commented 7 years ago

@NeilWJames Thank you for the confirmation.

@staeglis The URL /index.php/apps/user_saml/saml/sls was the single logout URL in Nextcloud 11.

LukasReschke commented 7 years ago

Regression introduced with https://github.com/nextcloud/server/commit/054e161eb5f4a5c5c13ee322ae8e93ce66f01b13.

KamilWo commented 7 years ago

Thank you for the fix @LukasReschke For the stable version 12.0.2, who needs SAML log out working, monkey patched solution until the update, then you can find in file nextcloud/lib/private/NavigationManager.php the line 'core.login.logout', (no. 196), comment that out in case you need it later, then put in next line: 'user_saml.SAML.singleLogoutService',. Hope this helps.

KamilWo commented 7 years ago

I confirm the fix is working fine. Thank you, much appreciated.

jetatar commented 6 years ago

I am have Nextcloud 12.0.4 configured with SSO. The bug remains and the above monkey patch is no longer relevant. Any additional info I can provide?

screen shot 2018-01-25 at 18 55 58