From 019e0f2af17286be08ab17c1c9e1d8ba871ec9b2 Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Wed, 23 Mar 2011 12:19:27 +0000 Subject: [PATCH] Keep improving security git-svn-id: https://svn.argeo.org/commons/trunk@4342 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc --- .../org/argeo/eclipse/ui/EclipseUiUtils.java | 37 ++++++++ .../META-INF/spring/commands.xml | 5 +- .../security/ui/admin/commands/NewUser.java | 24 +++++ .../admin/wizards/MainUserInfoWizardPage.java | 71 +++++++++++++++ .../ui/admin/wizards/NewUserWizard.java | 19 ++++ .../org/argeo/security/UserAdminService.java | 16 ++++ .../ldap/jcr/JcrUserDetailsContextMapper.java | 9 +- .../jcr/ThreadBoundJcrSessionFactory.java | 88 ++++++++++--------- 8 files changed, 224 insertions(+), 45 deletions(-) create mode 100644 eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/EclipseUiUtils.java create mode 100644 security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/NewUser.java create mode 100644 security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/MainUserInfoWizardPage.java create mode 100644 security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/NewUserWizard.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 index 000000000..333db5b88 --- /dev/null +++ b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/EclipseUiUtils.java @@ -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; + } + +} diff --git a/security/plugins/org.argeo.security.ui.admin/META-INF/spring/commands.xml b/security/plugins/org.argeo.security.ui.admin/META-INF/spring/commands.xml index 044fabf58..36aefbe03 100644 --- a/security/plugins/org.argeo.security.ui.admin/META-INF/spring/commands.xml +++ b/security/plugins/org.argeo.security.ui.admin/META-INF/spring/commands.xml @@ -4,9 +4,10 @@ xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd"> - - 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 index 000000000..de8dc9b54 --- /dev/null +++ b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/NewUser.java @@ -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 index 000000000..cf11f8517 --- /dev/null +++ b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/MainUserInfoWizardPage.java @@ -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 index 000000000..6e9704a49 --- /dev/null +++ b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/wizards/NewUserWizard.java @@ -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; + } + +} diff --git a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/UserAdminService.java b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/UserAdminService.java index e813e72a4..5fc482f90 100644 --- a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/UserAdminService.java +++ b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/UserAdminService.java @@ -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 this tip (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 this tip. + */ + 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 */ diff --git a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java index 6b166d5c5..6b729a19b 100644 --- a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java +++ b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java @@ -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 { diff --git a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/ThreadBoundJcrSessionFactory.java b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/ThreadBoundJcrSessionFactory.java index 1a37e3e85..d05d8450a 100644 --- a/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/ThreadBoundJcrSessionFactory.java +++ b/server/runtime/org.argeo.server.jcr/src/main/java/org/argeo/jcr/ThreadBoundJcrSessionFactory.java @@ -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 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 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 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(); } } -- 2.30.2