From 6e249e7cbddc0084e31a0328d03edeafaed1877d Mon Sep 17 00:00:00 2001 From: Adrian Damian <Adrian.Damian@nrc.ca> Date: Thu, 3 Sep 2015 14:28:27 -0700 Subject: [PATCH] Added support for proxy User (superuser) authentication --- .../nrc/cadc/ac/server/GroupPersistence.java | 16 --- .../nrc/cadc/ac/server/ldap/LdapGroupDAO.java | 6 +- .../ac/server/ldap/LdapGroupPersistence.java | 13 +-- .../ac/server/web/users/LoginServlet.java | 106 +++++++++++++++--- .../web/users/UserLoginServletTest.java | 106 ++++++++++++++++++ 5 files changed, 201 insertions(+), 46 deletions(-) create mode 100644 projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/UserLoginServletTest.java diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java index 4da9df74..a906d804 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/GroupPersistence.java @@ -173,21 +173,5 @@ public interface GroupPersistence<T extends Principal> Collection<Group> getGroups(T userID, Role role, String groupID) throws UserNotFoundException, GroupNotFoundException, TransientException, AccessControlException; - - /** - * Check whether the user is a member of the group. - * - * @param userID The userID. - * @param groupID The groupID. - * - * @return true or false - * - * @throws UserNotFoundException If the user is not found. - * @throws TransientException If an temporary, unexpected problem occurred. - * @throws AccessControlException If the operation is not permitted. - */ - boolean isMember(T userID, String groupID) - throws UserNotFoundException, TransientException, - AccessControlException; } diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java index 1cee54ae..c27fadd6 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupDAO.java @@ -126,6 +126,9 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO }; private LdapUserDAO<T> userPersist; + + // this gets filled by the LdapgroupPersistence + GroupDetailSelector searchDetailSelector; public LdapGroupDAO(LdapConfig config, LdapUserDAO<T> userPersist) { @@ -811,8 +814,7 @@ public class LdapGroupDAO<T extends Principal> extends LdapDAO } throw new RuntimeException("BUG: failed to extract group name from " + groupDN.toString()); } - // this gets filled by the LdapgroupPersistence - GroupDetailSelector searchDetailSelector; + private boolean isDetailedSearch(Group g, Role r) { diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java index 9f66002c..41196a39 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapGroupPersistence.java @@ -97,7 +97,7 @@ public class LdapGroupPersistence<T extends Principal> config = LdapConfig.getLdapConfig(); } - protected void setDetailSelector(GroupDetailSelector gds) + public void setDetailSelector(GroupDetailSelector gds) { this.detailSelector = gds; } @@ -256,14 +256,5 @@ public class LdapGroupPersistence<T extends Principal> userDAO.close(); } } - } - - public boolean isMember(T userID, String groupID) - throws UserNotFoundException, TransientException, - AccessControlException - { - return (new LdapUserPersistence<T>()).isMember(userID, groupID); - } - - + } } diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/LoginServlet.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/LoginServlet.java index eae23b58..ea59ec4f 100755 --- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/LoginServlet.java +++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/LoginServlet.java @@ -70,7 +70,10 @@ package ca.nrc.cadc.ac.server.web.users; import java.io.IOException; import java.security.AccessControlException; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; +import javax.security.auth.Subject; import javax.servlet.ServletConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -79,8 +82,10 @@ import javax.servlet.http.HttpServletResponse; import org.apache.log4j.Logger; +import ca.nrc.cadc.ac.Group; +import ca.nrc.cadc.ac.Role; import ca.nrc.cadc.ac.UserNotFoundException; -import ca.nrc.cadc.ac.server.GroupPersistence; +import ca.nrc.cadc.ac.server.GroupDetailSelector; import ca.nrc.cadc.ac.server.UserPersistence; import ca.nrc.cadc.ac.server.ldap.LdapGroupPersistence; import ca.nrc.cadc.ac.server.ldap.LdapUserPersistence; @@ -99,6 +104,8 @@ public class LoginServlet extends HttpServlet String proxyGroup; // only users in this group can impersonate other users String nonImpersonGroup; // users in this group cannot be impersonated + private static final String PROXY_ACCESS = "Proxy user access: "; + @Override public void init(final ServletConfig config) throws ServletException @@ -171,16 +178,24 @@ public class LoginServlet extends HttpServlet } catch (IllegalArgumentException e) { - log.debug(e.getMessage(), e); - logInfo.setMessage(e.getMessage()); + String msg = e.getMessage(); + if (msg.startsWith(PROXY_ACCESS)) + { + log.warn(msg, e); + } + else + { + log.debug(msg, e); + } + logInfo.setMessage(msg); response.setContentType(CONTENT_TYPE); - response.getWriter().write(e.getMessage()); + response.getWriter().write(msg); response.setStatus(400); } catch (AccessControlException e) { log.debug(e.getMessage(), e); - String message = "Invalid credentials"; + String message = e.getMessage(); logInfo.setMessage(message); response.setContentType(CONTENT_TYPE); response.getWriter().write(message); @@ -206,25 +221,82 @@ public class LoginServlet extends HttpServlet /** * Checks if user can impersonate another user */ - private void checkCanImpersonate(final String userID, final String proxyUser) + protected void checkCanImpersonate(final String userID, final String proxyUser) throws AccessControlException, UserNotFoundException, - TransientException + TransientException, Throwable { - GroupPersistence<HttpPrincipal> gp = new LdapGroupPersistence<HttpPrincipal>(); - - if (!gp.isMember(new HttpPrincipal(proxyUser), proxyGroup)) + final LdapGroupPersistence<HttpPrincipal> gp = + getLdapGroupPersistence(); + + + Subject proxySubject = new Subject(); + proxySubject.getPrincipals().add(new HttpPrincipal(proxyUser)); + try { - throw new AccessControlException(proxyUser + " as " + userID - + " failed: not in proxyGroup"); - } + Subject.doAs(proxySubject, new PrivilegedExceptionAction<Object>() + { + @Override + public Object run() throws Exception + { + if (gp.getGroups(new HttpPrincipal(proxyUser), Role.MEMBER, + proxyGroup).size() == 0) + { + throw new AccessControlException(PROXY_ACCESS + + proxyUser + " as " + userID + + " failed - not allowed to impersonate (" + + proxyUser + " not in " + proxyGroup + + " group)"); + } + return null; + } + }); - if (gp.isMember(new HttpPrincipal(userID), nonImpersonGroup)) + Subject userSubject = new Subject(); + userSubject.getPrincipals().add(new HttpPrincipal(userID)); + Subject.doAs(userSubject, new PrivilegedExceptionAction<Object>() + { + @Override + public Object run() throws Exception + { + if (gp.getGroups(new HttpPrincipal(userID), Role.MEMBER, + nonImpersonGroup).size() != 0) + { + throw new AccessControlException(PROXY_ACCESS + + proxyUser + " as " + userID + + " failed - non impersonable (" + userID + + " in " + nonImpersonGroup + " group)"); + } + return null; + } + }); + } + catch (PrivilegedActionException e) { - if (gp.isMember(new HttpPrincipal(proxyUser), proxyGroup)) + Throwable cause = e.getCause(); + if (cause != null) + { + throw cause; + } + Exception exception = e.getException(); + if (exception != null) { - throw new AccessControlException(proxyUser + " as " + userID - + " failed: non impersonable"); + throw exception; } + throw e; } } + + protected LdapGroupPersistence<HttpPrincipal> getLdapGroupPersistence() + { + LdapGroupPersistence<HttpPrincipal> gp = new LdapGroupPersistence<HttpPrincipal>(); + gp.setDetailSelector(new GroupDetailSelector() + { + @Override + public boolean isDetailedSearch(Group g, Role r) + { + return false; + } + }); + return gp; + } } diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/UserLoginServletTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/UserLoginServletTest.java new file mode 100644 index 00000000..99d060b3 --- /dev/null +++ b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/UserLoginServletTest.java @@ -0,0 +1,106 @@ +package ca.nrc.cadc.ac.server.web.users; + +import java.security.AccessControlException; +import java.util.Collection; +import java.util.HashSet; + +import org.easymock.EasyMock; +import org.junit.Test; + +import ca.nrc.cadc.ac.Group; +import ca.nrc.cadc.ac.Role; +import ca.nrc.cadc.ac.server.GroupDetailSelector; +import ca.nrc.cadc.ac.server.ldap.LdapGroupPersistence; +import ca.nrc.cadc.auth.HttpPrincipal; + +import static org.junit.Assert.fail; +import static org.junit.Assert.assertTrue; + +public class UserLoginServletTest +{ + @Test + public void getCheckCanImpersonate() throws Throwable + { + LoginServlet ls = new LoginServlet() + { + /** + * + */ + private static final long serialVersionUID = 1L; + + @Override + protected LdapGroupPersistence<HttpPrincipal> getLdapGroupPersistence() + { + proxyGroup = "proxyGroup"; + nonImpersonGroup = "niGroup"; + Collection<Group> proxyGroups = new HashSet<Group>(); + proxyGroups.add(new Group(proxyGroup)); + Collection<Group> niGroups = new HashSet<Group>(); + niGroups.add(new Group(nonImpersonGroup)); + LdapGroupPersistence<HttpPrincipal> mockGp = EasyMock + .createMock(LdapGroupPersistence.class); + mockGp.setDetailSelector(new GroupDetailSelector() + { + @Override + public boolean isDetailedSearch(Group g, Role r) + { + return false; + } + }); + try + { + EasyMock.expect( + mockGp.getGroups(new HttpPrincipal("proxyUser"), + Role.MEMBER, proxyGroup)).andReturn( + proxyGroups); + EasyMock.expect( + mockGp.getGroups(new HttpPrincipal("nonProxyUser"), + Role.MEMBER, proxyGroup)).andReturn( + new HashSet<Group>()); + EasyMock.expect( + mockGp.getGroups(new HttpPrincipal("user"), + Role.MEMBER, nonImpersonGroup)).andReturn( + new HashSet<Group>()); + EasyMock.expect( + mockGp.getGroups(new HttpPrincipal("niUser"), + Role.MEMBER, nonImpersonGroup)).andReturn( + niGroups); + EasyMock.replay(mockGp); + } catch (Exception e) + { + throw new RuntimeException(e); + } + return mockGp; + } + }; + // proxyUser can impersonate user + ls.checkCanImpersonate("user", "proxyUser"); + // nonProxyUser cannot impersonate + try + { + ls.checkCanImpersonate("user", "nonProxyUser"); + fail("AccessControlException expected"); + } catch (AccessControlException ex) + { + assertTrue(ex.getMessage().contains("not allowed to impersonate")); + } + // niUser cannot be impersonated + try + { + ls.checkCanImpersonate("niUser", "proxyUser"); + fail("AccessControlException expected"); + } catch (AccessControlException ex) + { + assertTrue(ex.getMessage().contains("non impersonable")); + } + // nonProxyUser cannot impersonate and niUser cannot be impersonated + try + { + ls.checkCanImpersonate("niUser", "nonProxyUser"); + fail("AccessControlException expected"); + } catch (AccessControlException ex) + { + assertTrue(ex.getMessage().contains("not allowed to impersonate")); + } + } +} -- GitLab