ory / fosite

Extensible security first OAuth 2.0 and OpenID Connect SDK for Go.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=fosite
Apache License 2.0
2.31k stars 359 forks source link

Is there a plan to support custom redirect_uri matching rules. #820

Open gitsang opened 1 month ago

gitsang commented 1 month ago

Preflight checklist

Ory Network Project

No response

Describe your problem

Should we providing the functionality of custom matching patterns for redirect_uri?

Describe your ideal solution

I certainly understand that using a complete match for security reasons is reasonable, but there are also some third-party systems that may have compatibility issues.

In OAuth Documents, using exact redirect URI matching is just Advise.

Anyway, at least I think providing the functionality of custom matching patterns is following the specifications.

Workarounds or alternatives

This is the alternatives solution for supporting pattern match directly.

Version

0.45

Additional Context

No response

james-d-elliott commented 1 month ago

It's not accurate to describe it as just advice. It's a strict requirement of both OAuth 2.0 and OpenID Connect 1.0.

In addition almost 100% of the Best Current Practice document is formally part of the OAuth 2.1 draft if it's not already part of OAuth 2.0 (in which case the Best Current Practice document serves as an ADR / rationale for the requirements at least in some cases).

Typically clients which require this, or providers that allow this, are objectively not compliant with the spec. Is that a reason to not implement it? Not really sure.

gitsang commented 1 month ago

It's not accurate to describe it as just advice. It's a strict requirement of both OAuth 2.0 and OpenID Connect 1.0.

In addition almost 100% of the Best Current Practice document is formally part of the OAuth 2.1 draft if it's not already part of OAuth 2.0 (in which case the Best Current Practice document serves as an ADR / rationale for the requirements at least in some cases).

Typically clients which require this, or providers that allow this, are objectively not compliant with the spec. Is that a reason to not implement it? Not really sure.

Thank you for your reply. However, there are still some third-party systems that do not compliant with the spec. And Unfortunately, due to historical reasons or business requirements, sometimes we have to make concessions for compatibility.

Is it worth sacrificing security and violating standards in exchange for compatibility? Or can we simply treat it as an insecure option (such as naming it 'InsecureRedirectUriMatching' or 'NonComplianceRedirectUriMatching')?

vivshankar commented 1 month ago

FWIW - We wrap Fosite to provide a RedirectURIMatchingStrategy for exactly this reason of being able to interoperate. There are some practical considerations around certain applications that have a large number of redirect URIs because of an RP designed to be an intermediary. An example of this is Artifactory, IIRC. Do I like this? No. Do I have to live with it? Yes :-)

From a product standpoint, we offer the option to choose the matching strategy but default to the prescribed approach of exact matching.

If there is interest and @aeneasr sees this as something that we can potentially add, I will be happy to contribute this.

james-d-elliott commented 1 month ago

Yeah no worries this makes sense. I think some examples of precisely the situations and URI's that may need specialized matching may be helpful.

I would also argue that absent a specific example the matching strategy (if implemented) should generally speaking only be utilized in this context to match the path of the URI, and the origin should be matched explicitly against the list of registered redirect URIs. Pattern matching (especially with regex) the origin due to its nature has a considerable security impact that doesn't exist with RFC3986 simple string comparison.

gitsang commented 1 month ago

In my scenario, most of the incompatible situations are caused by old internal systems and some outsourced services. A typical scenario is that some services will attach parameters in the redirect_uri instead of placing them under the "state" parameter. For example:

https://server.example.com/yauth/api/v1/oauth/authorize?
    response_type=code&
    client_id=424911365001&
    scope=openid&
    redirect_uri=https://app.example.com/callback?token=ey......&
    state=xxxxxxxx&
    nonce=0394852-3190485-2490358