Incorrect Scope Format in API Response: Single String Instead of Array for Multiple Scopes

[vamsii777]

There appears to be a discrepancy in how scopes are being handled within the Scope Validator of the vapor-oauth project. While the clientScopes are correctly retrieved as an array (e.g., ["admin", "openid"]), the scopes fetched via the API are concatenated into a single string ("admin,openid").

Specific Problem

The core issue arises from the way scopes are being processed and returned. The expected behavior is to have the scopes returned as an array of strings, which facilitates accurate scope comparisons. However, currently, the API merges all scopes into a single string, thus causing comparison mismatches and functional limitations.

Examples and Implications

• Example 1: As demonstrated in the first screenshot, clientScopes retrieves ["admin","openid"], which is the expected format.
• Example 2: The API response, as shown in the second screenshot, provides the scopes as "admin,openid". This format is technically correct but not suitable for direct comparisons.
• Failure Case: This issue is exemplified in the third screenshot, where an attempted comparison of individual scopes (e.g., "openid" against "openid, admin") fails due to the string format.

The consequence of this issue is significant: it restricts users to request only one scope at a time, limiting the functionality and flexibility of scope management within the application.

Suggested Fix

The API should be modified to split the scopes string into an array before any comparison or further processing. This would align the format with the expected array structure and ensure accurate scope comparisons and validations.

Additional Context

This issue was first identified in a discussion thread (refer to the original post by @mynona in https://github.com/vamsii777/vapor-oauth/discussions/5#discussioncomment-7938901). The provided screenshots and descriptions therein offer further insights into the problem and its implications.

[mynona]

Hi @vamsii777,

I checked the scope issue in more detail and found that there is no issue.

AuthorizeGetHandler.swift

private func validateRequest(_ request: Request) async throws -> (Response?, AuthorizationGetRequestObject?) 

(...)

let scopes: [String]

        if let scopeQuery: String = request.query[OAuthRequestParameters.scope] {
            scopes = scopeQuery.components(separatedBy: " ")
        } else {
            scopes = []
        }
(...)

As you can see it accepts scopes sent from the client only separated by spaces (" ").

I found different specifications as we use ["scope","scope"]; "scope,scope"; "scope scope" as possibilities. According to https://www.rfc-editor.org/rfc/rfc6749#section-3.3 it says:

The value of the scope parameter is expressed as a list of space- delimited, case-sensitive strings. The strings are defined by the authorization server. If the value contains multiple space-delimited strings, their order does not matter, and each string adds an additional access range to the requested scope.

I was not aware that we follow only this guideline and don't allow scopes as "scope,scope"

So, there is nothing that needs to be fixed and sorry that I created this confusion in the first place.

[mynona]

But there is one more topic that needs to be considered with OpenID Connect.

In the AccessToken we have also "aud" which represents the clientID.

Based on the userinfo endpoint specification https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse a client would for example request

"email phone address profile customized1 customized2"

The OpenID Provider should then return only the valid user attributes FOR THIS CLIENT.

At the moment we are returning all user attributes because the UserManager doesn't know anything except the userID:

func getUser(userID: String) async throws -> VaporOAuth.OAuthUser? {

If we would have an optional parameter clientID the UserManager could look up the scopes for this client (AccessToken aud) and check what parameters it should return.

What do you think about extending getUser:

getUser(userID: String, clientID: String?)

I believe it would not be good if the returned attributes are automatically managed as there are also customized scopes allowed and the OpenID specification describes that out of security reasons not all requested attributes per scope must be returned. So, this should be handled by the consumer = customized UserManager

[vamsii777]

@mynona so, about the scopes there's no need to change apart from that being string in the JWT right? The previous branch feature/jwt should be alright for the changes or fixes/incorrect-scope-format?

[mynona]

In my opinion, you improved the readability of the code a lot, because you do the mapping to [String] in the ScopeValidator. This is why the issue arose in the first place because it was not obvious where "admin openid" was mapped to ["admin", "openid"]. So, I think it is an improvement.

But my second comment is more important: Extending the UserManager to return user attributes based on clientID (and therefore, return only allowed attributes for this client)

Otherwise you would return data that should not even be shared with certain clients. It's all connected.

[mynona]

After putting more thought in this: the JWT doesn't need to be changed as the payload is created by the consumer of vapor/oauth. I think, this is perfect because then every user of vapor/oauth can configure the payload themselves (and have to take care that they implement it correctly).

This flexibility should remain on the consumer I guess.

[vamsii777]

Yes, agreed! Will check on https://github.com/vamsii777/vapor-oauth/issues/11#issuecomment-1890941857

[mynona]

Once more, sorry that I didn't see it in the first place that there happens a mapping based on spaces in the RequestObject

[vamsii777]

• Merged #15