Merge branch 'ac2' of /srv/cadc/git/wopencadc into ac2

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/GetUserAction.java

@@ -82,6 +82,7 @@ import java.util.Set;
 public class GetUserAction extends AbstractUserAction
 {
     private static final Logger log = Logger.getLogger(GetUserAction.class);
+
     private final Principal userID;
     private final String detail;
 
@@ -105,30 +106,33 @@ public class GetUserAction extends AbstractUserAction
 
         /**
          * 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 the calling Subject user is the notAugmentedX500User, AND it is
+         * a GET, call the userDAO to get the User with all identities.
          */
-        if (detail != null &&
-            detail.equalsIgnoreCase("identity") &&
-            isSubjectUser(principal))
+        if (isAugmentUser())
         {
-            Subject subject = Subject.getSubject(AccessController.getContext());
-            user = new User<Principal>(principal);
-            user.getIdentities().addAll(subject.getPrincipals());
+            log.debug("getting augmented user " + principal.getName());
+            user = userPersistence.getAugmentedUser(principal);
         }
 
         /**
          * 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.
+         * 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.
          */
-        else if (this.isAugmentUser)
+        else if (detail != null &&
+                 detail.equalsIgnoreCase("identity") &&
+                 isSubjectUser(principal))
         {
-            user = userPersistence.getAugmentedUser(principal);
+            log.debug("augmenting " + principal.getName() + " from subject");
+            Subject subject = Subject.getSubject(AccessController.getContext());
+            user = new User<Principal>(principal);
+            user.getIdentities().addAll(subject.getPrincipals());
         }
         else
         {
+            log.debug("getting user " + principal.getName());
             try
             {
                 user = userPersistence.getUser(principal);

projects/cadcAccessControl-Server/src/ca/nrc/cadc/ac/server/web/users/UserServlet.java

@@ -90,6 +90,7 @@ public class UserServlet extends HttpServlet
 
     private static final long serialVersionUID = 5289130885807305288L;
     private static final Logger log = Logger.getLogger(UserServlet.class);
+
     private String notAugmentedX500User;
 
     @Override
@@ -121,7 +122,6 @@ public class UserServlet extends HttpServlet
         {
             log.info(logInfo.start());
             AbstractUserAction action = factory.createAction(request);
-            SyncOutput syncOut = new SyncOutput(response);
 
             // Special case: if the calling subject has a servops X500Principal,
             // AND it is a GET request, do not augment the subject.
@@ -129,14 +129,17 @@ public class UserServlet extends HttpServlet
             if (action instanceof GetUserAction && isNotAugmentedSubject())
             {
                 subject = Subject.getSubject(AccessController.getContext());
+                log.debug("subject not augmented: " + subject);
                 action.setAugmentUser(true);
             }
             else
             {
                 subject = AuthenticationUtil.getSubject(request);
+                log.debug("augmented subject: " + subject);
             }
             logInfo.setSubject(subject);
 
+            SyncOutput syncOut = new SyncOutput(response);
             action.setLogInfo(logInfo);
             action.setSyncOut(syncOut);
             action.setAcceptedContentType(getAcceptedContentType(request));
@@ -251,13 +254,16 @@ public class UserServlet extends HttpServlet
     {
         boolean notAugmented = false;
         Subject subject = Subject.getSubject(AccessController.getContext());
+        log.debug("subject: " + subject);
         if (subject != null)
         {
+            log.debug("notAugmentedX500User" + notAugmentedX500User);
             for (Principal principal : subject.getPrincipals())
             {
                 if (principal instanceof X500Principal)
                 {
-                    if (principal.getName().equalsIgnoreCase(this.notAugmentedX500User))
+                    log.debug("principal: " + principal.getName());
+                    if (principal.getName().equalsIgnoreCase(notAugmentedX500User))
                     {
                         notAugmented = true;
                         break;

projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/GetUserActionTest.java

@@ -78,12 +78,14 @@ import ca.nrc.cadc.auth.HttpPrincipal;
 import ca.nrc.cadc.auth.NumericPrincipal;
 import org.junit.Test;
 
+import javax.security.auth.Subject;
 import javax.security.auth.x500.X500Principal;
 import javax.servlet.http.HttpServletResponse;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.Writer;
 import java.security.Principal;
+import java.security.PrivilegedExceptionAction;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -135,13 +137,85 @@ public class GetUserActionTest
 
     @Test
     public void writeUserWithDetailIdentity() throws Exception
+    {
+        final HttpPrincipal httpPrincipal = new HttpPrincipal("CADCtest");
+        final NumericPrincipal numericPrincipal = new NumericPrincipal(789);
+        final X500Principal x500Principal = new X500Principal("cn=foo,o=bar");
+
+        Subject testUser = new Subject();
+        testUser.getPrincipals().add(httpPrincipal);
+        testUser.getPrincipals().add(numericPrincipal);
+        testUser.getPrincipals().add(x500Principal);
+
+        Subject.doAs(testUser, new PrivilegedExceptionAction<Object>()
+        {
+            public Object run() throws Exception
+            {
+
+                final HttpServletResponse mockResponse = createMock(HttpServletResponse.class);
+                final UserPersistence<HttpPrincipal> mockUserPersistence =
+                    createMock(UserPersistence.class);
+
+
+                final GetUserAction testSubject = new GetUserAction(httpPrincipal, "identity")
+                {
+                    @Override
+                    UserPersistence<HttpPrincipal> getUserPersistence()
+                    {
+                        return mockUserPersistence;
+                    }
+                };
+
+                final User<HttpPrincipal> expected = new User<HttpPrincipal>(httpPrincipal);
+                expected.getIdentities().add(httpPrincipal);
+                expected.getIdentities().add(numericPrincipal);
+                expected.getIdentities().add(x500Principal);
+
+                StringBuilder sb = new StringBuilder();
+                UserWriter userWriter = new UserWriter();
+                userWriter.write(expected, sb);
+                String expectedUser = sb.toString();
+
+                final PersonalDetails personalDetails = new PersonalDetails("cadc", "test");
+                personalDetails.city = "city";
+                expected.details.add(personalDetails);
+
+                final PosixDetails posixDetails = new PosixDetails(123L, 456L, "/dev/null");
+                expected.details.add(posixDetails);
+
+                final Writer writer = new StringWriter();
+                final PrintWriter printWriter = new PrintWriter(writer);
+
+                mockResponse.setHeader("Content-Type", "text/xml");
+                expectLastCall().once();
+                expect(mockResponse.getWriter()).andReturn(printWriter).once();
+
+                replay(mockUserPersistence, mockResponse);
+
+                SyncOutput syncOutput = new SyncOutput(mockResponse);
+                testSubject.setSyncOut(syncOutput);
+                testSubject.doAction();
+
+                String actualUser = writer.toString();
+
+                assertEquals(expectedUser, actualUser);
+
+                verify(mockUserPersistence, mockResponse);
+
+                return null;
+            }
+        });
+    }
+
+    @Test
+    public void writeUserWithDetailDisplay() throws Exception
     {
         final HttpServletResponse mockResponse = createMock(HttpServletResponse.class);
         final UserPersistence<HttpPrincipal> mockUserPersistence =
             createMock(UserPersistence.class);
         final HttpPrincipal userID = new HttpPrincipal("CADCtest");
 
-        final GetUserAction testSubject = new GetUserAction(userID, "identity")
+        final GetUserAction testSubject = new GetUserAction(userID, "display")
         {
             @Override
             UserPersistence<HttpPrincipal> getUserPersistence()
@@ -151,17 +225,21 @@ public class GetUserActionTest
         };
 
         final User<HttpPrincipal> expected = new User<HttpPrincipal>(userID);
-        expected.getIdentities().add(new NumericPrincipal(789));
-        expected.getIdentities().add(new X500Principal("cn=foo,o=bar"));
+
+        final PersonalDetails personalDetails = new PersonalDetails("cadc", "test");
+        expected.details.add(personalDetails);
 
         StringBuilder sb = new StringBuilder();
         UserWriter userWriter = new UserWriter();
         userWriter.write(expected, sb);
         String expectedUser = sb.toString();
 
-        final PersonalDetails personalDetails = new PersonalDetails("cadc", "test");
-        personalDetails.city = "city";
-        expected.details.add(personalDetails);
+        Set<PersonalDetails> details = expected.getDetails(PersonalDetails.class);
+        PersonalDetails pd = details.iterator().next();
+        pd.city = "city";
+
+        expected.getIdentities().add(new NumericPrincipal(789));
+        expected.getIdentities().add(new X500Principal("cn=foo,o=bar"));
 
         final PosixDetails posixDetails = new PosixDetails(123L, 456L, "/dev/null");
         expected.details.add(posixDetails);
@@ -188,46 +266,40 @@ public class GetUserActionTest
     }
 
     @Test
-    public void writeUserWithDetailDisplay() throws Exception
+    public void writeAugmentedUser() throws Exception
     {
-        final HttpServletResponse mockResponse = createMock(HttpServletResponse.class);
-        final UserPersistence<HttpPrincipal> mockUserPersistence =
+        final UserPersistence<Principal> mockUserPersistence =
             createMock(UserPersistence.class);
-        final HttpPrincipal userID = new HttpPrincipal("CADCtest");
+        final HttpServletResponse mockResponse = createMock(HttpServletResponse.class);
 
-        final GetUserAction testSubject = new GetUserAction(userID, "display")
+        final HttpPrincipal userID = new HttpPrincipal("CADCtest");
+        final GetUserAction testSubject = new GetUserAction(userID, null)
         {
             @Override
-            UserPersistence<HttpPrincipal> getUserPersistence()
+            UserPersistence<Principal> getUserPersistence()
             {
                 return mockUserPersistence;
             }
         };
+        testSubject.setAugmentUser(true);
 
-        final User<HttpPrincipal> expected = new User<HttpPrincipal>(userID);
+        final NumericPrincipal numericPrincipal = new NumericPrincipal(789);
+        final X500Principal x500Principal = new X500Principal("cn=foo,o=bar");
 
-        final PersonalDetails personalDetails = new PersonalDetails("cadc", "test");
-        expected.details.add(personalDetails);
+        final User<Principal> expected = new User<Principal>(userID);
+        expected.getIdentities().add(userID);
+        expected.getIdentities().add(numericPrincipal);
+        expected.getIdentities().add(x500Principal);
 
         StringBuilder sb = new StringBuilder();
         UserWriter userWriter = new UserWriter();
         userWriter.write(expected, sb);
         String expectedUser = sb.toString();
 
-        Set<PersonalDetails> details = expected.getDetails(PersonalDetails.class);
-        PersonalDetails pd = details.iterator().next();
-        pd.city = "city";
-
-        expected.getIdentities().add(new NumericPrincipal(789));
-        expected.getIdentities().add(new X500Principal("cn=foo,o=bar"));
-
-        final PosixDetails posixDetails = new PosixDetails(123L, 456L, "/dev/null");
-        expected.details.add(posixDetails);
-
         final Writer writer = new StringWriter();
         final PrintWriter printWriter = new PrintWriter(writer);
 
-        expect(mockUserPersistence.getUser(userID)).andReturn(expected).once();
+        expect(mockUserPersistence.getAugmentedUser(userID)).andReturn(expected).once();
         mockResponse.setHeader("Content-Type", "text/xml");
         expectLastCall().once();
         expect(mockResponse.getWriter()).andReturn(printWriter).once();