From 7eab8a52b56a763e0d0e5153d298ce658e41a22f Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Tue, 15 Sep 2015 18:30:31 +0000 Subject: [PATCH] Change password working git-svn-id: https://svn.argeo.org/commons/trunk@8401 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc --- .../internal/auth/UserAdminLoginModule.java | 22 +-- org.argeo.eclipse.ui/bnd.bnd | 1 - .../ui/dialogs/ChangePasswordDialog.java | 94 ------------- .../osgi/useradmin/AbstractUserDirectory.java | 6 +- .../argeo/osgi/useradmin/LdapUserAdmin.java | 2 +- .../org/argeo/osgi/useradmin/LdifName.java | 2 +- .../org/argeo/osgi/useradmin/LdifUser.java | 54 ++++++++ .../META-INF/spring/commands.xml | 3 +- .../META-INF/spring/osgi.xml | 5 +- .../ui/commands/OpenChangePasswordDialog.java | 130 +++++++++++++++++- 10 files changed, 188 insertions(+), 131 deletions(-) delete mode 100644 org.argeo.eclipse.ui/src/org/argeo/eclipse/ui/dialogs/ChangePasswordDialog.java diff --git a/org.argeo.cms/src/org/argeo/cms/internal/auth/UserAdminLoginModule.java b/org.argeo.cms/src/org/argeo/cms/internal/auth/UserAdminLoginModule.java index 16a7d7265..5fca43be3 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/auth/UserAdminLoginModule.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/auth/UserAdminLoginModule.java @@ -1,8 +1,5 @@ package org.argeo.cms.internal.auth; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.Charset; import java.security.Principal; import java.util.Arrays; import java.util.Collections; @@ -22,8 +19,6 @@ import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; import javax.security.auth.x500.X500Principal; -import org.apache.commons.codec.binary.Base64; -import org.apache.commons.codec.digest.DigestUtils; import org.apache.jackrabbit.core.security.AnonymousPrincipal; import org.apache.jackrabbit.core.security.SecurityConstants; import org.apache.jackrabbit.core.security.principal.AdminPrincipal; @@ -113,15 +108,10 @@ public class UserAdminLoginModule implements LoginModule { else throw new CredentialNotFoundException("No credentials provided"); - // user = (User) userAdmin.getRole(username); user = userAdmin.getUser(null, username); if (user == null) return false; - - byte[] hashedPassword = ("{SHA}" + Base64 - .encodeBase64String(DigestUtils.sha1(toBytes(password)))) - .getBytes(); - if (!user.hasCredential("userpassword", hashedPassword)) + if (!user.hasCredential(null, password)) return false; } else // anonymous @@ -130,16 +120,6 @@ public class UserAdminLoginModule implements LoginModule { return true; } - private byte[] toBytes(char[] chars) { - CharBuffer charBuffer = CharBuffer.wrap(chars); - ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer); - byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), - byteBuffer.position(), byteBuffer.limit()); - Arrays.fill(charBuffer.array(), '\u0000'); // clear sensitive data - Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data - return bytes; - } - @Override public boolean commit() throws LoginException { if (authorization != null) { diff --git a/org.argeo.eclipse.ui/bnd.bnd b/org.argeo.eclipse.ui/bnd.bnd index bf8d37a8f..98a501e33 100644 --- a/org.argeo.eclipse.ui/bnd.bnd +++ b/org.argeo.eclipse.ui/bnd.bnd @@ -8,5 +8,4 @@ Import-Package: org.eclipse.core.commands,\ org.springframework.core.io.support,\ org.springframework.dao,\ javax.jcr.nodetype,\ - org.springframework.security.core,\ * diff --git a/org.argeo.eclipse.ui/src/org/argeo/eclipse/ui/dialogs/ChangePasswordDialog.java b/org.argeo.eclipse.ui/src/org/argeo/eclipse/ui/dialogs/ChangePasswordDialog.java deleted file mode 100644 index 6c492ce97..000000000 --- a/org.argeo.eclipse.ui/src/org/argeo/eclipse/ui/dialogs/ChangePasswordDialog.java +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright (C) 2007-2012 Argeo GmbH - * - * 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.argeo.eclipse.ui.dialogs; - -import org.argeo.ArgeoException; -import org.eclipse.jface.dialogs.IMessageProvider; -import org.eclipse.jface.dialogs.MessageDialog; -import org.eclipse.jface.dialogs.TitleAreaDialog; -import org.eclipse.swt.SWT; -import org.eclipse.swt.graphics.Point; -import org.eclipse.swt.layout.GridData; -import org.eclipse.swt.layout.GridLayout; -import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Control; -import org.eclipse.swt.widgets.Label; -import org.eclipse.swt.widgets.Shell; -import org.eclipse.swt.widgets.Text; -import org.springframework.security.provisioning.UserDetailsManager; - -/** Dialog to change the current user password */ -public class ChangePasswordDialog extends TitleAreaDialog { - private static final long serialVersionUID = -2447446550246803237L; - - private Text currentPassword, newPassword1, newPassword2; - private UserDetailsManager userDetailsManager; - - public ChangePasswordDialog(Shell parentShell, - UserDetailsManager securityService) { - super(parentShell); - this.userDetailsManager = securityService; - } - - protected Point getInitialSize() { - return new Point(400, 450); - } - - protected Control createDialogArea(Composite parent) { - Composite dialogarea = (Composite) super.createDialogArea(parent); - dialogarea.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); - Composite composite = new Composite(dialogarea, SWT.NONE); - composite.setLayout(new GridLayout(2, false)); - composite.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - currentPassword = createLP(composite, "Current password"); - newPassword1 = createLP(composite, "New password"); - newPassword2 = createLP(composite, "Repeat new password"); - - setMessage("Change password", IMessageProvider.INFORMATION); - parent.pack(); - return composite; - } - - @Override - protected void okPressed() { - if (!newPassword1.getText().equals(newPassword2.getText())) - throw new ArgeoException("Passwords are different"); - try { - userDetailsManager.changePassword(currentPassword.getText(), - newPassword1.getText()); - close(); - } catch (Exception e) { - MessageDialog.openError(newPassword1.getShell(), "Error", - "Cannot change password"); - e.printStackTrace(); - } - } - - /** Creates label and password. */ - protected Text createLP(Composite parent, String label) { - new Label(parent, SWT.NONE).setText(label); - Text text = new Text(parent, SWT.SINGLE | SWT.LEAD | SWT.PASSWORD - | SWT.BORDER); - text.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); - return text; - } - - protected void configureShell(Shell shell) { - super.configureShell(shell); - shell.setText("Change password"); - } - -} diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java index c7448b574..95e1fc0b6 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/AbstractUserDirectory.java @@ -56,11 +56,11 @@ public abstract class AbstractUserDirectory implements UserAdmin { private UserAdmin externalRoles; private List indexedUserProperties = Arrays.asList(new String[] { - "uid", "mail", "cn" }); + LdifName.uid.name(), LdifName.mail.name(), LdifName.cn.name() }); private String memberAttributeId = "member"; private List credentialAttributeIds = Arrays - .asList(new String[] { "userpassword" }); + .asList(new String[] { LdifName.userpassword.name() }); // private TransactionSynchronizationRegistry syncRegistry; // private Object editingTransactionKey = null; @@ -89,7 +89,7 @@ public abstract class AbstractUserDirectory implements UserAdmin { this.isReadOnly = readOnlyDefault(uri); else this.isReadOnly = new Boolean(isReadOnly); - + this.userObjectClass = LdapProperties.userObjectClass .getValue(properties); this.groupObjectClass = LdapProperties.groupObjectClass diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java index 7129cfb56..23a31fc87 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdapUserAdmin.java @@ -38,7 +38,7 @@ public class LdapUserAdmin extends AbstractUserDirectory { "com.sun.jndi.ldap.LdapCtxFactory"); connEnv.put(Context.PROVIDER_URL, getUri().toString()); connEnv.put("java.naming.ldap.attributes.binary", - LdifName.userPassword.name()); + LdifName.userpassword.name()); initialLdapContext = new InitialLdapContext(connEnv, null); // StartTlsResponse tls = (StartTlsResponse) ctx diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifName.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifName.java index 2614b6fa5..0534267b3 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifName.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifName.java @@ -9,7 +9,7 @@ import javax.naming.ldap.LdapName; */ public enum LdifName { // Attributes - cn, sn, uid, displayName, objectClass,userPassword, + cn, sn, uid, mail, displayName, objectClass, userpassword, // Object classes inetOrgPerson, organizationalPerson, person, groupOfNames, top; diff --git a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUser.java b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUser.java index 712ccdfde..d54964226 100644 --- a/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUser.java +++ b/org.argeo.security.core/src/org/argeo/osgi/useradmin/LdifUser.java @@ -1,5 +1,8 @@ package org.argeo.osgi.useradmin; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -15,6 +18,8 @@ import javax.naming.directory.Attributes; import javax.naming.directory.BasicAttribute; import javax.naming.ldap.LdapName; +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.codec.digest.DigestUtils; import org.argeo.osgi.useradmin.AbstractUserDirectory.WorkingCopy; class LdifUser implements DirectoryUser { @@ -64,6 +69,13 @@ class LdifUser implements DirectoryUser { @Override public boolean hasCredential(String key, Object value) { + if (key == null) { + // TODO check other sources (like PKCS12) + char[] password = toChars(value); + byte[] hashedPassword = hash(password); + return hasCredential(LdifName.userpassword.name(), hashedPassword); + } + Object storedValue = getCredentials().get(key); if (storedValue == null || value == null) return false; @@ -76,6 +88,41 @@ class LdifUser implements DirectoryUser { return false; } + /** Hash and clear the password */ + private byte[] hash(char[] password) { + byte[] hashedPassword = ("{SHA}" + Base64 + .encodeBase64String(DigestUtils.sha1(toBytes(password)))) + .getBytes(); + Arrays.fill(password, '\u0000'); + return hashedPassword; + } + + private byte[] toBytes(char[] chars) { + CharBuffer charBuffer = CharBuffer.wrap(chars); + ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer); + byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), + byteBuffer.position(), byteBuffer.limit()); + Arrays.fill(charBuffer.array(), '\u0000'); // clear sensitive data + Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data + return bytes; + } + + private char[] toChars(Object obj) { + if (obj instanceof char[]) + return (char[]) obj; + if (!(obj instanceof byte[])) + throw new IllegalArgumentException(obj.getClass() + + " is not a byte array"); + ByteBuffer fromBuffer = ByteBuffer.wrap((byte[]) obj); + CharBuffer toBuffer = Charset.forName("UTF-8").decode(fromBuffer); + char[] res = Arrays.copyOfRange(toBuffer.array(), toBuffer.position(), + toBuffer.limit()); + Arrays.fill(fromBuffer.array(), (byte) 0); // clear sensitive data + Arrays.fill((byte[]) obj, (byte) 0); // clear sensitive data + Arrays.fill(toBuffer.array(), '\u0000'); // clear sensitive data + return res; + } + @Override public LdapName getDn() { return dn; @@ -227,6 +274,13 @@ class LdifUser implements DirectoryUser { @Override public Object put(String key, Object value) { + if (key == null) { + // TODO persist to other sources (like PKCS12) + char[] password = toChars(value); + byte[] hashedPassword = hash(password); + return put(LdifName.userpassword.name(), hashedPassword); + } + userAdmin.checkEdit(); if (!isEditing()) startEditing(); diff --git a/org.argeo.security.ui.rap/META-INF/spring/commands.xml b/org.argeo.security.ui.rap/META-INF/spring/commands.xml index 3dd037a3c..0dc74d0f4 100644 --- a/org.argeo.security.ui.rap/META-INF/spring/commands.xml +++ b/org.argeo.security.ui.rap/META-INF/spring/commands.xml @@ -6,7 +6,8 @@ - + + diff --git a/org.argeo.security.ui.rap/META-INF/spring/osgi.xml b/org.argeo.security.ui.rap/META-INF/spring/osgi.xml index 916c2c692..84e5d7b8b 100644 --- a/org.argeo.security.ui.rap/META-INF/spring/osgi.xml +++ b/org.argeo.security.ui.rap/META-INF/spring/osgi.xml @@ -8,7 +8,6 @@ http://www.springframework.org/schema/beans/spring-beans-2.5.xsd" osgi:default-timeout="30000"> - + + \ No newline at end of file diff --git a/org.argeo.security.ui/src/org/argeo/security/ui/commands/OpenChangePasswordDialog.java b/org.argeo.security.ui/src/org/argeo/security/ui/commands/OpenChangePasswordDialog.java index f652ccde4..ed3e2c13c 100644 --- a/org.argeo.security.ui/src/org/argeo/security/ui/commands/OpenChangePasswordDialog.java +++ b/org.argeo.security.ui/src/org/argeo/security/ui/commands/OpenChangePasswordDialog.java @@ -15,22 +15,47 @@ */ package org.argeo.security.ui.commands; -import org.argeo.eclipse.ui.dialogs.ChangePasswordDialog; +import java.security.AccessController; + +import javax.naming.InvalidNameException; +import javax.naming.ldap.LdapName; +import javax.security.auth.Subject; +import javax.security.auth.x500.X500Principal; +import javax.transaction.UserTransaction; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.argeo.ArgeoException; 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.IMessageProvider; import org.eclipse.jface.dialogs.MessageDialog; +import org.eclipse.jface.dialogs.TitleAreaDialog; +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Point; +import org.eclipse.swt.layout.GridData; +import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Control; +import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Text; import org.eclipse.ui.handlers.HandlerUtil; -import org.springframework.security.provisioning.UserDetailsManager; +import org.osgi.service.useradmin.User; +import org.osgi.service.useradmin.UserAdmin; /** Opens the change password dialog. */ public class OpenChangePasswordDialog extends AbstractHandler { - private UserDetailsManager userDetailsManager; + private final static Log log = LogFactory + .getLog(OpenChangePasswordDialog.class); + private UserAdmin userAdmin; + private UserTransaction userTransaction; public Object execute(ExecutionEvent event) throws ExecutionException { ChangePasswordDialog dialog = new ChangePasswordDialog( - HandlerUtil.getActiveShell(event), userDetailsManager); + HandlerUtil.getActiveShell(event), userAdmin); if (dialog.open() == Dialog.OK) { MessageDialog.openInformation(HandlerUtil.getActiveShell(event), "Password changed", "Password changed."); @@ -38,8 +63,101 @@ public class OpenChangePasswordDialog extends AbstractHandler { return null; } - public void setUserDetailsManager(UserDetailsManager userDetailsManager) { - this.userDetailsManager = userDetailsManager; + protected void changePassword(char[] oldPassword, char[] newPassword) { + Subject subject = Subject.getSubject(AccessController.getContext()); + String name = subject.getPrincipals(X500Principal.class).iterator() + .next().toString(); + LdapName dn; + try { + dn = new LdapName(name); + } catch (InvalidNameException e) { + throw new ArgeoException("Invalid user dn " + name, e); + } + try { + userTransaction.begin(); + User user = (User) userAdmin.getRole(dn.toString()); + if (user.hasCredential(null, oldPassword)) { + user.getCredentials().put(null, newPassword); + } + userTransaction.commit(); + } catch (Exception e) { + try { + userTransaction.rollback(); + } catch (Exception e1) { + log.error("Could not roll back", e1); + } + if (e instanceof RuntimeException) + throw (RuntimeException) e; + else + throw new ArgeoException("Cannot change password", e); + } + } + + public void setUserAdmin(UserAdmin userDetailsManager) { + this.userAdmin = userDetailsManager; } + public void setUserTransaction(UserTransaction userTransaction) { + this.userTransaction = userTransaction; + } + + class ChangePasswordDialog extends TitleAreaDialog { + private static final long serialVersionUID = -6963970583882720962L; + private Text currentPassword, newPassword1, newPassword2; + + public ChangePasswordDialog(Shell parentShell, UserAdmin securityService) { + super(parentShell); + } + + protected Point getInitialSize() { + return new Point(400, 450); + } + + protected Control createDialogArea(Composite parent) { + Composite dialogarea = (Composite) super.createDialogArea(parent); + dialogarea.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, + true)); + Composite composite = new Composite(dialogarea, SWT.NONE); + composite.setLayout(new GridLayout(2, false)); + composite.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, + false)); + currentPassword = createLP(composite, "Current password"); + newPassword1 = createLP(composite, "New password"); + newPassword2 = createLP(composite, "Repeat new password"); + + setMessage("Change password", IMessageProvider.INFORMATION); + parent.pack(); + return composite; + } + + @Override + protected void okPressed() { + if (!newPassword1.getText().equals(newPassword2.getText())) + throw new ArgeoException("Passwords are different"); + try { + changePassword(currentPassword.getTextChars(), + newPassword1.getTextChars()); + close(); + } catch (Exception e) { + MessageDialog.openError(newPassword1.getShell(), "Error", + "Cannot change password"); + e.printStackTrace(); + } + } + + /** Creates label and password. */ + protected Text createLP(Composite parent, String label) { + new Label(parent, SWT.NONE).setText(label); + Text text = new Text(parent, SWT.SINGLE | SWT.LEAD | SWT.PASSWORD + | SWT.BORDER); + text.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false)); + return text; + } + + protected void configureShell(Shell shell) { + super.configureShell(shell); + shell.setText("Change password"); + } + + } } -- 2.30.2