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

Oauth2 support #2013

Merged
merged 56 commits into from
Aug 24, 2017
Merged

Oauth2 support #2013

merged 56 commits into from
Aug 24, 2017

Conversation

davigonz
Copy link
Contributor

@davigonz davigonz commented Aug 4, 2017

Fixes #1724
Requires owncloud/android-library#173

How Has This Been Tested?

Bugs & Improvements

@davivel
Copy link
Contributor

davivel commented Aug 7, 2017

Did code review and applied changes by myself.

👍

@pablocarmu , @SamuAlfageme , ready to test.

@SamuAlfageme
Copy link

SamuAlfageme commented Aug 10, 2017

[UX] - Clicking back button on new login view relaunches the URL input form:

Device: Nexus 6P (Android v7.1.2)
Related with: #2009

Steps to reproduce:

The initial 'connect' view The basic auth form
img1 login_form

@SamuAlfageme
Copy link

SamuAlfageme commented Aug 10, 2017

[WONTFIX] Clicking on any https:// link on the auth. webview causes a SSL handshake error

Device: Nexus 6P (Android v7.1.2)

Note: this is tightly related to specific proxy settings on the device

Since the webview does not have any inherent settings but the default, performing an SSL handshake (trying to display an https:// external URL) will throw an error. Some possible explanations:

  • Date and Time being sent from the webview are incorrect.
  • The webview is using SSLv3 instead of TLS 1.2 for the HTTP requests. (most likely)

e.g. clicking on the clickable part of the default login form: "ownCloud, webservices under your control" displays:

owncloud.org:443 (sni: owncloud.org): TlsException("SSL handshake error: 
    Error([(\'SSL routines\', \'ssl3_read_bytes\', \'sslv3 alert handshake failure\')],)",)',

... on said webview.

@SamuAlfageme
Copy link

SamuAlfageme commented Aug 11, 2017

[WONTFIX] Switching the scheme on the server URL does not generate a second request

Note: this is tightly related to specific proxy settings on the device

Device: Samsung Galaxy Tab S2 (Android v5.0.2)

When no scheme is specified in the server URL (e.g. demo.owncloud.com) the client appends http:// to the request; if it turns out to be wrong and the server is only accessible via https://, a 'Bad Gateway error' is returned. The client is not able to recover by appending the right scheme in the URL (i.e. the client does not generate a second GET request)

Steps to Reproduce

  1. Write demo.owncloud.com on the server URL input field - curl http://demo.owncloud.com
  2. Click 'Connect' returns 302: Found
  3. After the 'Bad Gateway' error is returned, insert the right scheme: https://demo.owncloud.com

Actual Result

No new GET request is generated and therefore 'Bad Request' error persists:

@SamuAlfageme
Copy link

SamuAlfageme commented Aug 11, 2017

[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)

@davivel davivel self-assigned this Aug 14, 2017
@davivel
Copy link
Contributor

davivel commented Aug 14, 2017

Jumping in to fix bugs,

@davivel
Copy link
Contributor

davivel commented Aug 14, 2017

Missing display name recovered, ready to test.

Hunting next ...

@davivel
Copy link
Contributor

davivel commented Aug 14, 2017

Fixed problem with silent refresh of access token found accidentally. :(

@davivel
Copy link
Contributor

davivel commented Aug 14, 2017

Problem with https:// browsing in WebView is a false positive introduced by mitmproxy.

@davigonz
Copy link
Contributor Author

davigonz commented Aug 18, 2017

[BUG] SAML session is not being expired

Steps

  1. Login with SAML
  2. Change SAML session lifetime in the server
  3. Perform any operation such as upload a file or refresh the folder

Current behaviour

Operation is performed successfully

Expected behaviour

App is redirected to login view, showing an error about the SAML session expiration

Device: Huawei 6P v7.

@davigonz
Copy link
Contributor Author

davigonz commented Aug 18, 2017

[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 Author

davigonz commented Aug 21, 2017

[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 and others added 21 commits August 23, 2017 08:38
@davivel
Copy link
Contributor

davivel commented Aug 23, 2017

This PR will be merge for a new beta release.

Pending test cases and bugs will be tracked in original issue, #1724 .

@davivel
Copy link
Contributor

davivel commented Aug 24, 2017

Merging for beta.

@davivel davivel merged commit 5b334c0 into master Aug 24, 2017
@davivel davivel deleted the oauth2_support branch August 24, 2017 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants