Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Current versions success page requires authentication #293

Open
TheOneRing opened this issue Jun 17, 2021 · 6 comments
Open

Comments

@TheOneRing
Copy link

TheOneRing commented Jun 17, 2021

Start the desktop client, add demo.owncloud.org.

Enter the credentials, authorise.
You will get redirected to https://demo.owncloud.com/login?redirect_url=%252Fapps%252Foauth2%252Fauthorization-successful
and get asked for the credentials again.
After entering them you get the login successful message.

Expected:
Getting to the "login successful" page without entering the credentials again.

@DeepDiver1975
Copy link
Member

unrelated to the oauth page controller - annotations for the success page have been untouched in the past 4 years
Screenshot from 2021-06-17 11-33-09

@IljaN
Copy link
Member

IljaN commented Jun 17, 2021

'http.cookie.samesite' => 'Lax' needs to be added to config.php. The root-cause is a recent change in browser behavior.
https://web.dev/schemeful-samesite/

localhost != demo.owncloud.com 💥

@C0rby
Copy link
Contributor

C0rby commented Jun 17, 2021

There are other possible fixes which would require code changes.

  1. Have a dedicated session cookie just for the oauth2 app. (Don't know if that is even possible in the OC architecture)
  2. relax it by default - and those people who want a super save oc can set the config switch to 'strict' <- proposed by @DeepDiver1975

Decision should be made by @micbar.

But the quick fix mentioned by @IljaN can be used for the upcoming 10.8 release. We should include it in the release notes.

@IljaN
Copy link
Member

IljaN commented Jun 17, 2021

I`d vote for option 2 unless 1 is easy. Correct me If I am wrong: The samesite change is done by browser vendors to prevent tracking via third-party cookies. So there should be no real implications for the Security of owncloud setting it to Lax.

@xoxys
Copy link
Contributor

xoxys commented Jun 18, 2021

@TheOneRing Config set on demo systems, could you test it again please?

@TheOneRing
Copy link
Author

Seems to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants