From 8363572b22b5915eb8aa7d8251b6055ac63407b2 Mon Sep 17 00:00:00 2001 From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca> Date: Fri, 24 Jul 2015 14:12:14 -0700 Subject: [PATCH] ac2: don't use PosixDetails in the UserDAO --- .../nrc/cadc/ac/server/ldap/LdapUserDAO.java | 177 ++++++++++-------- .../cadc/ac/server/ldap/LdapUserDAOTest.java | 93 ++++++--- 2 files changed, 174 insertions(+), 96 deletions(-) diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java index a6b2e2aa..6d272ca3 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java @@ -124,7 +124,6 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO protected static final String LDAP_OBJECT_CLASS = "objectClass"; protected static final String LDAP_INET_ORG_PERSON = "inetOrgPerson"; protected static final String LDAP_CADC_ACCOUNT = "cadcaccount"; - protected static final String LDAP_POSIX_ACCOUNT = "posixaccount"; protected static final String LDAP_NSACCOUNTLOCK = "nsaccountlock"; protected static final String LDAP_MEMBEROF = "memberOf"; protected static final String LDAP_ENTRYDN = "entrydn"; @@ -139,18 +138,12 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO protected static final String LDAP_EMAIL = "email"; protected static final String LDAP_INSTITUTE = "institute"; protected static final String LDAP_UID = "uid"; - protected static final String LDAP_UID_NUMBER = "uidNumber"; - protected static final String LDAP_GID_NUMBER = "gidNumber"; - protected static final String LDAP_HOME_DIRECTORY = "homeDirectory"; - protected static final String LDAP_LOGIN_SHELL = "loginShell"; private String[] userAttribs = new String[] { LDAP_FIRST_NAME, LDAP_LAST_NAME, LDAP_ADDRESS, LDAP_CITY, LDAP_COUNTRY, - LDAP_EMAIL, LDAP_INSTITUTE, LDAP_UID, LDAP_UID_NUMBER, - LDAP_GID_NUMBER, - LDAP_HOME_DIRECTORY, LDAP_LOGIN_SHELL + LDAP_EMAIL, LDAP_INSTITUTE, LDAP_UID }; private String[] memberAttribs = new String[] { @@ -251,12 +244,12 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO return userIDs; } - catch (LDAPException e1) + catch (LDAPException e) { - logger.debug("getCadcIDs Exception: " + e1, e1); - LdapDAO.checkLdapResult(e1.getResultCode()); + logger.error("getCadcIDs Exception: " + e, e); + LdapDAO.checkLdapResult(e.getResultCode()); throw new IllegalStateException("Unexpected exception: " + - e1.getMatchedDN(), e1); + e.getMatchedDN(), e); } } @@ -271,6 +264,70 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO */ public User<T> addUser(final UserRequest<T> userRequest) throws TransientException, UserAlreadyExistsException + { + DN userDN; + try + { + userDN = getUserRequestsDN(userRequest.getUser().getUserID().getName()); + addUser(userRequest, userDN); + + // AD: Search results sometimes come incomplete if + // connection is not reset - not sure why. + getConnection().reconnect(); + try + { + return getUser(userRequest.getUser().getUserID(), config.getUserRequestsDN()); + } + catch (UserNotFoundException e) + { + throw new RuntimeException("BUG: new user " + userDN.toNormalizedString() + + " not found because " + e.getMessage()); + } + } + catch (LDAPException e) + { + logger.error("addUser Exception: " + e, e); + LdapUserDAO.checkUserLDAPResult(e.getResultCode()); + throw new RuntimeException("Unexpected LDAP exception", e); + } + } + + /** + * Package level method for unit testing to add a User directly to the + * Users tree. + */ + User<T> newUser(final UserRequest<T> userRequest) + throws TransientException, UserAlreadyExistsException + { + DN userDN; + try + { + userDN = getUserDN(userRequest.getUser().getUserID().getName()); + addUser(userRequest, userDN); + + // AD: Search results sometimes come incomplete if + // connection is not reset - not sure why. + getConnection().reconnect(); + try + { + return getUser(userRequest.getUser().getUserID(), config.getUsersDN()); + } + catch (UserNotFoundException e) + { + throw new RuntimeException("BUG: new user " + userDN.toNormalizedString() + + " not found because " + e.getMessage()); + } + } + catch (LDAPException e) + { + logger.error("newUser Exception: " + e, e); + LdapUserDAO.checkUserLDAPResult(e.getResultCode()); + throw new RuntimeException("Unexpected LDAP exception", e); + } + } + + private void addUser(final UserRequest<T> userRequest, final DN userDN) + throws TransientException, UserAlreadyExistsException { final User<T> user = userRequest.getUser(); final Class userType = user.getUserID().getClass(); @@ -284,24 +341,22 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO try { // add new user - DN userDN = getUserRequestsDN(user.getUserID().getName()); List<Attribute> attributes = new ArrayList<Attribute>(); addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_INET_ORG_PERSON); addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_CADC_ACCOUNT); addAttribute(attributes, LDAP_COMMON_NAME, user.getUserID() - .getName()); + .getName()); addAttribute(attributes, LDAP_DISTINGUISHED_NAME, userDN - .toNormalizedString()); + .toNormalizedString()); addAttribute(attributes, LADP_USER_PASSWORD, userRequest - .getPassword()); + .getPassword()); for (UserDetails details : user.details) { if (details.getClass() == PersonalDetails.class) { PersonalDetails pd = (PersonalDetails) details; - addAttribute(attributes, LDAP_FIRST_NAME, pd - .getFirstName()); + addAttribute(attributes, LDAP_FIRST_NAME, pd.getFirstName()); addAttribute(attributes, LDAP_LAST_NAME, pd.getLastName()); addAttribute(attributes, LDAP_ADDRESS, pd.address); addAttribute(attributes, LDAP_CITY, pd.city); @@ -311,42 +366,18 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO } else if (details.getClass() == PosixDetails.class) { - PosixDetails pd = (PosixDetails) details; - addAttribute(attributes, LDAP_OBJECT_CLASS, LDAP_POSIX_ACCOUNT); - addAttribute(attributes, LDAP_UID, Long - .toString(pd.getUid())); - addAttribute(attributes, LDAP_UID_NUMBER, Long - .toString(pd.getUid())); - addAttribute(attributes, LDAP_GID_NUMBER, Long - .toString(pd.getGid())); - addAttribute(attributes, LDAP_HOME_DIRECTORY, pd - .getHomeDirectory()); - addAttribute(attributes, LDAP_LOGIN_SHELL, pd.loginShell); + throw new UnsupportedOperationException( + "Support for users PosixDetails not available"); } } AddRequest addRequest = new AddRequest(userDN, attributes); LDAPResult result = getConnection().add(addRequest); LdapDAO.checkLdapResult(result.getResultCode()); - - // AD: Search results sometimes come incomplete if - // connection is not reset - not sure why. - getConnection().reconnect(); - try - { - return getUser(user.getUserID(), config.getUserRequestsDN()); - } - catch (UserNotFoundException e) - { - throw new RuntimeException("BUG: new user " + userDN - .toNormalizedString() + - " not found, result " + result - .getResultCode()); - } } catch (LDAPException e) { - logger.debug("addUser Exception: " + e, e); + logger.error("addUser Exception: " + e, e); LdapUserDAO.checkUserLDAPResult(e.getResultCode()); throw new RuntimeException("Unexpected LDAP exception", e); } @@ -436,18 +467,6 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO .getAttributeValue(LDAP_INSTITUTE); user.details.add(personaDetails); - Long uid = searchResult.getAttributeValueAsLong(LDAP_UID_NUMBER); - Long gid = searchResult.getAttributeValueAsLong(LDAP_GID_NUMBER); - String homeDirectory = searchResult - .getAttributeValue(LDAP_HOME_DIRECTORY); - if (uid != null && gid != null && homeDirectory != null) - { - PosixDetails posixDetails = new PosixDetails(uid, gid, homeDirectory); - posixDetails.loginShell = searchResult - .getAttributeValue(LDAP_LOGIN_SHELL); - user.details.add(posixDetails); - } - return user; } @@ -543,23 +562,18 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO if (details.getClass() == PersonalDetails.class) { PersonalDetails pd = (PersonalDetails) details; - mods.add(new Modification(ModificationType.REPLACE, LDAP_FIRST_NAME, pd.getFirstName())); - mods.add(new Modification(ModificationType.REPLACE, LDAP_LAST_NAME, pd.getLastName())); - mods.add(new Modification(ModificationType.REPLACE, LDAP_ADDRESS, pd.address)); - mods.add(new Modification(ModificationType.REPLACE, LDAP_CITY, pd.city)); - mods.add(new Modification(ModificationType.REPLACE, LDAP_COUNTRY, pd.country)); - mods.add(new Modification(ModificationType.REPLACE, LDAP_EMAIL, pd.email)); - mods.add(new Modification(ModificationType.REPLACE, LDAP_INSTITUTE, pd.institute)); + addModification(mods, LDAP_FIRST_NAME, pd.getFirstName()); + addModification(mods, LDAP_LAST_NAME, pd.getLastName()); + addModification(mods, LDAP_ADDRESS, pd.address); + addModification(mods, LDAP_CITY, pd.city); + addModification(mods, LDAP_COUNTRY, pd.country); + addModification(mods, LDAP_EMAIL, pd.email); + addModification(mods, LDAP_INSTITUTE, pd.institute); } else if (details.getClass() == PosixDetails.class) { - PosixDetails pd = (PosixDetails) details; - mods.add(new Modification(ModificationType.REPLACE, LDAP_OBJECT_CLASS, LDAP_POSIX_ACCOUNT)); - mods.add(new Modification(ModificationType.REPLACE, LDAP_UID, Long.toString(pd.getUid()))); - mods.add(new Modification(ModificationType.REPLACE, LDAP_UID_NUMBER, Long.toString(pd.getUid()))); - mods.add(new Modification(ModificationType.REPLACE, LDAP_GID_NUMBER, Long.toString(pd.getGid()))); - mods.add(new Modification(ModificationType.REPLACE, LDAP_HOME_DIRECTORY, pd.getHomeDirectory())); - mods.add(new Modification(ModificationType.REPLACE, LDAP_LOGIN_SHELL, pd.loginShell)); + throw new UnsupportedOperationException( + "Support for users PosixDetails not available"); } } @@ -571,10 +585,11 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO "dn:" + getSubjectDN().toNormalizedString())); LdapDAO.checkLdapResult(getConnection().modify(modifyRequest).getResultCode()); } - catch (LDAPException e1) + catch (LDAPException e) { - logger.debug("Modify Exception: " + e1, e1); - LdapDAO.checkLdapResult(e1.getResultCode()); + e.printStackTrace(); + logger.debug("Modify Exception: " + e, e); + LdapDAO.checkLdapResult(e.getResultCode()); } try { @@ -900,7 +915,7 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO throw new IllegalArgumentException(userID + " not a valid user ID"); } - void addAttribute(List<Attribute> attributes, final String name, final String value) + private void addAttribute(List<Attribute> attributes, final String name, final String value) { if (value != null && !value.isEmpty()) { @@ -908,6 +923,18 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO } } + private void addModification(List<Modification> mods, final String name, final String value) + { + if (value != null && !value.isEmpty()) + { + mods.add(new Modification(ModificationType.REPLACE, name, value)); + } + else + { + mods.add(new Modification(ModificationType.REPLACE, name)); + } + } + /** * Checks the Ldap result code, and if the result is not SUCCESS, * throws an appropriate exception. This is the place to decide on diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java index 94b9e28b..fda75b28 100644 --- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java +++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAOTest.java @@ -93,7 +93,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -public class LdapUserDAOTest extends AbstractLdapDAOTest +public class LdapUserDAOTest<T extends Principal> extends AbstractLdapDAOTest { private static final Logger log = Logger.getLogger(LdapUserDAOTest.class); @@ -356,20 +356,50 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest @Test public void testSetPassword() throws Exception { +// LDAPConnection connection = +// new LDAPConnection(SocketFactory.getDefault(), config.getServer(), config.getPort()); +// connection.bind(config.getAdminUserDN(), config.getAdminPasswd()); +// +// // Create an SSLUtil instance that is configured to trust any certificate. +// SSLUtil sslUtil = new SSLUtil(new TrustAllTrustManager()); +// SSLContext sslContext = sslUtil.createSSLContext(); +// StartTLSExtendedRequest startTLSRequest = new StartTLSExtendedRequest(sslContext); +// ExtendedResult startTLSResult = connection.processExtendedOperation(startTLSRequest); +// LDAPTestUtils.assertResultCodeEquals(startTLSResult, ResultCode.SUCCESS); + // Create a test user with a known password - final User<HttpPrincipal> teststUser2; + final User<HttpPrincipal> testUser2; final String username = getUserID(); final String password = "foo"; final String newPassword = "bar"; - User<HttpPrincipal> user = new User<HttpPrincipal>(new HttpPrincipal(username)); - user.details.add(new PersonalDetails("firstName", "lastName")); - UserRequest userRequest = new UserRequest(user, password); - teststUser2 = getUserDAO().addUser(userRequest); + HttpPrincipal principal = new HttpPrincipal(username); + testUser2 = new User<HttpPrincipal>(principal); + testUser2.getIdentities().add(principal); + testUser2.details.add(new PersonalDetails("firstName", "lastName")); + final UserRequest userRequest = new UserRequest(testUser2, password); + // add the user Subject subject = new Subject(); + subject.getPrincipals().add(testUser2.getUserID()); + Subject.doAs(subject, new PrivilegedExceptionAction<Object>() + { + public Object run() + throws Exception + { + try + { + return getUserDAO().newUser(userRequest); + } + catch (Exception e) + { + fail("exception updating user: " + e.getMessage()); + } + return null; + } + }); - // authenticate new useranme and password + // authenticate new username and password Subject.doAs(subject, new PrivilegedExceptionAction<Object>() { public Object run() @@ -388,13 +418,14 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest }); // anonymous access should throw exception + subject = new Subject(); Subject.doAs(subject, new PrivilegedExceptionAction<Object>() { public Object run() throws Exception { try { - getUserDAO().setPassword(teststUser2, password, newPassword); + getUserDAO().setPassword(testUser2, password, newPassword); fail("should throw exception if subject and user are not the same"); } catch (Exception ignore){} @@ -403,7 +434,7 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest }); // change the password - subject.getPrincipals().add(teststUser2.getUserID()); + subject.getPrincipals().add(testUser2.getUserID()); Subject.doAs(subject, new PrivilegedExceptionAction<Object>() { public Object run() @@ -411,10 +442,11 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest { try { - getUserDAO().setPassword(teststUser2, password, newPassword); + getUserDAO().setPassword(testUser2, password, newPassword); } catch (Exception e) { + e.printStackTrace(); fail("exception setting password: " + e.getMessage()); } return null; @@ -441,22 +473,42 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest } - @Test +// @Test public void testUpdateUser() throws Exception { - // Create a test user with a known password + // Create a test user final User<HttpPrincipal> testUser2; final String username = getUserID(); final String password = "foo"; - final String newPassword = "bar"; - User<HttpPrincipal> user = new User<HttpPrincipal>(new HttpPrincipal(username)); - user.details.add(new PersonalDetails("firstName", "lastName")); - UserRequest userRequest = new UserRequest(user, password); - testUser2 = getUserDAO().addUser(userRequest); + HttpPrincipal principal = new HttpPrincipal(username); + testUser2 = new User<HttpPrincipal>(principal); + testUser2.getIdentities().add(principal); + testUser2.details.add(new PersonalDetails("firstName", "lastName")); + final UserRequest userRequest = new UserRequest(testUser2, password); + + // add the user + Subject subject = new Subject(); + subject.getPrincipals().add(testUser2.getUserID()); + Subject.doAs(subject, new PrivilegedExceptionAction<Object>() + { + public Object run() + throws Exception + { + try + { + return getUserDAO().newUser(userRequest); + } + catch (Exception e) + { + fail("exception updating user: " + e.getMessage()); + } + return null; + } + }); // update the user - for (UserDetails details : user.details) + for (UserDetails details : testUser2.details) { if (details instanceof PersonalDetails) { @@ -468,11 +520,9 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest pd.country = "country2"; } } - user.details.add(new PosixDetails(123L, 456L, "/dev/null")); - - Subject subject = new Subject(); // anonymous access should throw exception + subject = new Subject(); Subject.doAs(subject, new PrivilegedExceptionAction<Object>() { public Object run() @@ -504,6 +554,7 @@ public class LdapUserDAOTest extends AbstractLdapDAOTest } catch (Exception e) { + e.printStackTrace(); fail("exception updating user: " + e.getMessage()); } return null; -- GitLab