Hi, all! My update for previous week: 1) Some JASPIC architecture refactoring 2) Implemented both BASIC and DIGEST authentication modules 3) Added integration test for DIGEST module 4) Fixed some small bugs and issues 5) Updated javadocs
What about current status, I started working on FORM authentication module on Friday, however, I have faced some problems. FORM authentication mechanism looks harder than BASIC/DIGEST for me, at least when I looked at current implementation, so I had spent Friday and Monday digging FORM authentication specifications and FormAuthenticator class. I have some commits prepared but they are in very-very draft stage now, so I'd not pushed them yet. Also, I tried to start working on BASIC integration test, but FORM authentication looks like priority now. Thanks, Fjodor 2015-06-26 11:16 GMT+03:00 Mark Thomas <ma...@apache.org>: > On 25/06/2015 20:50, Fjodor Vershinin wrote: > > Hi! > > Fresh set of patches is ready. > > What has been done: > > 1) Added engine name to getVirtualServerName() > > 2) Implemented method for getting roles directly from Realm, > > 3) Authentication provider uses LoginConfig now, which gives us ability > to > > get different options directly from there. > > 4) Added test for validating DIGEST auth module, however one test case is > > ignored, because not implemented yet. > > 5) Updated some javadocs to make them more specific and clear. > > All looks good and patches applied. I added a few comments to some of > the patches. > > Thanks, > > Mark > > > > > > BR, > > Fjodor > > > > 2015-06-23 23:18 GMT+03:00 Mark Thomas <ma...@apache.org>: > > > >> On 23/06/2015 16:50, Fjodor Vershinin wrote: > >>> Hi there! > >>> > >>> > >>>> You still need to address the issue of a unique name for the JASPIC > app > >>>> context. > >>> > >>> I see your point. However, tomcat's implementation of uniqueness is > >> against > >>> JASPIC 1.1 specification. We must somehow document this feature. > >> > >> I'm reading that part of the spec now. > >> > >> Currently Tomcat returns the name of the host object (not necessarily > >> the DNS host name) for ServletContext.getVirtualServerName(). Reading > >> the Servlet spec more carefully, we can change that to > >> engine-name/host-name and still be specification compliant. That would > >> address the uniqueness issue for JASPIC as well as being a better > >> implementation for getVirtualServerName(). > >> > >>>>> 1) I have prepared mechanism for registration embedded JASPIC modules > >>>>> 2) Callback handler is singleton now > >>>>> 3) Implemented JAAS Subject's support (it turned out, that it is > >>>> mandatory). > >>>>> 4) BASIC and DIGEST authenticators has been ported to JASPIC > >>>>> I think these modules need to be carefully refactored though, then I > >> will > >>>>> prepare some tests. > >>>> > >>>> Why do you think these modules need to be refactored? Given the > security > >>>> nature of this code and that what you have currently is largely copied > >>>> directly from the existing implementations, I'd be wary of making any > >>>> changes without a good reason for doing so. > >>> > >>> Yes, we must be very careful with security implementations. However, I > >>> would decouple JASPIC code from authentication algorithms and put them > >> into > >>> separate classes. > >> > >> I'm on the fence on this. I don't see it as a priority unless it is > >> blocking something else. I'd file this under "come back to it if there > >> is time at the end". > >> > >>>> 5) Fixed some bugs in implementation, such as lack of session caching > >>>>> 6) Currently, I am working on some javadoc's, but I'll commit them > >> later. > >>>> > >>>> Remember, little and often is better than a few larger code dumps. The > >>>> recent commits have been fine but I would prefer to see 1 or 2 > commits a > >>>> day rather than a batch of 10+ commits once a week. > >>> > >>> > >>> I agree, however I was intensively using rebase and squashing for > commit > >>> rewriting in order to get "feature per commit". I think it depends on > >>> architectural tasks - currently we have architectural stuff done, so > next > >>> commits will require less rewriting. > >> > >> We don't have to merge into Tomcat until you are ready but it would be > >> nice to see how the work is developing. > >> > >>>> 1) I need some convenient way to get user roles from Realm. I assume, > >> that > >>>>> every Principal is GenericPrincipal, but I guess that's not right. > >>>> > >>>> What for? The best way to handle this depends on why/where that > >>>> information is needed. > >>> > >>> > >>> I need this info in order to construct GenericPrincipal using > callbacks. > >>> Currently, Realm is returning GenericPrincipal, however, implementation > >> is > >>> hidden behind Principal interface. I need to do casting to get > >>> GenericPrincipal object, because Principal doesn't have getRoles() > >> method. > >> > >> I suspect that was the case. Casting is going to be fragile for users > >> with custom realm implementation. I think what is required is a new > >> method on Realm: > >> > >> String[] getRoles(Principal) > >> > >> For the current realms this should be a trivial implementation in > >> RealmBase: > >> - cast to GenericPrincipal > >> - return getRoles() > >> > >> > >>>>> 2) We need find a easy way for configuring embedded JASPIC modules. > For > >>>>> example, form authentication requires login page and error page. I > >> think > >>>>> that these parameters can be passed to JASPIC provider directly, but > >> I'm > >>>>> not sure. > >>>> > >>>> Currently the ContextConfig registers a new TomcatAuthConfigProvider > for > >>>> each web application. > >>>> > >>>> The TomcatAuthConfigProvider creates (lazily) a TomcatAuthConfig. > >>>> > >>>> The TomcatAuthConfig creates (lazily) TomcatServerAuthContext with all > >>>> available modules. > >>>> > >>>> The TomcatAuthConfig then looks up the authentication type obtained > from > >>>> the request and maps it to the right module. > >>>> > >>>> Initialising all the modules when - typically - only one is required > >>>> looks wrong to me. I'd expect the ContextConfig to specify (possibly > >>>> even create and configure) the required modules and pass those to the > >>>> TomcatAuthConfigProvider instance for the web application. > >>> > >>> > >>> Yes, I agree, that it's better solution. I am not sure about > constructing > >>> auth modules in ContextConfig. May be we can pass LoginConfig info into > >>> provider, and construct modules inside. > >> > >> That sounds even better to me. > >> > >> Mark > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: dev-h...@tomcat.apache.org > >> > >> > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > > -- Thanks, Fjodor
