pulsejet / nextcloud-oidc-login

Nextcloud login via a single OpenID Connect 1.0 provider
https://apps.nextcloud.com/apps/oidc_login
GNU Affero General Public License v3.0
232 stars 63 forks source link

Refresh tokens #102

Open stevesobol opened 3 years ago

stevesobol commented 3 years ago

Good evening!

Question: Does your plugin handle refresh tokens properly?

I have my Nextcloud instance set to use my Keycloak server for authentication. Authentication works flawlessly. HOWEVER, despite my realm's session idle timeout being 14 days, and the max session length being 30 days (neither of which is overridden in the client I set up for use with Nextcloud), and despite having logged into two separate Keycloak sessions in that realm yesterday, if I pull up the URL of my Nextcloud server, I get redirected to my realm's login screen.

I don't see any mention of refresh tokens in your code, although the OIDC library you use does grab them and pass them back to the caller when it receives them.

Thanks :)

dseomn commented 3 years ago

I just spent some time looking into the session handling code in this plugin and in Nextcloud server, for a related issue. If I understand correctly, this plugin doesn't really affect Nextcloud's session handling, it just handles authentication at the beginning, and logout at the end. @pulsejet Is that right?

@stevesobol If that's right, I'm guessing you might want to increase the session_lifetime parameter. (But it also means that the Nextcloud session duration is mostly separate from the session with the OpenID Provider, which is unfortunate.)

stevesobol commented 3 years ago

You may be correct. I've already forked the Git repo with the intention of seeing how I can improve it, but I don't have the bandwidth to do that at this moment.

Wednesday, October 13, 2021, 7:34:37 PM, you wrote:

I just spent some time looking into the session handling code in this plugin and in Nextcloud server, for a related issue. If I understand correctly, this plugin doesn't really affect Nextcloud's session handling, it just handles authentication at the beginning, and logout at the end. @pulsejet Is that right? @stevesobol If that's right, I'm guessing you might want to increase the session_lifetime parameter. (But it also means that the Nextcloud session duration is mostly separate from the session with the OpenID Provider, which is unfortunate.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

stevesobol commented 3 years ago

@dseomn I am 100% sure you're correct. config.sample.php sets session_lifetime to 60 60 24... 24 hours.

This coincides with what I'm seeing, that I can log in, but I'm logged out the next day. This isn't a problem with the plugin, it's a configuration issue. I'll do some testing, but I'm pretty sure that this isn't something that has to be fixed.

dseomn commented 3 years ago

It's not a bug, but using refresh tokens seems like a really good feature to have. I'd rather not have such separate session validity in Nextcloud and the SSO system. Mind re-opening this?

stevesobol commented 3 years ago

To be honest, it probably is a good idea. I would like to (maybe) work on it, I just don't know if/when I will have the time. I'm putting it on my personal to-do list, but it's going to be down at the bottom...

stevesobol commented 2 years ago

It's looking like the session lifetime was indeed too short. I'm basing this on examining the expiration dates on the cookies Nextcloud sets when you use the web UI.

@dseomn:

(But it also means that the Nextcloud session duration is mostly separate from the session with the OpenID Provider, which is unfortunate.)

Yeah, but the OpenID session lifetime is configured completely separately (i.e., not in Nextcloud at all). Is this situation avoidable?

dseomn commented 2 years ago

I'd prefer that Nextcloud with this plugin used the OIDC refresh token for as long as it's valid (or until the user manually logs out of Nextcloud), and let the OpenID Provider control the Nextcloud session by invalidating the refresh and access tokens when it's time to end the session. At least with the provider I'm using (LemonLDAP::NG) I think it's pretty easy to tie the validity period of the refresh token to the session with the provider.