Rationalize fail-fast behavior in resolver
Description
Environment
Activity
Rod Widdowson October 3, 2019 at 12:11 PM
I'll leave open for another week or so, just in case something else happens
Rod Widdowson October 3, 2019 at 12:10 PM
I believe that this is (finally) done
Scott Cantor September 30, 2019 at 12:11 PM
Agree. The flows don't instantiate at startup, only at first execution. We have talked about various ways of exercising the login code, maybe even as part of initializing, but that's speculative and given my general feelings about failfast, I don't think it makes sense to apply it to this use case anyway.
Rod Widdowson September 28, 2019 at 3:26 PM
Adventures in failFast/AuthN land
If the IDP comes up and there is no LDAP the default behavior is for the login screen to prompt and the result to be a warning about pool exhaustion. Start the LDAP and try again and you login.
If the IDP comes up and there is no LDAP and the ConnectionPool is set to be failfast in LDAP-authn.xml.
If you have an existing session then things just carry on
If you don;t then you get a flowExecutionException (after the idp has queried the browser to get the session - I know this because my test browser has javascript off). Start the LDAP, get logged in
I can therefore think of no valid reason to control failfast on the authn LDAP connection pool. It isn't failfast and it just makes the behavior more smelly.
So the plan is to entirely remove the failfast setting from ldap.properties and related. These are user-only files so nothing is changed.
Rod Widdowson September 26, 2019 at 3:20 PM
I updated the documentation to reflect the failfast change and went to make the changed to attribute resolver but it gets complicated because {{idp.pool.LDAP.failFastInitialize }}is used by Authn. So I'll need to strip the use from Attribute resolver and introduce a new property for Authn. Or something.
An alternative is to leave the property in place but use it to drive the LDAPDataConnector rather than the pool. Its bruggy, but it fixed the issue.
One of the current hassles in the resolver is a lack of consistency in handling errors and determining how to expose them, including during config load, where we have a mandatory notion of a Validator vs. other places where flags are used to control the behavior.
While it's always possible to use failover connectors to mask most errors, that's not an ideal solution. I think we should have a more consistent approach to this that ties the Validator piece in or possibly just gets rid of it. I know of virtually nobody that really sees a win in having the IdP fail to start up because an LDAP server is down, but perhaps I'm still projecting there.
If we keep it, I also think the Validator concept should be non-mandatory and if not set simply treated as "don't validate". Having to explicitly designate a non-failfast validator is awkward.
If nothing else, a good review of all the error handling options and how they get used is a good documentation addition.