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_rzN
> mnSI73jgqTH09vSYyWlFa6U/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/afa4cecf626266
>>> d0ae400f806b0358ed7b07bc61
>>> 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/ms
>>> gid/jsr375-experts/CAL%2Bwt-5QmUqZiZ9V2uTYXAE-A%2B%2BFrzNwYK
>>> kYxmZBxaHyYLq15g%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/ms
>> gid/jsr375-experts/CAE%3D-AhB8_6DVbWMhixN_pDUuVFTPqEc3VkzhwP
>> GxW3UKHOfVyA%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.
>>
>
>