refresh_token is null when validating authentication code response

[justinmakaila]

Hi,
When making a request against a service, the service is responding like this:

      body: {
        access_token: '...',
        token_type: 'bearer',
        expires_in: 600,
        refresh_token: null,
        refresh_token_expires_in: null,
        scope: '...'
      }

I keep getting the error OperationProcessingError: "response" body "refresh_token" property must be a string which seems a bit strange... checking RFC 6749, E in the diagram states "...w/ Optional Refresh Token".

The library checks the refresh token as follows:

L3568

  if (json.refresh_token !== undefined) {
    assertString(json.refresh_token, '"response" body "refresh_token" property', INVALID_RESPONSE, {
      body: json,
    })
  }

I'm thinking that L3568 should be changed to:

if (json.refresh_token !== undefined && json.refresh_token !== null) {

to allow for null values.

Are there any objections to this PR? Or things that I'm missing that may not be considered?

Since it's such a small change, I've made it already locally, happy to file upon approval.

Thanks,
Justin

[panva]

Hi Justin,

when present in a JSON body, refresh token is required to be a string. It being present but having the JSON value null is not expected.

OpenID Connect Core specifically calls this out and further clarifies: Omitted parameters and parameters with no value SHOULD be omitted from the object and not represented by a JSON null value, unless otherwise specified.

[justinmakaila]

Ok, that all makes sense to me. Locally I have the function patched with the null check to get unblocked, and I've reached out to the maintainers of the service to see if they can make the change to omit the fields.

Thanks!

[justinmakaila]

Actually, one more thing to add @panva... It's not specified in the OAuth2.0 spec that the values must be omitted, it just mentions them being optional: https://www.rfc-editor.org/rfc/rfc6749#section-5.1

I skimmed the library, and I understand that the processGenericAccessTokenResponse is used in processAuthorizationCodeOAuth2Response and processAuthorizationCodeOpenIDResponse, however, the maintainers of the service I'm utilizing have mentioned that they are not aiming for OIDC-compliance, but just OAuth2.0.

I've sent them the OIDC Core article, and they're going to make the best decision for their service, but it has me wondering about other services that may only be concerned with OAuth2.0 and not OIDC...

I would think that most services would like to avoid sending unnecessary things over the wire, but I wonder if there should be an internal flag/option that's passed to processGenericAccessTokenResponse to add null checks in the OAuth2 use-case? I wonder if applying the OIDC consideration is too restrictive for that case?

[panva]

If you want an OAuth 2.0 syntax then that's defined in appendix-A.17 of RFC6749 - refresh token is non-empty string, not null.

[justinmakaila]

Got it, thanks for your prompt responses, very helpful.