Merge pull request #713 from spotify/more-auth-ITs

fix crtauth handshake bug; allow system tests to set environment variables

circle.sh

@@ -28,7 +28,7 @@ case "$1" in
     ;;
 
   dependencies)
-    mvn clean install -T 2 -Dmaven.javadoc.skip=true -DskipTests=true -B -V
+    mvn clean install -T 2 -Dmaven.javadoc.skip=true -DskipTests=true -Dinvoker.skip=true -B -V
 
     ;;
 

helios-authentication/src/main/java/com/spotify/helios/auth/AuthenticationPlugin.java

@@ -17,6 +17,9 @@
 
 package com.spotify.helios.auth;
 
+import java.util.Map;
+import java.util.Set;
+
 import io.dropwizard.jersey.setup.JerseyEnvironment;
 
 /**
@@ -39,7 +42,13 @@ public interface AuthenticationPlugin<C> {
    */
   String schemeName();
 
-  ServerAuthentication<C> serverAuthentication();
+  /**
+   * Create the ServerAuthentication instance to use with this plugin.
+   *
+   * @param environment the environment variables that the Helios Master was started with. Provided
+   *                    as a method parameter rather than System.getenv() for ease of testing.
+   */
+  ServerAuthentication<C> serverAuthentication(Map<String, String> environment);
 
   ClientAuthentication<C> clientAuthentication();
 
@@ -67,6 +76,17 @@ interface ServerAuthentication<C> {
      * for multi-stepped authentication handshakes.
      */
     void registerAdditionalJerseyComponents(JerseyEnvironment env);
+
+    /**
+     * Returns a Set of URI paths (such as <code>auth</code>) that should not require
+     * authentication. Note that the path as presented by Jersey will not have the leading
+     * <code>/</code>.
+     * <p>
+     * If the plugin registers any endpoints for authentication handshakes in {@link
+     * #registerAdditionalJerseyComponents(JerseyEnvironment)} then it must also return the path(s)
+     * to those endpoint(s) in this method.</p>
+     */
+    Set<String> unauthenticatedPaths();
   }
 
   interface ClientAuthentication<C> {

helios-authentication/src/main/java/com/spotify/helios/auth/SimpleServerAuthentication.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2014 Spotify AB.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.spotify.helios.auth;
+
+import com.google.common.collect.ImmutableSet;
+
+import com.spotify.helios.auth.AuthenticationPlugin.ServerAuthentication;
+
+import java.util.Set;
+
+import io.dropwizard.jersey.setup.JerseyEnvironment;
+
+/**
+ * A base authentication plugin class that can be used if the plugin does not need to add any
+ * additional Jersey components.
+ */
+public abstract class SimpleServerAuthentication<C> implements ServerAuthentication<C> {
+
+  @Override
+  public final void registerAdditionalJerseyComponents(final JerseyEnvironment env) {
+    // do nothing
+  }
+
+  @Override
+  public Set<String> unauthenticatedPaths() {
+    return ImmutableSet.of();
+  }
+}

helios-authentication/src/main/java/com/spotify/helios/auth/basic/BasicAuthenticationPlugin.java

@@ -18,11 +18,20 @@
 package com.spotify.helios.auth.basic;
 
 import com.google.auto.service.AutoService;
+import com.google.common.base.Throwables;
 
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.spotify.helios.auth.AuthenticationPlugin;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.Map;
+
 import io.dropwizard.auth.basic.BasicCredentials;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /** Proof of concept for the authentication plugin framework using HTTP Basic Auth. */
 @AutoService(AuthenticationPlugin.class)
 public class BasicAuthenticationPlugin implements AuthenticationPlugin<BasicCredentials> {
@@ -33,8 +42,28 @@ public String schemeName() {
   }
 
   @Override
-  public ServerAuthentication<BasicCredentials> serverAuthentication() {
-    return new BasicServerAuthentication();
+  public ServerAuthentication<BasicCredentials> serverAuthentication(
+      Map<String, String> environment) {
+
+    final String path = environment.get("AUTH_BASIC_USERDB");
+    checkNotNull(path,
+        "Environment variable AUTH_BASIC_USERDB not defined, required for "
+        + BasicAuthenticationPlugin.class.getSimpleName());
+
+    return new BasicServerAuthentication(readFileOfUsers(path));
+  }
+
+  private Map<String, String> readFileOfUsers(final String path) {
+    final File file = new File(path);
+    final ObjectMapper objectMapper = new ObjectMapper();
+    final TypeReference<Map<String, String>> typeToken = new TypeReference<Map<String, String>>() {
+    };
+
+    try {
+      return  objectMapper.readValue(file, typeToken);
+    } catch (IOException e) {
+      throw Throwables.propagate(e);
+    }
   }
 
   @Override

helios-authentication/src/main/java/com/spotify/helios/auth/basic/BasicServerAuthentication.java

@@ -17,48 +17,22 @@
 
 package com.spotify.helios.auth.basic;
 
-import com.google.common.base.Throwables;
+import com.spotify.helios.auth.SimpleServerAuthentication;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.spotify.helios.auth.AuthenticationPlugin.ServerAuthentication;
-
-import java.io.File;
-import java.io.IOException;
 import java.util.Map;
 
 import io.dropwizard.auth.basic.BasicCredentials;
-import io.dropwizard.jersey.setup.JerseyEnvironment;
-
-import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * A very simple implementation of ServerAuthentication to demonstrate how to implement an
  * authentication plugin. This version reads in a user "database" from a JSON file store at a path
  * configured by an environment variable, making it likely far too simple to be used for anything
  * but demonstrations.
  */
-public class BasicServerAuthentication implements ServerAuthentication<BasicCredentials> {
+public class BasicServerAuthentication extends SimpleServerAuthentication<BasicCredentials> {
 
   private final Map<String, String> users;
 
-  public BasicServerAuthentication() {
-    final String path = System.getenv("AUTH_BASIC_USERDB");
-    checkNotNull(path,
-        "Environment variable AUTH_BASIC_USERDB not defined, required for "
-        + BasicAuthenticationPlugin.class.getSimpleName());
-
-    File file = new File(path);
-    final ObjectMapper objectMapper = new ObjectMapper();
-
-    try {
-      this.users = objectMapper.readValue(file, new TypeReference<Map<String, String>>() {
-      });
-    } catch (IOException e) {
-      throw Throwables.propagate(e);
-    }
-  }
-
   public BasicServerAuthentication(Map<String, String> users) {
     this.users = users;
   }
@@ -68,8 +42,4 @@ public com.spotify.helios.auth.Authenticator<BasicCredentials> authenticator() {
     return new BasicAuthenticator(users);
   }
 
-  @Override
-  public void registerAdditionalJerseyComponents(JerseyEnvironment env) {
-    // nothing to add
-  }
 }

helios-authentication/src/test/java/com/spotify/helios/auth/AuthenticationPluginLoaderTest.java

@@ -23,6 +23,8 @@
 
 import org.junit.Test;
 
+import java.util.Map;
+
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertThat;
 
@@ -53,7 +55,7 @@ public String schemeName() {
     }
 
     @Override
-    public ServerAuthentication<String> serverAuthentication() {
+    public ServerAuthentication<String> serverAuthentication(Map<String, String> environment) {
       return null;
     }
 

helios-crtauth/src/main/java/com/spotify/helios/auth/crt/CrtAuthenticationPlugin.java

@@ -18,7 +18,6 @@
 package com.spotify.helios.auth.crt;
 
 import com.google.auto.service.AutoService;
-import com.google.common.annotations.VisibleForTesting;
 
 import com.spotify.crtauth.CrtAuthServer;
 import com.spotify.crtauth.keyprovider.KeyProvider;
@@ -32,32 +31,24 @@
 @AutoService(AuthenticationPlugin.class)
 public class CrtAuthenticationPlugin implements AuthenticationPlugin<CrtAccessToken> {
 
-  private final Map<String, String> environment;
-
-  public CrtAuthenticationPlugin() {
-    this(System.getenv());
-  }
-
-  @VisibleForTesting
-  protected CrtAuthenticationPlugin(Map<String, String> environment) {
-    this.environment = environment;
-  }
-
   @Override
   public String schemeName() {
     return "crtauth";
   }
 
   @Override
-  public ServerAuthentication<CrtAccessToken> serverAuthentication() {
+  public ServerAuthentication<CrtAccessToken> serverAuthentication(
+      Map<String, String> environment) {
+
     // only validate the presence of environment variables when this method is called, as opposed to
     // in the constructor, as the client-side code will not use the same environment variables
-    final String ldapUrl =  getRequiredEnv("CRTAUTH_LDAP_URL");
-    final String ldapSearchPath = getRequiredEnv("CRTAUTH_LDAP_SEARCH_PATH");
-    final String serverName = getRequiredEnv("CRTAUTH_SERVERNAME");
-    final String secret = getRequiredEnv("CRTAUTH_SECRET");
-    final String ldapFieldNameOfKey = getOptionalEnv("CRTAUTH_LDAP_KEY_FIELDNAME", "sshPublicKey");
-    final int tokenLifetimeSecs = getOptionalEnv("CRTAUTH_TOKEN_LIFETIME_SECS", 540);
+    final String ldapUrl = getRequired(environment, "CRTAUTH_LDAP_URL");
+    final String ldapSearchPath = getRequired(environment, "CRTAUTH_LDAP_SEARCH_PATH");
+    final String serverName = getRequired(environment, "CRTAUTH_SERVERNAME");
+    final String secret = getRequired(environment, "CRTAUTH_SECRET");
+    final String ldapFieldNameOfKey =
+        getOptional(environment, "CRTAUTH_LDAP_KEY_FIELDNAME", "sshPublicKey");
+    final int tokenLifetimeSecs = getOptional(environment, "CRTAUTH_TOKEN_LIFETIME_SECS", 540);
 
     final LdapContextSource contextSource = new LdapContextSource();
     contextSource.setUrl(ldapUrl);
@@ -80,24 +71,25 @@ public ServerAuthentication<CrtAccessToken> serverAuthentication() {
     return new CrtServerAuthentication(new CrtTokenAuthenticator(authServer), authServer);
   }
 
-  private String getEnv(String name, boolean required) {
+  private static String getEnv(Map<String, String> environment, String name, boolean required) {
     if (required && !environment.containsKey(name)) {
       throw new IllegalArgumentException("Environment variable " + name + " is required");
     }
     return environment.get(name);
   }
 
-  private String getRequiredEnv(String name) {
-    return getEnv(name, true);
+  private static String getRequired(Map<String, String> environment, String name) {
+    return getEnv(environment, name, true);
   }
 
-  private String getOptionalEnv(String name, String defaultValue) {
-    final String defined = getEnv(name, false);
+  private static String getOptional(Map<String, String> environment, String name,
+                                    String defaultValue) {
+    final String defined = getEnv(environment, name, false);
     return defined != null ? defined : defaultValue;
   }
 
-  private int getOptionalEnv(String name, int defaultValue) {
-    final String defined = getEnv(name, false);
+  private static int getOptional(Map<String, String> environment, String name, int defaultValue) {
+    final String defined = getEnv(environment, name, false);
     if (defined == null) {
       return defaultValue;
     }

helios-crtauth/src/main/java/com/spotify/helios/auth/crt/CrtHandshakeResource.java

@@ -31,9 +31,10 @@
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
 
-@Path("/_auth")
+@Path(CrtHandshakeResource.PATH)
 public class CrtHandshakeResource {
 
+  public static final String PATH = "_auth";
   private static final Logger log = LoggerFactory.getLogger(CrtHandshakeResource.class);
 
   private final CrtAuthServer authServer;

helios-crtauth/src/main/java/com/spotify/helios/auth/crt/CrtServerAuthentication.java

@@ -17,10 +17,14 @@
 
 package com.spotify.helios.auth.crt;
 
+import com.google.common.collect.ImmutableSet;
+
 import com.spotify.crtauth.CrtAuthServer;
 import com.spotify.helios.auth.AuthenticationPlugin.ServerAuthentication;
 import com.spotify.helios.auth.Authenticator;
 
+import java.util.Set;
+
 import io.dropwizard.jersey.setup.JerseyEnvironment;
 
 public class CrtServerAuthentication implements ServerAuthentication<CrtAccessToken> {
@@ -43,4 +47,8 @@ public void registerAdditionalJerseyComponents(JerseyEnvironment env) {
     env.register(new CrtHandshakeResource(this.authServer));
   }
 
+  @Override
+  public Set<String> unauthenticatedPaths() {
+    return ImmutableSet.of(CrtHandshakeResource.PATH);
+  }
 }

helios-crtauth/src/test/java/com/spotify/helios/auth/crt/CrtAuthenticationPluginTest.java

@@ -49,14 +49,14 @@ public void serverAuthentication_NoEnvironmentVariables() {
 
     final Map<String, String> env = ImmutableMap.of();
 
-    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin(env);
-    plugin.serverAuthentication();
+    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin();
+    plugin.serverAuthentication(env);
   }
 
   @Test
   public void serverAuthentication_AllRequiredArgs() {
-    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin(requiredEnvArgs);
-    plugin.serverAuthentication();
+    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin();
+    plugin.serverAuthentication(requiredEnvArgs);
   }
 
   @Test
@@ -66,7 +66,7 @@ public void serverAuthentication_BadlyFormattedTokenLifetime() {
     final Map<String, String> env = new HashMap<>(requiredEnvArgs);
     env.put("CRTAUTH_TOKEN_LIFETIME_SECS", "asdf");
 
-    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin(env);
-    plugin.serverAuthentication();
+    final CrtAuthenticationPlugin plugin = new CrtAuthenticationPlugin();
+    plugin.serverAuthentication(env);
   }
 }