Hi,
Okay, that's good to hear. I will remain a fan of the single method, but
let's compromise on this then. You are right that there are also advantages
in not re-using that single method. Thanks for letting me see that ;)
I just pulled in your PR in a local branch and will add the new
authorization method and code sketch as discussed here, and then push it to
a new proposal/feature branch. If the EG then agrees on it, I can merge
this branch into the master.
Kind regards,
Arjan
On Mon, Sep 19, 2016 at 10:21 PM, Rudy De Busscher <rdebusscher_at_gmail.com>
wrote:
> Arjan,
>
> I find it better than the single method interface:
>
> - We don't have to do the strange constructs with ValidationResult anymore
> to combine different results
> - The link between authentication and authorization and which methods
> needs to be used is much clearer.
> - Flexible as we can add the groups from any store which is marked as
> Authorization only to the result (for example LDAP or OAuth2 where groups
> are stored in the Database)
> - When you only have a single IdentityStore which does everything, the M1
> logic and implementation behavior is retained.
>
> Best regards
> Rudy
>
>
> On 19 September 2016 at 13:03, arjan tijms <arjan.tijms_at_gmail.com> wrote:
>
>> P.s.
>>
>> By making the "get groups only" operation a special case, we could
>> introduce a method like:
>>
>> List<String> getGroupsByCaller(CallerPrincipal caller);
>>
>> This would move a bit back to Alex' original proposal, and it wouldn't be
>> my first choice (I much prefer the single method IdentityStore), but could
>> be a good compromise.
>>
>> Rudy, would that work for you? Is there anyone else having an opinion
>> about this?
>>
>> The default multi-store algorithm would then be:
>>
>>
>> @Override
>> public CredentialValidationResult validate(Credential credential) {
>>
>> CredentialValidationResult validationResult = null;
>>
>> // "Authenticate" - Check credential
>>
>> for (IdentityStore authenticationIdentityStore :
>> authenticationIdentityStores) {
>> validationResult = authenticationIdentityStore.va
>> lidate(credential);
>> if (validationResult.getStatus() == VALID) {
>> break;
>> }
>> }
>>
>> if (validationResult.getStatus() != VALID) {
>> // No store authenticated, no need to continue
>> return validationResult;
>> }
>>
>> CallerPrincipal caller = validationResult.getCallerPrincipal();
>> List<String> groups = new ArrayList<>(validationResult.g
>> etCallerGroups());
>>
>>
>> // "Authorise" -> Get Groups
>>
>> for (IdentityStore authorizationIdentityStore :
>> authorizationIdentityStores) {
>> groups.addAll(authorizationIdentityStore.getGroupsByCaller(
>> caller));
>> }
>>
>> return new CredentialValidationResult(VALID, caller, groups);
>> }
>>
>>
>> Kind regards,
>> Arjan Tijms
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Sun, Sep 18, 2016 at 9:25 PM, arjan tijms <arjan.tijms_at_gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Sun, Sep 18, 2016 at 8:41 PM, Rudy De Busscher <rdebusscher_at_gmail.com
>>> > wrote:
>>>
>>>>
>>>> I was just looking at the actual looping algorithm again here, but
>>>>> wasn't it the idea that we would try all identity stores until one succeeds?
>>>>>
>>>>
>>>>
>>>> And if the first one that succeeds does only authentication, and not
>>>> authorization? We don't have any groups then.
>>>>
>>>
>>> No that's not the case, it can return the groups at that point.
>>>
>>> The first one that succeeds returns a CredentialValidationResult with a
>>> CallerPrincipal and optionally one or more groups.
>>>
>>> As mentioned above, the "loop over stores until valid" is probably the
>>>> more common and expected approach, and additionally will also prevent this
>>>> rather major issue.
>>>>
>>>> But then authorization and authentication can't be separated. Something
>>>> which we do al the time.
>>>>
>>>
>>> It can absolutely still be separated. An identity store is free to not
>>> return any groups if it's in the authentication list, while stores
>>> configured for the authorisation list obviously only do authorization.
>>>
>>> In the most common case with 1 identity store, the two operations are
>>> combined. In many cases this is the most efficient way (i.e. only one query
>>> to the backend needed).
>>>
>>> If there are 2 identity stores, 1 can be used for authentication and if
>>> that succeeds the other can be used to retrieve the groups from. Hence the
>>> two operations are separated. See the algorithm sketch I provided a little
>>> bit below. First all stores that can do authentication are consulted, after
>>> that, optionally, all stores that only provide the groups are consulted.
>>>
>>> (btw, taking terminology into account you could say that what we call
>>> "authorization" is not actually authorization here. The list of groups is
>>> just part of the authenticated identity. True authorization happens when
>>> the caller actually attempts to access a resource and the authorization
>>> module checks the permissions needed for that against the groups/roles that
>>> the user is in)
>>>
>>> Kind regards,
>>> Arjan Tijms
>>>
>>>
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> This ventures a bit more into the PAM/JAAS pipeline. Multiple stores
>>>>> (modules in PAM) have indeed been used for this, but this quickly becomes
>>>>> quite complex. I didn't quite understood your exact use case before (I
>>>>> thought you meant that 1 store had a limit on the domains used), but I now
>>>>> understand you want one store to limit the outcome of another existing
>>>>> store that was processed earlier in the pipeline.
>>>>>
>>>>> Given the PAM/JAAS pipeline difficulties, maybe we should use
>>>>> interceptors / decorators for this instead of the multi store feature? Both
>>>>> interceptors and decorators can do something before and after the validate
>>>>> method, and could thus either limit based on the incoming credential (look
>>>>> at incoming caller name), or it could limit based on the returned
>>>>> validation result.
>>>>>
>>>>> Alternatively some event with veto power could also be used here (and
>>>>> such event can then also be used to limit login-attempts by a single
>>>>> caller, or just prevent a specific caller from logging in when you can't
>>>>> remove the user from the source, such as with OAuth).
>>>>>
>>>>> (as an anecdote, years ago I tried to do a remember-me like feature
>>>>> using the PAM pipeline, where I thought of just adding another
>>>>> authentication module in a similar multi-module pipeline as we're using
>>>>> here that would remember the outcome of the module that came before it, but
>>>>> this just didn't work)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> So probably we need some configuration to be able to combine both use
>>>>>> cases in one algorithm.
>>>>>>
>>>>>
>>>>> As for the algorithm, what about something along the lines of:
>>>>>
>>>>> 1. Loop over all stores in order of priority that are configured to
>>>>> authenticate using the exact same initial credential until one succeeds. If
>>>>> one succeeds, break the loop.
>>>>>
>>>>> 3. If we have a valid outcome, seed the group collection with the
>>>>> groups from the store that succeeded and take the caller principal from
>>>>> that store. If we don't have a valid outcome, return immediately from the
>>>>> handler.
>>>>>
>>>>> 4. Loop over all stores that are set to authorize using the caller
>>>>> principal from the previous step. From each such store, add the groups it
>>>>> returns to the group collection.
>>>>>
>>>>> 5. As the final result, return the caller principal and the combined
>>>>> groups.
>>>>>
>>>>>
>>>>> For adding the groups to the group collection, it's not even really
>>>>> necessary to check any status. If no groups are available the empty list
>>>>> will be returned, which we can just add to the collection unconditionally.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> Kind regards,
>>>>> Arjan Tijms
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> *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/msgid/jsr375-experts/CAL%2Bwt-5Q
>>>>>> mUqZiZ9V2uTYXAE-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.
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>