user_saml icon indicating copy to clipboard operation
user_saml copied to clipboard

IdP initiated SSO

Open ghost opened this issue 7 years ago • 12 comments

I'd like to use IdP initated SSO with Nextcloud. The reason for this is that we have different servers/URLs for different customers and those should all be able to access Nextcloud.

Steps to reproduce

  1. Do NOT go to Nextcloud
  2. POST a SAML response to /index.php/apps/user_saml/saml/acs

Expected behaviour

I'd like the user to be logged in to Nextcloud, whatever the state of the user previously was (not logged in, logged in, expired session).

Problems and my fixes

Normally you'd get "null" as a response. Fix: comment out the following lines in assertionConsumerService, so it looks like this:

                // if(is_null($AuthNRequestID) || $AuthNRequestID === '') {
                //       return;
                // }

Then, logging in works. However, when you perform the SAML login again, you get an error "The user is already logged in" (or something like that). This is fixed by commenting out:

// @OnlyUnauthenticatedUsers

The code now looks like this:

         /**
         * @PublicPage
         * @NoCSRFRequired
         * @UseSession
         * // @OnlyUnauthenticatedUsers
         * @NoSameSiteCookieRequired
         *
         * @return Http\RedirectResponse|void
         */
        public function assertionConsumerService() {
                $AuthNRequestID = $this->session->get('user_saml.AuthNRequestID');
                // if(is_null($AuthNRequestID) || $AuthNRequestID === '') {
                //       return;
                // }

This seems to work fine, however I sometimes get an internal error, with in the logs:

Doctrine\DBAL\Exception\UniqueConstraintViolationException: An exception occurred while executing 'INSERT INTO "oc_authtoken"("uid","login_name","name","token","type","remember","last_activity","last_check") VALUES(?,?,?,?,?,?,?,?)' with params ["<SNIP>", "<SNIP>", "<SNIP>", "<SNIP>", 0, 0, 1523868098, 1523868098]: SQLSTATE[23505]: Unique violation: 7 ERROR: duplicate key value violates unique constraint "authtoken_token_index" DETAIL: Key (token)=(<SNIP>) already exists.

I expect this has to do with an expired session.

Questions

Now my actual question: is this a good way to add IdP initiated login to the user_saml plugin? Any security implications? How should I handle the expired session?

When I've got everything figured out, would you be interested in me adding a "Allow IdP initiated SSO" setting?

ghost avatar Apr 16 '18 10:04 ghost

I'm assuming by "different URLs/Servers", you mean different SAML entities? Each has it's own entityID? Why not use "Authentication via Environment Variable" part of user_saml and a different SP, such as Shibboleth SP which will support multiple IdPs i.e. Federated. Might need to add the Shibboleth EDS to do your discovery (pick which IdP to use)

jagland avatar Apr 23 '18 20:04 jagland

@gkrol-zermelo I am having the same issue. iDP initiated login gives me a 'null' response. Would you mind telling me the file location of the assertionConsumerService. I can't seem to find it, as I would like to implement your fix as well. Thanks.

Edit: Never mind found it. For anyone else it's at /var/www/nextcloud/apps/user_saml/lib/Controller then you can open "SAMLController.php" on lines 236 - 248

jwg18 avatar Mar 06 '19 14:03 jwg18

We have a similar need and explored this as well.


We also started by commenting out the @OnlyUnauthenticatedUsers annotation to allow an already logged-in user to come back to nextcloud from the IDP without getting a "User is already logged in error".

However, it creates an issue where two users share the same machine, Alice is logged in on Nextcloud (e.g., via IDP-initiated SSO), then Bob tries an IDP-initiated SSO login. In that scenario, what happens is that Bob arrives on Nextcloud, but still logged in as Alice (he has to manually disconnect Alice from Nextcloud, and login to Nextcloud again).

The quick&dirty fix we implemented was to logout and log back at the start of the "assertionConsumerService" method (with a hardcoded IDP equal to 1):

    public function assertionConsumerService() {
        $this->userSession->logout();
        $this->login(1);

        ...

There is a pull request related to this issue at #297. It does not remove the @OnlyUnauthenticatedUsers though, but adds a couple functionalities.

nbitouze avatar Apr 17 '19 15:04 nbitouze

Just stumbled upon this myself when federating Nextcloud with Azure AD Application Proxy...

It's been a year since the last comment - can this just get fixed or do I have to automate the workarounds into my docker image build pipeline? 😀

mateuszdrab avatar Jun 07 '20 03:06 mateuszdrab

It appears Nextcloud 19 generates this issue again..

mxmathieu avatar Jun 10 '20 11:06 mxmathieu

I don't think it was ever fixed. I am facing both the null issue and the user is already signed in issue.

mateuszdrab avatar Jun 10 '20 11:06 mateuszdrab

This is what I understand from this thread, however I did'nt do anything to fix this in 18.x. It worked out of the box.

mxmathieu avatar Jun 10 '20 11:06 mxmathieu

Do you mean SAML sign in worked overall or IdP initiated sign-in specifically?

mateuszdrab avatar Jun 10 '20 11:06 mateuszdrab

My case is just this : I used AAD for authentication, IdP didn't initiate sign-in. It worked with 18.x. With the 19 release it is now returning null as you described. The issue may be not well qualified on my side I agree...

mxmathieu avatar Jun 10 '20 11:06 mxmathieu

For me it returns null if I signin using IDP, if I'm already signed it, it returns user is already logged in. It's a mess and definitely needs to be fixed. It's a half-compatible SAML solution at the moment :)

mateuszdrab avatar Jun 11 '20 11:06 mateuszdrab

I've applied the tweaks from the OPs first post and indeed I can login via IdP first time and subsequent attempt also worked.

mateuszdrab avatar Jun 12 '20 16:06 mateuszdrab

Same Problem with 19.0.0

heino-vdbh avatar Jun 23 '20 08:06 heino-vdbh