Oauth2-server: Feature Request: Allow wildcard subdirectory for redirect uri

Created on 4 Feb 2020  路  6Comments  路  Source: thephpleague/oauth2-server

Hi all

We鈥檇 like to propose a change of behavior regarding Redirect URLs. We use the proposed method right now in a customized implementation and we are ready to create a PR from it. However, we would prefer to have approval that the PR would be merged before creating it.

Current behavior
Multiple redirect urls are allowed. Each URL is required to be listed exactly.

Proposed behavior
Same as current, but additionally, it鈥檚 allows to wildcard subdirectories.

Examples:
http://example.com/callback allows example.com/callback
http//example.com/callback/* allows every subdirectory of example.com/callback
http://example.com/callback, http//example.com/callback/* allows example.com/callback and every subdirectory of it.

Why should we add this?
This behavior is allowed in Github OAuth (docs) from where we got the idea.
This doesn鈥檛 break or change any existing implementation, but adds a new feature for developers, which would be helpful in some use cases.
From our point of view, there鈥檚 no security issue with this. As usually a developer has full control of all subdirectories available in his application, there is no room for an attack. We think, Github didn鈥檛 make this decision without heavy evaluation of possible security issues.

Where can you use this?
In our case, we鈥檇 like to support multiple versions of our frontend at the same time. As versioning, we use the commit id. This results in Frontend URLs like:
example.com/dist/{commit-id}/index.html
e.g. example.com/dist/47e9620/index.html or example.com/dist/637ac8d/index.html
This is very hard to achieve without wildcard. We would need to update the database record on every new commit.

Most helpful comment

Thank you. That citation is very helpful.

All 6 comments

I very much see the need for this request but I think it is unlikely we would accept this feature. We try to match the server as closely as possible to the specs and make the library flexible enough that if you want to deviate from this, you should be able to (with overrides or custom implementations).

I double checked the specs and it does specify that we require exact matches. Could you use the state parameter to achieve the same effect? You could possibly pass the versioning commit ID in the state parameter and get the server to handle the request differently depending on that.

Thank you very much for your quick response.

We thought of utilizing the state parameter, but we were not happy with the possible results. Therefore, as said, we already wrote a custom implementation for the AuthCodeGrant. It's this changes that we'd like to merge back to this repository. We thought that other developers could benefit from this, too, and beside it's a little easier for maintenance.

I think you refer to Section3.1.2.2, especially:

The authorization server SHOULD require the client to provide the
complete redirection URI [...]

However, we, and I guess also the Github devs, allowed ourselves to interpret the SHOULD as default, but not as explicit way to go. It leaves room for the wildcard path implementation.

I ask you to consider this, and based on it make your final decision. I can propose 3 possibilities:

  • Final decision that this is considered out of spec; then I鈥檓 happy that at least it's discussed in this thread.
  • We make a Pull Request that changes behavior of AuthCodeGrant to allow wildcards as proposed. This means, wildcards are still optional and need to be specified explicitly with the /* at the end of the path to work, but are then available by default.
  • We make a Pull Request that adds an additional class like WildcardPathAuthCodeGrant, which extends the existing implementation, and lets the developer decide which implementation he wants to use.

Hi @kohlerdominik - sorry but I think we wouldn't merge this in as it is not part of the OAuth spec. I've done some reading around and can't see this as a normal practice although I do appreciate that there are some implementations that allow this. I think, without this being something recommended in the OAuth spec, we would not merge this in.

For what it is worth, in the spec, I think the section you have referred to is saying that we should ask for a complete URI but if the server does not, we should default to the complete URI's provided by clients at the point of registration rather than relaxing how complete the URI is.

I'm sorry that we aren't going to proceed with this at this time but if the specs relax on this, then I will be happy to revisit it.

@Sephster I think there should be more discussion on this topic. Two points:

  1. It's not really accurate to say the requested wildcard feature is "not part of the OAuth spec". It's not _required_ by the OAuth spec, but it's permitted -- we're dealing here with a SHOULD requirement, not MUST. So, the decision to exclude it would be easier to understand if there was a more explicit justification -- for example, it's not a good practice for some reason, or a general decision has been made that this server will not implement anything that is permitted by the spec but not explicitly described in it.

  2. Allowing wildcard subdirectories goes against the SHOULD recommendation just discussed, so maybe the idea is to follow all SHOULD recommendations in the spec. However section 3.1.2.2 contains another SHOULD recommendation that this server does not follow:

If requiring the registration of the complete redirection URI is not possible, 
the authorization server SHOULD require the registration of the URI scheme, 
authority, and path (allowing the client to dynamically vary only the query 
component of the redirection URI when requesting authorization).

Currently it is not possible to vary the query component of the redirection URI -- it must be an exact match. Shouldn't at least this much be allowed?

I agree that a good general solution is to use state.

This RFC points to why we will avoid dynamic URIs https://tools.ietf.org/html/draft-ietf-oauth-security-topics-14#page-11. It is too fraught with dangers and abuse to warrant inclusion in the main branch.

Thank you. That citation is very helpful.

Was this page helpful?
0 / 5 - 0 ratings