Hi,
On Sat, Apr 1, 2017 at 6:57 AM, Ajay Reddy <areddy_at_us.ibm.com> wrote:
> Here are my initial set of comments/questions for the EDR1 review based on
> the current doc so far. Will continue to review. Thanks for all who have
> worked on this.
>
> 1) Authentication Mechanism:
>
> a) When the auth-method is set to AUTHMECH and the application does not
> provide its own *HttpAuthenticationMechanism* what is the expected
> behavior? Depends on the container (if it provides a system one or not)?
>
Perhaps we could do the following:
1. No login-config ellement at all -> JSR 375 checks for
*HttpAuthenticationMechanism.
*If no *HttpAuthenticationMechanism* found do nothing from the JSR 375
perspective. If *HttpAuthenticationMechanism found, install it (and mandate
or recommend writing a log message about this?)*
2. login-config element with any value other than AUTHMECH -> do nothing
from the JSR 375 perspective
3. login-config element with value AUTHMECH -> JSR 375 checks for
*HttpAuthenticationMechanism. *If no *HttpAuthenticationMechanism* found
throw an exception and abort the startup process
b) The HttpMessageContext description is missing in the doc.
>
The main description is in the JavaDoc, which is part of the spec
regardless of what's in the spec document. I'm not 100% sure when something
is sufficiently spec'ed when in the JavaDoc and when it actually must be in
the spec document as well.
>
> c) What is the difference in the expected behavior when the validate
> returns INVALID vs NOT_VALIDATED? Container dependent or anything that spec
> needs to address? Believe INVALID should be resulting in some re-login
> attempt but NOT_VALIDATED would have to throw an exception.
>
I think you're referring here to the CredentialValidationResult used with
the IdenityStore, aren't you? This one:
public enum Status {
/**
* Indicates that the credential could not be validated
*/
NOT_VALIDATED,
/**
* Indicates that the credential is not valid after a validation
* attempt.
*/
INVALID,
This originally came from Alex. The idea is that NOT_VALIDATED is returned
when the store is not capable of validating the given credential, i.e. has
no knowledge about how to validate it. This happens when for example a
TokenCredential is handed over to an IdentityStore that only knows about
CallerNamePasswordCredentials.
INVALID means the store recognised the credential, and it has decided that
it's an invalid one.
> 2) Identity Store:
>
> a) Since there is no authorization being performed by the identity store
> itself may be use AuthorizationInfo or UserGroups or something similar - as
> pointed out in the review comment.
> b) As the review comment suggests, Set would be more appropriate when
> returning the groups for both methods.
>
True, logically it's indeed a Set. Lists are nevertheless more commonly
used, but as there are 2 members suggesting it to be a Set I believe we
should change it to that, then.
>
> c) For the issue with callerPrincipal not being unique when getting
> groups, the unique identifier approach makes sense.
> d) The simple way to protect the getGroups call is to use a Java 2
> permission (when security manager is enabled) but very few folks enable
> Java 2 security.
>
Indeed, the reason few folks enable it is that enabling the (JavaSE)
SecurityManager checks a lot of things a lot of times. Basically every
input/output, low level socket operations and what have you. This may
severely slow things down. On top of that, quite a lot of code is hard to
get working correctly with the SecurityManager enabled, and worst of all,
potential violations are hard to test for.
That all said, there are already a couple of Java 2 permissions checked
(with security manager enabled) for setting the HttpAuthenticationMechanism
(or rather, the lower level constructs it depends on). If there's
sufficiently consensus about getGroups being sensitive, I guess it would
not hurt that much to add a Java 2 permission check.
> e) The configuration to make the build-it beans available is something
> specific to the containers/servers - i.e outside of the specification?
>
No, this is standardised via the XYZDefinition annotations. E.g.
@DataBaseIdentityStoreDefinition. We were still thinking though how to best
allow for additional vendor specific configuration. Something like
key/values may work, and of course a vendor can always introduce a second
vendor specific annotation that the implementation of that vendor picks up.
> f) Given the concern for Embedded IdentityStore, may be we can make just
> the LDAP IdentityStore as required and others optional for containers to
> implement since anyone can implement their own specific IdentityStore?
>
Though I understand the concern, I don't really think we should make it
optional. As mentioned, anyone can implement their own store anyway, so why
give up the convenience of setting up security initially in a trivial way?
As this proposal came initially from Reza (who's not on the EG but helped a
lot with setting up the JSR initially) we could ask him for his thoughts
here.
> g) For the LDAP IdentityStore annotations, one approach could be to use
> the LDAP filter format when searching the users/groups -something like
> (&(objectClass=person)(objectClass=user))? Wanted to get some thoughts on
> this.
>
> Some examples:
> userFilter="(&(uid=%v)(objectclass=person))" // to search for the user
> using the uid attribute in person ObjectClass.
> groupMemberFilter ="groupOfNames:member;groupOfUniqueNames:uniqueMember"
> // to search for groups for the user using the member and uniqueMember
> attributes in the groupOfNames and groupOfUniqueNames objectclasses
> respectively.
>
> These would be similar to callerQuery/groupsQuery in the database
> annotation.
>
> Also, given the fact that a user is represented in multiple ways in LDAP
> and can log-in using different formats (bob - bob_at_myCompany.com - cn=Bob
> Smith,ou=myCompany,o=us) we need a consistent way to return for the
> programmatic APIs like getUserPrincipal, getRemoteUser,
> getCallerPrincipal,(bob which is the uid - for eg). This might require
> another filter to get that information.
>
> For example (use the uid attribute from the objectClass person)
> userIdMap="person:uid"
>
> These filters can be directly passed to the LDAP servers during search and
> are flexible enough to handle any attribute that one wants to use. This
> would also reduce the # of methods.
>
> Should authenticateOnly and authorizeOnly be just authenticate and
> authorize/authorizeInfo/userGroups?
>
> Should baseDN be bindDN?
>
It all sounds good, but I have to say I'm not a huge LDAP expert. I created
the initial version of the store which just mirrored how I happened to have
set up security via LDAP only a couple of times. I do know there are almost
an endless different ways to represent data in LDAP, so would be absolutely
good if someone with more LDAP knowledge takes a very good look at this.
> 3) Security Context
>
> Not sure how much of the Security Context will be supported in this JSR
> given the resource/time issues that have been mentioned. The api-sec
> includes more methods than listed in the current doc. For example,
>
> a) getAllDeclaredCallerRoles - Expected to return all the assigned roles
> for the caller principal (directly and indirectly through groups) in the
> authorization configuration? If so, since the role to user/group mapping is
> at the application level, this can include additional roles that may not be
> relevant to this web resource - or some kind of filtering is expected?
>
Yes, the filtering is done for each role by checking if the user is
currently in that role. We should carefully craft the specification of what
this exactly is supposed to return, but simply said it are the static
roles, meaning the roles that are declared upfront in the *web* app and are
discovered during startup.
More formally, it are all the WebRoleRefPermission instances that the JACC
specification specifies to be collected, even though the implementation on
request by Will would not actually have to use JACC for this (if I'm not
mistaken the Servlet spec also refers to this collection algorithm, even
though Servlet containers are not required to actually use JACC).
JSR 375 as of now does not distinguish between roles that are available
globally and roles that are only valid in the context of a specific
Servlet. Since (correct me if I'm wrong) roles per Servlet are a Java EE
feature that is extremely little used in practice, this distinction is not
there. It could be added of course, at the cost of some extra complexity in
the API.
Alternatively we could warn that the list of roles is only valid for the
moment it has been requested and from the context which it is requested.
Note this is conceptually not much different from storing the result of a
request.isUserInRole call and re-using that outcome later.
> b) hasAccess* - what web resources are these? Same app only or any web app
> in the container? What are the typical use cases for this?
>
These are the resources as specified by JACC and Servlet specifications for
the resource parameter of the WebResourcePermission. Simply said, they are
the URLs or pages of a web application.
The typical use case is rendering a menu in a web application that only
includes the pages a caller has actually access to. Another use case is
rendering links to pages that the caller does not have access to using a
different color or with a lock symbol etc behind it.
Kind regards,
Arjan Tijms
>
>
> Will have to investigate these more but wanted to understand the usage/use
> cases first.
>
> Thanks for your time.
>
>
> Regards,
> Ajay Reddy
>
>
>