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

Add an option to set oAuth2 applications as trusted, for own external apps which uses oAuth2 sessions #46629

Closed
T0mWz opened this issue Jul 19, 2024 · 9 comments · Fixed by #49670
Assignees
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: authentication 🍀 2025-Spring
Milestone

Comments

@T0mWz
Copy link

T0mWz commented Jul 19, 2024

Is your feature request related to a problem? Please describe.
For connecting proprietary apps, exchanging a user session via oAuth2 is convenient.
However, now a user must explicitly always grant permission to external apps via oAuth2.
Would be nice that for own apps, for example, you can say they are trusted and you do not have to give explicit permission.

Describe the solution you'd like
Maybe an option to extend the oauth2 clients with an extra field; trusted, which is by default false and can be enabled if client is trusted by default.

@T0mWz T0mWz added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Jul 19, 2024
@jancborchardt
Copy link
Member

Seems sensible for a smoother experience to have some sort of allowlist. @sorbaugh @AndyScherzinger for Files planning.

@jancborchardt jancborchardt moved this to 🧭 Planning evaluation / ideas in 🖍 Design team Sep 13, 2024
@AndyScherzinger
Copy link
Member

Also looping in @nickvergessen and @julien-nc for a security perspective since I believe this is basically a trade off situation treating security for convinience (which doesn't invalidate the idea, just something to be clear about)

@joshtrichards joshtrichards changed the title [FeatureRequest] Add an option to set oAuth2 applications as trusted, for own external apps which uses oAuth2 sessions Add an option to set oAuth2 applications as trusted, for own external apps which uses oAuth2 sessions Sep 13, 2024
@sorbaugh sorbaugh moved this to 📄 To do (~10 entries) in 📁 Files team Oct 2, 2024
@sorbaugh sorbaugh added this to the Nextcloud 31 milestone Oct 2, 2024
@julien-nc
Copy link
Member

As far as I know (and checked), skipping the step which asks for the user's consent is not part of the OAuth "Authorization Code Grant" specifications.

Anyway, this would only be beneficial if the browser already has a valid Nextcloud session. But if the user is not authenticated yet, the login phase interrupts the flow anyway. So there is no guaranty that the flow will be transparent (require no user interaction) even if we allow to remove the consent step (grant access confirmation).

I think we should stick to the Oauth2 specifications.
@nickvergessen Wdyt?

@julien-nc
Copy link
Member

Security concern: Any entity in possession of the client ID and secret can silently connect as a user who currently has an active NC session in the browser.

@sorbaugh
Copy link
Contributor

@T0mWz regarding the comments from julien-nc, it might be worth to re-discuss this issue.

@T0mWz
Copy link
Author

T0mWz commented Oct 10, 2024

That's correct what @julien-nc mentioned. For that, they are the trusted clients and that choice should also be made very consciously. By default, you should not make a client trusted, but only if you're really sure who the client is and based on callback url.
We use it to implement integrated apps more seemless, where we retrieve user details via oAuth2.
Therefore, we deliberately want some (internal apps) as trusted, but also some (external) applications/clients certainly not.

@sorbaugh
Copy link
Contributor

Adding an option to specifically define an app as trusted while everything else needs confirmation by default would be reasonable.

As discussed, our compromise for remaining security concerns, in order to avoid "novice users" or "accidental users" clicking it, we would only allow changing that via OCC command as a tool for admins.

@come-nc
Copy link
Contributor

come-nc commented Nov 28, 2024

Questions:

  • Does the oauth protocol allow skipping the validation from backend or will we need a frontend change to autovalidate?
  • What is the code path for the oauth confirmation in the backend? Which endpoints are used?

@julien-nc
Copy link
Member

Does the oauth protocol allow skipping the validation from backend or will we need a frontend change to autovalidate?

Not sure what you mean.

What is the code path for the oauth confirmation in the backend? Which endpoints are used?

It uses the login flow:

  • entry point is the authorize page: /apps/oauth2/authorize
  • redirect to /login/flow
  • leads to login/flow/grant
  • click on "grant access" makes a POST request to /login/flow

@sorbaugh sorbaugh moved this from 📄 To do (~10 entries) to 🏗️ In progress in 📁 Files team Dec 9, 2024
@github-project-automation github-project-automation bot moved this from 🧭 Planning evaluation / ideas to 🎉 Done in 🖍 Design team Jan 7, 2025
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 📁 Files team Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: authentication 🍀 2025-Spring
Projects
Status: ☑️ Done
Status: 🎉 Done
Development

Successfully merging a pull request may close this issue.

7 participants