juhanakristian / remix-auth-microsoft

Microsoft authentication strategy for remix-auth
MIT License
37 stars 19 forks source link

fix scope bug introdcued by remix-auth-oauth2 version 1.11 #24

Closed bindermuehle closed 8 months ago

bindermuehle commented 8 months ago

The recent update in remix-auth-oauth2 has introduced a conflict with the library's scope setting functionality. Prior to version 1.11.0, the library was setting the scope in a way that is now being overwritten.

Steps to Reproduce:

Navigate to the remix-auth-oauth2 GitHub repository: Link to Repository Review the changes made in the recent update to the library's scope setting mechanism. Notice how the MicrosoftStrategy class sets this.scope as an array of scopes in the constructor: Link to Code Current Behavior:

In version 1.11.0 of remix-auth-oauth2, the scope is being set based on the variable this.scope, which is an array in the library. However, the array is being concatenated to form a comma-separated list. This approach is not supported by Microsoft, as they require a space-separated list for the scope parameter.

Expected Behavior:

To ensure compatibility with Microsoft's requirements, it is suggested that the this.scope property be modified to a space-separated string. This adjustment will allow for seamless functionality with both version 1.10 and 1.11 of the library.

Code Reference:

The problematic lines of code can be found in the following locations:

MicrosoftStrategy class constructor: Link to Code remix-auth-oauth2 library: Link to Code

Thanks in advance for your consideration

bindermuehle commented 8 months ago

I'd like to squash these commits, but I didn't see the option when I created the PR

juhanakristian commented 8 months ago

@bindermuehle Thanks for the PR