ory / hydra

The most scalable and customizable OpenID Certified™ OpenID Connect and OAuth Provider on the market. Become an OpenID Connect and OAuth2 Provider over night. Broad support for related RFCs. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.56k stars 1.5k forks source link

Default token lifespan config does not work #3360

Open nbrns opened 1 year ago

nbrns commented 1 year ago

Preflight checklist

Describe the bug

First of all: I use Hydra as a very flexible OAuth2 provider, which works great. Thanks for providing this great piece of software!

However, it appears that in v2.0.1 the default ttl settings for tokens/codes do not apply. When using the ttl.* settings in the Hydra config, newly created clients do not inherit these values. Instead, the lifespans are set to null.

I'm using the Java SDK on the backend side, which requires the lifespans in the authentication request to be set. The code fails when trying to get the login request due to failing validation.

I could verify this behaviour in a docker-compose setup as well as a k8s deployment via helm. For the reproduction I will use the 5 minute tutorial.

Reproducing the bug

Use docker-compose quickstart version 2.0.1. I'll stick mostly to the 5 minute tutorial.

  1. Extend quickstart/5-min/hydra.yml with the following settings (taken from hydra.yml reference in github repo):
    
    ...

ttl: login_consent_request: 1h access_token: 1h refresh_token: 720h id_token: 1h auth_code: 10m

...

2. Spin up Hydra

docker-compose up -f quickstart.yml hydra

3. Create client

code_client=$(docker-compose -f quickstart.yml exec hydra \ hydra create client \ --endpoint http://127.0.0.1:4445 \ --grant-type authorization_code,refresh_token \ --response-type code,id_token \ --format json \ --scope openid --scope offline \ --redirect-uri http://127.0.0.1:5555/callback)

code_client_id=$(echo $code_client | jq -r '.client_id') code_client_secret=$(echo $code_client | jq -r '.client_secret')

4. Inspect client

curl localhost:4445/admin/clients/$code_client_id | jq { "client_id": "a0d242da-d04b-4598-a51b-8b39477da5ae", "client_name": "", "redirect_uris": [ "http://127.0.0.1:5555/callback" ], "grant_types": [ "authorization_code", "refresh_token" ], "response_types": [ "code", "id_token" ], "scope": "openid offline", "audience": [], "owner": "", "policy_uri": "", "allowed_cors_origins": [], "tos_uri": "", "client_uri": "", "logo_uri": "", "contacts": [], "client_secret_expires_at": 0, "subject_type": "public", "jwks": {}, "token_endpoint_auth_method": "client_secret_basic", "request_object_signing_alg": "RS256", "userinfo_signed_response_alg": "none", "created_at": "2022-11-11T09:32:11Z", "updated_at": "2022-11-11T09:32:11.136038Z", "metadata": {}, "authorization_code_grant_access_token_lifespan": null, "authorization_code_grant_id_token_lifespan": null, "authorization_code_grant_refresh_token_lifespan": null, "client_credentials_grant_access_token_lifespan": null, "implicit_grant_access_token_lifespan": null, "implicit_grant_id_token_lifespan": null, "jwt_bearer_grant_access_token_lifespan": null, "refresh_token_grant_id_token_lifespan": null, "refresh_token_grant_access_token_lifespan": null, "refresh_token_grant_refresh_token_lifespan": null }


Notice that all token lifespans are null.

### Relevant log output

_No response_

### Relevant configuration

```yml
ttl:
  login_consent_request: 1h
  access_token: 1h
  refresh_token: 720h
  id_token: 1h
  auth_code: 10m

Version

v2.0.1

On which operating system are you observing this issue?

Windows

In which environment are you deploying?

Kubernetes with Helm

Additional Context

No response

aeneasr commented 1 year ago

When the client reports null, it means that the system default is being used. Only if the value is not null, the client overrides the default!

nbrns commented 1 year ago

Thanks for clarification @aeneasr. Then this is most likely a bug in the Java SDK?

When not explicitly overwriting the lifespans of a client, the

.getOAuth2LoginRequest(challenge) .getOAuth2ConsentRequest(challenge)

methods of the OAuth2 Api Client throw the following exception during object validation:

java.lang.IllegalArgumentException: Expected the field `authorization_code_grant_access_token_lifespan` to be a primitive type in the JSON string but got `null`

Original Stack Trace:
        at sh.ory.hydra.model.OAuth2Client.validateJsonObject(OAuth2Client.java:1564) ~[hydra-client-2.0.1.jar:na]
        at sh.ory.hydra.model.OAuth2ConsentRequest.validateJsonObject(OAuth2ConsentRequest.java:575) ~[hydra-client-2.0.1.jar:na]
        at sh.ory.hydra.model.OAuth2ConsentRequest$CustomTypeAdapterFactory$1.read(OAuth2ConsentRequest.java:624) ~[hydra-client-2.0.1.jar:na]
        at sh.ory.hydra.model.OAuth2ConsentRequest$CustomTypeAdapterFactory$1.read(OAuth2ConsentRequest.java:614) ~[hydra-client-2.0.1.jar:na]

See these validation lines: https://github.com/ory/hydra-client-java/blob/master/src/main/java/sh/ory/hydra/model/OAuth2Client.java#L1563

They expect a JsonPrimitive and null seems like no valid primitive.

scthi commented 1 year ago

This also applies to non primitive fields like contacts:

java.lang.IllegalArgumentException: Expected the field `contacts` to be an array in the JSON string but got `null`
    at sh.ory.hydra.model.OAuth2Client.validateJsonObject(OAuth2Client.java:1592)
    at sh.ory.hydra.model.OAuth2Client$CustomTypeAdapterFactory$1.read(OAuth2Client.java:1700)
    at sh.ory.hydra.model.OAuth2Client$CustomTypeAdapterFactory$1.read(OAuth2Client.java:1690)
    at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199)
    at com.google.gson.Gson.fromJson(Gson.java:1058)
    at com.google.gson.Gson.fromJson(Gson.java:1016)
    at com.google.gson.Gson.fromJson(Gson.java:959)
    at sh.ory.hydra.JSON.deserialize(JSON.java:186)
    at sh.ory.hydra.ApiClient.deserialize(ApiClient.java:957)
    at sh.ory.hydra.ApiClient.handleResponse(ApiClient.java:1165)
    at sh.ory.hydra.ApiClient.execute(ApiClient.java:1089)
    at sh.ory.hydra.api.OAuth2Api.createOAuth2ClientWithHttpInfo(OAuth2Api.java:611)
    at sh.ory.hydra.api.OAuth2Api.createOAuth2Client(OAuth2Api.java:590)
ardetrick commented 1 year ago

I was able to work around this issue by setting a value for all fields (I am not recommending these defaults, I set these for an integration test, not for production):

oAuth2Client.authorizationCodeGrantAccessTokenLifespan("1h");
oAuth2Client.authorizationCodeGrantRefreshTokenLifespan("1h");
oAuth2Client.authorizationCodeGrantIdTokenLifespan("1h");
oAuth2Client.clientCredentialsGrantAccessTokenLifespan("1h");
oAuth2Client.contacts(ImmutableList.of());
oAuth2Client.implicitGrantAccessTokenLifespan("1h");
oAuth2Client.implicitGrantIdTokenLifespan("1h");
oAuth2Client.jwtBearerGrantAccessTokenLifespan("1h");
oAuth2Client.refreshTokenGrantAccessTokenLifespan("1h");
oAuth2Client.refreshTokenGrantRefreshTokenLifespan("1h");
oAuth2Client.refreshTokenGrantIdTokenLifespan("1h");

When looking into the code (OAuth2Client.java), we can see here is where the exception is thrown:

      if (jsonObj.get("authorization_code_grant_access_token_lifespan") != null && !jsonObj.get("authorization_code_grant_access_token_lifespan").isJsonPrimitive()) {
        throw new IllegalArgumentException(String.format("Expected the field `authorization_code_grant_access_token_lifespan` to be a primitive type in the JSON string but got `%s`", jsonObj.get("authorization_code_grant_access_token_lifespan").toString()));
      }

If you look at the jsonObj instance, you can see it is an instance of JsonNull and isJsonPrimitive() evaluates to false.

The above were made with version 2.0.2 of the java client jar.

It looks like this issue has been fixed but not released yet.

https://github.com/ory/client-java/blob/a159fb81a618c3fd10ecc9792cb3d7c74e5c9b52/src/main/java/sh/ory/model/OAuth2Client.java#L1565

      if ((jsonObj.get("authorization_code_grant_id_token_lifespan") != null && !jsonObj.get("authorization_code_grant_id_token_lifespan").isJsonNull()) && !jsonObj.get("authorization_code_grant_id_token_lifespan").isJsonPrimitive()) {

^ note the additional check of isJsonNull().

When can we expect a release?