From ab5fa4a851c5512f1a31f90d034575b07dcf572e Mon Sep 17 00:00:00 2001
From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca>
Date: Wed, 2 Sep 2015 16:04:14 -0700
Subject: [PATCH] s1728: add check in UserServlet for a special user used to
 augment a user's identites.

---
 .../server/web/users/AbstractUserAction.java  | 11 ++-
 .../ac/server/web/users/GetUserAction.java    | 98 ++++++++-----------
 .../server/web/users/UserActionFactory.java   |  8 +-
 .../cadc/ac/server/web/users/UserServlet.java | 49 ++++++++--
 4 files changed, 91 insertions(+), 75 deletions(-)

diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java
index af79d45f..0bc73920 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/AbstractUserAction.java
@@ -108,7 +108,7 @@ public abstract class AbstractUserAction implements PrivilegedExceptionAction<Ob
     static final String DEFAULT_CONTENT_TYPE = "text/xml";
     static final String JSON_CONTENT_TYPE = "application/json";
 
-    protected String augmentUserDN;
+    protected boolean isAugmentUser;
     protected UserLogInfo logInfo;
     protected SyncOutput syncOut;
 
@@ -116,18 +116,19 @@ public abstract class AbstractUserAction implements PrivilegedExceptionAction<Ob
 
     AbstractUserAction()
     {
+        this.isAugmentUser = false;
     }
 
     public abstract void doAction() throws Exception;
 
-    public void setAugmentUserDN(final String dn)
+    public void setAugmentUser(final boolean isAugmentUser)
     {
-    	this.augmentUserDN = dn;
+    	this.isAugmentUser = isAugmentUser;
     }
     
-    public String getAugmentUserDN()
+    public boolean isAugmentUser()
     {
-    	return this.augmentUserDN;
+    	return this.isAugmentUser;
     }
     
     public void setLogInfo(UserLogInfo logInfo)
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
index f21870be..34a9f545 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java
@@ -98,74 +98,58 @@ public class GetUserAction extends AbstractUserAction
 
 	public void doAction() throws Exception
     {
-        User<Principal> user;
- 
-        if (isServops())
-        {
-        	Subject subject = new Subject();
-        	subject.getPrincipals().add(this.userID);
-        	user = (User<Principal>) Subject.doAs(subject, new PrivilegedExceptionAction<Object>()
-        	{
-				@Override
-				public Object run() throws Exception 
-				{
-					return getUser(userID);
-				}
-        		
-        	});
-        }
-        else
-        {
-        	user = getUser(this.userID);
-        }
-
+        User<Principal> user = getUser(this.userID);
         writeUser(user);
     }
 
     protected User<Principal> getUser(Principal principal) throws Exception
     {
         User<Principal> user;
+        final UserPersistence<Principal> userPersistence = getUserPersistence();
 
-        // For detail=identity, if the calling user is the same as the requested user,
-        // the calling user already has all principals for that user.
-        if (detail != null && detail.equalsIgnoreCase("identity") &&
-            isSubjectUser(principal.getName()))
+        /**
+         * Special case 1
+         * If detail=identity, AND if the calling Subject user is the same as
+         * the requested User, then return the User with the principals from the
+         * Subject which has already been augmented.
+         */
+        if (detail != null &&
+            detail.equalsIgnoreCase("identity") &&
+            isSubjectUser(principal))
         {
             Subject subject = Subject.getSubject(AccessController.getContext());
             user = new User<Principal>(principal);
             user.getIdentities().addAll(subject.getPrincipals());
         }
+
+        /**
+         * Special case 2
+         * If the calling Subject user is the notAugmentedX500User, AND it is
+         * a GET, call the userDAO to get the User with all identities.
+         */
+        else if (this.isAugmentUser)
+        {
+            user = userPersistence.getAugmentedUser(principal);
+        }
         else
         {
-            final UserPersistence<Principal> userPersistence = getUserPersistence();
             try
             {
                 user = userPersistence.getUser(principal);
-                if (detail != null)
+
+                // Only return user profile info, first and last name.
+                if (detail != null && detail.equalsIgnoreCase("display"))
                 {
-                    // Only return user principals
-                    if (detail.equalsIgnoreCase("identity"))
-                    {
-                        user.details.clear();
-                    }
-                    // Only return user profile info, first and last name.
-                    else if (detail.equalsIgnoreCase("display"))
+                    user.getIdentities().clear();
+                    Set<PersonalDetails> details = user.getDetails(PersonalDetails.class);
+                    if (details.isEmpty())
                     {
-                        user.getIdentities().clear();
-                        Set<PersonalDetails> details = user.getDetails(PersonalDetails.class);
-                        if (details.isEmpty())
-                        {
-                            String error = principal.getName() + " missing required PersonalDetails";
-                            throw new IllegalStateException(error);
-                        }
-                        PersonalDetails pd = details.iterator().next();
-                        user.details.clear();
-                        user.details.add(new PersonalDetails(pd.getFirstName(), pd.getLastName()));
-                    }
-                    else
-                    {
-                        throw new IllegalArgumentException("Illegal detail parameter " + detail);
+                        String error = principal.getName() + " missing required PersonalDetails";
+                        throw new IllegalStateException(error);
                     }
+                    PersonalDetails pd = details.iterator().next();
+                    user.details.clear();
+                    user.details.add(new PersonalDetails(pd.getFirstName(), pd.getLastName()));
                 }
             }
             catch (UserNotFoundException e)
@@ -176,24 +160,22 @@ public class GetUserAction extends AbstractUserAction
     	
     	return user;
     }
-    
-    protected boolean isServops()
+
+    protected boolean isSubjectUser(Principal userPrincipal)
     {
-    	boolean isServops = false;
-        AccessControlContext acc = AccessController.getContext();
-        Subject subject = Subject.getSubject(acc);
+    	boolean isSubjectUser = false;
+        Subject subject = Subject.getSubject(AccessController.getContext());
         if (subject != null)
         {
-        	for (Principal principal : subject.getPrincipals())
+        	for (Principal subjectPrincipal : subject.getPrincipals())
         	{
-        		if (principal.getName().equals(this.getAugmentUserDN()))
+        		if (subjectPrincipal.getName().equals(userPrincipal.getName()))
         		{
-        			isServops = true;
+                    isSubjectUser = true;
         			break;
         		}
         	}
         }
-
-        return found;
+        return isSubjectUser;
     }
 }
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserActionFactory.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserActionFactory.java
index 30583c7f..a369ac49 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserActionFactory.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserActionFactory.java
@@ -75,16 +75,14 @@ import ca.nrc.cadc.auth.HttpPrincipal;
 import ca.nrc.cadc.auth.IdentityType;
 import ca.nrc.cadc.auth.NumericPrincipal;
 import ca.nrc.cadc.auth.OpenIdPrincipal;
+import org.apache.log4j.Logger;
 
+import javax.security.auth.x500.X500Principal;
+import javax.servlet.http.HttpServletRequest;
 import java.io.IOException;
 import java.net.URL;
 import java.security.Principal;
 
-import javax.security.auth.x500.X500Principal;
-import javax.servlet.http.HttpServletRequest;
-
-import org.apache.log4j.Logger;
-
 
 public abstract class UserActionFactory
 {
diff --git a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserServlet.java b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserServlet.java
index 6fbb19c3..696c125f 100644
--- a/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserServlet.java
+++ b/projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserServlet.java
@@ -69,9 +69,13 @@
 package ca.nrc.cadc.ac.server.web.users;
 
 import java.io.IOException;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.Principal;
 import java.security.PrivilegedActionException;
 
 import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
@@ -90,7 +94,7 @@ public class UserServlet extends HttpServlet
 
     private static final long serialVersionUID = 5289130885807305288L;
     private static final Logger log = Logger.getLogger(UserServlet.class);
-    private String augmentUserDN;
+    private String notAugmentedX500User;
     
     @Override
     public void init(final ServletConfig config) throws ServletException
@@ -99,8 +103,8 @@ public class UserServlet extends HttpServlet
 
         try
         {
-        	this.augmentUserDN = config.getInitParameter(UserServlet.class.getName() + ".augmentUserDN");
-            log.info("augmentUserDN: " + augmentUserDN);
+        	this.notAugmentedX500User = config.getInitParameter(UserServlet.class.getName() + ".NotAugmentedX500User");
+            log.info("notAugmentedX500User: " + notAugmentedX500User);
         }
         catch(Exception ex)
         {
@@ -120,13 +124,23 @@ public class UserServlet extends HttpServlet
         try
         {
             log.info(logInfo.start());
-            Subject subject = AuthenticationUtil.getSubject(request);
-            logInfo.setSubject(subject);
-
             AbstractUserAction action = factory.createAction(request);
             SyncOutput syncOut = new SyncOutput(response);
 
-            action.setAugmentUserDN(this.augmentUserDN);
+            // Special case: if the calling subject has a servops X500Principal,
+            // AND it is a GET request, do not augment the subject.
+            Subject subject;
+            if (action instanceof GetUserAction && isNotAugmentedSubject())
+            {
+                subject = Subject.getSubject(AccessController.getContext());
+                action.setAugmentUser(true);
+            }
+            else
+            {
+                subject = AuthenticationUtil.getSubject(request);
+            }
+            logInfo.setSubject(subject);
+
             action.setLogInfo(logInfo);
             action.setSyncOut(syncOut);
             action.setAcceptedContentType(getAcceptedContentType(request));
@@ -236,4 +250,25 @@ public class UserServlet extends HttpServlet
             return AbstractUserAction.DEFAULT_CONTENT_TYPE;
         }
     }
+
+    protected boolean isNotAugmentedSubject()
+    {
+        boolean notAugmented = false;
+        Subject subject = Subject.getSubject(AccessController.getContext());
+        if (subject != null)
+        {
+            for (Principal principal : subject.getPrincipals())
+            {
+                if (principal instanceof X500Principal)
+                {
+                    if (principal.getName().equalsIgnoreCase(this.notAugmentedX500User))
+                    {
+                        notAugmented = true;
+                        break;
+                    }
+                }
+            }
+        }
+        return notAugmented;
+    }
 }
-- 
GitLab