Uncomplete OIDC dynamic client registration request [BUG]

TuxCoder commented 2 years ago

Behaviour

The android app tries to do a OIDC client registration with a custom OIDC provider. This works successful but the registered client has some "optimistic" assumptions.

Expected behaviour

The client does a full defined request, with all arguments.

Steps to reproduce

A bit hard to describe I used:

OCIS v 2.0.0beta5
Ory/Hydra as Oauth2 Provider
Custom identity provider

If you need a more precise description ask, it is my testsystem, can also give access if requested.

Can this problem be reproduced with the official owncloud server? (url: https://demo.owncloud.org, user: test, password: test) NO

Environment data

Android version: Lineageos 18.1

Device model:

Stock or customized system:

ownCloud app version: github - master

ownCloud server version: OCIS 2.0.0beta5

Logs

Web server error log

Insert your webserver log here

ownCloud log (data/owncloud.log)

Insert your ownCloud log here

Sample Patch (but quick and dirty):

--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/oauth/params/ClientRegistrationParams.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/oauth/params/ClientRegistrationParams.kt
@@ -46,6 +46,8 @@ data class ClientRegistrationParams(
             put(PARAM_CLIENT_NAME, clientName)
             put(PARAM_REDIRECT_URIS, JSONArray(redirectUris))
             put(PARAM_TOKEN_ENDPOINT_AUTH_METHOD, tokenEndpointAuthMethod)
+            put(PARAM_SCOPE, "openid offline_access email profile")
+            put("grant_types", JSONArray(arrayOf("authorization_code","refresh_token")))
         }.toString().toRequestBody(CONTENT_TYPE_JSON.toMediaType())

     companion object {
@@ -53,5 +55,6 @@ data class ClientRegistrationParams(
         private const val PARAM_CLIENT_NAME = "client_name"
         private const val PARAM_TOKEN_ENDPOINT_AUTH_METHOD = "token_endpoint_auth_method"
         private const val PARAM_REDIRECT_URIS = "redirect_uris"
+        private const val PARAM_SCOPE = "scope"
     }
 }

This patch addes the arguments:

scope - defines witch sopes should be allowed to request, default not defined in rfc
grant_type - default is only ["authorization_code"] but ist should be IMHO ["authorization_code","refresh_token"]

The current implementation assumes that the default parameter are correct but that is implementation specific. I could apply the patch above to a testbuild on my phone and it looks like it works. The client does registration a new OIDC client with the right paramters. as api reference I used: https://www.ory.sh/docs/hydra/reference/api and the rfc: https://datatracker.ietf.org/doc/html/rfc7591#section-2

I'm not so into Kotline, so I would let to a dev to do the nice implementation

michaelstingl commented 2 years ago

@d7oc maybe this is useful for you?

/cc involving the OIDC experts and the other clients… @DeepDiver1975 @wkloucek @TheOneRing @IljaN @felix-schwarz

DeepDiver1975 commented 2 years ago

Generally speaking I'd prefer to keep the params to a minimum. As per spec both params can be omitted.

I fear that this will break interop with other IdPs ....

TuxCoder commented 2 years ago

Hi @DeepDiver1975 I agree to keep it at a min. for that reason I would not define response_types nor token_endpoint_auth_method as the defaults are fine.

but in the rfc7591 is written: grant_types :

.... If omitted, the default behavior is that the client will use only the "authorization_code" Grant Type.

but the app requires ["authorization_code","refresh_token"]

same for scope, there is no default amount of scopes

.... If omitted, an authorization server MAY register a client with a default set of scopes.

but the app requires: "openid offline_access email profile", what is the default set?

breaking inter op: all standard conform implementation should be capable to handle this parameters, if not they are already broken and should be fixed

felix-schwarz commented 2 years ago

The iOS client gets away with not sending any grant_types, which - per the documentation you also quoted - really only means ["authorization_code"].

It can apparently omit the refresh_token grant type because including the offline_access scope already ensures it gets a refresh token:

This scope value requests that an OAuth 2.0 Refresh Token be issued that can be used to obtain an Access Token that grants access to the End-User's UserInfo Endpoint even when the End-User is not present (not logged in). (OpenID Connect Core 1.0 Spec)

The iOS client requests the openid offline_access email profile scopes.

TuxCoder commented 2 years ago

@felix-schwarz

This scope value requests that an OAuth 2.0 Refresh Token be issued that can be used to obtain an Access Token that grants access to the End-User's UserInfo Endpoint even when the End-User is not present (not logged in). (OpenID Connect Core 1.0 Spec)

that is true, but how to use a refresh_token if you can not do a grant_type=refresh_token at a token endpoint, This is probably a bit strange done in the standard, but IMHO you need grant_type=refresh_token to use the refresh_token, so its kind of required but not directly written. see: https://www.rfc-editor.org/rfc/rfc6749#section-6

grant_type=refresh_token is also used in the code: https://github.com/owncloud/android/blob/master/owncloudDomain/src/main/java/com/owncloud/android/domain/authentication/oauth/model/TokenRequest.kt#L41

felix-schwarz commented 2 years ago

@TuxCoder One could argue that offline_access implies that refresh_token requests must work because the OAuth 2.0 Refresh Token to be issued … can be used to obtain an Access Token that grants access – which is only possible with a refresh_token request.

But in the end, the specs IMO leave room for interpretation here, with good arguments for why ["authorization_code","refresh_token"] should be sent - and why refresh_token could also be omitted.

From a practical point of view: are there any OIDC implementations requiring that none, only one - or both be sent?

TuxCoder commented 2 years ago

Hello again,

IMHO it's pretty clear a upper standard can not invalid a lower standard, as OIDC is build on top of OAuth2, OIDC should not invalid Oauth2 standard.

From a practical point of view: are there any OIDC implementations requiring that none, only one - or both be sent? looks like the ocis implementation and keyclock does have the "right" default, even IMHO not standard conform.

I tested my patched client with the keyclock test system[1] and looks to work. also login on [2] works, this should be the owncloud one.

[1] https://ocis.ocis-keycloak.latest.owncloud.works/ [2] https://ocis.ocis-wopi.latest.owncloud.works

OCIS IDP: has no dynamic client registration at list I don't see one int the configuration file: https://ocis.ocis-wopi.latest.owncloud.works/.well-known/openid-configuration

Ory/Hydra: Does it IMHO correct, you need to request the grant_types" : [ "authorization_code", "refresh_token"],

Keyclock: keyclock has not even a setting to restrict refresh_token grant, it does implement the dynamic client registration invalid: Also scopes are different than in the app requested.

curl -D- -o- -XPOST -d'{"client_name":"test cli","redirect_uris":["localhost:8080/callback"],"application_type":"nativ"}' -H"content-
type: application/json" https://keycloak.ocis-keycloak.latest.owncloud.works/auth/realms/oCIS/clients-registrations/openid-connect
HTTP/2 201 
content-type: application/json
date: Sun, 31 Jul 2022 14:31:21 GMT
location: https://keycloak.ocis-keycloak.latest.owncloud.works/auth/realms/oCIS/clients-registrations/openid-connect/9d4ee5ca-36ee-42bd-9934-1c79f3c5eda2
referrer-policy: no-referrer
strict-transport-security: max-age=31536000; includeSubDomains
x-content-type-options: nosniff
x-frame-options: SAMEORIGIN
x-xss-protection: 1; mode=block
content-length: 1393

{
   "backchannel_logout_session_required" : false,
   "client_id" : "9d4ee5ca-36ee-42bd-9934-1c79f3c5eda2",
   "client_id_issued_at" : 1659277881,
   "client_name" : "test cli",
   "client_secret" : "X2N3rqsdk7suSorR8JogVWMYcOKzGFEp",
   "client_secret_expires_at" : 0,
   "frontchannel_logout_session_required" : false,
   "grant_types" : [
      "authorization_code",
      "refresh_token"
   ],
   "redirect_uris" : [
      "localhost:8080/callback"
   ],
   "registration_access_token" : "eyJhbGciOiJIUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJmMTg4OTgzOS1mZGIxLTRjM2EtOThiNi0xMzMwNWYxYjBmNzQifQ.eyJleHAiOjAsImlhdCI6MTY1OTI3Nzg4MSwianRpIjoiMWM2N2ZiMjAtZTkyMC00OWY2LWE3ZTYtZDM2MjYxZDRjYWFhIiwiaXNzIjoiaHR0cHM6Ly9rZXljbG9hay5vY2lzLWtleWNsb2FrLmxhdGVzdC5vd25jbG91ZC53b3Jrcy9hdXRoL3JlYWxtcy9vQ0lTIiwiYXVkIjoiaHR0cHM6Ly9rZXljbG9hay5vY2lzLWtleWNsb2FrLmxhdGVzdC5vd25jbG91ZC53b3Jrcy9hdXRoL3JlYWxtcy9vQ0lTIiwidHlwIjoiUmVnaXN0cmF0aW9uQWNjZXNzVG9rZW4iLCJyZWdpc3RyYXRpb25fYXV0aCI6ImFub255bW91cyJ9.w6HACiPOpNKvDMp8zC3iddD4Jrcpdad7FUlWgqaggWs",
   "registration_client_uri" : "https://keycloak.ocis-keycloak.latest.owncloud.works/auth/realms/oCIS/clients-registrations/openid-connect/9d4ee5ca-36ee-42bd-9934-1c79f3c5eda2",
   "request_uris" : [],
   "require_pushed_authorization_requests" : false,
   "response_types" : [
      "code",
      "none"
   ],
   "scope" : "address phone offline_access microprofile-jwt",
   "subject_type" : "public",
   "tls_client_certificate_bound_access_tokens" : false,
   "token_endpoint_auth_method" : "client_secret_basic"
}

owncloud / android