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.
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...”
>
>