Expose original and suppressed prompt modes on validated authorize requests

[rutgersc]

Which version of Duende IdentityServer are you using?
6.2.1

Which version of .NET are you using?
7.0

Describe the request

We have an implementation of ICustomAuthorizeRequestValidator which contains some logic depending on PromptModes.

Currently we use a workaround similar to the one introduced with #65. We are looking to remove our workaround and rely on the fix in IdentityServer. This works fine when accessing the promptModes from the returned result of IIdentityServerInteractionService.GetAuthorizationContextAsync, but the same is not true for the context passed into ICustomAuthorizeRequestValidator, as seen here. ValidateOptionalParametersAsync mutates the PromptModes list just before passing it to the custom validator.

The ValidatedAuthorizeRequest does have an OriginalPromptModes, but it is internal. Is it possible to make the promptModes available somehow?

To Reproduce

Same as in IdentityServer/IdentityServer4#5002, except instead of trying to access promptModes on the result of IIdentityServerInteractionService.GetAuthorizationContextAsync, try and access the prompmodes from within a ICustomAuthorizeRequestValidator.

[josephdecock]

We can consider changing the validated request to make the original prompt modes not be internal, but a quick way to get at the original prompt modes is to instantiate an AuthorizationRequest using its constructor that takes a ValidatedAuthorizeRequest as a parameter. That will copy the data that you need into the new AuthorizationRequest's PromptModes property.

[rutgersc]

Thanks, yes that is one way to get the prompt modes. We use reflection right now, but request.Raw could also be used for example.

Making the original prompt modes public would probably be the quick and easy way to go. Though I also have to admit it can be quite confusing to have the PromptModes of AuthorizationRequest be different from the one in ValidatedAuthorizeRequest, especially when you consider most people don't go browsing the implementation of a library. Or read documentation, for that matter.

Having request.PromptModes and request.SuppressedPromptModes be in sync with what is in the actual url would make more sense I think, instead of mutating request.PromptModes. Downside is that this requires more changes to the code that follows the removal.

[josephdecock]

Okay, thanks for your feedback! There are several constraints that we have to balance here:

• Avoid breaking changes to the semantics of the existing prompt modes (to be perfectly honest, this is our highest priority)
• Avoid confusing apis (to the uninitiated, having both PromptModes and OriginalPromptModes could be confusing?)
• Allow customizations like the one that you have

[rutgersc]

Looking at it, I would do something like rutgersc@4d22471

Considering that the semantics of prompt modes has been broken ever since the original change that made it so prompts were removed, this would restore prompt modes to what it once was.

It does not introduce any public change, except reverting the behavior of promt modes. People might be interested in the suppressed prompts, but I doubt it would be of any use.

[josephdecock]

One question that I have is - what is your goal? What are you trying to customize based on prompt modes? I ask because there might be a different approach that we can suggest.

[rutgersc]

Are you suggesting then that people should not use the promptmodes in a custom validator? Do you not agree this is a source of confusion?

We do some user/client/tenant validation conditionally on whether there is a prompt=login. We want to do this at the authorize request, not after it, so to us this seems like it would be the way to go.

[josephdecock]

Hey, sorry for the delay getting back to you @rutgersc. I was asking what your goal was because I'm interested in what you're trying to accomplish and trying to think about how to improve IdentityServer. I can see why prompt modes being changed would be a source of confusion. Let me give some more background about how IdentityServer is using them.

We use the prompt modes when we're deciding if we should show some interactive bit of the UI (login, consent, custom pages, etc). The code that decides if we should show UI is invoked multiple times throughout the authentication process. First, it is called in the original request to the authorize endpoint, and then it is called again each time that the interactive UI indicates that it has completed its work by redirecting back to the return url that we pass to interactive pages.

With that design, we need some way of tracking if we've respected the prompt=login parameter and already redirected to the login page because it is present. Otherwise, we'd keep sending the user back to the login page forever. Removing the prompt after it's been "used" (and the internal suppressed and original prompt modes) are how we track that.

So, what should be happening is that on the initial authorize request, your custom validator will see the prompt=login parameter, and can make use of it. It's only after the user submits the login page and it redirects back to the identity server middleware that your custom validator will run again, this time without the prompt=login.

So that leads me back to wanting to understand your use case. If the validation is that you only want to respect the prompt=login for certain clients, users, or tenants, it seems fine to only run that validation on the first pass through the validations.

[rutgersc]

Thanks for the background writeup. That's also my understanding of how the code came to be, by fixing the endless loop. The only thing I would want to emphasize here is that besides the authorize endpoint(s), ICustomAuthorizeRequestValidator.ValidateAsync(context) is also called when calling IIdentityServerInteractionService.GetAuthorizationContextAsync(string returnUrl). This may very well be the first point for applications to do something with the data associated with the authorization request. At least it is for us, so we expect both of these to behave the same when you consider the information that is in the requests.

Let me give a quick overview of our flow:

1. A /connect/authorize request arrives with prompt=login

AuthorizeRequestValidator is invoked and in turn calls ICustomAuthorizeRequestValidator.ValidateAsync(context)

• where context.Result.ValidatedRequest.PromptModes still contains prompt=login
• we do validations (client/tenant/consent) conditionally on whether or not this is a forced login via prompt=login
• AuthorizeInteractionResponseGenerator is called, it removes prompt=login from request.PromptModesand returns interactionResult.IsLogin=true in order to have the LoginPageResult redirect to _options.UserInteraction.LoginUrl.
• the url now has the suppressed_prompt added to the ReturnUrl

2. User is redirected to /Account/Login?ReturnUrl=/connect/authorize/callback?...&prompt=login&suppressed_prompt=login...

Our LoginController receives the returnUrl and passes it to identityServerInteraction.GetAuthorizationContextAsync(returnUrl). The goal is to start the login process and store information about the authorize request returned from GetAuthorizationContextAsync.

• GetAuthorizationContextAsync calls the custom validator again, but this time context.Result.ValidatedRequest.PromptModes does not contain prompt=login. It has been moved to ValidatedRequest.OriginalPromptModesMethod. This is what is confusing, I think people will expect this to also contain prompt=login. What identityServer does behind the scenes is less relevant than what the actual original authorize request requested, in my opinion. Again, our ICustomAuthorizeRequestValidator does the same validations as for /connect/authorize, except this time the result is different.
• the result of GetAuthorizationContextAsync DOES have all the expected promptModes.

3. The rest of the login process

I hope this somewhat clarifies my point of view. Did you check out my branch with the changes? It is a bigger change compared to exposing the OriginalPromptModes, but it feels like it would be less of a workaround.

it seems fine to only run that validation on the first pass through the validations.

I don't think that's the way to go, because we don't persist anything about the authorize request until /Account/Login?ReturnUrl can get to GetAuthorizationContextAsync. How would you suggest we check if this is the first pass?

[josephdecock]

Did you check out my branch with the changes?

Yes I did, and I appreciate you providing that.

How would you suggest we check if this is the first pass?

It depends on what the purpose of your validation is, and in the most general case, I acknowledge you can't know if this is the first pass (prompt=login was just never sent) or if the prompt was suppressed. However, if you need to enforce something like "Only allow prompt=login if ...", then your validation could do something like this:

if(prompt=login)
   check if we want to allow prompt=login to tell us to go to the login screen

On the first pass through, you'll respect prompt=login according to your custom validation. Once you've done that, on later calls you can dispense with that validation because you already have done the redirect to the login page that the prompt=login asked for.

[rutgersc]

How would you suggest we check if this is the first pass?

In the end I think this is less relevant to the discussion of the actual issue.

It's unfortunate that in order to get the request.PromptModes be in sync with what is in the actual authorize request url, you risk breaking people's code, since they may now already rely on the broken semantics. That is, if we agree that this is considered broken in the first place.

[brockallen]

Given the entirety of this conversation and the balance we need to maintain as explained above, the quick workaround (beyond what you're already doing) is to expose the OriginalPromptModes, agreed?

Also, can you please elaborate on the concrete logic you need that changes the validation based on the prompt mode? This will help to educate us in evaluating your request.

Thanks.

[rutgersc]

Given the entirety of this conversation and the balance we need to maintain as explained above, the quick workaround (beyond what you're already doing) is to expose the OriginalPromptModes, agreed?

That would be the least disruptive approach, yes.

Also, can you please elaborate on the concrete logic you need that changes the validation based on the prompt mode? This will help to educate us in evaluating your request.

The logic in this case is about whether or not to use the tenant claim of the currently logged in user. If the prompt mode is login, then that means a login is forced and so it does not do the validation for the client-tenant combination. It checks if the tenant is enabled, if it differs from the requested tenant, if the client is allowed/consented for this specific tenant etc.

[brockallen]

Ok, so yes, we will look to expose the "OriginalPromptModes" or some such. I agree, perhaps in hindsight it would have been better to have the "working set of modes" named something else, but customer feedback has informed us that breaking changes are discouraged.

Thanks

[rutgersc]

Sounds good, thanks as well.