From 4c8c237990cda2b1a9be35532796510d9d5734c5 Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Tue, 31 May 2011 16:11:17 +0000 Subject: [PATCH] Improve Security UI git-svn-id: https://svn.argeo.org/commons/trunk@4550 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc --- .../argeo/eclipse/ui/dialogs/SingleValue.java | 2 +- .../META-INF/spring/commands.xml | 1 + .../ui/admin/commands/RefreshUsersList.java | 44 ++++++++++++++++++- .../ui/admin/editors/DefaultUserMainPage.java | 1 + .../security/ui/rap/SecureEntryPoint.java | 21 +++++++++ .../ui/commands/OpenChangePasswordDialog.java | 7 ++- .../org/argeo/security/UserAdminService.java | 3 ++ .../ldap/ArgeoLdapUserDetailsManager.java | 24 +++++++++- .../ldap/jcr/JcrUserDetailsContextMapper.java | 26 ++++++++--- 9 files changed, 118 insertions(+), 11 deletions(-) diff --git a/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/dialogs/SingleValue.java b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/dialogs/SingleValue.java index d3e67978f..c15a76736 100644 --- a/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/dialogs/SingleValue.java +++ b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/dialogs/SingleValue.java @@ -14,7 +14,7 @@ import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.Text; -/** Dialog to change the current user password */ +/** Dialog retrieve a single value. */ public class SingleValue extends TitleAreaDialog { private Text valueT; private String value; 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 96d25ddb6..7f6781cc1 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 @@ -22,5 +22,6 @@ + diff --git a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/RefreshUsersList.java b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/RefreshUsersList.java index a66b79067..6cf9a7749 100644 --- a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/RefreshUsersList.java +++ b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/commands/RefreshUsersList.java @@ -1,5 +1,17 @@ package org.argeo.security.ui.admin.commands; +import java.util.Set; + +import javax.jcr.Node; +import javax.jcr.NodeIterator; +import javax.jcr.RepositoryException; +import javax.jcr.Session; +import javax.jcr.query.Query; + +import org.argeo.ArgeoException; +import org.argeo.jcr.ArgeoNames; +import org.argeo.jcr.ArgeoTypes; +import org.argeo.jcr.JcrUtils; import org.argeo.security.UserAdminService; import org.argeo.security.ui.admin.views.UsersView; import org.eclipse.core.commands.AbstractHandler; @@ -7,11 +19,37 @@ import org.eclipse.core.commands.ExecutionEvent; import org.eclipse.core.commands.ExecutionException; import org.eclipse.ui.handlers.HandlerUtil; -/** Refresh the main EBI list. */ +/** + * Refreshes the main EBI list, removing nodes which are not referenced by user + * admin service. + */ public class RefreshUsersList extends AbstractHandler { private UserAdminService userAdminService; + private Session session; public Object execute(ExecutionEvent event) throws ExecutionException { + Set users = userAdminService.listUsers(); + try { + Query query = session + .getWorkspace() + .getQueryManager() + .createQuery( + "select * from [" + ArgeoTypes.ARGEO_USER_HOME + + "]", Query.JCR_SQL2); + NodeIterator nit = query.execute().getNodes(); + while (nit.hasNext()) { + Node node = nit.nextNode(); + String username = node.getProperty(ArgeoNames.ARGEO_USER_ID) + .getString(); + if (!users.contains(username)) + node.remove(); + } + session.save(); + } catch (RepositoryException e) { + JcrUtils.discardQuietly(session); + throw new ArgeoException("Cannot list users", e); + } + userAdminService.synchronize(); UsersView view = (UsersView) HandlerUtil .getActiveWorkbenchWindow(event).getActivePage() @@ -24,4 +62,8 @@ public class RefreshUsersList extends AbstractHandler { this.userAdminService = userAdminService; } + public void setSession(Session session) { + this.session = session; + } + } \ No newline at end of file diff --git a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/editors/DefaultUserMainPage.java b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/editors/DefaultUserMainPage.java index cfaf6e4fb..22927bd11 100644 --- a/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/editors/DefaultUserMainPage.java +++ b/security/plugins/org.argeo.security.ui.admin/src/main/java/org/argeo/security/ui/admin/editors/DefaultUserMainPage.java @@ -118,6 +118,7 @@ public class DefaultUserMainPage extends FormPage implements ArgeoNames { email.getText()); userProfile.setProperty(Property.JCR_DESCRIPTION, description.getText()); + userProfile.getSession().save(); super.commit(onSave); if (log.isTraceEnabled()) log.trace("General part committed"); diff --git a/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java b/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java index 638b33a12..d38bd8bc0 100644 --- a/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java +++ b/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java @@ -8,11 +8,15 @@ import javax.security.auth.login.LoginException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.eclipse.equinox.security.auth.ILoginContext; +import org.eclipse.jface.dialogs.Dialog; +import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.rwt.RWT; import org.eclipse.rwt.lifecycle.IEntryPoint; import org.eclipse.rwt.service.SessionStoreEvent; import org.eclipse.rwt.service.SessionStoreListener; +import org.eclipse.swt.graphics.Image; import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.application.IWorkbenchWindowConfigurer; import org.eclipse.ui.application.WorkbenchAdvisor; @@ -21,6 +25,7 @@ import org.eclipse.ui.application.WorkbenchWindowAdvisor; public class SecureEntryPoint implements IEntryPoint, SessionStoreListener { private final static Log log = LogFactory.getLog(SecureEntryPoint.class); + @SuppressWarnings("unchecked") @Override public int createUI() { // 15 mins session timeout @@ -41,6 +46,8 @@ public class SecureEntryPoint implements IEntryPoint, SessionStoreListener { subject = loginContext.getSubject(); } catch (LoginException e) { log.error("Error when logging in.", e); + MessageDialog.openInformation(display.getActiveShell(), + "Login failed", "Login failed"); display.dispose(); RWT.getRequest().getSession().setMaxInactiveInterval(1); try { @@ -48,6 +55,7 @@ public class SecureEntryPoint implements IEntryPoint, SessionStoreListener { } catch (InterruptedException e1) { // silent } + // throw new RuntimeException("Login failed", e); return -1; } @@ -121,6 +129,19 @@ public class SecureEntryPoint implements IEntryPoint, SessionStoreListener { // log.debug("Workbench closed"); // } + static class FailedLogin extends MessageDialog { + + public FailedLogin(Shell parentShell, String dialogTitle, + Image dialogTitleImage, String dialogMessage, + int dialogImageType, String[] dialogButtonLabels, + int defaultIndex) { + super(parentShell, "Failed ", dialogTitleImage, dialogMessage, + dialogImageType, dialogButtonLabels, defaultIndex); + // TODO Auto-generated constructor stub + } + + } + @SuppressWarnings("rawtypes") private PrivilegedAction getRunAction(final Display display) { return new PrivilegedAction() { diff --git a/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/commands/OpenChangePasswordDialog.java b/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/commands/OpenChangePasswordDialog.java index ab52b116c..c23e52006 100644 --- a/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/commands/OpenChangePasswordDialog.java +++ b/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/commands/OpenChangePasswordDialog.java @@ -4,6 +4,8 @@ import org.argeo.security.ui.dialogs.ChangePasswordDialog; import org.eclipse.core.commands.AbstractHandler; import org.eclipse.core.commands.ExecutionEvent; import org.eclipse.core.commands.ExecutionException; +import org.eclipse.jface.dialogs.Dialog; +import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.ui.handlers.HandlerUtil; import org.springframework.security.userdetails.UserDetailsManager; @@ -14,7 +16,10 @@ public class OpenChangePasswordDialog extends AbstractHandler { public Object execute(ExecutionEvent event) throws ExecutionException { ChangePasswordDialog dialog = new ChangePasswordDialog( HandlerUtil.getActiveShell(event), userDetailsManager); - dialog.open(); + if (dialog.open() == Dialog.OK) { + MessageDialog.openInformation(HandlerUtil.getActiveShell(event), + "Password changed", "Password changed."); + } return null; } 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 964c9dff2..8844534c3 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 @@ -24,6 +24,9 @@ public interface UserAdminService extends UserDetailsManager { /* * USERS */ + /** List all users. */ + public Set listUsers(); + /** List users having this role (except the super user). */ public Set listUsersInRole(String role); diff --git a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/ArgeoLdapUserDetailsManager.java b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/ArgeoLdapUserDetailsManager.java index 040d650d7..392ac4a27 100644 --- a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/ArgeoLdapUserDetailsManager.java +++ b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/ArgeoLdapUserDetailsManager.java @@ -10,10 +10,13 @@ import java.util.Random; import java.util.Set; import java.util.TreeSet; +import org.argeo.ArgeoException; import org.argeo.security.UserAdminDao; import org.argeo.security.UserAdminService; import org.springframework.ldap.core.ContextSource; +import org.springframework.security.Authentication; import org.springframework.security.GrantedAuthority; +import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.encoding.PasswordEncoder; import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.ldap.LdapUserDetailsManager; @@ -41,7 +44,22 @@ public class ArgeoLdapUserDetailsManager extends LdapUserDetailsManager @Override public void changePassword(String oldPassword, String newPassword) { - super.changePassword(oldPassword, encodePassword(newPassword)); + Authentication authentication = SecurityContextHolder.getContext() + .getAuthentication(); + if (authentication == null) + throw new ArgeoException( + "Cannot change password without authentication"); + String username = authentication.getName(); + UserDetails userDetails = loadUserByUsername(username); + String currentPassword = userDetails.getPassword(); + if (currentPassword == null) + throw new ArgeoException("Cannot access current password"); + if (!passwordEncoder + .isPasswordValid(currentPassword, oldPassword, null)) + throw new ArgeoException("Old password invalid"); + // Spring Security LDAP 2.0 is buggy when used with OpenLDAP and called + // with oldPassword argument + super.changePassword(null, encodePassword(newPassword)); } public void newRole(String role) { @@ -58,6 +76,10 @@ public class ArgeoLdapUserDetailsManager extends LdapUserDetailsManager userAdminDao.deleteRole(role); } + public Set listUsers() { + return userAdminDao.listUsers(); + } + public Set listUsersInRole(String role) { Set lst = new TreeSet( userAdminDao.listUsersInRole(role)); 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 7e2d89e6d..ec4255af9 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 @@ -6,6 +6,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.Random; +import java.util.SortedSet; import java.util.concurrent.Executor; import javax.jcr.Node; @@ -64,6 +65,10 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper, public UserDetails mapUserFromContext(final DirContextOperations ctx, final String username, GrantedAuthority[] authorities) { + if (ctx == null) + throw new ArgeoException("No LDAP information found for user " + + username); + final StringBuffer userHomePathT = new StringBuffer(""); Runnable action = new Runnable() { public void run() { @@ -85,13 +90,20 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper, } // password - byte[] arr = (byte[]) ctx - .getAttributeSortedStringSet(passwordAttribute).first(); + SortedSet passwordAttributes = ctx + .getAttributeSortedStringSet(passwordAttribute); + String password; + if (passwordAttributes == null || passwordAttributes.size() == 0) { + throw new ArgeoException("No password found for user " + username); + } else { + byte[] arr = (byte[]) passwordAttributes.first(); + password = new String(arr); + // erase password + Arrays.fill(arr, (byte) 0); + } JcrUserDetails userDetails = new JcrUserDetails( - userHomePathT.toString(), username, new String(arr), true, - true, true, true, authorities); - // erase password - Arrays.fill(arr, (byte) 0); + userHomePathT.toString(), username, password, true, true, true, + true, authorities); return userDetails; } @@ -210,7 +222,7 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper, if (ldapAttribute.equals("description")) { String value = userProfile.getProperty(jcrProperty).getString(); - if(value.trim().equals("")) + if (value.trim().equals("")) return; } -- 2.30.2