I just read the Security API EDR 1 draft and had some comments...
Section 2.2 -
It wasn't clear on first reading who is supposed to provide
an implementation of the HttpAuthenticationMechanism interface.
I believe this is an extension point that allows applications to
supply their own authentication mechanism support, right?
Some requirements in this section use "MUST", but these aren't
requirements on implementors of the Security API, they're
"requirements" (and I use that word loosely here) on applications
that provide their own classes implementing HttpAuthenticationMechanism,
right? Really you're just telling them what to do if they want their
class to work as expected, but if they don't do what you recommend
they're not violating any spec requirement, they just have a poor
or unusual implementation. Applications are just users of the
Security API, not implementors of it, and we can't "require" them to
do anything.
At the top of page 5, it says "the methods will be invoked". That
seems like a case where it should say "MUST" since you're really
specifying what an implementation of the Security API is required to do.
Section 3.1 -
Again, it wasn't clear who provides an implementation of IdentityStore.
And again, I believe it's applications.
The IdentityStore/IdentityStoreHandler naming seems a little clumsy.
I would expect that the API that applications *use* would be named
IdentityStore. If an application needs to implement a particular
interface to plug into this mechanism, I would expect them to
implement IdentityStoreProvider or something like that.
Section 3.2.1 -
Returning a CredentialValidationResult where you have to check the
contained Status to see if it's valid seems pretty ugly. It would
be much better if a CredentialValidationResult was always a valid
object, with an exception thrown to indicate a failure and null
returned to indicate "I can't do that".
Section 3.2.2 -
The method name getGroupsByCallerPrincipal could just be getGroups
since it's clear that the argument is a CallerPrincipal and there
aren't other getGroup methods to confuse it with.
On the "unique identifier" issue...
The validate method can return an implementation-specific class
(including an implementation-specific CallerPrincipal class)
with no public constructor. The getGroups method can check that
the expected subclass is passed in, thus knowing that it only
came from a validate call. If necessary, the subclass can contain
any other kind of identification/validation information the
implementation wants, to make sure the principal can't be forged.
For the second issue, a security permission is probably appropriate.
Section 3.2.3 -
Can you use the @Priority annotation from Common Annotations instead
of having it be part of the interface? An annotation for the
ValidationType might work as well. You expect these values to be
constants for a given IdentityStore implementation, right?
Section 3.2.4 -
In the validate() method algorithm, first bullet, first sub-bullet,
only the *first* INVALID response is remembered, right?
Section 3.4 -
First sentence, what exactly is meant by "via configuration"?
When it talks about "caller data", it means data about users, right?
Typo - "All of all" -> "All of"
Section 3.4.1 -
I thought all passwords were required to be char[] instead of String?
Chapter 4 -
I understand that you're proposing to get rid of this. I see how the
caller data would be useful in many contexts, but I don't see how an
app would use the authenticate method; if it has access to the
HttpServletResponse, can't it just use the corresponding Servlet methods?
Typo - "SecurityContect" -> "SecurityContext", in several places
Typo - "provided be" -> "provided by"
I just started looking at the javadocs but my first feedback is that
there are *way* too many packages. 6 packages for less than 40 classes
seems quite excessive. I would be fine with having all these classes
in a single package, or maybe separating the "API" classes from the "SPI"
classes.