Consider allowing users to customize the BuildRedirectUri of AuthorizeHttpWriter

[DoxxableSetanta]

Hello, I like your new extensibility points for the IHttpResponseWriter, I'm looking at overriding the AuthorizeHttpWriter, like the person in this issue:

DuendeSoftware/products#1446

Can you consider changing BuildRedirectUri to be a virtual method? I'd like to send some custom query parameters to the client that happen after some business logic within our IdentityServer. I add them in a step before the login page using an override of AuthorizeInteractionResponseGenerator to add them to the ACR values of the login request, which is then persisted over to the connect/authorize/callback request.

However, when the 302 happens, these values are not persisted in the redirect.

This would be the same case if we were to send these from the client.

Or, if there's some other way of sending back custom values to the client when the authorize/endpoint request is completed but before the /token request, please let me know. I had considered modifying the redirectURI on the fly but this just breaks the client and would require further overrides.

Even if it's just a method of appending ACR values to the location value, that would be sufficient, thanks!

[AndersAbel]

Thank you for your suggestion. While we added some flexibility in DuendeSoftware/products#1446, we opted to only open up for extensions once we had actual requests for it. Opening up our internal implementations for extension/reuse by making them public and having protected virtual methods adds them to our API surface that must be kept stable. So before we open up anything, we need to be careful to ensure that the solution we create will be maintainable over time.

If I understand your current solution right, you are using the ACR values as a property back just because it's something available. Do you prefer to have these custom values as ACR values, or would you prefer another field/property bag to be available to transfer data to the custom IHttpResponseWriter?

I will discuss this request back with our product development team to get their opinion on possibilities.

[DoxxableSetanta]

Of course, I understand. You have a lot of extensibility points already littered throughout, so I'm curious if there's something I'm missing also in your API surface that would achieve what I want, but I don't think there is.

I'm open entirely to either possibility - I'm not wedded to the ACR values specifically being returned, but at least there being some hook where I can e.g. inspect the ACR values (or some other state of the request/httpcontext), to then add some custom params to the redirect URI in the location header, that would be great. As what I want is in the ACR values, I thought that might be easiest for you guys as it acts as a bag anyway, but it's not a hard requirement of mine if you want to decouple it from what's returned to the client.

Thanks for the speedy response!

[josephdecock]

I've been thinking about this problem, and I think we should make changes to IdentityServer to help you solve it. But I think that there might be a better way to achieve your goals than adding virtuals or making other changes to the API surface of the AuthorizeHttpWriter.

Generally speaking, I think of the HttpWriters as being responsible for the lowest level networking/"plumbing" details - things like serialization and HTTP headers and the like. The way that IdentityServer's engine is structured, the better place for the kind of logic you're interested in would be in a response generator. In a response generator, your custom logic that needs to decide which custom parameters are needed can deal with an object model that describes the intent, while being abstracted from the networking details of url encoding the parameters.

The AuthorizeResponse model doesn't currently support this, so to make this approach work, we'd add a dictionary for custom parameters to it. Your customized AuthorizeResponseGenerator could then add custom parameters there using whatever logic you need. Then IdentityServer itself would handle the rest. We'd pass the custom parameters through the rest of the pipeline, updating AuthorizeResult so it can carry the custom parameters for us and AuthorizeHttpWriter so that custom parameters would get included in the redirect url.

Does that make sense and seem like it would solve your problem? Does it seem like the right level of abstraction for you? If so, what's the time frame when you need this feature? And finally, out of interest, can you share what custom information you're passing back to the client? Is there some OAuth or OpenId Connect extension or profile that needs this? Thanks in advance!

[DoxxableSetanta]

Thanks for the speedy Response @josephdecock ! Sorry it was a bank holiday here in Ireland yesterday.

I agree that a response generator would be optimal - that was our first port of call in fact.

In terms of time-frame, we do have the workaround of rewriting the AuthorizeHttpWriter with our custom logic implemented, so we're not blocked on this, but we'd appreciate it whenever you can so our implementation is not as hacky and is a bit more stable.

As to why we want it - it's actually to identify tenant in our implementation of multitenancy as we're not allowed to use custom domains for our tenants, but we're still maintaining separate DBs/IoC containers per tenant. When doing the code+pcke grant type, we are able to successfully identify tenant as part of the connect/authorize request using the user's e-mail domain and resolve the correct tenant, but then we need to know the tenant ahead of time when making the /token request as otherwise it will fail, hence we pass the resolved tenant back to the client who can then pass it back to IdS in the subsequent request.