From e994b365d06bfa8e563dd227327a79a185cee4de Mon Sep 17 00:00:00 2001 From: Dustin Jenkins Date: Tue, 24 May 2022 15:19:13 -0700 Subject: [PATCH 1/3] Code cleanup and test compilation fixes. --- cadc-web-util/build.gradle | 4 +- .../accesscontrol/AccessControlClient.java | 192 +++++++++--------- .../cadc/config/ApplicationConfiguration.java | 12 +- .../ca/nrc/cadc/web/SubjectGenerator.java | 17 +- .../nrc/cadc/web/WebAccessControlFilter.java | 16 +- .../ca/nrc/cadc/web/SubjectGeneratorTest.java | 34 +--- settings.gradle | 2 +- 7 files changed, 120 insertions(+), 157 deletions(-) diff --git a/cadc-web-util/build.gradle b/cadc-web-util/build.gradle index 7fcb2ad..1271fbc 100644 --- a/cadc-web-util/build.gradle +++ b/cadc-web-util/build.gradle @@ -12,7 +12,7 @@ repositories { sourceCompatibility = 1.8 group = 'org.opencadc' -version = '1.2.12' +version = '1.2.13' description = 'OpenCADC Web UI utility library' def git_url = 'https://github.com/opencadc/web' @@ -21,7 +21,7 @@ dependencies { implementation 'org.apache.commons:commons-configuration2:2.3' implementation 'commons-beanutils:commons-beanutils:[1.9.3,)' - implementation 'org.opencadc:cadc-util:[1.6,)' + implementation 'org.opencadc:cadc-util:[1.6,2.0)' implementation 'org.opencadc:cadc-registry:[1.5.2,)' implementation 'org.opencadc:cadc-access-control-identity:[1.1.0,)' diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java b/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java index bb7f001..ceb9d66 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java @@ -3,6 +3,7 @@ import ca.nrc.cadc.auth.AuthMethod; import ca.nrc.cadc.auth.AuthenticationUtil; import ca.nrc.cadc.auth.HttpPrincipal; +import ca.nrc.cadc.auth.NotAuthenticatedException; import ca.nrc.cadc.net.HttpPost; import ca.nrc.cadc.net.ResourceNotFoundException; import ca.nrc.cadc.reg.Standards; @@ -12,7 +13,6 @@ import java.io.OutputStream; import java.net.URI; import java.net.URL; -import java.security.AccessControlException; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.Collections; @@ -24,7 +24,6 @@ import org.apache.log4j.Logger; - /** * Client to access a registered AC Web Service. */ @@ -35,8 +34,7 @@ public class AccessControlClient { private final URI groupManagementServiceURI; private static final Logger log = Logger.getLogger(AccessControlClient.class); - public AccessControlClient(final URI serviceURI) - throws IllegalArgumentException { + public AccessControlClient(final URI serviceURI) throws IllegalArgumentException { this(serviceURI, new RegistryClient()); } @@ -52,9 +50,8 @@ public AccessControlClient(final URI serviceURI) * @return URL for login */ private URL lookupLoginURL() { - return this.registryClient - .getServiceURL(this.groupManagementServiceURI, - Standards.UMS_LOGIN_01, AuthMethod.ANON); + return this.registryClient.getServiceURL(this.groupManagementServiceURI, Standards.UMS_LOGIN_01, + AuthMethod.ANON); } public String login(final String username, char[] password) { @@ -75,21 +72,19 @@ public String login(final String username, char[] password) { } case 401: { - throw new AccessControlException("Login denied"); + throw new NotAuthenticatedException("Login denied"); } default: { - throw new IllegalArgumentException( - String.format("Unable to login '%s'.\nServer error code: %d.", - username, statusCode)); + throw new IllegalArgumentException(String.format("Unable to login '%s'.\nServer error code: %d.", + username, statusCode)); } } } private URL lookupPasswordResetURL() { - return this.registryClient.getServiceURL( - this.groupManagementServiceURI, - Standards.UMS_RESETPASS_01, AuthMethod.TOKEN); + return this.registryClient.getServiceURL(this.groupManagementServiceURI, Standards.UMS_RESETPASS_01, + AuthMethod.TOKEN); } /** @@ -98,7 +93,6 @@ private URL lookupPasswordResetURL() { * @param newPassword The new password value. */ public void resetPassword(final char[] newPassword, final char[] token) { - final ByteArrayOutputStream out = new ByteArrayOutputStream(); final Map payload = new HashMap<>(); payload.put(CADC_PASSWORD_FIELD, new String(newPassword)); @@ -109,83 +103,79 @@ public void resetPassword(final char[] newPassword, final char[] token) { try { - Subject.doAs(subject, new PrivilegedExceptionAction() { - - @Override - public Void run() throws Exception { - final HttpPost thePost = postNoRedirect(lookupPasswordResetURL(), payload, headers); - int statusCode = thePost.getResponseCode(); - StringBuilder logStr = new StringBuilder(); - logStr.append("Unable to reset password"); - StringBuilder throwStr = new StringBuilder(); - throwStr.append("Unable to reset password"); - - // Gets added after specific message - StringBuilder reasonPart = new StringBuilder(); - if (statusCode != 200) { - reasonPart.append(" - "); - reasonPart.append(thePost.getResponseCode()); - reasonPart.append(": "); - reasonPart.append(thePost.getThrowable().toString()); + Subject.doAs(subject, (PrivilegedExceptionAction) () -> { + final HttpPost thePost = postNoRedirect(lookupPasswordResetURL(), payload, headers); + int statusCode = thePost.getResponseCode(); + StringBuilder logStr = new StringBuilder(); + logStr.append("Unable to reset password"); + StringBuilder throwStr = new StringBuilder(); + throwStr.append("Unable to reset password"); + + // Gets added after specific message + StringBuilder reasonPart = new StringBuilder(); + if (statusCode != 200) { + reasonPart.append(" - "); + reasonPart.append(thePost.getResponseCode()); + reasonPart.append(": "); + reasonPart.append(thePost.getThrowable().toString()); + } + String msg = ""; + + switch (statusCode) { + case 200: { + break; } - String msg = ""; - - switch (statusCode) { - case 200: { - break; - } - case 400: { - throwStr.append(": "); - throwStr.append(thePost.getThrowable().getMessage()); - logStr.append(msg); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new IllegalArgumentException(throwStr.toString()); - } - case 403: - case 401: { - msg = ": Login denied"; - throwStr.append(msg); - logStr.append(msg); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new AccessControlException(throwStr.toString()); - } - case 404: { - msg = ": Service unavailable"; - throwStr.append(msg); - logStr.append(msg); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new ResourceNotFoundException(throwStr.toString()); - } - case -1: { - throwStr.append(": Bad request"); - logStr.append(": Call not completed"); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new IllegalStateException(throwStr.toString()); - } - case 500: { - msg = ": Server error"; - throwStr.append(msg); - logStr.append(msg); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new IllegalStateException(throwStr.toString()); - } - default: { - msg = ": Unknown error"; - throwStr.append(msg); - logStr.append(msg); - logStr.append(reasonPart.toString()); - log.error(logStr.toString()); - throw new IllegalStateException(throwStr.toString()); - } + case 400: { + throwStr.append(": "); + throwStr.append(thePost.getThrowable().getMessage()); + logStr.append(msg); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new IllegalArgumentException(throwStr.toString()); + } + case 403: + case 401: { + msg = ": Login denied"; + throwStr.append(msg); + logStr.append(msg); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new NotAuthenticatedException(throwStr.toString()); + } + case 404: { + msg = ": Service unavailable"; + throwStr.append(msg); + logStr.append(msg); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new ResourceNotFoundException(throwStr.toString()); + } + case -1: { + throwStr.append(": Bad request"); + logStr.append(": Call not completed"); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new IllegalStateException(throwStr.toString()); + } + case 500: { + msg = ": Server error"; + throwStr.append(msg); + logStr.append(msg); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new IllegalStateException(throwStr.toString()); + } + default: { + msg = ": Unknown error"; + throwStr.append(msg); + logStr.append(msg); + logStr.append(reasonPart); + log.error(logStr.toString()); + throw new IllegalStateException(throwStr.toString()); } - - return null; } + + return null; }); } catch (PrivilegedActionException pea) { final Exception cause = pea.getException(); @@ -193,14 +183,11 @@ public Void run() throws Exception { // This is to make sure the right errors are propagated out if (cause == null) { log.error("Bug: Unknown error.", cause); - } - else if (cause instanceof AccessControlException) { - throw ((AccessControlException) cause); - } - else if (cause instanceof IllegalArgumentException) { + } else if (cause instanceof SecurityException) { + throw ((SecurityException) cause); + } else if (cause instanceof IllegalArgumentException) { throw ((IllegalArgumentException) cause); - } - else { + } else { throw new RuntimeException(cause); } } @@ -216,6 +203,7 @@ else if (cause instanceof IllegalArgumentException) { * @return Response status code. */ int post(final URL url, final Map payload, final OutputStream out) { + log.debug("Logging into " + url); final Map headers = Collections.emptyMap(); return post(url, payload, headers, out); } @@ -237,11 +225,15 @@ int post(final URL url, final Map payload, final Map curPrincipals = subject.getPrincipals(HttpPrincipal.class); + final HttpPrincipal[] principalArray = new HttpPrincipal[curPrincipals.size()]; + username = ((HttpPrincipal[]) curPrincipals.toArray(principalArray))[0].getName(); } else { username = null; } diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java b/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java index ff5c941..4d02d64 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java @@ -4,20 +4,15 @@ import ca.nrc.cadc.util.StringUtil; import java.net.URI; -import java.util.ArrayList; -import java.util.List; import org.apache.commons.configuration2.CombinedConfiguration; import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.PropertiesConfiguration; import org.apache.commons.configuration2.SystemConfiguration; -import org.apache.commons.configuration2.XMLConfiguration; import org.apache.commons.configuration2.builder.FileBasedConfigurationBuilder; import org.apache.commons.configuration2.builder.fluent.Parameters; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.tree.MergeCombiner; -import org.apache.commons.configuration2.tree.NodeCombiner; -import org.apache.commons.configuration2.tree.UnionCombiner; import org.apache.log4j.Logger; @@ -32,11 +27,12 @@ public ApplicationConfiguration(final String filePath) { combinedConfiguration.addConfiguration(new SystemConfiguration()); final Parameters parameters = new Parameters(); - final FileBasedConfigurationBuilder builder = new FileBasedConfigurationBuilder<>(PropertiesConfiguration.class).configure(parameters.properties() - .setFileName(filePath)); + final FileBasedConfigurationBuilder builder = + new FileBasedConfigurationBuilder<>(PropertiesConfiguration.class).configure( + parameters.properties().setFileName(filePath)); try { - combinedConfiguration.addConfiguration((Configuration) builder.getConfiguration()); + combinedConfiguration.addConfiguration(builder.getConfiguration()); } catch (ConfigurationException var5) { LOGGER.warn(String.format("No configuration found at %s.\nUsing defaults.", filePath)); } diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java index e4fefe6..b7bf44f 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java @@ -100,26 +100,23 @@ public SubjectGenerator() { * * @param principalExtractor The Principal Extractor to use. * @return Subject instance. Never null. - * * @throws IOException If the domain cannot be extracted from * the server name. */ public final Subject generate(final PrincipalExtractor principalExtractor) throws IOException { - final Subject subject = - AuthenticationUtil.getSubject(principalExtractor); - final Set cookieCredentials = - subject.getPublicCredentials(SSOCookieCredential.class); - final SSOCookieCredential cookieCredential = - cookieCredentials.isEmpty() ? null : cookieCredentials.toArray(new SSOCookieCredential[0])[0]; + final Subject subject = AuthenticationUtil.getSubject(principalExtractor); + final Set cookieCredentials = subject.getPublicCredentials(SSOCookieCredential.class); + final SSOCookieCredential cookieCredential = cookieCredentials.isEmpty() + ? null + : cookieCredentials.toArray(new SSOCookieCredential[0])[0]; if (cookieCredential != null) { final Set publicCred = new HashSet<>(); for (final String serverName : accessControlUtil.getSSOServers()) { - publicCred.add(new SSOCookieCredential( - cookieCredential.getSsoCookieValue(), - NetUtil.getDomainName(serverName))); + publicCred.add(new SSOCookieCredential(cookieCredential.getSsoCookieValue(), + NetUtil.getDomainName(serverName))); publicCred.add(AuthMethod.COOKIE); } diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java b/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java index 81cfcbe..9fb1380 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java @@ -68,6 +68,7 @@ package ca.nrc.cadc.web; +import ca.nrc.cadc.auth.AuthenticationUtil; import ca.nrc.cadc.auth.PrincipalExtractor; import ca.nrc.cadc.auth.ServletPrincipalExtractor; @@ -135,22 +136,17 @@ public void init(FilterConfig filterConfig) throws ServletException { * @param chain The chain to continue on to. */ @Override - public void doFilter(final ServletRequest request, - final ServletResponse response, - final FilterChain chain) - throws IOException, ServletException { + public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) + throws IOException, ServletException { final SubjectGenerator subjectGenerator = new SubjectGenerator(); final PrincipalExtractor principalExtractor = new ServletPrincipalExtractor((HttpServletRequest) request); final Subject subject = subjectGenerator.generate(principalExtractor); try { - Subject.doAs(subject, new PrivilegedExceptionAction() { - @Override - public Object run() throws Exception { - chain.doFilter(request, response); - return null; - } + Subject.doAs(subject, (PrivilegedExceptionAction) () -> { + chain.doFilter(request, response); + return null; }); } catch (Exception e) { throw new ServletException(e); diff --git a/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java b/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java index ca55f77..4550e57 100644 --- a/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java +++ b/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java @@ -70,6 +70,7 @@ import ca.nrc.cadc.accesscontrol.AccessControlUtil; +import ca.nrc.cadc.auth.CookiePrincipal; import ca.nrc.cadc.auth.HttpPrincipal; import ca.nrc.cadc.auth.PrincipalExtractor; import ca.nrc.cadc.auth.SSOCookieCredential; @@ -81,7 +82,9 @@ import java.util.List; import java.util.Set; +import ca.nrc.cadc.auth.SSOCookieManager; import ca.nrc.cadc.net.NetUtil; +import org.json.Cookie; import org.junit.Test; import static org.junit.Assert.*; @@ -96,48 +99,29 @@ public void generate() throws Exception { final PrincipalExtractor mockPrincipalExtractor = mock(PrincipalExtractor.class); final SubjectGenerator testSubject = new SubjectGenerator(mockAccessControlUtil); final Set domainServers = new HashSet<>(); - final List cookieCredentials = - Collections.singletonList(new SSOCookieCredential("cookievalue", - "anotherplace.com")); final Set principals = new HashSet<>(); + final HttpPrincipal httpPrincipal = new HttpPrincipal("USER"); - principals.add(new HttpPrincipal("USER")); + principals.add(httpPrincipal); + principals.add(new CookiePrincipal("CADC_SSO", new SSOCookieManager().generate(httpPrincipal))); domainServers.add("mysite.example.com"); domainServers.add("mysite.anotherplace.com"); domainServers.add("mysite.onemore.com"); when(mockAccessControlUtil.getSSOServers()).thenReturn(domainServers); - when(mockPrincipalExtractor.getSSOCookieCredentials()).thenReturn(cookieCredentials); when(mockPrincipalExtractor.getPrincipals()).thenReturn(principals); when(mockPrincipalExtractor.getCertificateChain()).thenReturn(null); - when(mockPrincipalExtractor.getDelegationToken()).thenReturn(null); final Subject subject = testSubject.generate(mockPrincipalExtractor); - final Set generatedCookieCredentials = - subject.getPublicCredentials(SSOCookieCredential.class); - final Set generatedCookieCredentialDomains = new HashSet<>(); + final Set cookieCredentials = subject.getPublicCredentials(SSOCookieCredential.class); - for (final SSOCookieCredential ssoCookieCredential - : generatedCookieCredentials) { - generatedCookieCredentialDomains.add( - ssoCookieCredential.getDomain()); - } - - final Set serverDomains = new HashSet<>(); - - for (final String domainServer : domainServers) { - serverDomains.add(NetUtil.getDomainName(domainServer)); - } - - assertEquals("Wrong domains.", serverDomains, - generatedCookieCredentialDomains); + // The main two in ac-domains.properties, as well as the three above + assertEquals("Wrong cookie count.", 5, cookieCredentials.size()); verify(mockAccessControlUtil, times(1)).getSSOServers(); - verify(mockPrincipalExtractor, times(1)).getSSOCookieCredentials(); verify(mockPrincipalExtractor, times(1)).getCertificateChain(); verify(mockPrincipalExtractor, times(1)).getPrincipals(); - verify(mockPrincipalExtractor, times(1)).getDelegationToken(); } } diff --git a/settings.gradle b/settings.gradle index f57eb68..6de682b 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,2 +1,2 @@ rootProject.name = 'opencadc-web' -include 'cadc-web-util', 'cadc-web-test' +include('cadc-web-util', 'cadc-web-test') From 1a8239333357eaeee7c4e0319d447cbfe7d3142c Mon Sep 17 00:00:00 2001 From: Dustin Jenkins Date: Tue, 24 May 2022 15:43:20 -0700 Subject: [PATCH 2/3] Linter fixes. --- .github/workflows/gradle.yml | 2 +- .../accesscontrol/AccessControlClient.java | 11 +++-- .../cadc/config/ApplicationConfiguration.java | 10 ++-- .../ca/nrc/cadc/web/SubjectGenerator.java | 2 +- .../nrc/cadc/web/WebAccessControlFilter.java | 41 ++++++++--------- .../java/org/opencadc/proxy/HttpProxy.java | 39 +++++++--------- .../java/org/opencadc/proxy/ProxyServlet.java | 46 +++++++++++++------ .../opencadc/proxy/ServiceParameterMap.java | 16 +++---- 8 files changed, 87 insertions(+), 80 deletions(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index bdf3561..622d3fe 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -12,6 +12,6 @@ jobs: with: java-version: 1.8 - name: Build cadc-web-util with Gradle - run: cd cadc-web-util && ../gradlew -i clean build test + run: cd cadc-web-util && ../gradlew -i clean build test javadoc checkstyleMain - name: Build cadc-web-test with Gradle run: cd cadc-web-test && ../gradlew -i clean build test diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java b/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java index ceb9d66..b7d1c7e 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/accesscontrol/AccessControlClient.java @@ -90,7 +90,8 @@ private URL lookupPasswordResetURL() { /** * Reset the password for the currently authenticated user. * - * @param newPassword The new password value. + * @param newPassword The new password value. + * @param token The secure pre-authorized token. */ public void resetPassword(final char[] newPassword, final char[] token) { final Map payload = new HashMap<>(); @@ -234,10 +235,10 @@ int post(final URL url, final Map payload, final Map payload, final Map headers) { final HttpPost post = new HttpPost(url, payload, false); diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java b/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java index 4d02d64..c5599a7 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/config/ApplicationConfiguration.java @@ -1,6 +1,5 @@ package ca.nrc.cadc.config; - import ca.nrc.cadc.util.StringUtil; import java.net.URI; @@ -64,12 +63,13 @@ public String lookup(String key, String defaultValue) { return configuration.getString(key, defaultValue); } - public String[] lookupAll(String key) { - return configuration.getStringArray(key); - } - @SuppressWarnings("unchecked") public T lookup(String key) { return (T) configuration.getProperty(key); } + + + public String[] lookupAll(String key) { + return configuration.getStringArray(key); + } } diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java index b7bf44f..5abbb7a 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java @@ -75,10 +75,10 @@ import ca.nrc.cadc.auth.SSOCookieCredential; import ca.nrc.cadc.net.NetUtil; -import javax.security.auth.Subject; import java.io.IOException; import java.util.HashSet; import java.util.Set; +import javax.security.auth.Subject; public class SubjectGenerator { diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java b/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java index 9fb1380..fe0afa6 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/web/WebAccessControlFilter.java @@ -68,35 +68,36 @@ package ca.nrc.cadc.web; -import ca.nrc.cadc.auth.AuthenticationUtil; import ca.nrc.cadc.auth.PrincipalExtractor; import ca.nrc.cadc.auth.ServletPrincipalExtractor; -import javax.security.auth.Subject; -import javax.servlet.*; -import javax.servlet.http.HttpServletRequest; import java.io.IOException; import java.security.PrivilegedExceptionAction; +import javax.security.auth.Subject; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; public class WebAccessControlFilter implements Filter { /** - * Called by the web container to indicate to a filter that it is - * being placed into service. + * Called by the web container to indicate to a filter that it is being placed into service. * - *

The servlet container calls the init - * method exactly once after instantiating the filter. The init + *

The servlet container calls the init method exactly once after instantiating the filter. The init * method must complete successfully before the filter is asked to do any - * filtering work. + * filtering work.

* - *

The web container cannot place the filter into service if the init - * method either + *

The web container cannot place the filter into service if the init method either

*
    - *
  1. Throws a ServletException - *
  2. Does not return within a time period defined by the web container + *
  3. Throws a ServletException + *
  4. Does not return within a time period defined by the web container *
* - * @param filterConfig Config for filters. Empty here. + * @param filterConfig Config for filters. Empty here. */ @Override public void init(FilterConfig filterConfig) throws ServletException { @@ -110,8 +111,7 @@ public void init(FilterConfig filterConfig) throws ServletException { * The FilterChain passed in to this method allows the Filter to pass * on the request and response to the next entity in the chain. * - *

A typical implementation of this method would follow the following - * pattern: + *

A typical implementation of this method would follow the following pattern:

*
    *
  1. Examine the request *
  2. Optionally wrap the request object with a custom implementation to @@ -131,16 +131,15 @@ public void init(FilterConfig filterConfig) throws ServletException { * next entity in the filter chain. *
* - * @param request Incoming ServletRequest. - * @param response ServletResponse to write to. - * @param chain The chain to continue on to. + * @param request Incoming ServletRequest. + * @param response ServletResponse to write to. + * @param chain The chain to continue on to. */ @Override public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { final SubjectGenerator subjectGenerator = new SubjectGenerator(); - final PrincipalExtractor principalExtractor = - new ServletPrincipalExtractor((HttpServletRequest) request); + final PrincipalExtractor principalExtractor = new ServletPrincipalExtractor((HttpServletRequest) request); final Subject subject = subjectGenerator.generate(principalExtractor); try { diff --git a/cadc-web-util/src/main/java/org/opencadc/proxy/HttpProxy.java b/cadc-web-util/src/main/java/org/opencadc/proxy/HttpProxy.java index afe166e..eafce57 100644 --- a/cadc-web-util/src/main/java/org/opencadc/proxy/HttpProxy.java +++ b/cadc-web-util/src/main/java/org/opencadc/proxy/HttpProxy.java @@ -70,20 +70,16 @@ package org.opencadc.proxy; import ca.nrc.cadc.auth.NotAuthenticatedException; -import ca.nrc.cadc.io.ByteLimitExceededException; import ca.nrc.cadc.net.ExpectationFailedException; -import ca.nrc.cadc.net.HttpDownload; +import ca.nrc.cadc.net.HttpGet; import ca.nrc.cadc.net.HttpTransfer; - import ca.nrc.cadc.net.PreconditionFailedException; -import ca.nrc.cadc.net.ResourceAlreadyExistsException; -import ca.nrc.cadc.net.ResourceNotFoundException; -import ca.nrc.cadc.net.TransientException; -import javax.servlet.http.HttpServletResponse; + import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.URL; import java.security.AccessControlException; +import javax.servlet.http.HttpServletResponse; public class HttpProxy extends HttpTransfer { @@ -96,22 +92,19 @@ public HttpProxy(final URL url, final HttpServletResponse response) { } @Override - public void prepare() - throws AccessControlException, NotAuthenticatedException, - ByteLimitExceededException, ExpectationFailedException, IllegalArgumentException, - PreconditionFailedException, ResourceAlreadyExistsException, ResourceNotFoundException, - TransientException, IOException, InterruptedException { + public void prepare() throws AccessControlException, NotAuthenticatedException, ExpectationFailedException, + IllegalArgumentException, PreconditionFailedException { throw new UnsupportedOperationException(); } - + /** * When an object implementing interface Runnable is used * to create a thread, starting the thread causes the object's * run method to be called in that separately executing * thread. - *

- * The general contract of the method run is that it may + * + *

The general contract of the method run is that it may * take any action whatsoever. * * @see Thread#run() @@ -125,17 +118,17 @@ public void run() { // jenkinsd 2019.04.04 // final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - final HttpDownload download = new HttpDownload(this.remoteURL, outputStream); - download.setFollowRedirects(this.followRedirects); - download.setRequestProperties(super.getRequestProperties()); - download.run(); + final HttpGet getRequest = new HttpGet(this.remoteURL, outputStream); + getRequest.setFollowRedirects(this.followRedirects); + getRequest.setRequestProperties(super.getRequestProperties()); + getRequest.run(); - final long contentLength = download.getContentLength(); - this.failure = download.getThrowable(); + final long contentLength = getRequest.getContentLength(); + this.failure = getRequest.getThrowable(); try { - response.setStatus(download.getResponseCode()); - response.setContentType(download.getContentType()); + response.setStatus(getRequest.getResponseCode()); + response.setContentType(getRequest.getContentType()); if (contentLength > 0L) { response.setContentLength(Long.valueOf(contentLength).intValue()); diff --git a/cadc-web-util/src/main/java/org/opencadc/proxy/ProxyServlet.java b/cadc-web-util/src/main/java/org/opencadc/proxy/ProxyServlet.java index 5d4fff3..d8a1c4e 100644 --- a/cadc-web-util/src/main/java/org/opencadc/proxy/ProxyServlet.java +++ b/cadc-web-util/src/main/java/org/opencadc/proxy/ProxyServlet.java @@ -77,17 +77,17 @@ import ca.nrc.cadc.reg.client.RegistryClient; import ca.nrc.cadc.util.StringUtil; -import javax.servlet.ServletOutputStream; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import javax.servlet.ServletOutputStream; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; /** @@ -143,18 +143,16 @@ private ServiceParameterMap buildServiceParameterMap(final HttpServletRequest re URL lookupServiceURL(final ServiceParameterMap serviceParameters) throws IOException { final RegistryClient registryClient = getRegistryClient(); - final URL serviceURL = registryClient.getServiceURL(serviceParameters.getURI(ServiceParameterName.RESOURCE_ID), - serviceParameters.getURI(ServiceParameterName.STANDARD_ID), - AuthMethod.valueOf( - serviceParameters - .get(ServiceParameterName.AUTH_TYPE) - .toUpperCase()), - serviceParameters - .getURI(ServiceParameterName.INTERFACE_TYPE_ID)); + final URL serviceURL = + registryClient.getServiceURL(serviceParameters.getURI(ServiceParameterName.RESOURCE_ID), + serviceParameters.getURI(ServiceParameterName.STANDARD_ID), + AuthMethod.valueOf(serviceParameters.get(ServiceParameterName.AUTH_TYPE) + .toUpperCase()), + serviceParameters.getURI(ServiceParameterName.INTERFACE_TYPE_ID)); if (serviceURL == null) { throw new IllegalArgumentException("No Service URL matching provided parameters:\n\n" - + serviceParameters.toString() + "\n\n"); + + serviceParameters + "\n\n"); } else { final URL serviceURLWithPath; @@ -170,7 +168,8 @@ URL lookupServiceURL(final ServiceParameterMap serviceParameters) throws IOExcep final String extraQuery = serviceParameters.get(ServiceParameterName.EXTRA_QUERY); serviceURLWithQuery = new URL(serviceURLWithPath, serviceURLWithPath.getPath() + (extraQuery.startsWith("?") ? extraQuery - : "?" + extraQuery)); + : "?" + + extraQuery)); } else { serviceURLWithQuery = serviceURLWithPath; } @@ -208,27 +207,33 @@ HttpPost getHttpPost(final URL url, final byte[] data, final String contentType) * data of unlimited length to the Web server a single time * and is useful when posting information such as * credit card numbers. + * *

When overriding this method, read the request data, * write the response headers, get the response's writer or output * stream object, and finally, write the response data. It's best * to include content type and encoding. When using a * PrintWriter object to return the response, set the * content type before accessing the PrintWriter object. + * *

The servlet container must write the headers before committing the * response, because in HTTP the headers must be sent before the * response body. + * *

Where possible, set the Content-Length header (with the * {@link ServletResponse#setContentLength} method), * to allow the servlet container to use a persistent connection * to return its response to the client, improving performance. * The content length is automatically set if the entire response fits * inside the response buffer. + * *

When using HTTP 1.1 chunked encoding (which means that the response * has a Transfer-Encoding header), do not set the Content-Length header. + * *

This method does not need to be either safe or idempotent. * Operations requested through POST can have side effects for * which the user can be held accountable, for example, * updating stored data or buying items online. + * *

If the HTTP POST request is incorrectly formatted, * doPost returns an HTTP "Bad Request" message. * @@ -286,6 +291,7 @@ HttpUpload getHttpUpload(final URL url, final InputStream inputStream) { * The PUT operation allows a client to * place a file on the server and is similar to * sending a file by FTP. + * *

When overriding this method, leave intact * any content headers sent with the request (including * Content-Length, Content-Type, Content-Transfer-Encoding, @@ -295,11 +301,13 @@ HttpUpload getHttpUpload(final URL url, final InputStream inputStream) { * (HTTP 501 - Not Implemented) and discard the request. * For more information on HTTP 1.1, see RFC 2616 * . + * *

This method does not need to be either safe or idempotent. * Operations that doPut performs can have side * effects for which the user can be held accountable. When using * this method, it may be useful to save a copy of the * affected URL in temporary storage. + * *

If the HTTP PUT request is incorrectly formatted, * doPut returns an HTTP "Bad Request" message. * @@ -332,10 +340,12 @@ HttpProxy getHttpProxy(final URL url, final HttpServletResponse response) { /** * Called by the server (via the service method) to * allow a servlet to handle a GET request. + * *

Overriding this method to support a GET request also * automatically supports an HTTP HEAD request. A HEAD * request is a GET request that returns no body in the * response, only the request header fields. + * *

When overriding this method, read the request data, * write the response headers, get the response's writer or * output stream object, and finally, write the response data. @@ -343,28 +353,34 @@ HttpProxy getHttpProxy(final URL url, final HttpServletResponse response) { * a PrintWriter object to return the response, * set the content type before accessing the * PrintWriter object. + * *

The servlet container must write the headers before * committing the response, because in HTTP the headers must be sent * before the response body. + * *

Where possible, set the Content-Length header (with the * {@link ServletResponse#setContentLength} method), * to allow the servlet container to use a persistent connection * to return its response to the client, improving performance. * The content length is automatically set if the entire response fits * inside the response buffer. + * *

When using HTTP 1.1 chunked encoding (which means that the response * has a Transfer-Encoding header), do not set the Content-Length header. + * *

The GET method should be safe, that is, without * any side effects for which users are held responsible. * For example, most form queries have no side effects. * If a client request is intended to change stored data, * the request should use some other HTTP method. + * *

The GET method should also be idempotent, meaning * that it can be safely repeated. Sometimes making a * method safe also makes it idempotent. For example, * repeating queries is both safe and idempotent, but * buying a product online or modifying data is neither * safe nor idempotent. + * *

If the request is incorrectly formatted, doGet * returns an HTTP "Bad Request" message. * diff --git a/cadc-web-util/src/main/java/org/opencadc/proxy/ServiceParameterMap.java b/cadc-web-util/src/main/java/org/opencadc/proxy/ServiceParameterMap.java index 45889e4..c4fc772 100644 --- a/cadc-web-util/src/main/java/org/opencadc/proxy/ServiceParameterMap.java +++ b/cadc-web-util/src/main/java/org/opencadc/proxy/ServiceParameterMap.java @@ -75,7 +75,7 @@ import java.util.HashMap; /** - * ServiceParamter name values are mandatory, and cannot be null. + * ServiceParameter name values are mandatory, and cannot be null. */ public class ServiceParameterMap extends HashMap { public final URI getURI(final ServiceParameterName serviceParameterNameKey) { @@ -89,10 +89,8 @@ public final URI getURI(final ServiceParameterName serviceParameterNameKey) { * * @param key key with which the specified value is to be associated * @param value value to be associated with the specified key - * @return the previous value associated with key, or - * null if there was no mapping for key. - * (A null return can also indicate that the map - * previously associated null with key.) + * @return the previous value associated with key, or null if there was no mapping for key. + * (A null return can also indicate that the map previously associated null with key.) */ @Override public String put(final ServiceParameterName key, final String value) { @@ -122,10 +120,10 @@ public String putFirst(final ServiceParameterName key, final String[] values) { /** * Returns a string representation of this map. The string representation * consists of a list of key-value mappings in the order returned by the - * map's entrySet view's iterator, enclosed in braces - * ("{}"). Adjacent mappings are separated by the characters - * ", " (comma and space). Each key-value mapping is rendered as - * the key followed by an equals sign ("=") followed by the + * map's entrySet view's iterator, enclosed in braces + * ("{}"). Adjacent mappings are separated by the characters + * ", " (comma and space). Each key-value mapping is rendered as + * the key followed by an equals sign ("=") followed by the * associated value. Keys and values are converted to strings as by * {@link String#valueOf(Object)}. * From dcea021e73195d5cb0f8ee31b3ff9fde4ab8fb64 Mon Sep 17 00:00:00 2001 From: Dustin Jenkins Date: Wed, 25 May 2022 15:46:35 -0700 Subject: [PATCH 3/3] Remove need for RSA keys. --- .../ca/nrc/cadc/web/SubjectGenerator.java | 6 ++- .../ca/nrc/cadc/web/SubjectGeneratorTest.java | 50 ++++++++++++++----- .../test/resources/LocalAuthority.properties | 15 ------ 3 files changed, 42 insertions(+), 29 deletions(-) delete mode 100644 cadc-web-util/src/test/resources/LocalAuthority.properties diff --git a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java index 5abbb7a..091d13b 100644 --- a/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java +++ b/cadc-web-util/src/main/java/ca/nrc/cadc/web/SubjectGenerator.java @@ -105,7 +105,7 @@ public SubjectGenerator() { */ public final Subject generate(final PrincipalExtractor principalExtractor) throws IOException { - final Subject subject = AuthenticationUtil.getSubject(principalExtractor); + final Subject subject = getSubject(principalExtractor); final Set cookieCredentials = subject.getPublicCredentials(SSOCookieCredential.class); final SSOCookieCredential cookieCredential = cookieCredentials.isEmpty() ? null @@ -125,4 +125,8 @@ public final Subject generate(final PrincipalExtractor principalExtractor) return subject; } + + Subject getSubject(final PrincipalExtractor principalExtractor) { + return AuthenticationUtil.getSubject(principalExtractor); + } } diff --git a/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java b/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java index 4550e57..3f00274 100644 --- a/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java +++ b/cadc-web-util/src/test/java/ca/nrc/cadc/web/SubjectGeneratorTest.java @@ -76,14 +76,18 @@ import ca.nrc.cadc.auth.SSOCookieCredential; import javax.security.auth.Subject; +import java.io.IOException; import java.security.Principal; +import java.util.Calendar; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.UUID; import ca.nrc.cadc.auth.SSOCookieManager; import ca.nrc.cadc.net.NetUtil; +import junit.framework.AssertionFailedError; import org.json.Cookie; import org.junit.Test; @@ -97,31 +101,51 @@ public class SubjectGeneratorTest { public void generate() throws Exception { final AccessControlUtil mockAccessControlUtil = mock(AccessControlUtil.class); final PrincipalExtractor mockPrincipalExtractor = mock(PrincipalExtractor.class); - final SubjectGenerator testSubject = new SubjectGenerator(mockAccessControlUtil); + final Subject testUser = new Subject(); + final String cookieToken = UUID.randomUUID().toString(); + final Calendar expiryCalendar = Calendar.getInstance(); + expiryCalendar.add(Calendar.MONTH, 1); + final SSOCookieCredential testSSOCookieCredential = + new SSOCookieCredential(cookieToken, "example.cadc.ca", expiryCalendar.getTime()); + testUser.getPublicCredentials().add(testSSOCookieCredential); + final SubjectGenerator testSubject = new SubjectGenerator(mockAccessControlUtil) { + @Override + Subject getSubject(PrincipalExtractor principalExtractor) { + return testUser; + } + }; final Set domainServers = new HashSet<>(); - final Set principals = new HashSet<>(); - final HttpPrincipal httpPrincipal = new HttpPrincipal("USER"); - - principals.add(httpPrincipal); - principals.add(new CookiePrincipal("CADC_SSO", new SSOCookieManager().generate(httpPrincipal))); - domainServers.add("mysite.example.com"); domainServers.add("mysite.anotherplace.com"); domainServers.add("mysite.onemore.com"); when(mockAccessControlUtil.getSSOServers()).thenReturn(domainServers); - when(mockPrincipalExtractor.getPrincipals()).thenReturn(principals); - when(mockPrincipalExtractor.getCertificateChain()).thenReturn(null); + + final Set allDomains = new HashSet<>(); + domainServers.forEach(domain -> { + try { + allDomains.add(NetUtil.getDomainName(domain)); + } catch (IOException ioException) { + throw new RuntimeException(ioException.getMessage(), ioException); + } + }); + allDomains.add(NetUtil.getDomainName(testSSOCookieCredential.getDomain())); final Subject subject = testSubject.generate(mockPrincipalExtractor); final Set cookieCredentials = subject.getPublicCredentials(SSOCookieCredential.class); - // The main two in ac-domains.properties, as well as the three above - assertEquals("Wrong cookie count.", 5, cookieCredentials.size()); + // The main one set, as well as the three above + assertEquals("Wrong cookie count.", allDomains.size(), cookieCredentials.size()); + cookieCredentials.forEach(cookieCredential -> { + try { + final String cookieCredDomain = NetUtil.getDomainName(cookieCredential.getDomain()); + assertTrue("Wrong domain (" + cookieCredDomain + ")", allDomains.contains(cookieCredDomain)); + } catch (IOException ioException) { + throw new AssertionFailedError(ioException.getMessage()); + } + }); verify(mockAccessControlUtil, times(1)).getSSOServers(); - verify(mockPrincipalExtractor, times(1)).getCertificateChain(); - verify(mockPrincipalExtractor, times(1)).getPrincipals(); } } diff --git a/cadc-web-util/src/test/resources/LocalAuthority.properties b/cadc-web-util/src/test/resources/LocalAuthority.properties deleted file mode 100644 index 1280dd7..0000000 --- a/cadc-web-util/src/test/resources/LocalAuthority.properties +++ /dev/null @@ -1,15 +0,0 @@ -# -# local authority map -# -# = -#ac = canfar.net -ivo://ivoa.net/std/GMS#groups-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/GMS#search-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#users-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#reqs-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#login-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#modpass-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#resetpass-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/UMS#whoami-0.1 = ivo://cadc.nrc.ca/ac -ivo://ivoa.net/std/CDP#delegate-1.0 = ivo://cadc.nrc.ca/cred -ivo://ivoa.net/std/CDP#proxy-1.0 = ivo://cadc.nrc.ca/cred