users@javaee-security-spec.java.net

[javaee-security-spec users] Re: Review feedback on the Early Draft from the NLJUG

From: Guillermo González de Agüero <z06.guillermo_at_gmail.com>
Date: Thu, 13 Apr 2017 16:56:47 +0000

Hi,

El jue., 13 de abril de 2017 18:29, arjan tijms <arjan.tijms_at_gmail.com>
escribió:

> Hi,
>
> First of all sorry for the late reply. This mail seemed to have missed my
> inbox completely.
>
> On Wed, Apr 5, 2017 at 3:53 PM, Maurice de Chateau <maurice_at_jpoint.nl>
> wrote:
>
>>
>> *Overall*
>> Q: There is no functionality present for managing identities in the
>> IdentityStore, e.g. for allowing new users to sign up in an application and
>> for having authorizations given or withdrawn from within an application. Is
>> this foreseen for a future version, or will this be left to non-standard
>> extensions?
>>
>
> It has been discussed extensively and the initial proposal even had a
> rather large interface to encompass this.
>
> However, in the majority of cases (that we found) the application knows
> what store is being used. I.e. the IdentityStore wraps a UserService,
> roughly as follows;
>
> @ApplicationScoped
> public class MyAppIdentityStore implements IdentityStore {
>
> @Inject
> private UserService userService;
>
> @Override
> public CredentialValidationResult
> validate(UsernamePasswordCredentialcredential) {
> return validate(() -> userService.getByEmailAndPassword(
> credential.getCaller(), credential.getPasswordAsString()));
> }
>
> // ...
> }
>
> So there the app already knows that users can be added via its own
> UserService, and the IdentityStore interface is, basically, a way to let
> the container interact with that.
>
> Now the other way around, that the container configured an unknown
> identity store, and that the application wants to add users to this unknown
> store in a portable way, was not specified. The reason, as with quite a lot
> of things, is related to available resources.
>
>
>
>
>
>
>> Q: Chapter 3 mentions user groups, and chapter 4 mentions roles for users
>> – however the relationship between these two concepts is not described
>> anywhere. Should the mechanism by which this relation (i.e. users/groups
>> mapping to roles/permissions) is configured be standardized too, or will
>> this be left to non-standard extensions?
>>
>
>
> This concerns the (in)famous group to role mapper.
>
> See:
>
> * https://java.net/jira/browse/JAVAEE_SECURITY_SPEC-8
> * https://java.net/jira/browse/JAVAEE_SECURITY_SPEC-27
>
> And for some more background:
>
> *
> http://arjan-tijms.omnifaces.org/2014/12/java-ee-authorization-jacc-revisited.html
>
> We absolutely wanted to have this covered, but unfortunately have not done
> so till this far and now we're running out of time pretty badly. I'd still
> like to have the most simple version of this covered though, which is
> standardising the default (1:1) mapping of groups to role.
>
>
>
>
>>
>> *Chapter 1 – Concepts*
>> Q: Why is the list, as proposed on the java.net project page, not used
>> here as a starting point?
>> Q Add terms/abbreviations that are frequently used on the mailing list?
>> E.g. SAM, HAM.
>>
>
> Certainly good idea to add these ones.
>
>
>
>>
>> *Chapter 2 – Authentication Mechanism*
>> Q: Will the HttpAuthenticationMechanism (or a similar) interface become
>> available for other inbound traffic handling container types? If not, why
>> not?
>>
>
> It's indeed the idea that an interface like HttpAuthenticationMechanism
> could become available later for other types of security influx. Currently
> it's only HttpAuthenticationMechanism since the web layer was prioritised,
> and for the Security API 1.0 we wanted to go for the lowest hanging fruit
> first.
>
>
>
>>
>> *Section 2.1 – Introduction*
>> Q: “… through the process of authentication”: isn’t this step actually
>> called identification? (and, as stated correctly afterwards, authentication
>> is the presenting of proof of identity)
>>
>
> Note, will take a look at this ;)
>
>
>
>>
>> *Section 2.2 – Interface and Theory of Operation*
>> Q: The last method is called cleanSubject(), yet the term “subject” isn’t
>> used anywhere else in the specification. Can another, already used term be
>> applied here as well, or is it a significantly different entity?
>>
>
> cleanSubject() is indeed a debatable term. Even to the point I already put
> it up for discussion a while ago, but few people on the list responded to
> this back then, so I left it as it was.
>
> The term comes from the ServerAuthModule (the SAM), which is the lower
> level artefact the HttpAuthenticationModule indirectly builds on.
>
> The cleanSubject() method responds to the HttpServletRequest#logout method
> call, so a more logical name could be logout as well.
>

What about clearCredentials()? cleanSubject sounds rather low level. I
guess posting on Twitter could bring some ideas.

Then again, "logout' kinda assumes that there's also a "login" method, yet
> there's isn't one. There's HttpServletRequest#authenticate that causes
> validateRequest to be called. There *is* an HttpServletRequest#login, which
> quite unfortunately isn't supported at the moment, but the idea would have
> been that it went straight to the IdentityStore (by-passing the
> authentication mechanism).
>

>
>
>> Q: How do the calls to these methods fit in the lifecycle of a client
>> session? Could this be illustrated in the specification?
>>
>
> These calls and how they fit in both the request processing and the http
> session (which I assume is what is meant with "client session" here) are
> specified by the JASPIC specification, which is referenced by the Java EE
> Security specification. I think though we could make this a little bit more
> clear.
>
>
>
>>
>> *Section 2.3 – Installation and Configuration*
>> Q: Would it be clearer what is meant by “enabled bean” and “normal
>> scoped” by referring to the defining sections in CDI specification?
>>
>
> Good point, noted.
>
>
>
>> Q: Why is it not specified how the configuration over which mechanism is
>> to be done?
>>
>
> I'm not 100% sure what is meant here. The spec provided authentication
> mechanisms are installed (enabled) and configured by the *Definition
> annotations. E.g. BasicAuthenticationMechanismDefinition. That annotation
> contains the configuration, e.g. the name of the HTTP Basic realm.
>
>
>
>
>>
>> *Chapter 3 – Identity Store*
>> Q: The concept of multiple identity stores is introduced, but are there
>> restrictions that should apply to each single store? Such as uniqueness of
>> each identity entry within that store, or at least consistency of which
>> entry is used for validation or returning groups for upon a given enquiry?
>>
>
> There are no restrictions at this point in time enforced by the spec for
> uniqueness of entries. In general this may be impossible, as a store could
> potentially back an external provider that has no option to list the
> identities it contains.
>
> There are facilities for marking some stores to be used for validation,
> and other stores to be used for returning groups.
>
> Also, the spec anticipates that the default algorithm may not be
> sufficient for all use cases, and allows this to be replaced fully by means
> of providing a (CDI) alternative for the IdentityHandler.
>
>
>
>
>>
>> *Section 3.2.1 – Validating Credentials*
>> Q: Wouldn’t it be better to indicate the validate() method as MANDATORY
>> for IdentityStores that declare themselves to have AUTHENTICATION or BOTH
>> capability (instead of OPTIONAL)?
>>
>
> These concepts are somewhat orthogonal. The default algorithm is to
> consult all stores that have been set to AUTHENTICATE or BOTH until one
> succeeds. This roughly corresponds to JAAS/PAM's OPTIONAL, but is not fully
> equivalent.
>
> Consult all until one succeeds is what's done by some other popular
> security frameworks as well, e.g. Spring Security.
>
> Using a custom IdentityHandler and keeping the metadata for which store is
> MANDATORY or OPTIONAL, etc "somewhere" third party code can basically
> emulate JAAS/PAM if it wants.
>
>
>> Q: Instead of providing a status value (or called “error code” in the
>> review section), shouldn’t the signature of the method indicate that
>> specific exceptions can be thrown in exceptional cases (such as the failure
>> causes mentioned in the review section)
>>
>
> The question is whether failure in authentication is really an exceptional
> case and thus warrants an exception instead of a code. I'm not sure about
> the term "error code". In JASPIC it's called an AuthStatus, hence the
> original wording used "status".
>
> So if the questions is; should the FAILURE status code be an exception?
> Then the answer is likely "no, I personally don't think so".
>
> However, if the question is more about "Should the method list the runtime
> exceptions it can throw", then the answer is that it probably should.
>
>
>
>
>>
>> *Section 3.2.2 – Retrieving Caller Information*
>> Q: Wouldn’t it be better to indicate the getGroupsByCallerPrincipal()
>> method as MANDATORY for IdentityStores that declare themselves to have
>> AUTHORIZATION or BOTH capability (instead of OPTIONAL)?
>>
>
> This is a very subtle one.
>
> Initially it's "no", since having groups is and always has been optionally
> in Java EE. A caller can be authenticated, yet not be in any group.
>
> So it's perfectly fine for identity stores to return no groups for a given
> caller principal.
>
> But... the subtlety here is that we may wish to distinguish between the
> store not knowing about the caller, and therefor not returning anything,
> and the store knowing the caller and having determined as a fact it has no
> groups.
>
>
>
>
>>
>> *Section 3.2.4 – Handling Multiple Identity Stores*
>> Q: How are different Credentials, spread over multiple identity stores,
>> for the same user correlated with a single CallerPrincipal (which currently
>> only contains a ‘name’ field)? How can it be prevented that Credentials for
>> different users are correlated to the same CallerPrincipal? How can such
>> consistency be enforced (or even be maintained)?
>>
>
> The spec does not enforce this. It's, at least for now, an application
> responsibility to ensure that if it wishes to use multiple identity store
> that they are compatible with each other.
>
> Look at if from a different perspective; if the application (or its ops
> team) configures a single identity store, it's their responsibility to
> ensure that store matches the application. I.e. one would not likely
> configure an identity store with public users in it for a company's
> external web app, to an internal office application.
>
> This is not unheard of, e.g. if I'm not mistaken then in JBoss/WildFly one
> can configure multiple stores for the certificate authentication mechanism.
> One can only validate the certificate, where the other can only provide the
> groups. There are, as far as I know, no specific framework facilities to
> make sure these two match.
>
> If an application so wishes the single name field can always encode
> additional information. It's an opaque string that each store and if needed
> the encompassing handler can interpret it in any way it likes.
>
>
>
>> Q: Shouldn’t the remark made in section 3.3, that equal priorities lead
>> to an undefined calling order, be moved to this section? It seems more
>> appropriate here.
>>
>
> Good point ;)
>
>
>> Q: The reasoning behind why the identity stores are filtered for the two
>> steps is missing (or implicit) from the process description. What is it,
>> and should it be added to the section for clarification?
>>
>
> The filtering is so that step 1 can easily consult all the stores that do
> AUTHENTICATE and step 2 the AUTHORISE ones. But an (independent)
> implementation does not necessarily have to do it this way.
>
>
>> Q: Shouldn’t the case where no VALID response was received, but errors
>> did occur (as described in the third bullet of the review section), result
>> in an exception being thrown?
>>
>
> I've to take a better look at this tbh, but in general if with "error" an
> exception is meant then it would abort the entire algorithm right away. A
> failure status code would mean the credential did not validate and is not
> considered an exception, and would result in an overal failure return.
>
>
>
>>
>> *Section 3.3 – Installation and Configuration*
>> Q: Ad review section: Isn’t it confusing or possibly even just wrong to
>> accept group data from an authentication-only identity store? Isn’t it so
>> that the validation type is under the control of the application, while the
>> process/implementation through which the group data is returned from such a
>> store may not be?
>>
>
> Yes & yes ;) Accepting group data from an authentication-only store is
> wrong and will be rectified.
>
> And indeed, the validation type is under the control of the application,
> but the internals of the store may not be, or even if it's application
> controlled it may be that the same store is used for 2 different situations.
>
> Btw, a few cosmetic changes will be made to the authentication type,
> basically it will become a return type of Set, and AUTHENTICATE and
> AUTHORISE will be called VALIDATE and GROUPS respectively.
>
>
>
>>
>> *Section 3.4 – Annotations and Built-in IdentityStore Beans*
>> Q: What is the reasoning behind the various default values for priority()
>> as used in the annotations? Why is the order defined like this?
>>
>
> I'm not sure, but great catch! I guess it may be a leftover from a quick
> initial test and was not adjusted afterwards.
>
>
>
>>
>> *Section 4.4 – Relationship to Other Specifications*
>> Q: Would it be a goal to have the existing ‘alternatives’ in the other
>> specifications be deprecated for Java EE 9?
>>
>
> The isCallerInRole etc methods from the securityContext could indeed be
> deprecated, but there's always the delicate issue of specs wanting to be
> both totally independent from Java EE and part of Java EE.
>
> In JSF for instance I have deprecated the native bean facility (in favour
> of the CDI one), but if Servlet would want to deprecate let alone remove
> HttpServletRequest#isUserInRole is questionable.
>
>
> Thanks a bunch for the review and detailed questions. Much appreciated :)
>
> Kind regards,
> Arjan Tijms
>
>
>
>
>
>
>>
>> *Typos in the specification:*
>>
>>
>> Section 2.1 – Introduction In the third paragraph, first line: “… the
>> iteraction...” should be “… the interaction...”
>>
>> Third paragraph, third line: “… the construction an...” should be “… the
>> construction of an…”
>> Section 2.2 – Interface and Theory of Operation Page 5, summary of
>> cleanSubject(): “… the logout method...” should be “… the logout()
>> method...”
>>
>> Page 5, paragraph directly after the summary: redundant use of “for
>> example” (twice), where “etc.” of “such as...” is used
>> Section 2.4 – Relationship to other specifications Second paragraph,
>> third line: “… method of servlet filter…” should be “… method of a servlet
>> filter...”
>>
>> Second paragraph, fifth line: “… validateRequest…” should be “…
>> validateRequest()...”
>>
>> Third paragraph, first line: “… build-in...” should be “… built-in…”
>> Section 3.1 – Introduction Third paragraph: “A primary advantage...”
>> should be “The primary advantage...”(?)
>>
>> Fourth paragraph, second line: “… passed in to it...” should be “… passed
>> into it...”
>> Section 3.2.1 – Validating Credentials Page 9, section under review
>> section: missing _ before “validate(Credential)_” to make it italic
>> Section 3.2.4 – Handling Multiple Identity Stores Second paragraph,
>> fourth line: “… qualier…” should be “… qualifier...”
>>
>> Page 12, one-but-last line: “… passed in to this method...” should be “…
>> passed into this method…”
>> Section 3.4 – Annotations and Built-in IdentityStore Beans Page 15,
>> mid-way: “All of all these beans...” should be “All of these beans...”
>> Chapter 4 – Security Context Various instances on pages 17 and 18:
>> “SecurityContect” should be “SecurityContext”
>> Section 4.2 – Testing for Caller Data Last paragraph, second line: “…
>> role determinatino...” should be “… role determination...”
>>
> Regards,

Guillermo González de Agüero

>
>>