hi andreas!

[EMAIL PROTECTED] wrote:
Author: andreas
Date: Fri Jun  9 02:01:58 2006
New Revision: 412987

URL: http://svn.apache.org/viewvc?rev=412987&view=rev
Log:
Use separate usecase for changing password with check of old password. This 
avoids the security issue that the checkPassword parameter can be set by the 
client.

after some testing, this seems to fix the issue i reported for good.


one minor thing remains: the usecase handler for non-admin users still allows to set userId to arbitrary users. no security implication here, because they will be prompted for the password of the user accound they try to hijack, but still...

as a java exercise, i hacked together an alternate version that uses an abstract class AbstractChangePassword that has all the common features, and two derived classes ChangePassword and ChangePasswordAdmin that each add their own extensions. (i chose to do it this way because each of these classes has features that the other hasn't, so there was no obvious way to do it with inheritance.)

could you comment on this? it might be a little over-engineered, but i want to get some hands-on experience regarding oo design.

a patch is attached, but it does not work, since i'm still stuck with another problem: i want the AbstractChangePassword to initialize "user" with the userId of the currently logged in user, but i can't seem to find out where to get that kind of information... i tried
  Map objectModel = ContextHelper.getObjectModel(getContext());
  Request request = ObjectModelHelper.getRequest(objectModel);
  this.user = Identity.getIdentity(request.getSession(true)).getUser();
but that gives an npe since getContext returns null.

can anyone help me here?


tia,


jörn







--
"Open source takes the bullshit out of software."
        - Charles Ferguson on TechnologyReview.com

--
Jörn Nettingsmeier, EDV-Administrator
Institut für Politikwissenschaft
Universität Duisburg-Essen, Standort Duisburg
Mail: [EMAIL PROTECTED], Telefon: 0203/379-2736
Index: java/org/apache/lenya/cms/ac/usecases/UserPassword.java
===================================================================
--- java/org/apache/lenya/cms/ac/usecases/UserPassword.java	(revision 412994)
+++ java/org/apache/lenya/cms/ac/usecases/UserPassword.java	(working copy)
@@ -1,94 +0,0 @@
-/*
- * Copyright  1999-2004 The Apache Software Foundation
- *
- *  Licensed under the Apache License, Version 2.0 (the "License");
- *  you may not use this file except in compliance with the License.
- *  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- *  Unless required by applicable law or agreed to in writing, software
- *  distributed under the License is distributed on an "AS IS" BASIS,
- *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- *  See the License for the specific language governing permissions and
- *  limitations under the License.
- *
- */
-package org.apache.lenya.cms.ac.usecases;
-
-import org.apache.lenya.ac.User;
-
-/**
- * Usecase to change a user's password.
- */
-public class UserPassword extends AccessControlUsecase {
-
-    protected static final String NEW_PASSWORD = "password";
-    protected static final String CONFIRM_PASSWORD = "confirmPassword";
-
-    /**
-     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doCheckExecutionConditions()
-     */
-    protected void doCheckExecutionConditions() throws Exception {
-        super.doCheckExecutionConditions();
-        
-        if (this.user == null) {
-            addErrorMessage("The user ID has to be provided when executing this usecase.");
-            return;
-        }
-
-        checkNewPassword(this);
-    }
-
-    /**
-     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doExecute()
-     */
-    protected void doExecute() throws Exception {
-        super.doExecute();
-        this.user.setPassword(getParameterAsString(NEW_PASSWORD));
-    }
-
-    private User user;
-    
-    protected User getUser() {
-        return this.user;
-    }
-
-    /**
-     * @see org.apache.lenya.cms.usecase.Usecase#setParameter(java.lang.String, java.lang.Object)
-     */
-    public void setParameter(String name, Object value) {
-
-        super.setParameter(name, value);
-
-        if (name.equals(UserProfile.USER_ID)) {
-            String userId = (String) value;
-            this.user = getUserManager().getUser(userId);
-            if (this.user == null) {
-                throw new RuntimeException("User [" + userId + "] not found.");
-            }
-        }
-    }
-
-    /**
-     * Checks a password and a confirmed password.
-     * @param usecase The usecase.
-     */
-    protected static void checkNewPassword(AccessControlUsecase usecase) {
-        String password = usecase.getParameterAsString(UserPassword.NEW_PASSWORD);
-        String confirmPassword = usecase.getParameterAsString(UserPassword.CONFIRM_PASSWORD);
-
-        if (!password.equals(confirmPassword)) {
-            usecase.addErrorMessage("Password and confirmed password are not equal.");
-        }
-
-        if (password.length() < 6) {
-            usecase.addErrorMessage("The password must be at least six characters long.");
-        }
-
-        if (!password.matches(".*\\d.*")) {
-            usecase.addErrorMessage("The password must contain at least one number.");
-        }
-    }
-
-}
\ No newline at end of file
Index: java/org/apache/lenya/cms/ac/usecases/ChangePassword.java
===================================================================
--- java/org/apache/lenya/cms/ac/usecases/ChangePassword.java	(revision 0)
+++ java/org/apache/lenya/cms/ac/usecases/ChangePassword.java	(revision 0)
@@ -0,0 +1,35 @@
+/*
+ * Copyright  1999-2004 The Apache Software Foundation
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+package org.apache.lenya.cms.ac.usecases;
+
+/**
+ * Usecase to change a user's password. The old password is checked.
+ */
+public class ChangePassword extends AbstractChangePassword {
+
+    protected static final String OLD_PASSWORD = "oldPassword";
+
+    protected void doCheckExecutionConditions() throws Exception {
+        super.doCheckExecutionConditions();
+        String oldPassword = getParameterAsString(OLD_PASSWORD);
+        boolean authenticated = getUser().authenticate(oldPassword);
+        if (!authenticated) {
+            addErrorMessage("The old password is not correct.");
+        }
+    }
+
+}
Index: java/org/apache/lenya/cms/ac/usecases/ChangePasswordAdmin.java
===================================================================
--- java/org/apache/lenya/cms/ac/usecases/ChangePasswordAdmin.java	(revision 0)
+++ java/org/apache/lenya/cms/ac/usecases/ChangePasswordAdmin.java	(revision 0)
@@ -0,0 +1,51 @@
+/*
+ * Copyright  1999-2004 The Apache Software Foundation
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+package org.apache.lenya.cms.ac.usecases;
+
+/**
+ * Usecase to change a user's password.
+ */
+public class ChangePasswordAdmin extends AbstractChangePassword {
+
+    /**
+     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doCheckExecutionConditions()
+     */
+    protected void doCheckExecutionConditions() throws Exception {
+        super.doCheckExecutionConditions();
+        if (getUser() == null) {
+            addErrorMessage("The user ID has to be provided when executing this usecase.");
+            return;
+        }
+        checkNewPassword(this);
+    }
+
+    /**
+     * @see org.apache.lenya.cms.usecase.Usecase#setParameter(java.lang.String, java.lang.Object)
+     */
+    public void setParameter(String name, Object value) {
+
+        super.setParameter(name, value);
+
+        if (name.equals(UserProfile.USER_ID)) {
+            String userId = (String) value;
+            setUser(getUserManager().getUser(userId));
+            if (getUser() == null) {
+                throw new RuntimeException("User [" + userId + "] not found.");
+            }
+        }
+    }
+}
\ No newline at end of file
Index: java/org/apache/lenya/cms/ac/usecases/UserPasswordWithCheck.java
===================================================================
--- java/org/apache/lenya/cms/ac/usecases/UserPasswordWithCheck.java	(revision 412994)
+++ java/org/apache/lenya/cms/ac/usecases/UserPasswordWithCheck.java	(working copy)
@@ -1,37 +0,0 @@
-/*
- * Copyright  1999-2004 The Apache Software Foundation
- *
- *  Licensed under the Apache License, Version 2.0 (the "License");
- *  you may not use this file except in compliance with the License.
- *  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- *  Unless required by applicable law or agreed to in writing, software
- *  distributed under the License is distributed on an "AS IS" BASIS,
- *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- *  See the License for the specific language governing permissions and
- *  limitations under the License.
- *
- */
-package org.apache.lenya.cms.ac.usecases;
-
-/**
- * Usecase to change a user's password. The old password is checked.
- */
-public class UserPasswordWithCheck extends UserPassword {
-
-    protected static final String OLD_PASSWORD = "oldPassword";
-
-    protected void doCheckExecutionConditions() throws Exception {
-
-        super.doCheckExecutionConditions();
-
-        String oldPassword = getParameterAsString(OLD_PASSWORD);
-        boolean authenticated = getUser().authenticate(oldPassword);
-        if (!authenticated) {
-            addErrorMessage("The old password is not correct.");
-        }
-    }
-
-}
Index: java/org/apache/lenya/cms/ac/usecases/AbstractChangePassword.java
===================================================================
--- java/org/apache/lenya/cms/ac/usecases/AbstractChangePassword.java	(revision 0)
+++ java/org/apache/lenya/cms/ac/usecases/AbstractChangePassword.java	(revision 0)
@@ -0,0 +1,87 @@
+/*
+ * Copyright  1999-2004 The Apache Software Foundation
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+package org.apache.lenya.cms.ac.usecases;
+
+import org.apache.lenya.ac.User;
+
+import org.apache.lenya.ac.Identity;
+import java.util.Map;
+import org.apache.cocoon.components.ContextHelper;
+import org.apache.cocoon.environment.ObjectModelHelper;
+import org.apache.cocoon.environment.Request;
+
+/**
+ * Usecase to change a user's password.
+ */
+public abstract class AbstractChangePassword extends AccessControlUsecase {
+
+    private User user;
+
+    protected static final String NEW_PASSWORD = "password";
+    protected static final String CONFIRM_PASSWORD = "confirmPassword";
+
+    public AbstractChangePassword() {
+        super();
+        //hmm. for some reason, getContext seems to return null.
+        Map objectModel = ContextHelper.getObjectModel(getContext());
+        Request request = ObjectModelHelper.getRequest(objectModel);
+        this.user = Identity.getIdentity(request.getSession(true)).getUser();
+    }
+    /**
+     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doCheckExecutionConditions()
+     */
+    protected void doCheckExecutionConditions() throws Exception {
+        super.doCheckExecutionConditions();
+        checkNewPassword(this);
+    }
+
+    /**
+     * @see org.apache.lenya.cms.usecase.AbstractUsecase#doExecute()
+     */
+    protected void doExecute() throws Exception {
+        super.doExecute();
+        this.user.setPassword(getParameterAsString(NEW_PASSWORD));
+    }
+
+    protected void setUser(User newuser){
+        this.user = newuser;
+    }
+
+    protected User getUser() {
+        return this.user;
+    }
+    /**
+     * Checks a password and a confirmed password.
+     * @param usecase The usecase.
+     */
+    protected static void checkNewPassword(AccessControlUsecase usecase) {
+        String password = usecase.getParameterAsString(NEW_PASSWORD);
+        String confirmPassword = usecase.getParameterAsString(CONFIRM_PASSWORD);
+
+        if (!password.equals(confirmPassword)) {
+            usecase.addErrorMessage("Password and confirmed password are not equal.");
+        }
+
+        if (password.length() < 6) {
+            usecase.addErrorMessage("The password must be at least six characters long.");
+        }
+
+        if (!password.matches(".*\\d.*")) {
+            usecase.addErrorMessage("The password must contain at least one number.");
+        }
+    }
+}
\ No newline at end of file
Index: impl/java/org/apache/lenya/cms/ac/usecases/AddUser.java
===================================================================
--- impl/java/org/apache/lenya/cms/ac/usecases/AddUser.java	(revision 412994)
+++ impl/java/org/apache/lenya/cms/ac/usecases/AddUser.java	(working copy)
@@ -77,7 +77,7 @@
         }
 
         else {
-            UserPassword.checkNewPassword(this);
+            ChangePassword.checkNewPassword(this);
         }
 
     }
@@ -109,7 +109,7 @@
             String ldapId = getParameterAsString(LDAP_ID);
             user = new LDAPUser(configDir, userId, email, ldapId, getLogger());
         } else {
-            String password = getParameterAsString(UserPassword.NEW_PASSWORD);
+            String password = getParameterAsString(AbstractChangePassword.NEW_PASSWORD);
             user = new FileUser(configDir, userId, fullName, email, "");
             user.setName(fullName);
             user.setPassword(password);
Index: webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePassword.xconf
===================================================================
--- webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePassword.xconf	(revision 412994)
+++ webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePassword.xconf	(working copy)
@@ -23,7 +23,7 @@
   <xconf xpath="/cocoon/usecases" unless="/cocoon/usecases/[EMAIL PROTECTED] = 'admin.changePassword']">
 
     <component-instance name="admin.changePassword" logger="lenya.admin"
-      class="org.apache.lenya.cms.ac.usecases.UserPasswordWithCheck">
+      class="org.apache.lenya.cms.ac.usecases.ChangePassword">
       <view template="usecases/admin/changePassword.jx" menu="true">
         <tab group="admin" name="users"/>
         <parameter name="checkPassword" value="true"/>
Index: webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePasswordAdmin.xconf
===================================================================
--- webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePasswordAdmin.xconf	(revision 412994)
+++ webapp/lenya/config/cocoon-xconf/usecases/admin/usecase-admin-changePasswordAdmin.xconf	(working copy)
@@ -22,7 +22,7 @@
 
   <xconf xpath="/cocoon/usecases" unless="/cocoon/usecases/[EMAIL PROTECTED] = 'admin.changePasswordAdmin']">
 
-    <component-instance name="admin.changePasswordAdmin" logger="lenya.admin" class="org.apache.lenya.cms.ac.usecases.UserPassword">
+    <component-instance name="admin.changePasswordAdmin" logger="lenya.admin" class="org.apache.lenya.cms.ac.usecases.ChangePasswordAdmin">
       <view template="usecases/admin/changePassword.jx" menu="true">
         <tab group="admin" name="users"/>
         <parameter name="checkPassword" value="false"/>

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to