Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/action/TicketRequired.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/action/TicketRequired.java (revision 0) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/action/TicketRequired.java (revision 0) @@ -0,0 +1,33 @@ +package net.sourceforge.stripes.action; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import net.sourceforge.stripes.controller.Ticket; + +/** + * Method-level annotation Indicating that a handler method requires a valid + * transaction {@link Ticket} before access is granted. The "ticket" is a + * parameter containing a unique identifier, generated by FormTag. + * + * @see net.sourceforge.stripes.controller.TicketInspector + * @author Andrew Jaquith + */ +@Retention(RetentionPolicy.RUNTIME) +@Target( { ElementType.METHOD }) +@Documented +@Inherited +public @interface TicketRequired { + /** + * Returns the number of seconds before the ticket expires, in seconds. If + * this attribute is not specified, the ticket will expire by default in 1 + * hour (720 seconds). + * + * @return the timeout for the ticket, in seconds + */ + int expires() default 720; +} Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/config/DefaultConfiguration.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/config/DefaultConfiguration.java (revision 1114) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/config/DefaultConfiguration.java (working copy) @@ -37,6 +37,7 @@ import net.sourceforge.stripes.controller.LifecycleStage; import net.sourceforge.stripes.controller.NameBasedActionResolver; import net.sourceforge.stripes.controller.ObjectFactory; +import net.sourceforge.stripes.controller.TicketInspector; import net.sourceforge.stripes.controller.multipart.DefaultMultipartWrapperFactory; import net.sourceforge.stripes.controller.multipart.MultipartWrapperFactory; import net.sourceforge.stripes.exception.DefaultExceptionHandler; @@ -491,6 +492,7 @@ Map> interceptors = new HashMap>(); addInterceptor(interceptors, new BeforeAfterMethodInterceptor()); addInterceptor(interceptors, new HttpCacheInterceptor()); + addInterceptor(interceptors, new TicketInspector()); return interceptors; } Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/InvalidTicketException.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/InvalidTicketException.java (revision 0) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/InvalidTicketException.java (revision 0) @@ -0,0 +1,26 @@ +package net.sourceforge.stripes.controller; + +/** + * Exception that Indicates an invalid attempt to access an event handler method + * protected by a {@link net.sourceforge.stripes.action.TicketRequired} + * annotation. The exception is be thrown in two cases: when the required + * {@link Ticket} is missing, or when the Ticket is present but has expired. + * + * @author Andrew Jaquith + * + */ +public class InvalidTicketException extends Exception { + + private static final long serialVersionUID = -987484091709542279L; + + /** + * Constructs a new InvalidTicketException with a supplied message. + * + * @param message + * the message + */ + public InvalidTicketException(String message) { + super(message); + } + +} Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/StripesConstants.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/StripesConstants.java (revision 1114) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/StripesConstants.java (working copy) @@ -50,6 +50,13 @@ String URL_KEY_FLASH_SCOPE_ID = "__fsk"; /** + * The name of a URL parameter that holds {@link net.sourceforge.stripes.controller.Ticket} + * values, as generated by {@link net.sourceforge.stripes.tag.FormTag} and evaluated by + * {@link TicketInspector}. + */ + String URL_KEY_TICKET = "_ticket"; + + /** * An immutable set of URL keys or request parameters that have special meaning to Stripes and * as a result should not be referenced in binding, validation or other other places that * work on the full set of request parameters. @@ -58,7 +65,8 @@ Literal.set(StripesConstants.URL_KEY_SOURCE_PAGE, StripesConstants.URL_KEY_FIELDS_PRESENT, StripesConstants.URL_KEY_FLASH_SCOPE_ID, - StripesConstants.URL_KEY_EVENT_NAME)); + StripesConstants.URL_KEY_EVENT_NAME, + StripesConstants.URL_KEY_TICKET)); /** * The name under which the ActionBean for a request is stored as a request attribute before * forwarding to the JSP. @@ -85,6 +93,12 @@ String REQ_ATTR_TAG_STACK = "__stripes_tag_stack"; /** + * The attribute key that is used to store the Set of tickets stored for the user's + * HttpSession. + */ + String REQ_ATTR_TICKETS = "__stripes_tickets"; + + /** * The name of a request parameter that holds a Map of flash scopes keyed by the * hash code of the request that generated them. */ Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/Ticket.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/Ticket.java (revision 0) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/Ticket.java (revision 0) @@ -0,0 +1,120 @@ +package net.sourceforge.stripes.controller; + +import javax.servlet.http.HttpSession; + +import net.sourceforge.stripes.action.ActionBean; +import net.sourceforge.stripes.util.ConcurrentHashSet; +import net.sourceforge.stripes.util.CryptoUtil; + +/** + * Event "ticket" generated by the + * {@link net.sourceforge.stripes.tag.FormTag} and stashed in the user's + * HttpSession, where it is later checked for by the {@link TicketInspector}. + * Tickets are meant to be used once for a particular event, and expire + * after a defined interval. Tickets are valid for a target ActionBean + * class and event handler methods. + * @see TicketInspector + * @author Andrew Jaquith + * + */ +public class Ticket { + + private final Class beanclass; + + private final String[] handlers; + + private final long creation; + + private final String ticket; + + /** + * Creates a new Ticket, valid for an ActionBean and array of handler + * methods. + * + * @param beanclass + * the ActionBean class containing the method the Ticket can be + * used with + * @param handlers + * the handler methods the Ticket can be used with + */ + private Ticket(Class beanclass, String... handlers ) { + this.beanclass = beanclass; + this.handlers = new String[handlers.length]; + for ( int i = 0; i < handlers.length; i++ ) + { + this.handlers[i] = handlers[i]; + } + this.creation = System.currentTimeMillis(); + this.ticket = CryptoUtil.encrypt(String.valueOf(creation)); + } + + /** + * Returns the creation time stamp, in milliseconds. + * + * @return the creation time + */ + public long creationTime() { + return creation; + } + + /** + * Returns the ticket value, which is simply the encoded value of the + * expiration time, after being encrypted by + * {@link CryptoUtil#encrypt(String)}. The actual value of the ticket does + * not matter; it merely needs to be unique. + * + * @return the ticket value + */ + public String value() { + return ticket; + } + + /** + * Returns the handler methods that the Ticket is valid for. + * + * @return the handler method name + */ + public String[] getHandlers() { + return handlers; + } + + /** + * Returns the ActionBean class the Ticket is valid for. + * + * @return the bean class + */ + public Class getBeanClass() { + return beanclass; + } + + /** + * Factory method that creates a new event Ticket and stashes it in the + * user's HttpSession. Although {@link FormTag} is the preferred method for + * issuing Tickets, this method may be called to create Tickets outside of + * this context, for example in test cases. + * + * @param session + * the user's HTTP session + * @param beanclass + * the ActionBean class containing the method the Ticket can be + * used with + * @param handlers + * the handler methods the Ticket can be used with + * @return the new Ticket + */ + @SuppressWarnings("unchecked") + public static Ticket issueTicket( HttpSession session, Class beanClass, String... events ) + { + ConcurrentHashSet tickets = + (ConcurrentHashSet) session.getAttribute(StripesConstants.REQ_ATTR_TICKETS); + if ( tickets == null ) + { + tickets = new ConcurrentHashSet(); + session.setAttribute(StripesConstants.REQ_ATTR_TICKETS, tickets); + } + + Ticket ticket = new Ticket( beanClass, events ); + tickets.add(ticket); + return ticket; + } +} Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/TicketInspector.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/TicketInspector.java (revision 0) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/controller/TicketInspector.java (revision 0) @@ -0,0 +1,149 @@ +package net.sourceforge.stripes.controller; + +import java.lang.reflect.Method; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; + +import net.sourceforge.stripes.action.ActionBeanContext; +import net.sourceforge.stripes.action.Resolution; +import net.sourceforge.stripes.action.TicketRequired; +import net.sourceforge.stripes.util.ConcurrentHashSet; + +/** + *

+ * {@link Interceptor} that determines whether an ActionBean possesses the + * correct {@link Ticket} to execute an event, if the handler method + * specifies one is required via the {@link net.sourceforge.stripes.action.TicketRequired} + * annotation. + *

+ *

+ * Tickets are "issued" (created by) by + * {@link net.sourceforge.stripes.tag.FormTag}. They are "redeemed" for particular + * events when TicketInspector intercepts access to a protected ActionBean + * event handler method. Access is granted if the user possesses the Ticket + * and it has not expired. To "possess" the Ticket, the user's HttpServletRequest + * must pass back to TicketInspector a parameter value that matches one generated by + * FormTag. Stripes stores the current set of unexpired Tickets in the user's + * HttpSession. + *

+ *

+ * Tickets may be redeemed once. After TicketInterceptor redeems the ticket, it is + * removed from the HttpSession cache. + *

+ *

+ * If the user does not possess the correct Ticket, or if the Ticket has + * expired, TicketInspector throws a {@link InvalidTicketException}. + *

+ *

+ * Transaction tickets are most commonly used to prevent double-posting of form + * contents. They can also be used to ensure that a "view JSP" is accessed + * before submitting a form, thus preventing common cross-site request forgery + * (CSRF) attacks. To use transaction tickets, you must do three things: + *

+ *
    + *
  • Annotate an event handler method with a {@linkplain TicketRequired} + * annotation, optionally with an expires attribute that + * specifies the validity, in seconds, of the Ticket. The default is 720 seconds + * (1 hour).
    • + *
  • Render an HTTP form using the <stripes:form> (aka + * {@link FormTag}), using a beanclass or action + * attribute that resolves to a valid ActionBean.
    • + *
  • Inside that form, supply a submit button using + * <stripes:submit> (aka + * {@link net.sourceforge.stripes.tag.InputSubmitTag}}. The name + * attribute passed to the button must match the name of a valid event.
    • + *
+ *

+ * That's it. Practically speaking, if you already use the Stripes taglibs in your JSPs, + * you just need to add the {@link TicketRequired} annotation to the handler + * method. + *

+ * + * @author Andrew Jaquith + * + */ +@Intercepts( { LifecycleStage.HandlerResolution }) +public class TicketInspector implements Interceptor { + + /** + * Inspects the resolved ActionBean handler method to determine whether a + * transaction {@link Ticket} is required, and if so, checks if the user + * possesses it. If not, an {@link InvalidTicketException} is thrown. + * + * @throws InvalidTicketException + * if the user does not possess the correct ticket + */ + @SuppressWarnings("unchecked") + public Resolution intercept(ExecutionContext context) throws Exception { + + // Execute all other interceptors first + context.proceed(); + + // Is a ticket required to execute the event handler? + Method method = context.getHandler(); + boolean ticketRequired = method.isAnnotationPresent(TicketRequired.class); + + // If ticket required, expiration time is as annotated, or default of 30 + // mins + long expiry = 1000 * (ticketRequired ? method.getAnnotation(TicketRequired.class).expires() : 1800); + + // Rip through stashed tickets and see if we find one that matches, + // pruning any expired tickets as we go + ActionBeanContext abc = context.getActionBeanContext(); + HttpServletRequest request = abc.getRequest(); + HttpSession session = request.getSession(); + ConcurrentHashSet stashedTickets = (ConcurrentHashSet) session + .getAttribute(StripesConstants.REQ_ATTR_TICKETS); + String[] userTickets = request.getParameterValues(StripesConstants.URL_KEY_TICKET); + boolean hasValidTicket = false; + + if (stashedTickets != null) { + for (Ticket ticket : stashedTickets) { + boolean ticketExpired = ticket.creationTime() + expiry < System.currentTimeMillis(); + + // Does the ticket match (beanclass, event name, ticket value)? + if (ticketRequired && !hasValidTicket) { + for (String userTicket : userTickets) { + boolean ticketMatch = ticket.value().equals(userTicket); + boolean beanClassMatch = context.getActionBean().getClass().equals(ticket.getBeanClass()); + if (ticketMatch && beanClassMatch) { + for (String handler : ticket.getHandlers()) { + if (handler.equals(abc.getEventName())) { + hasValidTicket = true; + } + break; + } + } + } + } + + // Prune any expired tickets + if (hasValidTicket || ticketExpired) { + stashedTickets.remove(ticket); + } + + // If user had the ticket but it expired, tell 'em + if (ticketRequired && ticketExpired) { + throw new InvalidTicketException("Ticket expired!"); + } + } + } + + // If user was supposed to have the ticket but didn't, throw exception + if (ticketRequired && !hasValidTicket) { + throw new InvalidTicketException( + "Here's the deal. The event handler method requires the form POST or GET " + + "to contain a parameter with the valid, single-use ticket ID in it, which should " + + "have been set by a previous . You don't have it, which means " + + "you submited the form more than once. It's also possible you submitted the form " + + "in a unit test. You might also be an evil person. Whatever the reason, this is " + + "the end of the transaction. That is all."); + } + + // Always return nothing + return null; + + } + +} Index: /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/tag/FormTag.java =================================================================== --- /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/tag/FormTag.java (revision 1114) +++ /Users/arj/workspace/stripes-svn/stripes/src/net/sourceforge/stripes/tag/FormTag.java (working copy) @@ -19,6 +19,7 @@ import net.sourceforge.stripes.controller.StripesConstants; import net.sourceforge.stripes.controller.StripesFilter; import net.sourceforge.stripes.controller.ActionResolver; +import net.sourceforge.stripes.controller.Ticket; import net.sourceforge.stripes.exception.StripesJspException; import net.sourceforge.stripes.util.CryptoUtil; import net.sourceforge.stripes.util.HtmlUtil; @@ -258,6 +259,9 @@ HttpServletRequest request = (HttpServletRequest) getPageContext().getRequest(); out.write(CryptoUtil.encrypt(HttpUtil.getRequestedServletPath(request))); out.write("\" />"); + + // Write out the hidden ticket field if the target ActionBean handler method requires one + writeTicketField(); if (isWizard()) { writeWizardFields(); @@ -394,6 +398,43 @@ return clazz.getAnnotation(Wizard.class) != null; } + + /** + * Writes out the hidden field for this form's transaction + * {@link net.sourceforge.stripes.controller.Ticket}, + * if one or more handler methods possesses a + * {@link net.sourceforge.stripes.action.TicketRequired} annotation. + * Note that the hidden field is written only if the submit location + * resolves to an ActionBean. + * @throws IOException if the output cannot be written + */ + protected void writeTicketField() throws IOException { + JspWriter out = getPageContext().getOut(); + + // If form submit location doesn't resolve to an ActionBean, exit silently + if ( actionBeanClass == null ) { + return; + } + + // Find all of the submit tags, and extract the name fields (==event names) + Set events = new HashSet(); + for ( Map.Entry> field : fieldsPresent.entrySet() ) { + if (InputSubmitTag.class.equals(field.getValue())) { + events.add(field.getKey() ); + } + } + String[] eventArray = events.toArray(new String[events.size()]); + + //Write out a hidden field with the transaction ticket value in it + HttpServletRequest request = (HttpServletRequest)this.pageContext.getRequest(); + out.write("
"); + out.write(""); + } /** * Writes out hidden fields for all fields that are present in the request but are not Index: /Users/arj/workspace/stripes-svn/tests/src/net/sourceforge/stripes/controller/TicketTests.java =================================================================== --- /Users/arj/workspace/stripes-svn/tests/src/net/sourceforge/stripes/controller/TicketTests.java (revision 0) +++ /Users/arj/workspace/stripes-svn/tests/src/net/sourceforge/stripes/controller/TicketTests.java (revision 0) @@ -0,0 +1,133 @@ +package net.sourceforge.stripes.controller; + +import net.sourceforge.stripes.StripesTestFixture; +import net.sourceforge.stripes.action.ActionBean; +import net.sourceforge.stripes.action.ActionBeanContext; +import net.sourceforge.stripes.action.DefaultHandler; +import net.sourceforge.stripes.action.ForwardResolution; +import net.sourceforge.stripes.action.HandlesEvent; +import net.sourceforge.stripes.action.Resolution; +import net.sourceforge.stripes.action.TicketRequired; +import net.sourceforge.stripes.exception.StripesServletException; +import net.sourceforge.stripes.mock.MockRoundtrip; +import net.sourceforge.stripes.mock.MockServletContext; + +import org.testng.Assert; +import org.testng.annotations.Test; + +/** + * Tests transaction tickets. + * + * @author Andrew Jaquith + */ +public class TicketTests { + + public static class TicketBean implements ActionBean { + + private ActionBeanContext context = null; + + public ActionBeanContext getContext() { + return context; + } + + public void setContext(ActionBeanContext context) { + this.context = context; + } + + @DefaultHandler + @HandlesEvent("unprotected") + public Resolution unprotected() { + return new ForwardResolution("/Success.jsp"); + } + + @TicketRequired + @HandlesEvent("save") + public Resolution save() { + return new ForwardResolution("/Success.jsp"); + } + + @TicketRequired + @HandlesEvent("edit") + public Resolution edit() { + return new ForwardResolution("/Success.jsp"); + } + + @TicketRequired(expires =2) + @HandlesEvent("timedEdit") + public Resolution timedEdit() { + return new ForwardResolution("/Success.jsp"); + } + } + + /** Helper method to create a roundtrip with the test TicketBean class. */ + protected MockRoundtrip getRoundtrip() { + MockServletContext context = StripesTestFixture.getServletContext(); + return new MockRoundtrip(context, TicketBean.class); + } + + /** Verifies that we can execute unproteced methods without any issues. */ + @Test(groups = "fast") + public void unprotected() throws Exception { + MockRoundtrip trip = getRoundtrip(); + trip.execute(); + Assert.assertEquals("/Success.jsp", trip.getDestination()); + } + + /** Execute a protected method without a Ticket; we expect to see an exception. */ + @Test(groups = "fast") + public void protectedNoTicket() throws Exception { + MockRoundtrip trip = getRoundtrip(); + try { + trip.execute("save"); + Assert.fail(); + } catch (StripesServletException e) { + // Excellent. This is what we expect. + } + } + + /** Execute a protected method with a valid Ticket; should proceed normally.. */ + @Test(groups = "fast") + public void protectedWithTicket() throws Exception { + MockRoundtrip trip = getRoundtrip(); + Ticket ticket = Ticket.issueTicket(trip.getRequest().getSession(), TicketBean.class, "save", "edit"); + try { + trip.addParameter(StripesConstants.URL_KEY_TICKET, ticket.value()); + trip.execute("save"); + // Execution should proceed normally because we have the ticket! + } catch (StripesServletException e) { + Assert.fail(); + } + } + + /** Execute a protected method with a Ticket issued for another method in the same bean; we expect an exception. */ + @Test(groups = "fast") + public void protectedMismatchedTicket() throws Exception { + MockRoundtrip trip = getRoundtrip(); + Ticket ticket = Ticket.issueTicket(trip.getRequest().getSession(), TicketBean.class, "save"); + try { + trip.addParameter(StripesConstants.URL_KEY_TICKET, ticket.value()); + trip.execute("edit"); + Assert.fail(); + } catch (StripesServletException e) { + // Excellent. This is what we expect. + } + } + + /** Execute a protected method with an expired Ticket; we expect an exception. */ + @Test(groups = "fast") + public void expiredTicket() throws Exception { + MockRoundtrip trip = getRoundtrip(); + Ticket ticket = Ticket.issueTicket(trip.getRequest().getSession(), TicketBean.class, "timedEdit"); + + // Wait 3 seconds + Thread.sleep( 3000 ); + + try { + trip.addParameter(StripesConstants.URL_KEY_TICKET, ticket.value()); + trip.execute("timedEdit"); + Assert.fail(); + } catch (StripesServletException e) { + // Excellent. This is what we expect. + } + } +}