Address issues with Mixed Authentication provider.

Summary:
When we merged in the changes for MixedAuthenticationProvider, we
did not realize a couple of issues.

MAP accepts an authType that is ',' seperated. Which allows
for "file,ldap". However the config validation was not updated to
account for this.

This manifested in the unit tests with mixed mode test "file,ldap"
where ldap configuration was not provided.

This commit addresses these issues, when specifying "file,ldap"
authentication mode, both file and ldap configuration MUST be
valid.

app/collins/util/security/AuthenticationProvider.scala

@@ -26,7 +26,7 @@ trait AuthenticationProvider {
                                     }
                                   }
                                 )
-  def authType: String
+  def authType: Array[String]
   def authenticate(username: String, password: String): Option[User]
   def useCachedCredentials: Boolean = AuthenticationProviderConfig.cacheCredentials
   def cacheTimeout: Long = AuthenticationProviderConfig.cacheTimeout
@@ -55,8 +55,8 @@ object AuthenticationProvider {
   lazy private val permissionsCache =
     ConfigCache.create(AuthenticationProviderConfig.cachePermissionsTimeout, PermissionsLoader())
 
-  def get(name: String): AuthenticationProvider = {
-    new MixedAuthenticationProvider(name)
+  def get(types: Array[String]): AuthenticationProvider = {
+    new MixedAuthenticationProvider(types)
   }
 
   def permissions(concern: String): Option[Set[String]] = {

app/collins/util/security/AuthenticationProviderConfig.scala

@@ -13,7 +13,7 @@ object AuthenticationProviderConfig extends Configurable {
   def cacheTimeout = getMilliseconds("cacheTimeout").getOrElse(0L)
   def cachePermissionsTimeout = getMilliseconds("cachePermissionsTimeout").getOrElse(30000L)
   def permissionsFile = getString("permissionsFile")(ConfigValue.Required).get
-  def authType = getString("type", "default").toLowerCase
+  def authType = getString("type", "default").split(",").map(_.trim.toLowerCase)
 
   override protected def validateConfig() {
     File.requireFileIsReadable(permissionsFile)
@@ -30,7 +30,7 @@ object FileAuthenticationProviderConfig extends Configurable {
   def userfile = getString("userfile")(ConfigValue.Required).get
 
   override protected def validateConfig() {
-    if (AuthenticationProviderConfig.authType == "file") {
+    if (AuthenticationProviderConfig.authType.contains("file")) {
       logger.debug("User authentication file " + userfile)
       File.requireFileIsReadable(userfile)
     }
@@ -64,7 +64,7 @@ object LdapAuthenticationProviderConfig extends Configurable {
   def isRfc2307Bis = schema == RFC_2307_BIS
 
   override protected def validateConfig() {
-    if (AuthenticationProviderConfig.authType == "ldap") {
+    if (AuthenticationProviderConfig.authType.contains("ldap")) {
       host
       if (!ValidSchemas.contains(schema)) {
         throw globalError("%s is not one of %s".format(

app/collins/util/security/FileAuthenticationProvider.scala

@@ -11,7 +11,7 @@ import sun.misc.BASE64Encoder
 class FileAuthenticationProvider() extends AuthenticationProvider {
 
   def userfile = FileAuthenticationProviderConfig.userfile
-  override val authType = "file"
+  override val authType = Array("file")
 
   lazy private val userCache = ConfigCache.create(10000L, FileUserLoader())
 

app/collins/util/security/LdapAuthenticationProvider.scala

@@ -17,7 +17,7 @@ import javax.naming.directory.SearchResult
 class LdapAuthenticationProvider extends AuthenticationProvider {
 
   val config = LdapAuthenticationProviderConfig
-  override val authType = "ldap"
+  override val authType = Array("ldap")
 
   // LDAP values
   def host = config.host

app/collins/util/security/MixedAuthenticationProvider.scala

@@ -10,17 +10,12 @@ class AuthenticationException(msg: String) extends Exception(msg)
 /**
  * Provides authentication by a variety of methods
  */
-class MixedAuthenticationProvider(types: String) extends AuthenticationProvider {
+class MixedAuthenticationProvider(types: Array[String]) extends AuthenticationProvider {
 
   /**
    * @return The authorization type provided by this class
    */
-  def authType: String = types
-
-  /**
-   * Ordered list of permitted authentication types
-   */
-  private val configuredTypes = types.split(",").toSeq.map(_.trim)
+  def authType = types
 
   /**
    * Attempt to authenticate user by each method specified in :types
@@ -30,7 +25,7 @@ class MixedAuthenticationProvider(types: String) extends AuthenticationProvider
     logger.debug("Beginning to try authentication types")
 
     // Iterate over the types lazily, such that if one method passes, iteration stops
-    configuredTypes.view.flatMap({
+    authType.flatMap({
       case "default" => {
         logger.trace("mock authentication type")
         val defaultProvider = AuthenticationProvider.Default

app/collins/util/security/MockAuthenticationProvider.scala

@@ -4,7 +4,7 @@ import collins.models.User
 import collins.models.UserImpl
 
 class MockAuthenticationProvider extends AuthenticationProvider {
-  override val authType = "default"
+  override val authType = Array("default")
 
   val users = Map(
     "blake" -> UserImpl("blake", "admin:first", Set("engineering","Infra","ops"), 1024, false),

test/collins/util/security/AuthenticationProviderSpec.scala

@@ -19,7 +19,7 @@ object AuthenticationProviderSpec extends Specification with collins.ResourceFin
     "work with file based auth" in new WithApplication(FakeApplication(additionalConfiguration=Map(
         "authentication.file.userfile" -> authFile.getAbsolutePath
       ))) {
-      val provider = AuthenticationProvider.get("file")
+      val provider = AuthenticationProvider.get(Array("file"))
 
       val users = Seq(
         ("blake", "password123", Set("engineering")),

test/collins/util/security/MixedAuthenticationProviderSpec.scala

@@ -6,78 +6,37 @@ import collins.util.config.ConfigurationException
 import org.specs2.mutable.Specification
 import collins.ResourceFinder
 import play.api.test.WithApplication
+import play.api.test.FakeApplication
 
 class MixedAuthenticationProviderSpec extends Specification with ResourceFinder {
 
   "Authentication" should {
-    "Authenticate users in htaccess type files" in new WithApplication {
-      authUser()
-    }
-
-    "Stop iterating if a user is found" in new WithApplication {
-      shortCircuit()
-    }
 
-    "Return None if no method succeeds" in new WithApplication {
-      authBadUser()
+    "Return None if no method succeeds" in new WithApplication(FakeApplication(additionalConfiguration = Map(
+      "authentication.type" -> "default"
+    ))) {
+      val user = User.authenticate("blake", "badpassword")
+      user must beNone
     }
 
-    "Attempt each method in order" in new WithApplication {
-      allTypes()
+    "Find user in default when auth mode is default and file" in new WithApplication(FakeApplication(additionalConfiguration = Map(
+      "authentication.type" -> "default,file",
+      "authentication.permissionsFile" -> "conf/permissions.yaml",
+      "authentication.file.userfile" -> findResource("htpasswd_users").getAbsolutePath
+    ))) {
+      // user is in default provider
+      val user = User.authenticate("test", "fizz")
+      (user must beSome[User]) and (user.get.username must_== "test")
     }
 
-    "Fail when bad type specified" in new WithApplication {
-      enforceKnownTypes()
-    }
-  }
-
-  def configure() {
-    val authFile = findResource("htpasswd_users")
-    val configData = Map(
-      "authentication.type" -> "file",
+    "Find user in file when auth mode is default and file" in new WithApplication(FakeApplication(additionalConfiguration = Map(
+      "authentication.type" -> "default,file",
       "authentication.permissionsFile" -> "conf/permissions.yaml",
-      "userfile" -> authFile.getAbsolutePath
-    )
-
-    val config = Configuration.from(configData)
-    collins.util.config.AppConfig.globalConfig = Some(config)
-    FileAuthenticationProviderConfig.initialize()
-  }
-
-  def authUser() = {
-    configure()
-
-    val prov = new MixedAuthenticationProvider("file")
-    val user = prov.authenticate("blake", "password123")
-
-    (user must beSome[User]) and (user.get.username must_== "blake")
-  }
-
-  def shortCircuit() = {
-    // If ldap is attempted, this will fail because there is no configuration registered
-    // ...thus this test passes by virtue of such an exception not being raised
-    val prov = new MixedAuthenticationProvider("file, ldap")
-    val user = prov.authenticate("blake", "password123")
-
-    (user must beSome[User]) and (user.get.username must_== "blake")
-  }
-
-  def authBadUser() = {
-    val prov = new MixedAuthenticationProvider("file")
-    val user = prov.authenticate("blake", "badpassword")
-
-    user must beNone
-  }
-
-  def allTypes() = {
-    val prov = new MixedAuthenticationProvider("file, default, ldap")
-    val user = prov.authenticate("blake", "admin:first")
-
-    (user must beSome[User]) and (user.get.username must_== "blake")
-  }
-
-  def enforceKnownTypes() = {
-    val prov = new MixedAuthenticationProvider("unknown")
-    prov.authenticate("", "") must throwAn[AuthenticationException](message = "Invalid authentication type provided: unknown")
+      "authentication.file.userfile" -> findResource("htpasswd_users").getAbsolutePath
+    ))) {
+      // user is in file
+      val user = User.authenticate("testuser2", "testpassword")
+      (user must beSome[User]) and (user.get.username must_== "testuser2")
+    }
   }
 }

test/resources/htpasswd_users

@@ -1,2 +1,3 @@
 blake:{SHA}y/2sYAj5yrQIN4TL0YdPdmGNKpc=:engineering
 testuser:{SHA}+jiiBeYrEknnkk2N2sh+NXMnnXE=:ny,also
+testuser2:{SHA}i7YRj4/Wk1rQh2o740pxfTJwj/0=:ny,also