oauth2 icon indicating copy to clipboard operation
oauth2 copied to clipboard

Improve non-registered client workflows

Open SamuAlfageme opened this issue 8 years ago • 5 comments

Steps to Reproduce:

  1. (e.g.) Use a client with either/both OAuth's id/secret not present on server's DB to try to authenticate.
  2. 'Request not valid' view is displayed:

Clicking on 'Back' button causes:

Current Behavior

The whole files webUI to be displayed. While the client is idle for a redirection that it's never going to happen.

Expected Behavior

  • A. Do not even display the login form for unregistered clients (i.e. https://<server>/index.php/login?redirect_url=%252Findex.php%252Fapps%252Foauth2%252Fauthorize%253Fresponse_type%253Dcode%2526client_id%<non-registered-id>redirect_uri%253Dhttp%253A%252F%252Flocalhost%253A54883 to display an error, not the form)

  • B. Perform a redirection with information useful the clients can listen to:

This becomes really handy on mobile clients (or any client, actually, since it could be used to notify of flow termination) to avoid loading the whole files webUI on the authentication webview. Clients will receive the error message while they listen for protocol redirections and use that message to enhance UX telling what went wrong.

{
    "message_url": "https://<server>/index.php/apps/oauth2/authorization-error",
    "error": "The client does not appear to be registered on the OAuth2 application"
    "user_id": "<username>"
}

cc/ @davivel @nasli

SamuAlfageme avatar Aug 14 '17 11:08 SamuAlfageme

related to #58

DeepDiver1975 avatar Aug 16 '17 15:08 DeepDiver1975

is this a release blocker ? is this causing any kind of harm apart from confusing users ?

PVince81 avatar Aug 31 '17 09:08 PVince81

@PVince81 nope, this one isn't. See https://github.com/owncloud/oauth2/issues/63#issuecomment-319038407 for the ones that I consider to be blockers - mainly https://github.com/owncloud/oauth2/issues/74 (https://github.com/owncloud/core/issues/28860 & https://github.com/owncloud/core/pull/28733 are just pending to be tested)

SamuAlfageme avatar Aug 31 '17 10:08 SamuAlfageme

-> p2. We use p1 for release blockers.

PVince81 avatar Aug 31 '17 10:08 PVince81

Yet one more example on why we need to do this; when the client is not registered and the browser does not hold a valid session - the login form is displayed but missing an equivalent to the "application X is requesting access to your account" info message:

... at this point, we could simply disable the login form since all that valid credentials can do is to display the "Request not valid" message from https://github.com/owncloud/oauth2/issues/77#issue-250006603 - and confusing users.

cc/ @DeepDiver1975

SamuAlfageme avatar Mar 07 '18 08:03 SamuAlfageme