[Bug]: Access forbidden - State token missing in Nextcloud Desktop

nextcloud / user_oidc

OIDC connect user backend for Nextcloud

GNU Affero General Public License v3.0

89 stars 36 forks source link

[Bug]: Access forbidden - State token missing in Nextcloud Desktop #972

Closed joselameira closed 3 weeks ago

joselameira commented 3 weeks ago

How to use GitHub

Please use the 👍 reaction to show that you are affected by the same issue.
Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
Subscribe to receive notifications on status change and new comments.

Steps to reproduce

Have OIDC configured (using Keycloak for instance)
Make sure you're logged out from Nextcloud on browser
Try to login with Nextcloud Desktop using the OIDC IdP

Expected behaviour

Actual behaviour

joselameira commented 3 weeks ago

This happens because stateToken is missing while redirecting back to Nextcloud on GET call to

https:///apps/user_oidc/code?state=...&session_state=...&iss=...&code=...

This returns 303 - /login/v2/grant

instead of the complete URL with path and Query String (that was correctly passed at the beginning of the processes).

It seems related to commit 99f7b730e7911459de22898e431cfcf525f0393a

I've prepared a pull request https://github.com/nextcloud/user_oidc/pull/971 that fixes this issue

xataxxx commented 3 weeks ago

Thank you @joselameira and @julien-nc

I've encountered this same bug after upgrading to the latest nextcloud desktop and nextcloud 30 + all the app updates and couldn't actually pinpoint the reason for the bug, I found a temporary workaround, but this does seem like the right fix for it.

xataxxx commented 3 weeks ago

Fun fact, this patch and the latest 6.1.2 did NOT fix the same error message for my OIDC use case with Talk Desktop. I'll dig a bit further and attempt to get some specific log data of what's going on.

joselameira commented 3 weeks ago

Hi @xataxxx

That's weird. Just installed Talk Desktop 0.39.0, and Talk Mobile 20.0.2 (on Android) and it's working on both of them.

julien-nc commented 3 weeks ago

Same here. I tested with and without the PR. I'm positive the PR fixed the issue of the missing GET parameters in the redirect URL during the login flow (tested with the desktop client).

xataxxx commented 3 weeks ago

My bad, you are correct, 6.1.2 does fix things. Since I had been debugging I had managed to accidentally overwrite the 6.1.2 LoginController file with the previous version before the update.

julien-nc commented 3 weeks ago

@xataxxx Thanks for the feedback. Closing this then.