authenticate endpoint still expects scope as a string instead of string[]

[Valerionn]

Specify your setup

Irrelevant

Describe the bug

According to the Documentation and the Typescript typings, OAuth2Server.authenticate expects options.scope, which is of type string[]. However, in authenticate-handler.js, this scope is passedd to parseScope [source], which expects scope to be a string:

    if (typeof requestedScope !== 'string') {
      throw new InvalidScopeError('Invalid parameter: `scope`');
    }

To Reproduce

Try to pass a scope-Array to authenticate

Expected behavior

No error thrown, but instead an error would be thrown if I wouldn't pass an array.

[jankapunkt]

@Valerionn thank you for reporting. From what I see this must be still wrong in the types and docs, since scope must be a string, according to the standard.

// CC @dhensby

[dhensby]

Ah, a good spot.

As I remember, our intention was that this should be an array of strings (my understanding was we wanted the developer's interface to deal with arrays of strings for scopes, but the HTTP interactions are a single string with space delimited scopes (as per the spec)). Implementing that intention as a "bug fix" will be quite disruptive, though...

I see three choices:

1. Update the docs to say it's a string and leave it at that
2. Update the code to accept an array of scopes and release a new major version
3. Update the code to accept either a string or an array and release a minor version (with a view to remove the string support in the future / next major).

[jankapunkt]

This is the related PR: #267

• Scope can be sent from clients either as string or array of strings.
• Internally scope is managed as array of strings.
• scope in return values from handlers are still array of string
• scope in response body scope will always be a string

@Valerionn so the parseScope expects a string and will at the end split it into an array. Model-implementations will receive and return arrays while authenticate should indeed expect a string.

From what I see this rather calls for a fix in the docs.

[Valerionn]

@jankapunkt A fix in the docs and Typescript types sounds good to me as well (then I won't have to manually cast the type there). I just found it weird that the library expects a string[] for scope everywhere else except at this call (and additionally, the typing is currently wrong as well).

Btw. (regarding types), the BaseModel.getClient type defines the method as getClient(clientId: string, clientSecret: string) but should be getClient(clientId: string, clientSecret: string | null) (as correctly described in the Docs).

If you want to, I can make a PR with the doc + typing changes so they match the code (I just didn't want to make a PR yet because I didn't know the intended behavior).