Modified: incubator/jspwiki/trunk/src/java/org/apache/wiki/action/GroupActionBean.java URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/action/GroupActionBean.java?rev=884993&r1=884992&r2=884993&view=diff ============================================================================== --- incubator/jspwiki/trunk/src/java/org/apache/wiki/action/GroupActionBean.java (original) +++ incubator/jspwiki/trunk/src/java/org/apache/wiki/action/GroupActionBean.java Fri Nov 27 20:33:40 2009 @@ -21,26 +21,36 @@ package org.apache.wiki.action; +import java.security.Permission; import java.security.Principal; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; +import net.sourceforge.stripes.action.*; +import net.sourceforge.stripes.controller.LifecycleStage; +import net.sourceforge.stripes.exception.StripesRuntimeException; +import net.sourceforge.stripes.validation.*; + import org.apache.wiki.WikiEngine; +import org.apache.wiki.WikiSession; +import org.apache.wiki.auth.AuthorizationManager; +import org.apache.wiki.auth.NoSuchPrincipalException; +import org.apache.wiki.auth.PrincipalComparator; import org.apache.wiki.auth.WikiSecurityException; import org.apache.wiki.auth.authorize.Group; import org.apache.wiki.auth.authorize.GroupManager; import org.apache.wiki.auth.authorize.Role; import org.apache.wiki.auth.permissions.GroupPermission; import org.apache.wiki.auth.permissions.WikiPermission; +import org.apache.wiki.log.Logger; +import org.apache.wiki.log.LoggerFactory; import org.apache.wiki.ui.stripes.HandlerPermission; import org.apache.wiki.ui.stripes.LineDelimitedTypeConverter; +import org.apache.wiki.ui.stripes.WikiInterceptor; import org.apache.wiki.ui.stripes.WikiRequestContext; -import net.sourceforge.stripes.action.*; -import net.sourceforge.stripes.exception.StripesRuntimeException; -import net.sourceforge.stripes.validation.*; - - /** * <p> * Views, creates, modifies and deletes @@ -51,8 +61,7 @@ * <li>{...@link #create()} - redirects to the <code>/CreateGroup.jsp</code> * display page</li> * <li>{...@link #view()} - views a Group</li> - * <li>{...@link #save()} - saves an existing Group</li> - * <li>{...@link #save()} - saves a new Group</li> + * <li>{...@link #save()} - saves a Group</li> * <li>{...@link #delete()} - deletes a Group</li> * </ul> * <p> @@ -65,20 +74,88 @@ * </p> * <p> * All of the event handlers require the user to possess an appropriate - * {...@link org.apache.wiki.auth.permissions.GroupPermission} to execute. See - * each handler method's {...@link org.apache.wiki.ui.stripes.HandlerPermission} - * for details. + * {...@link org.apache.wiki.auth.permissions.GroupPermission} to execute. See each + * handler method's {...@link org.apache.wiki.ui.stripes.HandlerPermission} for + * details. * </p> */ @UrlBinding( "/Group.action" ) public class GroupActionBean extends AbstractActionBean { + private static final String DEFAULT_NEW_GROUP_NAME = "MyGroup"; + + private static final Comparator<? super Principal> PRINCIPAL_COMPARATOR = new PrincipalComparator(); + private Group m_group = null; + private static Logger log = LoggerFactory.getLogger( GroupActionBean.class ); + private List<Principal> m_members = new ArrayList<Principal>(); /** - * Forwards the user to the <code>/CreateGroup.jsp</code> display JSP. + * <p> + * When a group is saved, this method checks that the user has the correct + * permissions. For a new group that does not exist, the user must possess + * the WikiPermission {...@link WikiPermission#CREATE_GROUPS_ACTION}. If the + * group already exists, the user must possess GroupPermission with target + * {...@link GroupPermission#EDIT_ACTION} for the group. This method intercepts + * the Stripes request lifecycle and executes after + * {...@link LifecycleStage#BindingAndValidation}, which ensures that the + * handler method is known and that all values are bound. + * </p> + * <p> + * Now, you might be asking yourself, why go to all this trouble when we + * have a perfectly good permission-checking method in + * {...@link WikiInterceptor}? The answer is that the permission is different + * depending on whether the group is being created or edited. It's not + * possible to specify more than one {...@link HandlerPermission} in a method, + * so this method does that instead. + * </p> + */ + @Before( stages = LifecycleStage.CustomValidation ) + public Resolution checkSavePermission() + { + // If not the save event, don't continue + String handler = getContext().getEventName(); + if( !"save".equals( handler ) ) + { + return null; + } + + // Create the required permission + Permission permission; + if( isNew() ) + { + permission = new WikiPermission( "*", WikiPermission.CREATE_GROUPS_ACTION ); + } + else + { + permission = new GroupPermission( "*:" + m_group.getName(), GroupPermission.EDIT_ACTION ); + } + + // Does the user have it? + WikiSession session = getContext().getWikiSession(); + AuthorizationManager mgr = getContext().getEngine().getAuthorizationManager(); + boolean allowed = mgr.checkPermission( session, permission ); + + // If not allowed, redirect to login page with all parameters intact; + // otherwise proceed + Resolution r = null; + if( !allowed ) + { + r = new RedirectResolution( LoginActionBean.class ); + ((RedirectResolution) r).includeRequestParameters( true ); + if( log.isDebugEnabled() ) + { + log.debug( "CheckSavePermission rejected access to ActionBean=" + this.getClass().getCanonicalName() + ", method=" + + handler ); + } + } + return r; + } + + /** + * Redirects the user to the <code>/EditGroup.jsp</code> display JSP. * * @return {...@link net.sourceforge.stripes.action.ForwardResolution} to the * display JSP @@ -88,13 +165,17 @@ @WikiRequestContext( "createGroup" ) public Resolution create() { - return new ForwardResolution( "/CreateGroup.jsp" ); + String groupName = m_group == null ? DEFAULT_NEW_GROUP_NAME : m_group.getName(); + RedirectResolution r = new RedirectResolution( "/EditGroup.jsp" ); + r.addParameter( "group", groupName ); + r.addParameter( "members", getContext().getWikiSession().getUserPrincipal() ); + return r; } /** - * Handler method that deletes the group supplied by {...@link #getGroup()}. - * If the GroupManager throws a WikiSecurityException because it cannot - * delete the group for some reason, the exception is re-thrown as a + * Handler method that deletes the group supplied by {...@link #getGroup()}. If + * the GroupManager throws a WikiSecurityException because it cannot delete + * the group for some reason, the exception is re-thrown as a * StripesRuntimeException. <code>/Group.jsp</code>. * * @throws StripesRuntimeException if the group cannot be deleted for any @@ -118,6 +199,20 @@ return new RedirectResolution( ViewActionBean.class ); } + /** + * Forwards the user to the <code>/EditGroup.jsp</code> display JSP. + * + * @return {...@link net.sourceforge.stripes.action.ForwardResolution} to the + * display JSP + */ + @HandlesEvent( "edit" ) + @HandlerPermission( permissionClass = WikiPermission.class, target = "*", actions = WikiPermission.CREATE_GROUPS_ACTION ) + @WikiRequestContext( "editGroup" ) + public Resolution edit() + { + return new ForwardResolution( "/EditGroup.jsp" ).addParameter( "group", m_group.getName() ); + } + public Group getGroup() { return m_group; @@ -128,9 +223,30 @@ return m_members; } + /** + * Returns {...@code true} if the group returned by {...@link #getGroup()} does + * not exist. + * + * @return the result + */ public boolean isNew() { - return(m_group.getCreated() == null); + if( m_group == null ) + { + return true; + } + + GroupManager mgr = getContext().getEngine().getGroupManager(); + try + { + mgr.getGroup( m_group.getName() ); + return false; + } + catch( NoSuchPrincipalException e ) + { + // Group not created yet + } + return true; } /** @@ -139,13 +255,11 @@ * set automatically by the * {...@link org.apache.wiki.ui.stripes.PrincipalTypeConverter}. * - * @return {...@link net.sourceforge.stripes.action.ForwardResolution} to the + * @return {...@link net.sourceforge.stripes.action.RedirectResolution} to the * view page. * @throws WikiSecurityException if the group cannot be saved for any reason */ @HandlesEvent( "save" ) - @HandlerPermission( permissionClass = GroupPermission.class, target = "${group.name}", actions = GroupPermission.EDIT_ACTION ) - @WikiRequestContext( "editGroup" ) public Resolution save() throws WikiSecurityException { GroupManager mgr = getContext().getEngine().getGroupManager(); @@ -154,28 +268,13 @@ } /** - * Saves a new wiki Group. The members of the group to be saved are returned - * by the method {...@link #getMembers()}; these members are usually set - * automatically by the - * {...@link org.apache.wiki.ui.stripes.PrincipalTypeConverter}. This method - * functions identically to {...@link #save()} except that the - * {...@link org.apache.wiki.auth.permissions.GroupPermission} required to - * execute it is different. + * Sets the Group used by this ActionBean. When the Group is assigned, the + * list of Principals returned by {...@link #getMembers()} is cleared and + * populated by the members of the Group. * - * @return {...@link net.sourceforge.stripes.action.ForwardResolution} to the - * view page. - * @throws WikiSecurityException if the group cannot be saved for any reason + * @param group the group */ - @HandlesEvent( "saveNew" ) - @HandlerPermission( permissionClass = WikiPermission.class, target = "*", actions = WikiPermission.CREATE_GROUPS_ACTION ) - public Resolution saveNew() throws WikiSecurityException - { - GroupManager mgr = getContext().getEngine().getGroupManager(); - mgr.setGroup( getContext().getWikiSession(), m_group ); - return new RedirectResolution( GroupActionBean.class, "view" ).addParameter( "group", m_group.getName() ); - } - - @Validate( required = true ) + @Validate( required = true, on = "view,edit,delete,save" ) public void setGroup( Group group ) { m_group = group; @@ -188,13 +287,15 @@ /** * Sets the member list for the ActionBean, and for the underlying Group. + * The member is sorted when it is assigned. * * @param members the members, separated by carriage returns */ - @Validate( required = true, on = "save,saveNew,delete", converter = LineDelimitedTypeConverter.class ) + @Validate( required = true, on = "save", converter = LineDelimitedTypeConverter.class ) public void setMembers( List<Principal> members ) { m_members = members; + Collections.sort( m_members, PRINCIPAL_COMPARATOR ); m_group.clear(); for( Principal member : members ) { @@ -202,10 +303,15 @@ } } - @ValidationMethod( on = "save, saveNew" ) - public void validateBeforeSave( ValidationErrors errors ) + /** + * Validates that the group name is legal. + * + * @param errors the current set of validation errors + */ + @ValidationMethod( on = "save", when = ValidationState.NO_ERRORS ) + public void validateIsLegalName( ValidationErrors errors ) { - // Name cannot be one of the restricted names either + // Name cannot be one of the restricted names String name = m_group.getName(); if( Role.isReservedName( name ) ) { @@ -222,10 +328,9 @@ * {...@link net.sourceforge.stripes.validation.LocalizableError} with key * <code>editgroup.illegalname</code> -- this method forwards to the error * page. Lastly, if {...@link #getGroup()} returns <code>null</code> because - * the user supplied no <code>group</code> parameter, this method - * redirects to {...@link #create()} with a suggested sample group name - * <code>Group<em>x</em></code>, where <em>x</em> is an integer - * value. + * the user supplied no <code>group</code> parameter, this method redirects + * to {...@link #create()} with a suggested sample group name + * <code>Group<em>x</em></code>, where <em>x</em> is an integer value. */ @DefaultHandler @DontValidate
Added: incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/GroupFormatter.java URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/GroupFormatter.java?rev=884993&view=auto ============================================================================== --- incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/GroupFormatter.java (added) +++ incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/GroupFormatter.java Fri Nov 27 20:33:40 2009 @@ -0,0 +1,72 @@ +package org.apache.wiki.ui.stripes; + +import java.util.Locale; + +import net.sourceforge.stripes.format.Formatter; + +import org.apache.wiki.auth.authorize.Group; + +/** + * Stripes Formatter class that formats a supplied wiki {...@link Group}, with the result + * being the name of the group. + */ +public class GroupFormatter implements Formatter<Group> +{ + /** + * {...@inheritdoc} + * <p> + * This implementation produces a formatted String representing the {...@link Group} + * according to the following rules: + * </p> + * <ul> + * <li>if the Group is {...@code null}, returns the empty string</li> + * <li>otherwise, the Group name is returned</li> + * </ul> + */ + public String format( Group input ) + { + return input == null ? "" : input.getName(); + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing. + * </p> + */ + public void init() + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing; the page name is always used. + * </p> + */ + public void setFormatPattern( String formatPattern ) + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing; the group name is always used. + * </p> + */ + public void setFormatType( String formatType ) + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing, because the Locale has no effect on the + * output. + * </p> + */ + public void setLocale( Locale locale ) + { + } + +} Added: incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/PrincipalFormatter.java URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/PrincipalFormatter.java?rev=884993&view=auto ============================================================================== --- incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/PrincipalFormatter.java (added) +++ incubator/jspwiki/trunk/src/java/org/apache/wiki/ui/stripes/PrincipalFormatter.java Fri Nov 27 20:33:40 2009 @@ -0,0 +1,71 @@ +package org.apache.wiki.ui.stripes; + +import java.security.Principal; +import java.util.Locale; + +import net.sourceforge.stripes.format.Formatter; + +/** + * Stripes Formatter class that formats a supplied {...@link Principal}, with the result + * being the name of the Principal. + */ +public class PrincipalFormatter implements Formatter<Principal> +{ + /** + * {...@inheritdoc} + * <p> + * This implementation produces a formatted String representing the {...@link Principal} + * according to the following rules: + * </p> + * <ul> + * <li>if the Principal is {...@code null}, returns the empty string</li> + * <li>otherwise, the result of {...@link Principal#getName()} is returned</li> + * </ul> + */ + public String format( Principal input ) + { + return input == null ? "" : input.getName(); + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing. + * </p> + */ + public void init() + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing; the Principal name is always used. + * </p> + */ + public void setFormatPattern( String formatPattern ) + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing; the Principal name is always used. + * </p> + */ + public void setFormatType( String formatType ) + { + } + + /** + * {...@inheritdoc} + * <p> + * This implementation does nothing, because the Locale has no effect on the + * output. + * </p> + */ + public void setLocale( Locale locale ) + { + } + +} Modified: incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/GroupActionBeanTest.java URL: http://svn.apache.org/viewvc/incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/GroupActionBeanTest.java?rev=884993&r1=884992&r2=884993&view=diff ============================================================================== --- incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/GroupActionBeanTest.java (original) +++ incubator/jspwiki/trunk/tests/java/org/apache/wiki/action/GroupActionBeanTest.java Fri Nov 27 20:33:40 2009 @@ -20,11 +20,18 @@ */ package org.apache.wiki.action; +import java.security.Principal; +import java.util.List; import java.util.Properties; +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestSuite; +import net.sourceforge.stripes.mock.MockRoundtrip; +import net.sourceforge.stripes.validation.ValidationErrors; + import org.apache.wiki.TestEngine; import org.apache.wiki.WikiSession; -import org.apache.wiki.action.GroupActionBean; import org.apache.wiki.auth.NoSuchPrincipalException; import org.apache.wiki.auth.SessionMonitor; import org.apache.wiki.auth.Users; @@ -33,12 +40,6 @@ import org.apache.wiki.auth.authorize.GroupDatabase; import org.apache.wiki.auth.authorize.GroupManager; -import junit.framework.Test; -import junit.framework.TestCase; -import junit.framework.TestSuite; -import net.sourceforge.stripes.mock.MockRoundtrip; -import net.sourceforge.stripes.validation.ValidationErrors; - public class GroupActionBeanTest extends TestCase { @@ -171,8 +172,10 @@ assertNull( null ); // Verify we are directed to the wiki front page + GroupActionBean bean = trip.getActionBean( GroupActionBean.class ); + ValidationErrors errors = bean.getContext().getValidationErrors(); + assertEquals( 0, errors.size() ); assertEquals( "/Wiki.action", trip.getDestination() ); - } public void testSaveExistingGroup() throws Exception @@ -181,32 +184,40 @@ MockRoundtrip trip = m_engine.authenticatedTrip( Users.JANNE, Users.JANNE_PASS, GroupActionBean.class ); WikiSession wikiSession = SessionMonitor.getInstance( m_engine ).find( trip.getRequest().getSession() ); - // Create a new group + // Create a new group using tha back-end APIs so that it exists already GroupManager mgr = m_engine.getGroupManager(); - Group group = mgr.getGroup( "TestGroup", true ); + Group group = mgr.getGroup( "TestSaveExistingGroup", true ); group.add( new WikiPrincipal( "Janne Jalkanen" ) ); group.add( new WikiPrincipal( "Princess Buttercup" ) ); mgr.setGroup( wikiSession, group ); // Make sure the group saved correctly - group = mgr.getGroup( "TestGroup" ); + group = mgr.getGroup( "TestSaveExistingGroup" ); assertNotNull( group ); - // Now, try to save the group with some new members - trip.setParameter( "group", "TestGroup" ); + // Now, via the UI, try to save the group with some new members + trip.setParameter( "group", "TestSaveExistingGroup" ); trip.setParameter( "members","Princess Buttercup\nInigo Montoya\nMiracle Max" ); + trip.setParameter( "new", "false" ); trip.execute( "save" ); // Verify we are directed to the group-view page GroupActionBean bean = trip.getActionBean( GroupActionBean.class ); ValidationErrors errors = bean.getContext().getValidationErrors(); assertEquals( 0, errors.size() ); - assertEquals( "/Group.action?view=&group=TestGroup", trip.getDestination() ); + assertEquals( "/Group.action?view=&group=TestSaveExistingGroup", trip.getDestination() ); + + // Verify the Group members were set on the Actionbean correctly (sorted!) + List<Principal> members = bean.getMembers(); + assertEquals( 3, members.size() ); + assertEquals( new WikiPrincipal( "Inigo Montoya" ), members.get( 0 ) ); + assertEquals( new WikiPrincipal( "Miracle Max" ), members.get( 1 ) ); + assertEquals( new WikiPrincipal( "Princess Buttercup" ), members.get( 2 ) ); // Verify the Group saved correctly group = bean.getGroup(); assertNotNull( group ); - assertEquals( "TestGroup", group.getName() ); + assertEquals( "TestSaveExistingGroup", group.getName() ); assertEquals( 3, group.members().length ); assertTrue( group.isMember( new WikiPrincipal( "Princess Buttercup" ) ) ); assertTrue( group.isMember( new WikiPrincipal( "Inigo Montoya" ) ) ); @@ -215,12 +226,13 @@ public void testSaveNewGroup() throws Exception { - // Start with an authenticated user - MockRoundtrip trip = m_engine.authenticatedTrip( Users.JANNE, Users.JANNE_PASS, GroupActionBean.class ); + // Start with a guest session + MockRoundtrip trip = m_engine.guestTrip( GroupActionBean.class ); - // Try to save the group - trip.setParameter( "group", "TestGroup" ); - trip.setParameter( "members","Princess Buttercup\nInigo Montoya\nMiracle Max" ); + // Try to save the group (make sure Janne is in the list!) + trip.setParameter( "group", "TestSaveNewGroup" ); + trip.setParameter( "members","Janne Jalkanen\nPrincess Buttercup\nInigo Montoya\nMiracle Max" ); + trip.setParameter( "new", "true" ); trip.execute( "save" ); // Should NOT succeed because the save event requires Edit permissions, which we don't have @@ -229,23 +241,33 @@ assertEquals( 0, errors.size() ); assertEquals( "/Login.action", trip.getDestination().substring( 0, 13 ) ); - // Try again with the 'saveNew' event + // Try saving again, this time authenticated trip = m_engine.authenticatedTrip( Users.JANNE, Users.JANNE_PASS, GroupActionBean.class ); - trip.setParameter( "group", "TestGroup" ); - trip.setParameter( "members","Princess Buttercup\nInigo Montoya\nMiracle Max" ); - trip.execute( "saveNew" ); + trip.setParameter( "group", "TestSaveNewGroup" ); + trip.setParameter( "members","Janne Jalkanen\nPrincess Buttercup\nInigo Montoya\nMiracle Max" ); + trip.setParameter( "new","true" ); + trip.execute( "save" ); // Verify we are directed to the view page bean = trip.getActionBean( GroupActionBean.class ); errors = bean.getContext().getValidationErrors(); assertEquals( 0, errors.size() ); - assertEquals( "/Group.action?view=&group=TestGroup", trip.getDestination() ); + assertEquals( "/Group.action?view=&group=TestSaveNewGroup", trip.getDestination() ); + + // Verify the Group members were set on the Actionbean correctly (sorted!) + List<Principal> members = bean.getMembers(); + assertEquals( 4, members.size() ); + assertEquals( new WikiPrincipal( "Inigo Montoya" ), members.get( 0 ) ); + assertEquals( new WikiPrincipal( "Janne Jalkanen" ), members.get( 1 ) ); + assertEquals( new WikiPrincipal( "Miracle Max" ), members.get( 2 ) ); + assertEquals( new WikiPrincipal( "Princess Buttercup" ), members.get( 3 ) ); // Verify the Group saved correctly Group group = bean.getGroup(); assertNotNull( group ); - assertEquals( "TestGroup", group.getName() ); - assertEquals( 3, group.members().length ); + assertEquals( "TestSaveNewGroup", group.getName() ); + assertEquals( 4, group.members().length ); + assertTrue( group.isMember( new WikiPrincipal( "Janne Jalkanen" ) ) ); assertTrue( group.isMember( new WikiPrincipal( "Princess Buttercup" ) ) ); assertTrue( group.isMember( new WikiPrincipal( "Inigo Montoya" ) ) ); assertTrue( group.isMember( new WikiPrincipal( "Miracle Max" ) ) );
