jsr375-experts@javaee-security-spec.java.net

[jsr375-experts] Re: Updated Multi IdentityStore Proposal

From: Ivar Grimstad <ivar.grimstad_at_gmail.com>
Date: Mon, 05 Sep 2016 09:08:05 +0000

Werner,

I have a talk at 11:30 on Wednesday, so you may say that's a conflict :)

Ivar

On Mon, Sep 5, 2016 at 11:02 AM Werner Keil <werner.keil_at_gmail.com> wrote:

>
> On Mon, Sep 5, 2016 at 11:01 AM, Werner Keil <werner.keil_at_gmail.com>
> wrote:
>
>> Ivar/all,
>>
>> I know you already have a Hackergarten on MVC
>> https://docs.google.com/spreadsheets/d/1JidhkO_kzVfZApzl_rzNmnSI73jgqTH09vSYyWlFa6U/edit#gid=0
>>
>> Since the slots became practically full, I offered JSR 375 / Soteria with
>> both of us as host in the last one Wed from 10-12. I may have a slight
>> conflict because JSON-B Spec Lead Dmitry does an Oracle talk on Java EE
>> Configuration and I may not be the only one keen to hear what they have in
>> mind (but e.g. Anatole from Tamaya should also be able to attend it)
>>
>> If you absolutely can't make it, we may have to move it e.g. by merging
>> it into another one (Anatole on Wed also offers JSR 354 and Tamaya,
>> depending on who's interested in what, we could do that, too if Wed was too
>> inconvenient)
>>
>> Arjan and e.g. Dutch or Swedish JUGs (Ivar) are already pretty active, at
>> least a few members. It would be great to get more of them, so Hackergarten
>> in addition to an Oracle talk (likely more theoretical than hands-on;-)
>> sounds like a good idea to present some things and flesh-out others.
>>
>> Regards,
>>
>> Werner
>>
>>
>> On Sun, Sep 4, 2016 at 9:28 PM, arjan tijms <arjan.tijms_at_gmail.com>
>> wrote:
>>
>>> Hi Rudy,
>>>
>>> Looks much improved, thanks!
>>>
>>> I have a few comments, but some are just cosmetic and could be looked at
>>> later.
>>>
>>> The most important one concerns the IdentityStore interface, it now has
>>> this method:
>>>
>>> public CredentialValidationResult validate(Credential credential,
>>> CallerPrincipal callerPrincipal) {
>>>
>>> I wonder, would it be possible to omit the CallerPrincipal argument
>>> here? There's already a caller in the Credential, and I'm afraid this would
>>> confuse people who have to implement the interface. I may also give the
>>> impression to implementors that they have to take the history of what
>>> happened before with other stores somehow into account. I think it would be
>>> best if the store itself is totally unaware of the concept of multiple
>>> stores, since that's what the handler should do. Since the handler is
>>> replaceable, a store implementation can't assume anything about it.
>>>
>>> In the implementations of the provided stores (e.g. the
>>> EmbeddedIdentityStore), there's this comment with some code below it.
>>>
>>> // We check also if caller != null to be sure the Authentication by
>>> another IdentityStore succeeded.
>>>
>>> If you look at the code flow that applies to it's:
>>>
>>> String caller = null;
>>> ...
>>> // We are Authorize Only mode, so get the caller determined
>>> previously.
>>> if (callerPrincipal != null) {
>>> caller = callerPrincipal.getName();
>>> }
>>>
>>> if (authenticated && caller != null) {
>>> ...
>>> }
>>>
>>> return INVALID_RESULT;
>>>
>>> So it callerPrincipal or callerPrincipal.getName() would be null here,
>>> the store would return INVALID_RESULT. But in that case, wouldn't it be
>>> better if the handler did not call the store at all? That would safe the
>>> store from having to check this case or even having to think of what other
>>> stores before it did. In other words, define in the contract and spec that
>>> if the store is used for authorization, the passed in caller is not null.
>>> Would that make sense?
>>>
>>> The idea here would be that a store is kept as simple as it can possibly
>>> be, as an often heard complaint is that all proprietary stores are quite
>>> complex. The multi-store algorithm should then 100% be with the handler.
>>>
>>> A somewhat cosmetic and implementation thing, but maybe still worth it
>>> to look at is this pattern in the stores:
>>>
>>> private EmbeddedIdentityStoreDefinition embeddedIdentityStoreDefinition;
>>>
>>> private ValidationType validationType;
>>>
>>> private void determineValidationType() {
>>> validationType = ValidationType.BOTH;
>>> if (embeddedIdentityStoreDefinition.authenticateOnly()) {
>>> validationType = ValidationType.AUTHENTICATION;
>>> } else {
>>> if (embeddedIdentityStoreDefinition.authorizeOnly()) {
>>> validationType = ValidationType.AUTHORIZATION;
>>> }
>>> }
>>> }
>>>
>>> public ValidationType validationType() {
>>> return validationType;
>>> }
>>>
>>> Seeing this code repeating, maybe it's better to have ValidationType
>>> directly on the EmbeddedIdentityStoreDefinition (and other definitions)?
>>>
>>> Then it could just be:
>>>
>>> public ValidationType validationType() {
>>> return embeddedIdentityStoreDefinition.validationType();
>>> }
>>>
>>> Another small thing, also cosmetic mostly, but there's now this kind of
>>> check often:
>>>
>>> if (validationType == ValidationType.AUTHENTICATION || validationType ==
>>> ValidationType.BOTH) {
>>> ...
>>> }
>>>
>>> What about making two helper methods on the ValidationType enum like e.g.
>>>
>>> public boolean isAuthentication() {
>>> return isOneOf(this, AUTHENTICATION, BOTH);
>>> }
>>>
>>> then the code using it could just do:
>>>
>>> if (validationType.isAuthentication()) {
>>> ...
>>> }
>>>
>>> Hope this feedback is useful ;)
>>>
>>> Kind regards,
>>> Arjan Tijms
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Sun, Sep 4, 2016 at 8:07 PM, Rudy De Busscher <rdebusscher_at_gmail.com>
>>> wrote:
>>>
>>>> Hi all,
>>>>
>>>> This is the updated multi IdentityStore based on earlier feedback on
>>>> this mailing list.
>>>>
>>>>
>>>> *Why multiple stores?*
>>>> There are various use cases where multiple identity stores can be
>>>> useful.
>>>> And it allows us to decouple the authentication and authorization parts
>>>> of the current mechanism. So that groups can be retrieved by another store
>>>> then the credential checking.
>>>>
>>>> *Examples*
>>>> * When your application should allow users which are defined in
>>>> multiple stores, like an LDAP for your internal users and a Database store
>>>> for the external ones.
>>>> * Users are authenticated by an OAuth2 provider and the groups are
>>>> retrieved from a database table.
>>>> * The first IdentityStore based on OAuth2 allows any Google account,
>>>> but the second store limits the access to a certain google domain.
>>>>
>>>> *IdentityStore interface*
>>>>
>>>> * CredentialValidationResult validate(Credential credential,
>>>> CallerPrincipal callerPrincipal);*
>>>>
>>>> * default int priority() {*
>>>> * return 100;*
>>>> * }*
>>>>
>>>> * default ValidationType validationType() {*
>>>> * return ValidationType.BOTH;*
>>>> * }*
>>>>
>>>> *validate method*
>>>>
>>>> the CallerPrincipal parameter contains previous validated Caller
>>>> information (authentication) if any. Value can be null.
>>>>
>>>> *ValidationType method*
>>>>
>>>> ValidationType contains meta information for the IdentityStore handler.
>>>> It defines if the store does authentication only
>>>> (ValidationType.AUTHENTICATION), authorization only
>>>> (ValidationType.AUTHORIZATION) or both; the default.
>>>> This information is used by the handler to define if the store should
>>>> be called. For example, the default IdentityStore handler does not call a
>>>> store who does Authorization only when no successful authentication took
>>>> place.
>>>>
>>>> *priority method *
>>>>
>>>> determines the order in the case when there are multiple ones. They are
>>>> ordered by ascending values, meaning that the IdentityStore with the lowest
>>>> value is processed first.
>>>>
>>>> *Status rules*
>>>>
>>>> Stores should return the following Status (contained in the
>>>> CredentialValidationResult result parameter)
>>>> *NOT_VALIDATED*: Type of Credential isn't compatible with the ones
>>>> expected by the store.
>>>> *INVALID* : In case the store does Authentication, the validation of
>>>> the credential failed.
>>>> *AUTHENTICATED* : In case the store does Authentication only and the
>>>> validation succeeded.
>>>> *VALID* : In case the store does Authorization and groups are supplied
>>>> for the Caller (can be an empty list indicating no groups assigned to
>>>> caller)
>>>>
>>>> So When Store does handle both (Authentication and Authorization) it
>>>> should return NOT_VALIDATED, INVALID or VALID
>>>> In case store does Authentication only; it is NOT_VALIDATED, INVALID or
>>>> AUTHENTICATED
>>>> In case store does Authorization only; it is NOT_VALIDATED or VALID
>>>> (the assumption is that when store can handle the type of Credentials, the
>>>>
>>>> *IdentityStore Handler *
>>>>
>>>> Default IdentityStore Handler uses the following algorithm
>>>> 1) Order all identityStores based on the priority value
>>>> 2) Loop over all IdentityStores unless one of the stores return the
>>>> status INVALID
>>>> 3) Call a certain IdentityStore when
>>>> * The Store does Authentication, regardless of the result of the
>>>> calls to previous IdentityStores.
>>>> * The Store does Authorization only and another Store already
>>>> successful validated the Credentials (Result is AUTHENTICATED or VALID)
>>>> 4) The result of the call to the IdentityStore is combined with
>>>> 'previous' result of the there calls to the IdentityStores.
>>>> * Groups are added to the already defined groups of the caller.
>>>>
>>>> *Issues*
>>>>
>>>> The current default IdentityStore Handler handler can't handle this use
>>>> case
>>>> - Caller can be authenticated by one IdentityStore (for example LDAP or
>>>> Database)
>>>>
>>>> This is because the loop stops when an INVALID result is returned by a
>>>> IdentityStore. This is required to veto another successful authentication
>>>> like in this use case
>>>> - The first IdentityStore based on OAuth2 allows any Google account,
>>>> but the second store limits the access to a certain google domain.
>>>>
>>>> So probably we need some configuration to be able to combine both use
>>>> cases in one algorithm.
>>>>
>>>> *Code status*
>>>>
>>>> The current code needs probably some more tests and verification. I try
>>>> to write them in the following weeks.
>>>>
>>>> Changes for this updated proposal can be viewed in this commit
>>>> https://github.com/rdebusscher/soteria/commit/afa4cecf626266d0ae400f806b0358ed7b07bc61
>>>> And are added to the first pull request to the Soteria repository.
>>>>
>>>> Best regards
>>>> Rudy
>>>>
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "Java EE Security API - JSR 375 - Experts" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>> an email to jsr375-experts+unsubscribe_at_googlegroups.com.
>>>> To post to this group, send email to jsr375-experts_at_googlegroups.com.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/jsr375-experts/CAL%2Bwt-5QmUqZiZ9V2uTYXAE-A%2B%2BFrzNwYKkYxmZBxaHyYLq15g%40mail.gmail.com
>>>> <https://groups.google.com/d/msgid/jsr375-experts/CAL%2Bwt-5QmUqZiZ9V2uTYXAE-A%2B%2BFrzNwYKkYxmZBxaHyYLq15g%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>> .
>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "Java EE Security API - JSR 375 - Experts" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to jsr375-experts+unsubscribe_at_googlegroups.com.
>>> To post to this group, send email to jsr375-experts_at_googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/jsr375-experts/CAE%3D-AhB8_6DVbWMhixN_pDUuVFTPqEc3VkzhwPGxW3UKHOfVyA%40mail.gmail.com
>>> <https://groups.google.com/d/msgid/jsr375-experts/CAE%3D-AhB8_6DVbWMhixN_pDUuVFTPqEc3VkzhwPGxW3UKHOfVyA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>>
>