Hi,

El mar., 8 ago. 2017 a las 11:37, Will Hopkins (<will.hopkins@...>) escribió:

I've been looking at the pile of bugs currently filed against the default LdapIdentityStore in Soteria, and have noticed a couple broad themes. I'd appreciate some feedback on these ASAP, as we need to decide on a consistent approach so we don't end up with wildly inconsistent behavior for different scenarios.

I'm compiling a list in issue #165, but want to call your attention to a couple items in particular:

• Returning NOT_VALIDATED vs. throwing an exception from validate():

My understanding to date has been that NOT_VALIDATED has a very specific meaning -- i.e., the identity store didn't attempt to validate the credential, because it doesn't handle that credential type. Errors -- network errors, config errors, etc. -- always result in a runtime exception being thrown.

This is reasonable if we expect errors to be rare, but the behavior isn't documented -- since there are no checked Exceptions, and we didn't add an explicit javadoc reference to runtime exceptions -- and it seems to happen pretty frequently since config problems don't typically fail deployment. This is the source of a lot of the outstanding bugs.

On balance, I'm leaning toward returning NOT_VALIDATED rather than throwing exceptions, and some fixes have already been made that catch exceptions rather than letting them propagate out of validate(). That said, I don't have really strong feelings either way -- we just need to pick one way or the other and handle errors consistently.

I prefer to throw exceptions at this moment so we can decide on a future version wether it's preferrable to create a VALIDATION_ERROR result, or if deployment validation is achievable and validation errors become rare.

IMO the whole "configuration errors" thing needs some more discussion and that's the main origin of these exceptions. A future version of the spec could probably add a new return value for network and pure runtime errors, but I wouldn't do it at this time.

• Default searchFilters:

There's no default searchFilter for caller lookup, but there is one for group lookup. We should probably handle both the same way -- either provide a default, or don't. The right way to provide defaults would probably be on the annotation attributes, rather than buried in internal code, but it's too late for that change.

Given that, my bias is to remove the default group search filter, rather than add a new one for callers. Note that any default filter should probably include objectclass filtering, which is fairly straightforward for groups, but not quite as straightforward for callers -- there's a wider variety of "person" objectclasses, including, apparently, (objectclass=computer), which is necessary for some LDAPs but won't work for others.

I can't unfortunately comment on this since I have no LDAP experience.

Thanks for any thoughts you may have,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803