Allow arbitrary pushed redirect_uris #1458
Conversation
Spec allows an arbitrary redirect uri to be pushed: https://datatracker.ietf.org/doc/html/rfc9126#name-management-of-client-redire
I prefer opt-in. I want people to know they're allowing this. |
| /// </summary> | ||
| public bool IsPushedAuthorizationRequest => PushedAuthorizationReferenceValue is not null; | ||
| public AuthorizeRequestType AuthorizeRequestType { get; set; } |
There was a problem hiding this comment.
Maybe make it overly explicit that it's set to Authorize by default?
There was a problem hiding this comment.
Meh, I now see this is used in many places... we can just rely upon the default enum values, I suppose.
|
Let's schedule a call to pair review this, please. |
|
This is ready for another review. |
| // is valid to use a pushed redirect uri that is not | ||
| // pre-arranged. | ||
| if (context.Client.RedirectUris?.Any() == false && | ||
| !context.Client.RequirePushedAuthorization) |
There was a problem hiding this comment.
Confidential, not require par, right?
There was a problem hiding this comment.
Right, forgot about this one!
| /// Initializes a new instance of the <see cref="StrictRedirectUriValidator" />. | ||
| /// </summary> | ||
| /// <param name="options"></param> | ||
| public StrictRedirectUriValidator(IdentityServerOptions options) |
There was a problem hiding this comment.
Maybe optional for backwards compat, and if null then we assume no PAR? The DI folks will love us. Or add a 2nd default ctor with the same behavior.
There was a problem hiding this comment.
I went with the optional parameters here (and similarly in the app auth ctor)
|
👍 |
Spec allows an arbitrary redirect uri to be pushed: https://datatracker.ietf.org/doc/html/rfc9126#name-management-of-client-redire
This is implemented as a new redirect uri validator that is passed the necessary context to determine if redirection is needed.
Question: should this behavior always be enabled, or be opt in?