Keep improving security
authorMathieu Baudier <mbaudier@argeo.org>
Wed, 23 Mar 2011 12:19:27 +0000 (12:19 +0000)
committerMathieu Baudier <mbaudier@argeo.org>
Wed, 23 Mar 2011 12:19:27 +0000 (12:19 +0000)
git-svn-id: https://svn.argeo.org/commons/trunk@4342 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc

eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/EclipseUiUtils.java [new file with mode: 0644]
security/plugins/org.argeo.security.ui.admin/META-INF/spring/commands.xml
security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/NewUser.java [new file with mode: 0644]
security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/MainUserInfoWizardPage.java [new file with mode: 0644]
security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/NewUserWizard.java [new file with mode: 0644]
security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/UserAdminService.java
security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java
server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/ThreadBoundJcrSessionFactory.java

diff --git a/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/EclipseUiUtils.java b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/EclipseUiUtils.java
new file mode 100644 (file)
index 0000000..333db5b
--- /dev/null
@@ -0,0 +1,37 @@
+package org.argeo.eclipse.ui;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.ModifyListener;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.Text;
+
+/** Utilities to simplify UI development. */
+public class EclipseUiUtils {
+       /**
+        * Create a label and a text field for a grid layout, the text field grabing
+        * excess horizontal
+        * 
+        * @param parent
+        *            the parent composite
+        * @param label
+        *            the lable to display
+        * @param modifyListener
+        *            a {@link ModifyListener} to listen on events on the text, can
+        *            be null
+        * @return the created text
+        */
+       public static Text createGridLT(Composite parent, String label,
+                       ModifyListener modifyListener) {
+               Label lbl = new Label(parent, SWT.LEAD);
+               lbl.setText(label);
+               lbl.setLayoutData(new GridData(SWT.RIGHT, SWT.CENTER, false, false));
+               Text txt = new Text(parent, SWT.LEAD | SWT.BORDER);
+               txt.setLayoutData(new GridData(SWT.LEFT, SWT.CENTER, true, false));
+               if (txt != null)
+                       txt.addModifyListener(modifyListener);
+               return txt;
+       }
+
+}
index 044fabf582d39a3a8a516df80a54d3701fd5a8ad..36aefbe036cd12ea344bde473edc26e187b47437 100644 (file)
@@ -4,9 +4,10 @@
        xsi:schemaLocation="http://www.springframework.org/schema/beans
         http://www.springframework.org/schema/beans/spring-beans.xsd">
 
-       <bean id="openArgeoUserEditor" class="org.argeo.security.ui.admin.commands.OpenArgeoUserEditor"
+       <bean id="openArgeoUserEditor"
+               class="org.argeo.security.ui.admin.commands.OpenArgeoUserEditor"
                scope="prototype" />
-       <bean id="newArgeoUserEditor" class="org.argeo.security.ui.admin.commands.OpenArgeoUserEditor"
+       <bean id="newArgeoUserEditor" class="org.argeo.security.ui.admin.commands.NewUser"
                scope="prototype" />
        <bean id="addRole" class="org.argeo.security.ui.admin.commands.AddRole"
                scope="prototype">
diff --git a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/NewUser.java b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/NewUser.java
new file mode 100644 (file)
index 0000000..de8dc9b
--- /dev/null
@@ -0,0 +1,24 @@
+package org.argeo.security.ui.admin.commands;
+
+import org.argeo.security.ui.admin.wizards.NewUserWizard;
+import org.eclipse.core.commands.AbstractHandler;
+import org.eclipse.core.commands.ExecutionEvent;
+import org.eclipse.core.commands.ExecutionException;
+import org.eclipse.jface.wizard.WizardDialog;
+import org.eclipse.ui.handlers.HandlerUtil;
+
+/** Command handler to set visible or open a Argeo user. */
+public class NewUser extends AbstractHandler {
+
+       public Object execute(ExecutionEvent event) throws ExecutionException {
+               try {
+                       NewUserWizard newUserWizard = new NewUserWizard();
+                       WizardDialog dialog = new WizardDialog(
+                                       HandlerUtil.getActiveShell(event), newUserWizard);
+                       dialog.open();
+               } catch (Exception e) {
+                       throw new ExecutionException("Cannot open editor", e);
+               }
+               return null;
+       }
+}
diff --git a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/MainUserInfoWizardPage.java b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/MainUserInfoWizardPage.java
new file mode 100644 (file)
index 0000000..cf11f85
--- /dev/null
@@ -0,0 +1,71 @@
+package org.argeo.security.ui.admin.wizards;
+
+import org.argeo.eclipse.ui.EclipseUiUtils;
+import org.argeo.security.UserAdminService;
+import org.eclipse.jface.wizard.WizardPage;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.ModifyEvent;
+import org.eclipse.swt.events.ModifyListener;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Text;
+
+public class MainUserInfoWizardPage extends WizardPage implements
+               ModifyListener {
+       private Text username, firstName, lastName, primaryEmail;
+
+       public MainUserInfoWizardPage() {
+               super("Main");
+               setTitle("Required Information");
+       }
+
+       @Override
+       public void createControl(Composite parent) {
+               parent.setLayout(new FillLayout());
+               Composite composite = new Composite(parent, SWT.NONE);
+               composite.setLayout(new GridLayout(2, false));
+               username = EclipseUiUtils.createGridLT(composite, "Username", this);
+               primaryEmail = EclipseUiUtils.createGridLT(composite, "Email", this);
+               firstName = EclipseUiUtils.createGridLT(composite, "First name", this);
+               lastName = EclipseUiUtils.createGridLT(composite, "Last name", this);
+               setControl(composite);
+       }
+
+       @Override
+       public void modifyText(ModifyEvent event) {
+               String message = checkComplete();
+               if (message != null)
+                       setMessage(message, WizardPage.ERROR);
+               else {
+                       setMessage("Complete", WizardPage.INFORMATION);
+                       setPageComplete(true);
+               }
+       }
+
+       /** @return error message or null if complete */
+       protected String checkComplete() {
+               if (!username.getText().matches(UserAdminService.USERNAME_PATTERN))
+                       return "Wrong user name format, should be lower case, between 3 and 15 characters with only '_' as acceptable special character.";
+               if (!primaryEmail.getText().matches(UserAdminService.EMAIL_PATTERN))
+                       return "Not a valid email address";
+               if (firstName.getText().trim().equals(""))
+                       return "Specify a first name";
+               if (lastName.getText().trim().equals(""))
+                       return "Specify a last name";
+               return null;
+       }
+
+       @Override
+       public boolean canFlipToNextPage() {
+               // TODO Auto-generated method stub
+               return super.canFlipToNextPage();
+       }
+
+       @Override
+       public boolean isPageComplete() {
+               // TODO Auto-generated method stub
+               return super.isPageComplete();
+       }
+
+}
diff --git a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/NewUserWizard.java b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/NewUserWizard.java
new file mode 100644 (file)
index 0000000..6e9704a
--- /dev/null
@@ -0,0 +1,19 @@
+package org.argeo.security.ui.admin.wizards;
+
+import org.eclipse.jface.wizard.Wizard;
+
+public class NewUserWizard extends Wizard {
+       private MainUserInfoWizardPage mainUserInfo;
+
+       @Override
+       public void addPages() {
+               mainUserInfo = new MainUserInfoWizardPage();
+               addPage(mainUserInfo);
+       }
+
+       @Override
+       public boolean performFinish() {
+               return false;
+       }
+
+}
index e813e72a4fd99dc468917b4dcd38331e74f85f59..5fc482f903fdb4028c2698232b56f1a9124e61de 100644 (file)
@@ -4,6 +4,22 @@ import java.util.List;
 import java.util.Set;
 
 public interface UserAdminService {
+       /**
+        * Usernames must match this regexp pattern ({@value #USERNAME_PATTERN}).
+        * Thanks to <a href=
+        * "http://www.mkyong.com/regular-expressions/how-to-validate-username-with-regular-expression/"
+        * >this tip</a> (modified to remove '-')
+        */
+       public final static String USERNAME_PATTERN = "^[a-z0-9_]{3,15}$";
+
+       /**
+        * Email addresses must match this regexp pattern ({@value #EMAIL_PATTERN}.
+        * Thanks to <a href=
+        * "http://www.mkyong.com/regular-expressions/how-to-validate-email-address-with-regular-expression/"
+        * >this tip</a>.
+        */
+       public final static String EMAIL_PATTERN = "^[_A-Za-z0-9-]+(\\.[_A-Za-z0-9-]+)*@[A-Za-z0-9]+(\\.[A-Za-z0-9]+)*(\\.[A-Za-z]{2,})$";
+
        /*
         * USERS
         */
index 6b166d5c5613dfd58483ccc2382758d40e2e5c1a..6b729a19b5db255bb7367becb139ef601c17df7e 100644 (file)
@@ -75,8 +75,11 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper,
 
                if (SecurityContextHolder.getContext().getAuthentication() == null) {
                        // authentication
-                       systemExecutor.execute(action);
-                       JcrUtils.logoutQuietly(session);
+                       try {
+                               systemExecutor.execute(action);
+                       } finally {
+                               JcrUtils.logoutQuietly(session);
+                       }
                } else {
                        // authenticated user
                        action.run();
@@ -120,7 +123,7 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper,
                        if (log.isDebugEnabled())
                                log.debug("Mapped " + ctx.getDn() + " to " + userProfile);
                        return userHomePath;
-               } catch (RepositoryException e) {
+               } catch (Exception e) {
                        JcrUtils.discardQuietly(session);
                        throw new ArgeoException("Cannot synchronize JCR and LDAP", e);
                } finally {
index 1a37e3e855f5edb0cc5fc43570ec7921d21fa584..d05d8450a4608f94c5f6e54f228aec1e3d0e2b63 100644 (file)
@@ -74,7 +74,10 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
 
        /** Logs in to the repository using various strategies. */
        protected Session login() {
-               // discard sesison previoussly attached to this thread
+               if (!isActive())
+                       throw new ArgeoException("Thread bound session factory inactive");
+
+               // discard session previously attached to this thread
                Thread thread = Thread.currentThread();
                if (activeSessions.containsKey(thread.getId())) {
                        Session oldSession = activeSessions.remove(thread.getId());
@@ -129,6 +132,9 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
        }
 
        public synchronized void destroy() throws Exception {
+               if (activeSessions.size() == 0)
+                       return;
+
                if (log.isDebugEnabled())
                        log.debug("Cleaning up " + activeSessions.size()
                                        + " active JCR sessions...");
@@ -138,7 +144,6 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
                        sess.logout();
                }
                activeSessions.clear();
-               monitoringThread.join(1000);
        }
 
        protected Boolean isActive() {
@@ -150,6 +155,39 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
                notifyAll();
        }
 
+       protected synchronized void removeSession(Thread thread) {
+               if (!isActive())
+                       return;
+               activeSessions.remove(thread.getId());
+               threads.remove(thread);
+       }
+
+       protected synchronized void cleanDeadThreads() {
+               if (!isActive())
+                       return;
+               Iterator<Thread> it = threads.iterator();
+               while (it.hasNext()) {
+                       Thread thread = it.next();
+                       if (!thread.isAlive() && isActive()) {
+                               if (activeSessions.containsKey(thread.getId())) {
+                                       Session session = activeSessions.get(thread.getId());
+                                       activeSessions.remove(thread.getId());
+                                       session.logout();
+                                       if (log.isDebugEnabled())
+                                               log.debug("Cleaned up JCR session (userID="
+                                                               + session.getUserID() + ") from dead thread "
+                                                               + thread.getId());
+                               }
+                               it.remove();
+                       }
+               }
+               try {
+                       wait(1000);
+               } catch (InterruptedException e) {
+                       // silent
+               }
+       }
+
        public Class<? extends Session> getObjectType() {
                return Session.class;
        }
@@ -202,18 +240,13 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
 
                        Object ret = method.invoke(threadSession, args);
                        if ("logout".equals(method.getName())) {
-                               synchronized (ThreadBoundJcrSessionFactory.this) {
-                                       session.remove();
-                                       Thread thread = Thread.currentThread();
-                                       if (isActive()) {
-                                               activeSessions.remove(thread.getId());
-                                               threads.remove(thread);
-                                       }
-                                       if (log.isTraceEnabled())
-                                               log.trace("Logged out JCR session (userId="
-                                                               + threadSession.getUserID() + ") on thread "
-                                                               + thread.getId());
-                               }
+                               session.remove();
+                               Thread thread = Thread.currentThread();
+                               removeSession(thread);
+                               if (log.isTraceEnabled())
+                                       log.trace("Logged out JCR session (userId="
+                                                       + threadSession.getUserID() + ") on thread "
+                                                       + thread.getId());
                        }
                        return ret;
                }
@@ -225,32 +258,7 @@ public class ThreadBoundJcrSessionFactory implements FactoryBean,
                @Override
                public void run() {
                        while (isActive()) {
-                               Iterator<Thread> it = threads.iterator();
-                               while (it.hasNext()) {
-                                       Thread thread = it.next();
-                                       if (!thread.isAlive() && isActive()) {
-                                               if (activeSessions.containsKey(thread.getId())) {
-                                                       Session session = activeSessions
-                                                                       .get(thread.getId());
-                                                       activeSessions.remove(thread.getId());
-                                                       session.logout();
-                                                       if (log.isDebugEnabled())
-                                                               log.debug("Cleaned up JCR session (userID="
-                                                                               + session.getUserID()
-                                                                               + ") from dead thread "
-                                                                               + thread.getId());
-                                               }
-                                               it.remove();
-                                       }
-                               }
-
-                               synchronized (ThreadBoundJcrSessionFactory.this) {
-                                       try {
-                                               ThreadBoundJcrSessionFactory.this.wait(1000);
-                                       } catch (InterruptedException e) {
-                                               // silent
-                                       }
-                               }
+                               cleanDeadThreads();
                        }
                }