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.

Sign up for GitHub

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

Jump to bottom

feat(oauth): Allow to skip grant step for selected applications #49670

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat(oauth): Allow to skip grant step for selected applications #49670

wants to merge 3 commits into from
+85 −59

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Dec 5, 2024 •
edited
Loading

Summary

Before After
request from application Copie d'écran_20241205_145140 request from application Copie d'écran_20241205_145140
Copie d'écran_20241209_145559 skipped
If not logged in, login screen Copie d'écran_20241209_145625 If not logged in, login screen Copie d'écran_20241209_145625
Copie d'écran_20241209_150703 skipped

To get the skipped version, I ran: occ config:app:set oauth2 autoGrantApplications --type array --value '["me"]', with me being the name associated to my client in oc_oauth2_clients table.

TODO

  • Is client name the right thing to match against? (Especially, is that safe?) It should be good, clients (applications) are added and controlled by the admin.
  • Is it okay to store the list of bypassing apps in appconfig?
  • I suppose ClientFlowLoginV2Controller should be adapted as well? Why are there 2 of those? Not needed, oauth2 uses v1 only
  • Why is there a step before login screen? It is now skipped as well for configured applications.

Checklist

@come-nc come-nc added the 2. developing Work in progress label Dec 5, 2024
@come-nc come-nc added this to the Nextcloud 31 milestone Dec 5, 2024
@come-nc come-nc self-assigned this Dec 5, 2024
@come-nc come-nc requested a review from nickvergessen December 5, 2024 16:33
@come-nc come-nc changed the title feat(oauth): Allow bypass of grant step for selected applications feat(oauth): Allow to skip grant step for selected applications Dec 5, 2024
@come-nc come-nc force-pushed the feat/allow-oauth-grant-bypass branch from 05a5de3 to af256b9 Compare December 9, 2024 15:55
nickvergessen
nickvergessen reviewed Dec 9, 2024
View reviewed changes
apps/oauth2/lib/Controller/LoginRedirectorController.php
]
);
if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'autoGrantApplications', []))) {
/* See ClientFlowLoginController::showAuthPickerPage */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* See ClientFlowLoginController::showAuthPickerPage */
/** @see ClientFlowLoginController::showAuthPickerPage */

To make it clickable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

@come-nc come-nc

Labels
2. developing Work in progress
Projects
📁 Files team
Status: 🏗️ In progress
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

Add an option to set oAuth2 applications as trusted, for own external apps which uses oAuth2 sessions
2 participants
@come-nc @nickvergessen