-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider refresh method on IBackChannelAuthenticationRequestStore #1588
Comments
This is a breaking change. I do see/understand the idea of adding a RefreshState method, but that feels a tad too implicit. I'd prefer making this more first class. What would be better is for the IBackChannelAuthenticationRequestStore to really break down the BackChannelAuthenticationRequest data into two halves -- the polling related data and the user related response. This would then need two APIs to load this data. One is cheap, one is expensive. Is this worth re-architecting this? Also, could the IBackChannelAuthenticationRequestStore not be so naive, and instead use caching and check the HTTP response headers to know how stale its own data is (etag or whatnot)? |
My suggested design was intended to not be a breaking change. If the status is stored together with the other information in the database the current flow would work. This would be a way to offer a separation for those cases where it's needed. For the implementation it would have to be special (possibly derived) interface that contains the new method to not be a code level breaking change. |
But the proposed change would be a leaky abstraction, I think. |
Hi, to implement the polling, we went ahead and injected our own BackchannelAuthenticationRequestIdValidator.cs, checking the status of the authentication (calling a 3rd party) here: IdentityServer/src/IdentityServer/Validation/Default/BackchannelAuthenticationRequestIdValidator.cs Line 77 in f01e45f It feels like a good fit to call the identity providers we integrate with, when our RPs are calling our /token endpoint as the moment to check the status of an authentication, instead of coming up with our own scheduling/polling mechanism on the side. Specially because some of these IDPs charge for every call we make to them, so we don't want to inquire the status of an authentication if the RP is not going to call /token for that login anymore (*). We first tried to add that status check call under our own IBackChannelAuthenticationRequestStore.GetByInternalIdAsync() but ran into the issue described in the op: we loose all the validation (client binding, lifetime and throttling). Anyway, fiddling with our own BackchannelAuthenticationRequestIdValidator feels like fighting Duende and we see it only as a temporary implementation. (*): We had to come with our own cancel flow, as some IDPs are national security systems and they only allow one login at a time for a given user, so if a user starts a login with us but changes their mind and try to login to another site, they will get blocked for 5 minutes because the 1st (incomplete) login was not properly cancelled. @brockallen I'm not sure I understand what you are proposing by "Also, could the IBackChannelAuthenticationRequestStore not be so naive, and instead use caching and check the HTTP response headers to know how stale its own data is (etag or whatnot)?". |
The CIBA flow can be used for interacting with eid-solutions such as Mobilt BankID in Sweden or MitID in Denmark. When using those the status of an authentication request is acquired through an external API call. The naive implementation would be to make a custom
IBackChannelAuthenticationRequestStore
that calls the external API on every Get* operation to ensure that up to date data is returned.The drawback with this is that the external API is called even before basic client validation and throttling rules have been applied. There should be a way to run the basic validation and throttling code first and only after those pass call the external API.
An implementation suggestion is to add a RefreshState() method to the IBackChannelAuthenticationRequestStore. It would be invoked by the BackchannelAuthenticationRequestIdValidator after basic validation (client binding, lifetime and throttling) have been done but before the IsComplete flag is checked. The default implementation would be a no-op.
The text was updated successfully, but these errors were encountered: