From 9af3038e343f76a5c3226a858115d7c66458f0f8 Mon Sep 17 00:00:00 2001 From: Jeff Burke <Jeff.Burke@nrc-cnrc.gc.ca> Date: Thu, 3 Sep 2015 10:54:37 -0700 Subject: [PATCH] s1728: fixed unit tests. --- .../ac/server/web/users/GetUserAction.java | 30 +++-- .../cadc/ac/server/web/users/UserServlet.java | 10 +- .../server/web/users/GetUserActionTest.java | 124 ++++++++++++++---- 3 files changed, 123 insertions(+), 41 deletions(-) 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 b5bb4a7e..b03b9a52 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 @@ -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); 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 99ab94ac..49a4bd5d 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 @@ -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; diff --git a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/GetUserActionTest.java b/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/GetUserActionTest.java index 77271949..8c77cf53 100644 --- a/projects/cadcAccessControl-Server/test/src/ca/nrc/cadc/ac/server/web/users/GetUserActionTest.java +++ b/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(); -- GitLab