From de8a5b9097942daa3087b1b5de08552caf5027aa Mon Sep 17 00:00:00 2001 From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca> Date: Thu, 17 Mar 2016 13:59:47 -0700 Subject: [PATCH] s1890: update UserServlet such that only a privileged user can create another user --- .../nrc/cadc/ac/server/ldap/LdapUserDAO.java | 2 +- .../nrc/cadc/ac/server/web/UserServlet.java | 33 +++++++++++++++++-- .../server/web/users/AbstractUserAction.java | 11 +++++++ .../ac/server/web/users/CreateUserAction.java | 6 ++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java index c0c5e4af..aeb81ba6 100755 --- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java +++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/ldap/LdapUserDAO.java @@ -322,7 +322,7 @@ public class LdapUserDAO extends LdapDAO } catch (LDAPException e) { - logger.error("addUserRequest Exception: " + e, e); + logger.error("addUser Exception: " + e, e); LdapUserDAO.checkUserLDAPResult(e.getResultCode()); throw new RuntimeException("Unexpected LDAP exception", e); } diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/UserServlet.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/UserServlet.java index e25d24df..931a1db3 100644 --- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/UserServlet.java +++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/UserServlet.java @@ -84,6 +84,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import ca.nrc.cadc.ac.server.web.users.CreateUserAction; import org.apache.log4j.Logger; import ca.nrc.cadc.ac.server.PluginFactory; @@ -177,15 +178,41 @@ public class UserServlet extends HttpServlet { log.info(logInfo.start()); AbstractUserAction action = factory.createAction(request); + log.debug("create action " + action.getClass().getSimpleName()); action.setAcceptedContentType(getAcceptedContentType(request)); log.debug("content-type: " + getAcceptedContentType(request)); profiler.checkpoint("created action"); - // Special case: if the calling subject has a privileged X500Principal, - // AND it is a GET request, do not augment the subject. Subject subject; Subject privilegedSubject = getPrivilegedSubject(request); - if (action instanceof GetUserAction && privilegedSubject != null) + log.debug("privileged subject: " + privilegedSubject); + + // If the calling subject is not a PrivilegedSubject, + // AND it is a PUT request, throw an AccessControlException + if (action instanceof CreateUserAction) + { + profiler.checkpoint("check non-privileged user"); + if (privilegedSubject == null) + { + action.setPrivilegedSubject(false); + subject = AuthenticationUtil.getSubject(request); + logInfo.setSubject(subject); + log.debug("augmented subject: " + subject); + profiler.checkpoint("augment subject"); + } + else + { + action.setPrivilegedSubject(true); + log.debug("subject not augmented: " + privilegedSubject); + subject = privilegedSubject; + logInfo.setSubject(privilegedSubject); + profiler.checkpoint("set privileged user"); + } + } + + // If the calling subject has a privileged X500Principal, + // AND it is a GET request, do not augment the subject. + else if (action instanceof GetUserAction && privilegedSubject != null) { profiler.checkpoint("check privileged user"); subject = Subject.getSubject(AccessController.getContext()); diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java index 288caf91..b582a04b 100644 --- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java +++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java @@ -105,6 +105,7 @@ public abstract class AbstractUserAction implements PrivilegedExceptionAction<Ob private Profiler profiler = new Profiler(AbstractUserAction.class); protected boolean isAugmentUser; + protected boolean isPrivilegedSubject; protected UserLogInfo logInfo; protected SyncOutput syncOut; protected UserPersistence userPersistence; @@ -128,6 +129,16 @@ public abstract class AbstractUserAction implements PrivilegedExceptionAction<Ob return this.isAugmentUser; } + public void setPrivilegedSubject(final boolean isPrivilegedSubject) + { + this.isPrivilegedSubject = isPrivilegedSubject; + } + + public boolean isPrivilegedSubject() + { + return this.isPrivilegedSubject; + } + public void setLogInfo(UserLogInfo logInfo) { this.logInfo = logInfo; diff --git a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java index 3809b519..eb82732f 100644 --- a/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java +++ b/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/CreateUserAction.java @@ -71,6 +71,7 @@ package ca.nrc.cadc.ac.server.web.users; import ca.nrc.cadc.ac.User; import java.io.InputStream; +import java.security.AccessControlException; public class CreateUserAction extends AbstractUserAction { @@ -85,6 +86,11 @@ public class CreateUserAction extends AbstractUserAction public void doAction() throws Exception { + if (!isPrivilegedSubject) + { + throw new AccessControlException("non-privileged user cannot create a user"); + } + final User user = readUser(this.inputStream); userPersistence.addUser(user); -- GitLab