-
Notifications
You must be signed in to change notification settings - Fork 668
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
oAuth2: Show error message or logout when there was a problem with the server token interaction #7212
Comments
@DeepDiver1975 @ogoffart What is the recommended behavior for "disabled user" in OAuth 2.0? Any hints in the RFC? /cc @felix-schwarz @davigonz |
The clients should show an authentication failed error in my opinion, I've performed the same steps in the Android app and behaves that way. |
It appears from what i can see from the client log, that the token refreshing still work, but the new auth token is not working, letting the client believe that it has expire and need to ask for a new one. Also the message from the server log
Looks like the bug where the server is stripping some header. |
@michaelstingl Just checked and I couldn't find any hints at the recommended behaviour in the RFC. The new iOS SDK will return an "authorization failed" error upon trying to connect, which will prompt the new app to bring up an alert
Ignore will continue in Offline mode. Edit will bring up the editing panel for the connection where the user gets a chance to re-create their authentication data (similar to when originally setting up the connection). |
So the plan is that the server will return a 400 http code with an "unauthorized_client" error if the user is disabled. This is based on https://tools.ietf.org/html/rfc6749#section-4.1.2.1 and https://tools.ietf.org/html/rfc6749#section-5.2 I'll add a "error_description: disabled user" mainly for debugging |
Does it behave better with owncloud/oauth2#209 ? |
I think so. With the fix in the oAuth app, the desktop waits for the user to be authenticated via browser, so there shouldn't be any problem. Client side, if it can't get a token for any reason I think the client should wait for user action. Maybe showing an error saying "I can't get a valid token, try reauthenticating" or something like that might be a good idea. |
Well, the problem is that the client gets a token, but it still gets a forbidden error when it tries to use it to access to webdav. |
So in summary, what remains to be done in the client? |
I think the client can't know the real status, so the waiting state is probably the best we can do. @jvillafanez recommendations? |
Somehow, the server would need to connect to the localhost (redirect_uri) notifying the error. I do not think there is a oauth2 workflow for that, so that's not really possible without extending the protocol. |
Most of the flow is handled by the browser... so yes, we'd need to send something from the server to the redirect_uri... sounds too complex with very little reward. An alternative in the client could be to set a timeout of 1 minute, so if the user hasn't authorized the app at that point the client could assume that the user hasn't authorized: it could change the message and ignore any request to the redirect_uri until the user clicks again in the link (requesting or refreshing a new token). |
@HanaGemela @ogoffart What do you think? Indicate there's an error? |
@michaelstingl I'm kind of OK with current behaviour. It is not ideal but it tells you to go to the browser and the browser shows a proper error message |
Expected behaviour
Desktop client syncs normally
Actual behaviour
Desktop client tries indefinitely to obtain an oAuth token
Steps to reproduce
Server configuration
Operating system: ubuntu 16.04 (docker)
Web server: apache 2.4.29
Database: mysql
PHP version: 7.1
ownCloud version: 10.1.0
Storage backend (external storage): no external storage, local primary storage
Client configuration
Client version: 2.5.4 (build 515)
Operating system: ubuntu 18.04
OS language:
Qt version used by client package (Linux only, see also Settings dialog):
Client package (From ownCloud or distro) (Linux only): owncloud
Installation path of client:
Logs
Please use Gist (https://gist.github.com/) or a similar code paster for longer
logs.
Template for output < 10 lines
owncloud --logwindow
orowncloud --logfile log.txt
(On Windows using
cmd.exe
, you might need to firstcd
into the ownCloud directory)(See also http://doc.owncloud.org/desktop/2.2/troubleshooting.html#client-logfile )
Web server error log:
Server logfile: ownCloud log (data/owncloud.log):
The text was updated successfully, but these errors were encountered: