Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Add redirect_uri parameter to ErrorUrl #1564

Closed
MNF opened this issue Sep 26, 2017 · 7 comments
Closed

Add redirect_uri parameter to ErrorUrl #1564

MNF opened this issue Sep 26, 2017 · 7 comments
Assignees

Comments

@MNF
Copy link
Contributor

MNF commented Sep 26, 2017

On error page, based on IdentityServerHost, we want to add a link to the client site, e.g. "You can return to your site".
We want to use redirect_uri parameter, but it's not available on HomeController.Error action.
I found that redirection to error page implemented in
IdentityServer4\Endpoints\Results\AuthorizeResult.cs

async Task RedirectToErrorPageAsync(HttpContext context)
{
…
 var errorUrl = _options.UserInteraction.ErrorUrl;
          var url = errorUrl.AddQueryString(_options.UserInteraction.ErrorIdParameter, message.Id);
          context.Response.RedirectToAbsoluteUrl(url);
}

I want to add to url RedirectUri
url = url.AddQueryString("redirect_uri", Response.RedirectUri);
and read it in HomeController.Error action.
Is it a good idea, or you see some issues with it or can suggest alternatives?
If you think, that it is ok, will it be useful for other users? Do you want me to create pull request?

@MNF MNF changed the title Add redirect_uri parameter to Add redirect_uri parameter to ErrorUrl Sep 26, 2017
@brockallen
Copy link
Member

No, we don't pass it along because in an error state we don't know if the redirect uri is safe or not. For example, is the reason we're showing the error page because the redirect uri is invalid?

@MNF
Copy link
Contributor Author

MNF commented Sep 26, 2017

If I will add validation that redirect_uri is in a list of allowed redirect uris for the client, will it be safe to use such uri? The link will appear for most of the errors except invalid redirect uri.

@brockallen
Copy link
Member

We can have a look to see if we think this is safe or not.

@brockallen
Copy link
Member

I submitted a PR for this. It needs scrutiny.

@brockallen brockallen added this to the 2.1 milestone Dec 19, 2017
@brockallen
Copy link
Member

@leastprivilege i put it back on the 2.1 milestone for discussion so we can decide to include or punt.

brockallen added a commit that referenced this issue Dec 20, 2017
* Add redirect_uri parameter to ErrorUrl #1564

* also include response mode to error page
@brockallen brockallen removed this from the 2.1 milestone Dec 21, 2017
@brockallen
Copy link
Member

Done

@lock
Copy link

lock bot commented Jan 13, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants