fusionauth-typescript-client icon indicating copy to clipboard operation
fusionauth-typescript-client copied to clipboard

exchangeRefreshTokenForAccessToken breaks compatibility with node-client

Open ulybu opened this issue 5 years ago • 6 comments

So I don't know if the current behavior is good or not, but I know it differs from the node-client and breaks compatibility.

I use to call the following without error with the node-client

await fusionAuthClient.exchangeRefreshTokenForAccessToken(
  refreshToken, clientID, clientSecret
)

Now the same code gives me the following error:

ClientResponse {
  statusCode: 400,
  exception: {
    error: 'invalid_scope',
    error_description: 'The scope provided is different than the scope used to generate the refresh token.'
  }
}

To fix it, I need to modify my code and add the scope parameter:

await fusionAuthClient.exchangeRefreshTokenForAccessToken(
  refreshToken, clientID, clientSecret, 'offline_access'
)

I'm raising that as an issue because the readme in node-client implies that there will be compatibility: https://github.com/FusionAuth/fusionauth-node-client#deprecation-warning

You can just change your dependency and should have no problems using it with node as the package is still javascript but with additional type information. If you run into problems then let us know over there.

ulybu avatar Jun 11 '20 11:06 ulybu

Hiya!

Thanks for letting us know.

What version of the node client were you running previously? What version of the typescript client are you moving to?

mooreds avatar Jun 11 '20 17:06 mooreds

Hi,

"@fusionauth/node-client": "1.16.0" => "@fusionauth/typescript-client": "1.17.0"

@mooreds edited, copy/pasted twice the same lib :top:

ulybu avatar Jun 12 '20 11:06 ulybu

Hi,

I think it could come from the fact that in the node-client lib the body is explicitly serialized with ~JSON.stringify~ queryString.stringify, which will effectively remove any undefined prop, like for instance scope if not provided to exchangeRefreshTokenForAccessToken

https://github.com/FusionAuth/fusionauth-node-client/blob/0a5c326adc097c85e3572244f70f25088e3de061/lib/RESTClient.js#L74

setFormBody: function(body) {
    this.body = queryString.stringify(body);
    this.header('Content-Type', 'application/x-www-form-urlencoded');
    this.header('Content-Length', Buffer.byteLength(this.body));
    return this;
},

Whereas in typescript-client (this repo) the missing ~JSON~ serialization means we're going to keep that scope key explicitly set to undefined.

https://github.com/FusionAuth/fusionauth-typescript-client/blob/0c0c36b8ddedb1b472a17e82dedba4e942658a0e/src/DefaultRESTClient.ts#L85

Edit: JSON.stringify => queryString.stringify. Issue is still the same, queryString also removed any undefined props

ulybu avatar Jun 22 '20 10:06 ulybu

That sure looks like a incompatibility to me. I'll take a look. We also welcome pull requests if you want to just patch DefaultRESTClient.ts.

mooreds avatar Jun 23 '20 17:06 mooreds

We also welcome pull requests if you want to just patch DefaultRESTClient.ts

Ugh, that seems too risky to me because I don't know the reason why the dropped the queryString.stringify in the first place. I can't just assume that's an oversight, right? I might very well be breaking something for someone else by adding queryString.stringify

I could submit a PR and let you guys deal with that risk assessment?

ulybu avatar Jun 26 '20 12:06 ulybu

No worries, I ended up submitting a PR (#32), so no effort needed on your part. Thanks again for reporting this.

mooreds avatar Jun 26 '20 16:06 mooreds

Closing this because #32 is merged, as well as the fact that the node client has been deprecated for 3+ years.

mooreds avatar Jun 27 '24 21:06 mooreds