lelylan / simple-oauth2

A simple Node.js client library for Oauth2
http://lelylan.github.com/simple-oauth2/
1.6k stars 292 forks source link

Support query params in tokenPath & AuthorizePath #335

Closed tanjacky closed 4 years ago

tanjacky commented 4 years ago

Context

What are you trying to achieve or the steps to reproduce?

Trying to integrate with Microsoft Dynamics 365 Business Central. Their documentation suggests we use:

    auth: {
      tokenHost: 'https://login.windows.net/',
      tokenPath: `${process.env.TENANT_DOMAIN}/oauth2/token?resource=https://api.businesscentral.dynamics.com`,
      authorizeHost: 'https://login.windows.net/',
      authorizePath: `${process.env.TENANT_DOMAIN}/oauth2/authorize?resource=https://api.businesscentral.dynamics.com`,
    }

What was the result you got?

The authorization URL returned looks like:

https://login.windows.net/<retracted>/oauth2/authorize?resource=https://api.businesscentral.dynamics.com?response_type=code&client_id=<retracted>&redirect_uri=<retracted>

Notice there are two ? in the URL

What result did you expect?

I expect the authorization URL to be:

https://login.windows.net/<retracted>/oauth2/authorize?resource=https://api.businesscentral.dynamics.com&response_type=code&client_id=<retracted>&redirect_uri=<retracted>
jonathansamines commented 4 years ago

@tanjacky Please take a look at #324. I have a solution to the problem in #332, but will require a major version bump in it's current form. Query params on the token path should work, please confirm if they don't.

tanjacky commented 4 years ago

@jonathansamines Thanks for that, did not realize an issue was already created.

Query params on the token path should work, please confirm if they don't.

Yes, it does work. There was a bug in my code which led me to believe it did not. You can close this and I will keep an eye out for v5. Thanks!