On Fri, Oct 30, 2009 at 2:29 PM, Marvin Addison <[email protected]>wrote:
> I would like to propose a consistent strategy for CAS logging and > exception handling in core CAS operations: > > 1. Log at DEBUG level the entry into all CentralAuthenticationService > methods. > If anything they should most likely be trace, which is what they are in CAS4 (we apply it via Aspects). CAS4 actually is slightly more intelligent (we do TRACE for just about every method other than setters) and INFO for the main interfaces. > 2. Log error/failure conditions at INFO or higher. > That's what we do. Though people have differing definitions of error/failure conditions. ;-) We've actually found the easiest way to diagnose any problem we have is to actually just look at the audit data provided by Inspektr. Maybe its just the types of issues we encounter. Most of our issues come in with users attached vs. service tickets. I.e. user XXXX said they had a problem. So we look up user XXXX in our audit logs and go well their authentication failed so they just kept typing their password in wrong. Or, well, myRutgers validated the ticket at that time, so its not an issue are on our end. > 3. Place detailed information about failure conditions into > exceptions so it may be communicated to logs and/or services; ideally, > both. > You example below is partially incorrect. Each of those ticket exceptions has a code associated with it. That code is mapped and internationalized to an error message listed in the CAS specification which is sent back to the client (which I'm sure, despite the fact that you state error messages aren't sent back to the client, you've seen these ;-)). The errors are listed here: http://developer.jasig.org/source/browse/jasigsvn/cas3/trunk/cas-server-webapp/src/main/webapp/WEB-INF/classes/messages.properties?r=46974 There are also corresponding ones in most if not all of the other messages files for the various languages. For the most part I believe people have been happy with the error messages returned. The biggest one had always been the one about service urls not matching so we had augmented that one to display both between quotes so that you can see any differences (such as whitespace). Cheers, Scott > > There are cases for all 3 at present in the codebase, but there is > inconsistency. I'm confident that consistent adherence to the above > strategy would benefit troubleshooting. In particular, it would > greatly benefit diagnosis of service ticket validation failures, which > is difficult at present. The detailed causes of failures are logged, > but not included in the exception: > > if (serviceTicket == null) { > if (log.isDebugEnabled()) { > log.debug("ServiceTicket [" + serviceTicketId > + "] does not exist."); > } > throw new InvalidTicketException(); > } > > I imagine most folks do what we do and put org.jasig.cas at INFO, and > completely miss the important information logged at DEBUG that would > point to the core problem that the ticket was not found in the > registry. Note also that this detailed information never makes it to > ServiceValidateController via the exception, and is therefore not > communicated in the XML service validation response payload. It may > be a design feature to omit ticket failure details to prevent leakage > that could facilitate attacks, but it's my opinion that in most cases > it would be both helpful and neutral with respect to security to > include it. A pattern like the following would support fixing both > shortcomings: > > if (serviceTicket == null) { > final String message = "ServiceTicket [" + serviceTicketId + > "] does not exist."; > log.info(message); > throw new InvalidTicketException(message); > } > > I would really like to see the exception message make it to the CAS > service ticket validation response. If that happens, the above > pattern may be inadequate since it does not accommodate > internationalization of the error message detail. But hopefully it > illustrates the point. > > M > > -- > You are currently subscribed to [email protected] as: > [email protected] > To unsubscribe, change settings or access archives, see > http://www.ja-sig.org/wiki/display/JSG/cas-dev > -- You are currently subscribed to [email protected] as: [email protected] To unsubscribe, change settings or access archives, see http://www.ja-sig.org/wiki/display/JSG/cas-dev
