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

[BUG] Migration OAuth2 - OIDC #824

Open
jesmrec opened this issue Oct 21, 2020 · 16 comments
Open

[BUG] Migration OAuth2 - OIDC #824

jesmrec opened this issue Oct 21, 2020 · 16 comments
Labels
Type:bug Something isn't working
Milestone

Comments

@jesmrec
Copy link
Contributor

jesmrec commented Oct 21, 2020

Problem comes from owncloud/openidconnect#66 (comment), in the scope of the OAuth2 - OIDC migration. These are the steps (let me know if you need an environment):

  1. Connect the app to an OAuth2 server (everything goes fine)
  2. Server changes the authentication method from OAuth2 to OIDC
  3. OAuth2 is no longer available, so next request returns 401
  4. OAuth2 token endpoint is called to get authorization, but this is not longer available either. It returns 302, pointing to the same server's URL + \login
  5. Kopano is available and credentials asked

After that, list of files is shown in the browser, not in the app. Callback is lost somewhere.

With both auth methods enabled at the time, editing the account gives the same result (files in browser)

Installing from scratch, everything is correct.

I can't assure 100% the problem is in the app, so, feel free to close if it is not.

@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 23, 2020

I have taken some logs with the steps above:

ownCloud_23_Oct_2020_at_14_34_10.log.txt

i hope it is useful

@felix-schwarz
Copy link
Contributor

Thanks for the log!

Here's what I found:

  1. When receiving a 401 that can't be resolved (f.ex. via a token refresh), the app will:
  • check for the available authentication methods
  • check if the previously used authentication method is still available
    • if it is still available, go through re-authentication using the same authentication method
    • if it is no longer available, go through re-authentication using the best authentication method available
  1. OAuth2 is detected by checking for Bearer in the Www-Authenticate header of the response for a PROPFIND on /remote.php/dav/files. However, OIDC will also add this. So, when receiving this header at 2020-10-23 14:35:27.696:
Www-Authenticate: Bearer realm="ownCloud", Basic realm="ownCloud", charset="UTF-8"

… the SDK will detect this as both OAuth2 and (since .well-known/openid-configuration also returned valid data) OIDC being available - when in fact only OIDC is available.

  1. Since the app believes that OAuth2 is still available, it starts a web session for OAuth2 at /index.php/apps/oauth2/authorize to resolve the error. ownCloud, however, likely redirects that to /login, so that you end up in the normal web UI login flow - and then in the web app.

Possible solution

With Bearer in the Www-Authenticate header no longer uniquely pointing to OAuth2 support, but also possibly indicating OIDC support or OAuth2 and OIDC support instead, a solution could be to take into account the response by the OAuth2 token endpoints: if they return a redirect, assume OAuth2 is not installed and do not report it as available.

@felix-schwarz
Copy link
Contributor

Ok, so I implemented the solution suggested above locally, with 30x and 404 responses resulting in OAuth2 being unavailable.

Testing with ocis-latest.owncloud.com, however, I get a 401 response for https://ocis-latest.owncloud.com/index.php/apps/oauth2/api/v1/token from the server, not a 302 or 404. So OAuth2 is considered available for that instance even with the suggested solution implemented.

Does the ocis-lastest instance also support OAuth2 - or just OIDC? (cc @DeepDiver1975)

@michaelstingl
Copy link
Contributor

@felix-schwarz I think ocis-latest.owncloud.com is not in good shape. Check ocis.owncloud.works, but they also had some hiccups the last few days. oCIS is OIDC-only, no OAuth 2.0. (I think Basic Auth is still working)

@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 26, 2020

Thanks for checking. Where (branch) can i check your solution @felix-schwarz?

@felix-schwarz
Copy link
Contributor

@michaelstingl I get a 401 response with ocis.owncloud.works as well:

# REQUEST ---------------------------------------------------------
URL:         https://ocis.owncloud.works/index.php/apps/oauth2/api/v1/token
Error:       -
Req Signals: (null)
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
GET /index.php/apps/oauth2/api/v1/token HTTP/1.1
Host: ocis.owncloud.works
[Redirect Policy: forbidden]
X-Request-ID: E88B1964-4658-4805-87E5-38B10DCEE565
User-Agent: ownCloudApp/11.4.2 (App/173; iOS/14.1; iPhone)

Response:

# RESPONSE --------------------------------------------------------
Method:      GET
URL:         https://ocis.owncloud.works/index.php/apps/oauth2/api/v1/token
Request-ID:  E88B1964-4658-4805-87E5-38B10DCEE565
Error:       -
Req Signals: (null)
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
401 UNAUTHORIZED
Date: Tue, 27 Oct 2020 00:07:09 GMT
Www-Authenticate: Basic realm="ocis.owncloud.works", Bearer realm="ocis.owncloud.works"
Content-Length: 0
-----------------------------------------------------------------

So it looks like detecting a lack of support for OAuth2 can only be determined for OC10 instances right now, but not OCIS, where the legacy token endpoint for OAuth2 returns a 401 response even if OAuth2 is not available.

@jesmrec I actually wanted to commit it to develop, but it looks like it slipped into feature/settings-metadata instead. To use it

  • checkout the app's milestone/11.4.2
  • in ios-sdk checkout the feature/settings-metadata branch

@felix-schwarz
Copy link
Contributor

Two additional thoughts here:

  • if it turns out that there is no way to detect missing OAuth2 support when OIDC support is active, another way to trigger a migration from OAuth2 to OIDC should be to limit the available authentication methods via MDM, so that OAuth2 is no longer seen as an option by the app.
  • another possible solution could be to automatically consider OAuth2 to be unavailable if OIDC support was detected.

@jesmrec jesmrec added the Type:bug Something isn't working label Oct 28, 2020
@jesmrec jesmrec added this to the 11.4.2-Current milestone Oct 28, 2020
@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 28, 2020

QA checks, in different related scenarios:

  • OAuth2 -> OIDC (hard switch: disable OAuth2 and enable OIDC at the time): detected new auth method (OIDC), Sign in and correct authentication and redirection to the app where the files are listed ✅

  • OAuth2 -> OIDC (smooth switch: login with OAuth2, then OIDC is enabled). ❌ Files are shown in the browser. These are the steps to reproduce:

  1. Add an OAuth2 account
  2. Enable OIDC in the server, so both OIDC and OAuth2 are enabled at the time
  3. Go to list of bookmarks, and edit the current one (OIDC is correctly detected)
  4. Enter credentials

Current: not redirected to the app. @felix-schwarz is this scenario feasible to fix?

@felix-schwarz
Copy link
Contributor

felix-schwarz commented Oct 29, 2020

@jesmrec Regarding Basic -> OIDC:

In normal usage, the app won't notice or scan for OIDC unless Basic Auth fails with a 401 response. And it would stick to Basic Auth unless the Www-Authenticate header would no longer include Basic.

Inside bookmark editing, right now the UI represents the status quo of the bookmark and allows editing of its parameters. If you edit the URL (f.ex. remove, then re-add a trailing /), however, it will again scan for available authentication methods and should offer the best available auth method then.

What could be done: add an entry with which it's possible to check for available authentication methods and either
a) pick the most preferred one automatically
b) present the user with an alert from which they can choose which auth method they want (sorted in order of preference)


I wouldn't consider any of this to be feasible for migrating whole instances, however.

Likewise, disabling authentication methods on the server to force the client to switch authentication methods could lock out client software that only supports Basic Auth and/or OAuth2.

The best solution IMO would be to provide information through the capabilities on migration paths for authentication methods, i.e.

"auth-migrations" : {
    "basic-auth" : [ "oidc", "oauth2" ],
    "oauth2" : [ "oidc" ]
}

This would indicate to clients that:

  • if they use basic-auth, they should migrate users to oidc or oauth2 (with oidc being preferred and oauth2 provided as a fallback for clients not supporting oidc)
  • if they use oauth2, they should migrate users to oidc if the client supports it

@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 29, 2020

Inside bookmark editing, right now the UI represents the status quo of the bookmark and allows editing of its parameters. If you edit the URL (f.ex. remove, then re-add a trailing /), however, it will again scan for available authentication methods and should offer the best available auth method then.

What could be done: add an entry with which it's possible to check for available authentication methods and either
a) pick the most preferred one automatically
b) present the user with an alert from which they can choose which auth method they want (sorted in order of preference)

Does this affect the fact that the list of files is shown in the browser? the point is, that the editing action moves correctly from OAuth2 to OIDC in the URL/auth method recognition, but at the end there is no callback to the app.

Here it is the performance. Initial account is OAuth2, and when editing, OIDC is already available:

Oct-29-2020 14-55-57

@felix-schwarz
Copy link
Contributor

@jesmrec Likely a redirect happening to the regular login page here, so that after Authorization, you get forwarded to the web UI rather than a token returned to the app.

Did you take a log for that session by any chance? Hard to definitely tell what's happening under the hood without.

@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 30, 2020

Here you have logs of that scenario

ownCloud_30_Oct_2020_at_08_58_19.log.txt

If you need more info, please ping me.

@felix-schwarz
Copy link
Contributor

@jesmrec That log doesn't use the SDK version with the improvements. To use that, in the ios-app folder in Terminal, type:

cd ios-sdk
git pull
git checkout feature/settings-metadata

After that, if you compile and run the app, it includes the changes in auth detection.

If the changes are relevant to the 11.4.2 release, I can quickly backport them to the develop branch for usage in that release.

@jesmrec
Copy link
Contributor Author

jesmrec commented Oct 30, 2020

will recheck, probably something went wrong through the submodule checkout.

@felix-schwarz
Copy link
Contributor

felix-schwarz commented Nov 2, 2020

@jesmrec FWIW I now back ported the changes from the feature/settings-metadata branch to the develop branch.

Additionally, I added enforcement for the connection.connection-allowed-authentication-methods MDM setting for existing bookmarks (also develop / milestone/11.4.2 branches):

If you set connection.connection-allowed-authentication-methods to [com.owncloud.openid-connect] (f.ex. via env vars):

  • the user will be informed that the existing authentication method is no longer allowed
  • no requests are sent to the server and the core switches to "offline" mode

Bildschirmfoto 2020-11-02 um 13 04 44

@jesmrec
Copy link
Contributor Author

jesmrec commented Nov 3, 2020

Thanks for the new stuff. I got new logs of the problem described here

App commit: 2aca714
SDK commit: 4854f1c (pointed from app, backported from feature/settings-metadata. In any case, i've tested with such branch and same result happened)

ownCloud_3_Nov_2020_at_12_31_04.log.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants