svn commit: r597062 - /lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java

Author: andreas
Date: Wed Nov 21 05:02:39 2007
New Revision: 597062

URL: http://svn.apache.org/viewvc?rev=597062&view=rev
Log:
Move condition checks to AccessControl.checkExecutionConditions() to avoid flow errors

Modified:
    lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java

Modified: lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java
URL: http://svn.apache.org/viewvc/lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java?rev=597062&r1=597061&r2=597062&view=diff
==============================================================================
--- lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java (original)
+++ lenya/trunk/src/modules-core/administration/java/src/org/apache/lenya/cms/ac/usecases/AccessControl.java Wed Nov 21 05:02:39 2007
@@ -185,35 +185,16 @@
         }
     }
 
-    /**
-     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doExecute()
-     */
-    public void doExecute() throws Exception {
-        super.doExecute();
-        if (getParameterAsString("change_ssl") != null) {
-            String ssl = getBooleanCheckboxParameter("ssl");
-            setSSLProtected(Boolean.valueOf(ssl).booleanValue());
-        }
-
+    protected void doCheckExecutionConditions() throws Exception {
+        super.doCheckExecutionConditions();
         for (int i = 0; i < types.length; i++) {
             for (int j = 0; j < operations.length; j++) {
                 String type = types[i];
                 String paramName = operations[j] + "Credential_" + type;
                 if (getParameterAsString(paramName) != null) {
                     String roleId = getParameterAsString(ROLE);
-                    String method = getParameterAsString(METHOD);
-
                     String id = getParameterAsString(type);
-                    Accreditable item = null;
-                    if (type.equals(USER)) {
-                        item = getUserManager().getUser(id);
-                    } else if (type.equals(GROUP)) {
-                        item = getGroupManager().getGroup(id);
-                    } else if (type.equals(IPRANGE)) {
-                        item = getIpRangeManager().getIPRange(id);
-                    } else if (type.equals(WORLD)) {
-                        item = World.getInstance();
-                    }
+                    Accreditable item = getAccreditable(type, id);
                     if (item == null) {
                         addErrorMessage("no_such_accreditable", new String[] { type, id });
                     } else {
@@ -224,15 +205,64 @@
                         if (!role.isAssignable()) {
                             addErrorMessage("cannot-assign-role", new String[] { roleId });
                         }
-                        manipulateCredential(item, role, operations[j], method);
-                        setParameter(SUB_CREDENTIALS, getSubtreeCredentials());
+                        if (operations[j].equals(ADD)) {
+                            ModifiablePolicy policy = getPolicy();
+                            if (containsCredential(policy, item, role)) {
+                                addErrorMessage("credential-already-contained",
+                                        new String[] { ((Item) item).getId(), role.getId() });
+                            }
+                        }
+                    }
+                    if (hasErrors()) {
+                        deleteParameter(paramName);
                     }
+                }
+            }
+        }
+    }
+
+    /**
+     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doExecute()
+     */
+    public void doExecute() throws Exception {
+        super.doExecute();
+        if (getParameterAsString("change_ssl") != null) {
+            String ssl = getBooleanCheckboxParameter("ssl");
+            setSSLProtected(Boolean.valueOf(ssl).booleanValue());
+        }
+
+        for (int i = 0; i < types.length; i++) {
+            for (int j = 0; j < operations.length; j++) {
+                String type = types[i];
+                String paramName = operations[j] + "Credential_" + type;
+                if (getParameterAsString(paramName) != null) {
+                    String roleId = getParameterAsString(ROLE);
+                    String method = getParameterAsString(METHOD);
+                    String id = getParameterAsString(type);
+                    Accreditable item = getAccreditable(type, id);
+                    Role role = getRoleManager().getRole(roleId);
+                    manipulateCredential(item, role, operations[j], method);
+                    setParameter(SUB_CREDENTIALS, getSubtreeCredentials());
                     deleteParameter(paramName);
                 }
             }
         }
     }
 
+    protected Accreditable getAccreditable(String type, String id) {
+        Accreditable item = null;
+        if (type.equals(USER)) {
+            item = getUserManager().getUser(id);
+        } else if (type.equals(GROUP)) {
+            item = getGroupManager().getGroup(id);
+        } else if (type.equals(IPRANGE)) {
+            item = getIpRangeManager().getIPRange(id);
+        } else if (type.equals(WORLD)) {
+            item = World.getInstance();
+        }
+        return item;
+    }
+
     /**
      * Returns if one of the ancestors of this URL is SSL protected.
      * 
@@ -281,8 +311,7 @@
      */
     protected void setSSLProtected(boolean ssl) throws ProcessingException {
         try {
-            ModifiablePolicy policy = (ModifiablePolicy) getPolicyManager().buildSubtreePolicy(
-                    getAccreditableManager(), getPolicyURL());
+            ModifiablePolicy policy = getPolicy();
             policy.setSSL(ssl);
             getPolicyManager().saveSubtreePolicy(getPolicyURL(), policy);
         } catch (AccessControlException e) {
@@ -309,19 +338,11 @@
      */
     protected void manipulateCredential(Accreditable accreditable, Role role, String operation,
             String method) throws ProcessingException {
-        ModifiablePolicy policy = null;
         try {
-            policy = (ModifiablePolicy) getPolicyManager().buildSubtreePolicy(
-                    getAccreditableManager(), getPolicyURL());
+            ModifiablePolicy policy = getPolicy();
             
             if (operation.equals(ADD)) {
-                if (containsCredential(policy, accreditable, role)) {
-                    addErrorMessage("credential-already-contained",
-                            new String[] { ((Item) accreditable).getId(), role.getId() });
-                }
-                else {
-                    policy.addRole(accreditable, role, method);
-                }
+                policy.addRole(accreditable, role, method);
             } else if (operation.equals(DELETE)) {
                 policy.removeRole(accreditable, role);
             } else if (operation.equals(UP)) {
@@ -334,6 +355,11 @@
         } catch (Exception e) {
             throw new ProcessingException("Manipulating credential failed: ", e);
         }
+    }
+
+    protected ModifiablePolicy getPolicy() throws AccessControlException {
+        return (ModifiablePolicy) getPolicyManager().buildSubtreePolicy(
+                getAccreditableManager(), getPolicyURL());
     }
 
     protected boolean containsCredential(ModifiablePolicy policy, Accreditable accreditable, Role role)