jasig-cas IRC Logs-2011-08-29

Owned by Apereo Infrastructure
Last updated: Aug 29, 2011Version comment
11 min read

[13:01:40 CDT(-0500)] <apetro> checking in
[13:02:00 CDT(-0500)] <wgthom> hola
[13:02:42 CDT(-0500)] <apetro> drafted a CAS manual Docbook chapter on hardening the CAS server, but I've broken the PDF build of the doco somehow
[13:03:01 CDT(-0500)] <apetro> resorting to manual binary search for the problem (smile)
[13:03:15 CDT(-0500)] <yann2> "hardening"? HA-wise? security-wise?
[13:03:34 CDT(-0500)] <apetro> security-wise
[13:03:41 CDT(-0500)] <yann2> k
[13:03:59 CDT(-0500)] <serac> present
[13:04:00 CDT(-0500)] <apetro> whois yann2
[13:04:11 CDT(-0500)] <yann2> random user (smile)
[13:05:59 CDT(-0500)] <apetro> I'm a little concerned that fighting with XML-content-in-DocBook is going to prove ongoingly painful.
[13:06:13 CDT(-0500)] <apetro> but that's a tangent that perhaps we should table.
[13:06:35 CDT(-0500)] <wgthom> https://wiki.jasig.org/display/CASUM/LPPE+Module+Integration+Notes
[13:06:41 CDT(-0500)] <serac> I'm quite interested in the problem, though. Ping me privately about it if you want.
[13:07:15 CDT(-0500)] <wgthom> post some more notes about the integration exercise there
[13:07:18 CDT(-0500)] <wgthom> posted
[13:08:01 CDT(-0500)] <serac> Can you summarize briefly why there's a need to extend CASImpl?
[13:08:07 CDT(-0500)] <wgthom> yes
[13:08:40 CDT(-0500)] <wgthom> public Principal getPrincipal(String id) {
[13:08:41 CDT(-0500)] <wgthom> Principal principal = ((TicketGrantingTicket) ticketRegistry.getTicket(id)).getAuthentication().getPrincipal();
[13:08:41 CDT(-0500)] <wgthom>
[13:08:41 CDT(-0500)] <wgthom> return principal;
[13:08:41 CDT(-0500)] <wgthom> }
[13:08:58 CDT(-0500)] <serac> Ah that's right – you mentioned that previously and I forgot. sry
[13:09:04 CDT(-0500)] <wgthom> np
[13:09:50 CDT(-0500)] <serac> Why not make TicketRegistry a dependency of this machinery and use the getTicket(id) method there?
[13:11:15 CDT(-0500)] <wgthom> I guess it could. remember I didn't write any of this code. not going to have a lot of background on the approach
[13:11:37 CDT(-0500)] <wgthom> i'm wondering of there is a better approach to get the principal then adding the method to CASImpl
[13:11:40 CDT(-0500)] <battags> that errorProcessor looked like it was fun to configure
[13:12:01 CDT(-0500)] <serac> wgthom: I just recommended an alternative.
[13:12:10 CDT(-0500)] <wgthom> lol
[13:12:13 CDT(-0500)] <serac> (wink)
[13:12:22 CDT(-0500)] <wgthom> yep…good feedback.
[13:12:45 CDT(-0500)] <wgthom> getPrincipal is only called on LdapPwdAuthenticationViaFormAction
[13:13:01 CDT(-0500)] <serac> Figured it was a well-defined point.
[13:13:22 CDT(-0500)] <serac> Just as easy to have a reference to TicketRegistry as CentralAuthenticationService, then, I'd imagine.
[13:13:31 CDT(-0500)] <wgthom> Get the user principal and put it into the scope of the WebFlow so we can use it when we check for password warnings
[13:13:31 CDT(-0500)] <wgthom> context.getFlowScope().put("principal",
[13:13:31 CDT(-0500)] <wgthom> this.centralAuthenticationService.getPrincipal(WebUtils.getTicketGrantingTicketId(context)));
[13:13:57 CDT(-0500)] <serac> Yup.
[13:14:10 CDT(-0500)] <serac> So we're here to talk about reconciling this approach with that in CAS4.
[13:14:12 CDT(-0500)] <serac> battags: Want to lead the discussion on CAS4 approach?
[13:15:31 CDT(-0500)] <battags> I can
[13:15:34 CDT(-0500)] <battags> but I will be sporatic
[13:15:40 CDT(-0500)] <battags> so apologies in advance
[13:15:46 CDT(-0500)] <serac> We'll cope.
[13:15:47 CDT(-0500)] <serac> np
[13:16:02 CDT(-0500)] <battags> and by CAS4 I mean cas3/trunk (wink)
[13:16:19 CDT(-0500)] <serac> Right – good clarification for folks following along at home.
[13:17:25 CDT(-0500)] <battags> the main interface is this at the moment
[13:17:25 CDT(-0500)] <battags> https://source.jasig.org/cas3/trunk/cas-server-support-ldap/src/main/java/org/jasig/cas/server/authentication/GeneralSecurityExceptionTranslator.java
[13:17:39 CDT(-0500)] <battags> ties in with the LDAP authentication handlers AND Spring LDAP
[13:17:54 CDT(-0500)] <battags> to translate the errors from the LDAP server to Java general security exceptions
[13:18:10 CDT(-0500)] <battags> it relies on the fact that we changed AuthenticationHandler API to use exceptions
[13:18:31 CDT(-0500)] <battags> Microsoft AD example: https://source.jasig.org/cas3/trunk/cas-server-support-ldap/src/main/java/org/jasig/cas/server/authentication/MicrosoftActiveDirectoryGeneralSecurityExceptionTranslator.java
[13:18:56 CDT(-0500)] <serac> I don't see anything LDAP-specific in this interface. Seems it could be used for password expiration functionality in arbitrary auth handlers.
[13:19:10 CDT(-0500)] <battags> potentially
[13:19:15 CDT(-0500)] <battags> the only LDAP specific thing is this
[13:19:16 CDT(-0500)] <battags> https://source.jasig.org/cas3/trunk/cas-server-support-ldap/src/main/java/org/jasig/cas/server/authentication/GeneralSecurityExceptionTranslatorAuthenticationErrorCallback.java
[13:19:56 CDT(-0500)] <battags> these are more LDAP specific though
[13:19:56 CDT(-0500)] <battags> https://source.jasig.org/cas3/trunk/cas-server-support-ldap/src/main/java/org/jasig/cas/server/authentication/GeneralSecurityExceptionTranslator.java
[13:20:03 CDT(-0500)] <battags> if you look at the exceptions we translate
[13:20:17 CDT(-0500)] <battags> I thought about making it super generic
[13:20:21 CDT(-0500)] <battags> and I think I blew my mind
[13:20:27 CDT(-0500)] <battags> some may argue I haven't been right since
[13:21:07 CDT(-0500)] <battags> so I don't know how it fits with the LDAP policy stuff that someone mentioned OpenLDAP does
[13:21:30 CDT(-0500)] <wgthom> sorry had a call…catching up
[13:21:39 CDT(-0500)] <serac> battags: I believe it goes something like this:
[13:22:13 CDT(-0500)] <serac> scratch that
[13:22:14 CDT(-0500)] <serac> No idea.
[13:22:20 CDT(-0500)] <battags> the suspense is killing me
[13:22:32 CDT(-0500)] <serac> I shouldn't make uneducated guesses.
[13:22:35 CDT(-0500)] <serac> So I stopped.
[13:22:48 CDT(-0500)] <serac> I have only a vague idea of how OpenLDAP will work.
[13:23:10 CDT(-0500)] <battags> what we need is an OpenLDAP expert
[13:23:24 CDT(-0500)] <battags> maybe drew is
[13:23:37 CDT(-0500)] <serac> I have one here, but we've only talked briefly about this stuff.
[13:23:53 CDT(-0500)] <battags> how does he or she feel about contributing to an open source project? (smile)
[13:24:25 CDT(-0500)] <serac> Good. Just bugged him about it.
[13:24:53 CDT(-0500)] <battags> so I think there are two parts to the LDAP support
[13:24:59 CDT(-0500)] <battags> (1) is actually blocking authentication
[13:25:04 CDT(-0500)] <battags> err ticket creation
[13:25:12 CDT(-0500)] <battags> and (2) is warning messages
[13:25:30 CDT(-0500)] <battags> I think the LDAP support in CAS4 solves (1) and it might be easy enough to back port
[13:25:40 CDT(-0500)] <battags> I think we get into sketchy situations trying to support (2)
[13:25:53 CDT(-0500)] <battags> so CAS4 does have support for warnings
[13:26:01 CDT(-0500)] <battags> but its a pretty extensive API change
[13:26:17 CDT(-0500)] <battags> which wouldn't fit in anyone's definition of a minor release
[13:26:20 CDT(-0500)] <battags> and I have a pretty liberal definition
[13:26:21 CDT(-0500)] <serac> I have a hunch folks are as interested in (2), if not more so, than (1).
[13:26:56 CDT(-0500)] <serac> I do like the generality of the approach.
[13:27:32 CDT(-0500)] <serac> Change the javax.naming exceptions to Exception and you've got a generic approach for any handler provided you can create an exception to describe the problem.
[13:27:46 CDT(-0500)] <wgthom> scott do you have a good understanding of the contributed model and how it differs from what you've done in cas4
[13:28:01 CDT(-0500)] <battags> I did at the time
[13:28:05 CDT(-0500)] <battags> my memory only retains so much though
[13:28:49 CDT(-0500)] <wgthom> did you see this: https://wiki.jasig.org/display/CASUM/LPPE+Module+Integration+Notes
[13:29:48 CDT(-0500)] <wgthom> At the moment I generally think of the module as breaking down into 3 chunks
[13:30:16 CDT(-0500)] <wgthom> (0) translating ldap messages
[13:30:33 CDT(-0500)] <wgthom> (1) possibly blocking ST creation
[13:30:38 CDT(-0500)] <wgthom> (2) UX
[13:30:40 CDT(-0500)] <battags> your lists start with (0)? (smile)
[13:30:47 CDT(-0500)] <serac> haha
[13:30:55 CDT(-0500)] <wgthom> something like that anyway
[13:31:08 CDT(-0500)] <wgthom> you're saying cas4 has support designed for all of that?
[13:31:13 CDT(-0500)] <serac> How do we do (1)?
[13:31:26 CDT(-0500)] <wgthom> and that 0 and 1 might be able to backpoint under a minor release ?
[13:31:27 CDT(-0500)] <serac> You're already authenticated by the point of ST creation.
[13:31:41 CDT(-0500)] <battags> wgthom: I'll say that we have better support. I won't say its perfect without more eyes
[13:31:51 CDT(-0500)] <wgthom> ok
[13:31:57 CDT(-0500)] <battags> at a high level we need the following though
[13:32:03 CDT(-0500)] <battags> and I typically start counting from one (smile)
[13:32:22 CDT(-0500)] <battags> (1) translate authentication errors into something the CAS server can respond to generically
[13:32:56 CDT(-0500)] <battags> (2) support displaying information to the user about their account generically
[13:33:09 CDT(-0500)] <battags> I've only got two items
[13:33:16 CDT(-0500)] <battags> because they're really high level
[13:33:21 CDT(-0500)] <wgthom> ok
[13:33:32 CDT(-0500)] <serac> (1) will be a alot more straightforward if we make the change to the AH API to throw instead of return bools.
[13:34:04 CDT(-0500)] <battags> that is in 4 and could be backported depending on our definition of minor
[13:34:37 CDT(-0500)] <serac> I believe we can do it in a non-breaking way. If yes after some research, +1 for putting that into 3.5.
[13:35:11 CDT(-0500)] <serac> Once we do that it really reduces down the the handlers throwing precise exceptions about the cause of auth failure.
[13:35:32 CDT(-0500)] <apetro> I'm missing something. AuthenticationHandler already throws AuthenticationException . https://source.jasig.org/cas3/branches/cas-3_4_x_maintenance/cas-server-3.4.2/cas-server-core/src/main/java/org/jasig/cas/authentication/handler/AuthenticationHandler.java
[13:35:32 CDT(-0500)] <battags> for (1) though we also need to have a generic API for translation
[13:35:50 CDT(-0500)] <battags> but it also returns boolean
[13:36:15 CDT(-0500)] <apetro> indeed. It can return false if it has nothing intelligent to say. It can throw an AuthenticationException if it does have something intelligent to say.
[13:36:24 CDT(-0500)] <serac> No.
[13:36:25 CDT(-0500)] <serac> No.
[13:36:26 CDT(-0500)] <serac> No.
[13:36:30 CDT(-0500)] <battags> we'd also have to essentially mirror the GeneralSecurityException chain
[13:36:33 CDT(-0500)] <apetro> So the change is more to, within the current APIs, start to implement AuthenticationHandler to say something intelligent.
[13:36:57 CDT(-0500)] <apetro> * @throws AuthenticationException An AuthenticationException can contain
[13:36:57 CDT(-0500)] <apetro> * details about why a particular authentication request failed.
[13:36:58 CDT(-0500)] <serac> Implementers ought not have to guess what to do to signal success.
[13:37:18 CDT(-0500)] <apetro> there's no guessing. Returning true (and not failing with an exception) signals success.
[13:37:19 CDT(-0500)] <serac> Throw a meaningful exception to signal failure, otherwise simply return.
[13:37:39 CDT(-0500)] <serac> Returning exclusively true and void are the same thing.
[13:37:49 CDT(-0500)] <serac> Except the latter is infinitely clearer.
[13:38:05 CDT(-0500)] <apetro> indeed. But leaving it alone breaks zero existing AuthenticationHandler implementations. The margin of clarity here is too small to break an API over.
[13:38:31 CDT(-0500)] <serac> Totally disagree.
[13:38:47 CDT(-0500)] <serac> We can ensure API backward compatibility via other means.
[13:39:22 CDT(-0500)] <apetro> AuthenticationHandler developers have to do heroics to signal interesting failures.
[13:39:23 CDT(-0500)] <serac> We've already had this discussion for CAS4 and agreed to do void throws GeneralSecurityException.
[13:39:37 CDT(-0500)] <apetro> Returning false is a simple way to signal that they have not done those heroics.
[13:39:52 CDT(-0500)] <serac> It's not heroic to force implementers to be specific about the cause of failure.
[13:39:55 CDT(-0500)] <apetro> Ah, yes. In a revolution, I agree making this margin of clarity is worth having for the API change.
[13:40:01 CDT(-0500)] <apetro> In evolution, I don't.
[13:40:14 CDT(-0500)] <serac> Again: we can ensure API backward compatibility via other means.
[13:40:48 CDT(-0500)] <apetro> I don't expect AuthenticationHandler API to change in a 3.x series, and this clarity isn't by itself worth the revolution to CAS 4.
[13:40:49 CDT(-0500)] <serac> Simply create a second method signature and delegate to it with all handlers that ship with CAS.
[13:41:39 CDT(-0500)] <serac> And mark it @Deprecated to prepare for its dissolution in CAS4.
[13:42:04 CDT(-0500)] <serac> That sounds both polite and reasonable.
[13:42:52 CDT(-0500)] <apetro> it feels like complexity that's not worth its weight.
[13:43:22 CDT(-0500)] <apetro> but in order to make best use of our time... besides the tradeoffs between return true and return void and the tradeoffs of changing AuthenticationHandler API or not, what else is there to discuss here?
[13:43:23 CDT(-0500)] <serac> There is huge value here in terms of expressiveness.
[13:43:25 CDT(-0500)] <battags_> sorry I think my connection died
[13:43:38 CDT(-0500)] <joachimf> hi everyone
[13:43:44 CDT(-0500)] <wgthom> howdy
[13:43:46 CDT(-0500)] <serac> Use of GeneralSecurityException is powerful.
[13:44:22 CDT(-0500)] <battags_> apparently my last comments didn't make it in
[13:44:30 CDT(-0500)] <serac> nope
[13:44:58 CDT(-0500)] <wgthom> for what it's worth the contributed module doesn't modify the AH API
[13:45:25 CDT(-0500)] <serac> I'm trying to reconcile the two approaches and making the change is one way.
[13:45:29 CDT(-0500)] <serac> Not the only way I suppose.
[13:45:36 CDT(-0500)] <apetro> wgthom, what does the contributed module do?
[13:46:01 CDT(-0500)] <apetro> how does it signal information gleaned from the AuthenticationHandler for use in the web flow, for use in the UI, when the authentication doesn't fail outright?
[13:46:03 CDT(-0500)] <wgthom> trying to figure that out! (smile)
[13:46:13 CDT(-0500)] <battags_> I want to pause for a moment because there appears to be resistance to making some of the CAS4 changes available sooner
[13:46:58 CDT(-0500)] <wgthom> in the forked BindLdapAuthNHandler:
[13:46:59 CDT(-0500)] <wgthom> } catch (final Exception e) {
[13:46:59 CDT(-0500)] <wgthom> String details = e.getMessage();
[13:46:59 CDT(-0500)] <wgthom> this.log.debug("LDAP server returned exception message: " + details);
[13:46:59 CDT(-0500)] <wgthom> // Call Treatment chain
[13:47:00 CDT(-0500)] <wgthom> errorProcessor.processErrorDetail(details);
[13:47:01 CDT(-0500)] <wgthom> // if we catch an exception, just try the next cn
[13:47:01 CDT(-0500)] <wgthom> } finally {
[13:47:02 CDT(-0500)] <wgthom> LdapUtils.closeContext(test);
[13:47:02 CDT(-0500)] <wgthom> }
[13:47:03 CDT(-0500)] <wgthom> }
[13:48:02 CDT(-0500)] <serac> battags point is important
[13:48:43 CDT(-0500)] <serac> I thought there was general agreement to backport or pull in API changes that are valuable or otherwise make sense for 3.5.
[13:49:16 CDT(-0500)] <battags_> wgthom: just to follow up on you, errorProcessor throws exceptions
[13:49:43 CDT(-0500)] <wgthom> yes public boolean processErrorDetail(final String in_detail) throws AuthenticationException {
[13:49:55 CDT(-0500)] <apetro> I resist paying the cost of a revolution without commensurate benefits. Changing AuthenticationHandler for CAS 4 makes sense. It's there that we should consider interesting approaches to backwards compatibility via adaptor. But I expect the most-written-to API in CAS to change between 3 and 4. I don't expect that change in 3.5, and this feature by itself isn't worth the revolution to CAS 4.
[13:50:12 CDT(-0500)] <serac> So it looks a lot like CAS4 approach conceptually – generate and translate exceptions.
[13:50:46 CDT(-0500)] <apetro> indeed. Piggybacking on the existing API support for throwing AuthenticationException.
[13:52:03 CDT(-0500)] <serac> This looks like a great opportunity for AH to simply throw a meaningful exception on the real cause of auth failure.
[13:52:33 CDT(-0500)] <apetro> AH is already allowed to simply throw a meaningful exception on the real cause of the failure, when it has meaning to communicate
[13:53:24 CDT(-0500)] <serac> That begs the question: when it it not acceptable to communicate precisely?
[13:55:51 CDT(-0500)] <wgthom> one way to look at the effort is what is the smallest change necessary. the contrib code doesn't modify the AH api…so we not that is not strictly neccesaary
[13:56:24 CDT(-0500)] <wgthom> one might still elect to take the opportunity for change for other reasons no doubt
[13:56:25 CDT(-0500)] <serac> Sure.
[13:56:38 CDT(-0500)] <serac> wgthom: That's my perspective.
[13:56:58 CDT(-0500)] <serac> This is one concrete use case for making the CAS4 AH API changes sooner than later.
[13:56:59 CDT(-0500)] <wgthom> almost out of time...
[13:57:33 CDT(-0500)] <wgthom> yes, but would it be consistent with a minor release
[13:57:47 CDT(-0500)] <serac> If we preserve backward compatibility, yes.
[13:57:54 CDT(-0500)] <wgthom> and reasonbale to take in given limited resources, time, etc
[13:58:00 CDT(-0500)] <serac> I've provided what I hope is areasonable proposal for that.
[13:59:22 CDT(-0500)] <serac> I will try to make time to provide a patch showing the changes to the bind handlers with the CAS4 GeneralSecurityExceptionTranslator.
[13:59:38 CDT(-0500)] <battags_> @serac I can work with you on that
[13:59:38 CDT(-0500)] <apetro> I hear the proposal, but I continue to believe it incurs more complexity than it is worth. It's better to leave this API alone until a revolutionary change in APIs is more generally justified.
[13:59:41 CDT(-0500)] <serac> If I don't get to it by Sunday, then go ahead.
[13:59:48 CDT(-0500)] <wgthom> ok. in the meantime I'll continue to pare down that I have and understand what's there
[13:59:49 CDT(-0500)] <serac> with current approach
[13:59:50 CDT(-0500)] <apetro> My time is up; I thank you for yours.
[14:00:07 CDT(-0500)] <wgthom> hey that's my liine
[14:00:11 CDT(-0500)] <serac> (wink)
[14:00:29 CDT(-0500)] <wgthom> and try to minimize changes…like the CASImpl one
[14:00:36 CDT(-0500)] <serac> Yeah, sounds good.
[14:00:50 CDT(-0500)] <wgthom> thanks for the chat…productive.
[14:01:08 CDT(-0500)] <serac> I just gave myself more work (sad)
[15:10:22 CDT(-0500)] <wgthom> ll
[15:33:37 CDT(-0500)] <apetro> serac so here's an example of what I'm talking about wrt XML in docbook being difficult. https://source.jasig.org/cas3/branches/cas-3_4_x_maintenance/cas-server-3.4.2/cas-server-documentation/src/docbkx/hardening.xml
[15:34:16 CDT(-0500)] <serac> What I was imagining.
[15:35:14 CDT(-0500)] <serac> This may force the issue of sucking in code snippets programmatically by a code reference.
[15:36:23 CDT(-0500)] <serac> Not sure that would be appropriate in this case, though, since the snippets are so brief.
[15:51:47 CDT(-0500)] <wgthom> forked LdapCasImpl gone… (smile)
[15:56:06 CDT(-0500)] <serac> Nice