nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.64k stars 4.09k forks source link

Simplify OAuth2 login flow #17165

Open kinolaev opened 5 years ago

kinolaev commented 5 years ago
**Is your feature request related to a problem? Please describe.** When user trying to login using OAuth2 client, he sees three screens: - authpicker page - login page - grant page First is completely useless (#17136), third must be showed only one time. **Describe the solution you'd like** I suggest to create table to store scopes that user has granted to client and skip grant page if no new scopes were requested. Also user must be able to revoke access to some or all scopes through web ui. **Describe alternatives you've considered** Alternatively we can check scopes from previous sessions (column `scope` in `authtoken` table). I would like to create PR, but firstly I want to discuss need I create new table or use `authtoken`. Personally I prefer first variant because (1) `authtoken` doesn't have client id (only name) (2) even if user revokes all sessions it doesn't mean that user want to revoke access for client, so sessions and scopes must be stored separately (3) oauth sessions have small expiration period (1 hour, column `expires` in `authtoken`) and it is strange to check expired sessions. **Additional context** ![0-authpicker](https://user-images.githubusercontent.com/1247759/64979932-771a2280-d8c1-11e9-9725-0769118acaa9.png) ![1-login](https://user-images.githubusercontent.com/1247759/64979933-771a2280-d8c1-11e9-9247-e0bed3f7ef1b.png) ![2-grant](https://user-images.githubusercontent.com/1247759/64979934-771a2280-d8c1-11e9-942c-eab1720c221d.png)
sunjam commented 5 years ago

Perhaps @jancborchardt could advise you about a pull request. See this active pr for polishing the login process for reference

kesselb commented 5 years ago

@sunjam there is already some discussion over here: https://github.com/nextcloud/server/pull/17136

skjnldsv commented 5 years ago

Shall we close then?

kinolaev commented 5 years ago

Hello @skjnldsv, it is not critical for me now because I decided to create nextcloud app instead of separate app that can authenticate through nextcloud. But I still think that if you want to promote nextcloud as identity provider for other apps, you need to simplify oauth2 flow. Ideally a user that already authenticated on nextcloud and previously grants permissions to oauth2 client schould only see loader and redirection to client page instead of dialogs above. I you don't agree, then you can close this. Otherwise, I want to see your recommendations about things mentioned in the first message and I think I will have free time in January to make PR for this. Anyway - thank you for the great opensource software!)

skjnldsv commented 5 years ago

Hey :) I have really no strong preferences as I'm realy not specialised in this area. But since there was some discussion overlapping this thread here: #17136 I assumed we should maybe close to avoid duplicate topics ;)

kinolaev commented 5 years ago

Yes, there is some overlapping with #17136 (mentioned in the first message), but PR #17136 is about skiping authpicker page and this issue is also about skiping grant page (when user previously grants persmissions). So we can use this issue as central discussion about oauth2 flow, PR #17136 - for authpicker page and new PR - for grant page.

skjnldsv commented 5 years ago

Let's leave it opened then :)

jancborchardt commented 5 years ago

Totally agree that the first screen is completely useless, always confuses me as it basically duplicates the 3rd screen.

MichaelDaum commented 3 years ago

The third dialog can be spared as well when I already granted access to the app.

naturzukunft commented 7 months ago

We also would like to have this solved. it's more than confusing for users.

jancborchardt commented 3 months ago

@sorbaugh @AndyScherzinger who can advise on feasibility and workload here? :)