From 7cbc62a834e9e4aec5c331ce10791b3139c64d13 Mon Sep 17 00:00:00 2001
From: Brian Major <major.brian@gmail.com>
Date: Wed, 30 Nov 2016 16:02:11 -0800
Subject: [PATCH] issue-19 - WhoAmIServlet supports different AuthMethods
---
cadc-access-control-server/build.gradle | 2 +-
.../nrc/cadc/ac/server/web/WhoAmIServlet.java | 59 ++++++++++++++-----
.../ac/server/ldap/AbstractLdapDAOTest.java | 2 +-
.../cadc/ac/server/web/WhoAmIServletTest.java | 29 +++++++--
.../groups/RemoveGroupMemberActionTest.java | 2 +-
5 files changed, 72 insertions(+), 22 deletions(-)
diff --git a/cadc-access-control-server/build.gradle b/cadc-access-control-server/build.gradle
index 8d4b6b5c..53822b19 100644
--- a/cadc-access-control-server/build.gradle
+++ b/cadc-access-control-server/build.gradle
@@ -13,7 +13,7 @@ repositories {
sourceCompatibility = 1.7
group = 'org.opencadc'
-version = '1.1.2'
+version = '1.1.3'
dependencies {
compile 'log4j:log4j:1.2.+'
diff --git a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/WhoAmIServlet.java b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/WhoAmIServlet.java
index a4978660..13db215b 100644
--- a/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/WhoAmIServlet.java
+++ b/cadc-access-control-server/src/main/java/ca/nrc/cadc/ac/server/web/WhoAmIServlet.java
@@ -71,9 +71,10 @@ package ca.nrc.cadc.ac.server.web;
import java.io.IOException;
import java.net.URI;
import java.net.URL;
-import java.util.Set;
+import java.security.Principal;
import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -84,6 +85,7 @@ import org.apache.log4j.Logger;
import ca.nrc.cadc.auth.AuthMethod;
import ca.nrc.cadc.auth.AuthenticationUtil;
import ca.nrc.cadc.auth.HttpPrincipal;
+import ca.nrc.cadc.auth.NumericPrincipal;
import ca.nrc.cadc.log.ServletLogInfo;
import ca.nrc.cadc.reg.Standards;
import ca.nrc.cadc.reg.client.LocalAuthority;
@@ -99,7 +101,7 @@ public class WhoAmIServlet extends HttpServlet
{
private static final Logger log = Logger.getLogger(WhoAmIServlet.class);
- static final String USER_GET_PATH = "/%s?idType=HTTP";
+ static final String USER_GET_PATH = "/%s?idType=%s";
/**
* Handle a /whoami GET operation.
@@ -119,19 +121,23 @@ public class WhoAmIServlet extends HttpServlet
log.info(logInfo.start());
try
{
- final Subject currentSubject = getSubject(request);
- final Set<HttpPrincipal> currentWebPrincipals =
- currentSubject.getPrincipals(HttpPrincipal.class);
-
- if (currentWebPrincipals.isEmpty())
+ final Subject subject = getSubject(request);
+ AuthMethod authMethod = getAuthMethod(subject);
+ if (AuthMethod.ANON.equals(authMethod))
{
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ return;
}
- else
+
+ Principal principal = getPrincipalForRestCall(subject);
+ if (principal == null)
{
- redirect(response, currentWebPrincipals.toArray(
- new HttpPrincipal[1])[0], request.getScheme());
+ response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+ response.getWriter().print("No supported identities for /whoami");
+ return;
}
+
+ redirect(response, principal, authMethod);
}
catch (IllegalArgumentException e)
{
@@ -154,12 +160,37 @@ public class WhoAmIServlet extends HttpServlet
}
}
+ private Principal getPrincipalForRestCall(Subject s)
+ {
+ // TODO: Would be better to add this type of checking to AuthenticationUtil.
+ // But: a better fix would probably be to remove the userid and
+ // authentication type from the get user REST call. Only the
+ // calling user is allowed to get that user, so the information
+ // is already available.
+
+ if (!s.getPrincipals(HttpPrincipal.class).isEmpty())
+ return s.getPrincipals(HttpPrincipal.class).iterator().next();
+
+ if (!s.getPrincipals(X500Principal.class).isEmpty())
+ return s.getPrincipals(X500Principal.class).iterator().next();
+
+ if (!s.getPrincipals(NumericPrincipal.class).isEmpty())
+ return s.getPrincipals(NumericPrincipal.class).iterator().next();
+
+ return null;
+ }
+
public URI getServiceURI(URI standard)
{
LocalAuthority localAuthority = new LocalAuthority();
return localAuthority.getServiceURI(standard.toString());
}
+ public AuthMethod getAuthMethod(Subject subject)
+ {
+ return AuthenticationUtil.getAuthMethod(subject);
+ }
+
/**
* Forward on to the Service's user endpoint.
*
@@ -167,21 +198,19 @@ public class WhoAmIServlet extends HttpServlet
* @param webPrincipal The HttpPrincipal instance.
* @param scheme The scheme
*/
- void redirect(final HttpServletResponse response,
- final HttpPrincipal webPrincipal,
- final String scheme) throws IOException
+ void redirect(HttpServletResponse response, Principal principal, AuthMethod authMethod) throws IOException
{
final RegistryClient registryClient = getRegistryClient();
URI umsServiceURI = getServiceURI(Standards.UMS_WHOAMI_01);
log.debug("ums service uri: " + umsServiceURI);
- final URL serviceURL = registryClient.getServiceURL(umsServiceURI, Standards.UMS_USERS_01, AuthMethod.CERT);
+ final URL serviceURL = registryClient.getServiceURL(umsServiceURI, Standards.UMS_USERS_01, authMethod);
final URL redirectURL = new URL(serviceURL.toExternalForm() + USER_GET_PATH);
// Take the first one.
final String redirectUrl =
- String.format(redirectURL.toString(), webPrincipal.getName());
+ String.format(redirectURL.toString(), principal.getName(), AuthenticationUtil.getPrincipalType(principal));
final URI redirectURI = URI.create(redirectUrl);
log.debug("redirecting to " + redirectURI.toASCIIString());
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/AbstractLdapDAOTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/AbstractLdapDAOTest.java
index 592b6b12..53f99bdd 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/AbstractLdapDAOTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/ldap/AbstractLdapDAOTest.java
@@ -131,7 +131,7 @@ public class AbstractLdapDAOTest
public static void setUpBeforeClass()
throws Exception
{
- Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
+ Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
System.setProperty(PropertiesReader.class.getName() + ".dir", "src/test/resources");
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/WhoAmIServletTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/WhoAmIServletTest.java
index 6b374453..a2f215b4 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/WhoAmIServletTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/WhoAmIServletTest.java
@@ -79,6 +79,7 @@ import java.net.URL;
import java.security.PrivilegedExceptionAction;
import javax.security.auth.Subject;
+import javax.security.auth.x500.X500Principal;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -114,10 +115,23 @@ public class WhoAmIServletTest
}
@Test
- public void doGet() throws Exception
+ public void doGetWithHttpPrincipal() throws Exception
{
final Subject subject = new Subject();
subject.getPrincipals().add(new HttpPrincipal("CADCtest"));
+ doGet(subject, AuthMethod.PASSWORD, "CADCtest", "http");
+ }
+
+ @Test
+ public void doGetWithCert() throws Exception
+ {
+ final Subject subject = new Subject();
+ subject.getPrincipals().add(new X500Principal("CN=cadcauthtest1_24c,OU=cadc,O=hia,C=ca"));
+ doGet(subject, AuthMethod.CERT, "CN=cadcauthtest1_24c,OU=cadc,O=hia,C=ca", "x500");
+ }
+
+ public void doGet(final Subject subject, final AuthMethod authMethod, final String restUserid, final String restType) throws Exception
+ {
final RegistryClient mockRegistry = createMock(RegistryClient.class);
final WhoAmIServlet testSubject = new WhoAmIServlet()
@@ -145,6 +159,12 @@ public class WhoAmIServletTest
{
return URI.create("ivo://example.org/ums");
}
+
+ @Override
+ public AuthMethod getAuthMethod(Subject subject)
+ {
+ return authMethod;
+ }
};
final HttpServletRequest mockRequest =
@@ -155,9 +175,10 @@ public class WhoAmIServletTest
expect(mockRequest.getPathInfo()).andReturn("users/CADCtest").once();
expect(mockRequest.getMethod()).andReturn("GET").once();
expect(mockRequest.getRemoteAddr()).andReturn("mysite.com").once();
- expect(mockRequest.getScheme()).andReturn("http");
- mockResponse.sendRedirect("/ac/users/CADCtest?idType=HTTP");
+ String redirect = "/ac/users/" + restUserid + "?idType=" + restType;
+ log.debug("expected redirect: " + redirect);
+ mockResponse.sendRedirect(redirect);
expectLastCall().once();
URI umsServiceURI = URI.create("ivo://example.org/ums");
@@ -166,7 +187,7 @@ public class WhoAmIServletTest
// "http", "/%s?idType=HTTP")).
// andReturn(new URL("http://mysite.com/ac/users/CADCtest?idType=HTTP")).once();
- expect(mockRegistry.getServiceURL(umsServiceURI, Standards.UMS_USERS_01, AuthMethod.CERT))
+ expect(mockRegistry.getServiceURL(umsServiceURI, Standards.UMS_USERS_01, authMethod))
.andReturn(new URL("http://mysite.com/ac/users")).once();
replay(mockRequest, mockResponse, mockRegistry);
diff --git a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
index 8cdc25f9..a8d2c7d5 100644
--- a/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
+++ b/cadc-access-control-server/src/test/java/ca/nrc/cadc/ac/server/web/groups/RemoveGroupMemberActionTest.java
@@ -96,7 +96,7 @@ public class RemoveGroupMemberActionTest
@BeforeClass
public static void setUpClass()
{
- Log4jInit.setLevel("ca.nrc.cadc.ac", Level.DEBUG);
+ Log4jInit.setLevel("ca.nrc.cadc.ac", Level.INFO);
}
@Test
--
GitLab