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

Authentication based in token (OAuth2) #1724

Closed
31 tasks done
jesmrec opened this issue Jun 29, 2016 · 27 comments
Closed
31 tasks done

Authentication based in token (OAuth2) #1724

jesmrec opened this issue Jun 29, 2016 · 27 comments

Comments

@jesmrec
Copy link
Collaborator

jesmrec commented Jun 29, 2016

#Login will be done via OAuth2 protocol when server version supports it.

AC:

  • Setup params
    • Move oauth2_configuration.xml params to setup.xml
    • Move grant_type and response_type from setup.xml to code as variables and delete unneeded param oauth2_scope
    • Delete oauth2_redirect_scheme param
  • Including OAuth2 authentication method
    • Extend detection of authentication method to consider servers accepting more than one method at the same time (more than one challenge header can be received in responses).
    • Compare authentication methods supported by the server and the app, and choose one of them to start with.
    • Embed log-in process into WebView instead of trusting in external web browser
    • Get OAuth2 acess token from captured redirection
    • Silently use refresh token to get an access token on expiration instead of redirecting to log-in view.
    • Redirect to log-in view / notification on top bar bar if refresh token doesn't get a new access token.
    • Grant that when log-in view is opened with a given account, the only possible authentication method is the same previously used for that account.
    • Show an error if the user enters credentials of other user when updating an OAuth2 account
    • Allow user to select authentication method (OAuth2 or Basic Auth) - NICE2HAVE [WON'T FIX]
  • UI/UX
    • Make some modifications in the view either portrait or landscape mode.
    • Show only url field in the first app launch and adapt user and password fields depending on the authentication method.
    • [ ] Show always the embedded refresh button so that the user knows what to do next after having written down the url and connect button is disabled? Already implemented on iOS. NICE2HAVE
    • Grant that 'enter' key in soft keyboard starts server check after URL is typed
    • Connect button doesn't look like disabled, use other color or show a toast when the user clicks on it.
    • Delete progress message about OAuth2 log-in when web dialog is cancelled

Forget the following description and comments about it. Jump after #1724 (comment)

Server version 9.1 will include this new feature that will make mobile apps adapt its login view to this new authentication mode.

The user generates a token in the web admin console (personal section). That token identifies unequivocally the account, so using only the token the user can login in his/her account. He could use the usual method user/password as well.

To do:

Actually, if the token is input in the password field the authentication works, but as username is displayed the string that the user input in the correspondent field, whatever he/she inputs. With the same token we can add so many instances of the same accounts as we wish.
Only one instance of each account, with the username/displayname of the user should be displayed.

Doubts:
- How to take the authentication method to be used: from a branding variable (as, for example, saml), or asking the server for that.
- Login view: what to do with username field. If the user input a token in password field, the username field is irrelevant, but must remain due to accounts in other servers that do not require tokens.
- Edit credentials: for a new generated token? tokens are overrided, a new one invalidates the previous in spite of the accounts added with those tokens keeps on being valid. Changing the account password invalidate all the generated tokens.


BUGS & IMPROVEMENTS

@davivel
Copy link
Contributor

davivel commented Jul 5, 2016

My POV:

  • Our target should be OC server 9.2, that includes a mechanism to query what are the valid authentication methods for a server; see Client auth detection core#24995
  • The option to use a generated token as a password in mobile instead of a real password should be considered right now an experimental feature; as such, any inconsistency derived from it should be ignored, unless a security threat is involved.

@davivel
Copy link
Contributor

davivel commented Jul 5, 2016

Calling in @nasli , @rperezb and @owncloud/android-developers

@davivel davivel removed the 1 - To do label Jul 14, 2016
@davivel
Copy link
Contributor

davivel commented Aug 4, 2016

Taking as a reference owncloud/client#4798.

The general idea would be:

  • The user enters the server URL in the log-in activity
  • After URL check is passed, the app ask the server to generate an authentication token
  • The app opens a WebView in a dialog, passing the token as a GET parameter
  • The user authenticates the token by signing in in the ownCloud web application using the configured server auth (user/pass, shibboleth, etc) and then confirm that he wants the client to have access
  • The app, meanwhile, monitors the URLs loaded in the WebView to detect when the access is granted

The last step specially will require coordination with server side.

@davivel davivel added this to the backlog milestone Aug 4, 2016
@davivel davivel modified the milestones: 2.3.0, backlog Aug 12, 2016
@davivel davivel modified the milestones: 2.2.0, 2.3.0 Oct 25, 2016
@davivel davivel modified the milestones: backlog, 2.2.0 Nov 23, 2016
@davivel
Copy link
Contributor

davivel commented Nov 23, 2016

Needs redefinition of scope.

@davivel davivel changed the title Authentication based in token Authentication based in token (OAuth2) Nov 30, 2016
@davivel davivel modified the milestones: 2.3.0, backlog Nov 30, 2016
@davivel davivel modified the milestones: 2.4.0, 2.3.0 Mar 9, 2017
@butonic
Copy link
Member

butonic commented Mar 22, 2017

https://github.com/owncloud/oauth2 is avalable in 9.1, see owncloud/core#26742

@davivel
Copy link
Contributor

davivel commented Mar 29, 2017

Tested. POC mainly works, though we need to expand it to make it ready-to-production, and there are some point to improve also in backend (see https://github.com/owncloud/oauth2/issues) .

Will update first comment to track tasks for the Android client.

@davivel
Copy link
Contributor

davivel commented Apr 11, 2017

Sorry, but we need to drop issues from milestone 2.4.0 to get it out next to OC 10 server.

This time we'll move topics to backlog instead of directly to next release, so that we can schedule 2.5.0 properly.

@davivel davivel modified the milestones: backlog, 2.4.0 Apr 11, 2017
@davivel davivel modified the milestones: 2.5.0, backlog May 26, 2017
@davivel davivel removed the delayed label May 31, 2017
@davigonz
Copy link
Contributor

[IMPROVEMENT] [UI] Password field in OAuth webview is covered by the keyboard

Steps

  1. Open the app in a tablet in landscape mode
  2. Introduce server and press connect button
  3. A webview asking for credentials appears
  4. Press username field, the keyboard appears, introduce the data
  5. There's no way to introduce the password easily because the field is covered by the keyboard

Current behaviour

Password field is covered by the keyboard.

oauth_tablet

Expected behaviour

Webview is resized properly, staying completely above the keyboard or showing a clear scrollbar.

Nexus 10 v5.0.2

@davigonz
Copy link
Contributor

[BUG] Basic login is shown in OAuth webview when changing auth endpoint to an incorrect one

Steps

  1. Go to setup.xml file and change oauth2_url_endpoint_auth to an incorrect one such as index.php/apps/oauth2/authoriinstead of index.php/apps/oauth2/authorize
  2. Open the app, write the server and press the connect button.

Current behaviour

Step 1: Enter credentials Step 2: Login successful
screenshot_2017-08-21-10-04-10 screenshot_2017-08-21-10-14-24

Expected behaviour

Show an error about the incorrect auth endpoint

Devices:

  • Samsung Galaxy Note 4 v6
  • Samsung Galaxy Tab S v6

@davigonz
Copy link
Contributor

[BUG] OAuth2 accounts are missing the display_name on the account construct

Device: Samsung Galaxy S7 edge (Android v7.0) / Samsung Galaxy Tab S2 (Android v5.0.2)

This screenshot compares a Basic Auth. account (the second one) to one set with OAuth:

I'm wondering if this can have collaterals later on (i.e. on token revoked, maybe the app will cross the new display_name with an empty string to determine if the user logged in the webview is the same that did before) - Haven't verified this scenario yet. (TC47)

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 23, 2017

[Typo]

Type correct URL of a OAuth2 server and tap on Connect . While the webview is loading the following text appears:

Connecting to oAuth2 server...

The correct way is:

Connecting to OAuth2 server...

(O in capitals)

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 23, 2017

[BUG] Deletion of the client in WebUI

Steps:

  1. In WebUI, delete the Android OAuth2 authorized client
  2. In app, try to login. Credentials input

Current behaviour

Error is shown in webview (this is correct). Tapping on Back, the webview shows the file list of the user.

Expected behaviour

Webview is closed and login view is focused again. The user can input other URL.

Tested with Samsung Galaxy S7, Samsung Galaxy Tab S2

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 23, 2017

[BUG] OAuth2 disabled

Steps:

  1. Login in OAuth2 account
  2. In server, disable OAuth2 app (sudo -u www-data php /opt/owncloud/occ app:disable oauth2)

Current behaviour

User is redirected to login view. Credentials are input, but user is redirected again in a loop to login view

Expected behaviour

Account becomes "basic auth" after input credentials. (maybe other solutions fit as well)

Tested with Samsung Galaxy S7, Samsung Galaxy Tab S2

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 23, 2017

[BUG] Video streaming

Video streaming does not work in OAuth2 servers. Error is displayed: This video cannot be reproduced and starts to download.

@jesmrec jesmrec reopened this Aug 24, 2017
@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 24, 2017

[BUG] Basic upgrade

  1. Install in device oc-android-2.4.0 attaching a basic auth account
  2. Upgrade to current branch

Current behaviour:

screen shot 2017-08-24 at 08 45 04

Expected behaviour

app correctly upgraded

Tested with Samsung Galaxy S7 v7, Samsung Galaxy Tab S2 v5

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 24, 2017

[BUG]** "Inverse" upgrade

1. Login in a OAuth2 account
2. Disable OAuth2

Current behaviour

Redirected to login view. Input credentials and redirected again to login view in a loop.

Expected behaviour

Redirected to login view, input credentials and app works properly

Tested with Samsung Galaxy S7 v7

Duplicates #1724 (comment)

@davivel
Copy link
Contributor

davivel commented Aug 25, 2017

@jesmrec , about the problem with basic upgrade: #1724 (comment)

I accidentally changed the appId for debug builds, so you will not be able to upgrade the app from 2.4.0-debug to 2.4.0+OAuth-debug in no device.

To test upgrades you will need to generate release APKs for both the previous version and the current one.

Sorry for the inconvenience.

@davivel
Copy link
Contributor

davivel commented Aug 25, 2017

@davigonz, @jesmrec , about the problem of wrong path to authorization endpoint: #1724 (comment) .

There is nothing we can do to detect the error. We can't monitor all the information in the webview, and there is not way to check that the URL initially loaded really corresponds to an OAuth authorization endpoint.

In any case, this is not something we should worry about. The only way to get a wrong authorization endpoint is with a mistake in build time. Any custom client that needs a custom endpoint should be tested after the build and the error should be detected before releasing it.

IMHO, there is nothing to do here.

@davivel
Copy link
Contributor

davivel commented Aug 28, 2017

Relaunch of log-in view after clicking BACK has nothing to do with OAuth2 -> out of scope.

Softkeyboard hidding input fields in log-in view prevented with auto-resize of dialog on keyboard events.

No more bugs pending of development.

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 29, 2017

[BUG] Crash after deletion of the client in WebUI

Steps:

  1. Add one OAuth2 account. Only one.
  2. In WebUI, delete the Android OAuth2 authorized client
  3. In app, try to login. Credentials input, no authorization granted and redirect to login view.

Current behaviour

App crashes in login view.

Expected behaviour

No crash

Stack trace:

08-29 10:48:20.646 29103-29103/com.owncloud.android.debug E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.owncloud.android.debug, PID: 29103
java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String[] java.lang.String.split(java.lang.String)' on a null object reference
at com.owncloud.android.lib.common.authentication.oauth.OAuth2QueryParser.parse(OAuth2QueryParser.java:48)
at com.owncloud.android.authentication.AuthenticatorActivity.getOAuth2AccessTokenFromCapturedRedirection(AuthenticatorActivity.java:726)
at com.owncloud.android.authentication.AuthenticatorActivity.onGetCapturedUriFromOAuth2Redirection(AuthenticatorActivity.java:1811)
at com.owncloud.android.authentication.OAuthWebViewClient$1.run(OAuthWebViewClient.java:81)
at android.os.Handler.handleCallback(Handler.java:751)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:154)
at android.app.ActivityThread.main(ActivityThread.java:6692)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1468)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1358)

Tested with Samsung Galaxy S7, Samsung Galaxy Tab S2

@davivel
Copy link
Contributor

davivel commented Aug 29, 2017

Crash fixed.

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 29, 2017

related with #1724 (comment)

apk is generated with the following name:

android_null-release.apk

i guess it is not matter of OAuth2, but you said you change the id, and not sure it is affected.

@davivel

@davivel
Copy link
Contributor

davivel commented Aug 30, 2017

@jesmrec , that's a bug in the naming of the APK after the update of the package name, but has no impact on the installation.

@jesmrec
Copy link
Collaborator Author

jesmrec commented Aug 31, 2017

Approved. Great job team!!

@davivel
Copy link
Contributor

davivel commented Aug 31, 2017

Created new PRs with bug fixing:

Library: owncloud/android-library#174
App: #2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants