- general refactoring

1. simpler and more uniform verifications of authen. and author.
instaclustr · Dec 4, 2018 · fac0469 · fac0469
1 parent 1124a6f
commit fac0469
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 129 deletions.
diff --git a/build.gradle b/build.gradle
@@ -1,19 +1,20 @@
 buildscript {
-    ext.kotlin_version = '1.2.70'
+    ext.kotlin_version = '1.3.10'
     ext.kafka_version = '2.0.0'
-    ext.unboundid_version = '4.0.8'
+    ext.unboundid_version = '4.0.9'
     ext.caffeine_version = '2.6.2'
-    ext.jackson_version = '2.9.6'
-    ext.spek_version = '1.1.5'
+    ext.jackson_version = '2.9.7'
+    ext.spek_version = '2.0.0-rc.1'
     ext.kluent_version = '1.39'
     ext.slf4j_version = '1.7.5'
 }
 
 plugins{
-    id "org.jetbrains.kotlin.jvm" version "1.2.70"
+    id "org.jetbrains.kotlin.jvm" version "1.3.10"
     id 'org.jmailen.kotlinter' version '1.15.0'
     id 'maven-publish'
-    id 'com.github.johnrengelman.shadow' version '2.0.4'
+    id 'com.github.johnrengelman.shadow' version '4.0.3'
+    id 'com.github.ben-manes.versions' version '0.20.0'
 }
 
 group 'no.nav.common.security'
@@ -24,6 +25,8 @@ version '2.0_0.56'
 
 sourceCompatibility = 1.8
 
+test { useJUnitPlatform { includeEngines 'spek2' } }
+
 repositories {
     maven { url "https://dl.bintray.com/spekframework/spek-dev" }
     jcenter()
@@ -43,10 +46,10 @@ dependencies {
     implementation "com.github.ben-manes.caffeine:caffeine:$caffeine_version"
 
     testImplementation "org.amshove.kluent:kluent:$kluent_version"
-    testImplementation ('org.spekframework.spek2:spek-dsl-jvm:2.0.0-alpha.2')  {
+    testImplementation ("org.spekframework.spek2:spek-dsl-jvm:$spek_version")  {
         exclude group: 'org.jetbrains.kotlin'
     }
-    testRuntimeOnly ('org.spekframework.spek2:spek-runner-junit5:2.0.0-alpha.2') {
+    testRuntimeOnly ("org.spekframework.spek2:spek-runner-junit5:$spek_version") {
         exclude group: 'org.jetbrains.kotlin'
     }
 

diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-4.10-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
diff --git a/src/main/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthentication.kt b/src/main/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthentication.kt
@@ -49,37 +49,25 @@ class SimpleLDAPAuthentication : AuthenticateCallbackHandler {
         callbacks?.other<NameCallback, PlainAuthenticateCallback>()?.let { throw UnsupportedCallbackException(it) }
     }
 
-    private fun userInCache(username: String, password: String): Boolean =
-            LDAPConfig.getByClasspath().let { ldapConfig ->
-                ldapConfig.toUserDNNodes(username).fold(false) { exists, uDN ->
-                    exists || LDAPCache.userExists(uDN, password) }.also { if (it) log.debug("$username is cached") }
-            }
+    private fun userInCache(userDNs: List<String>, password: String): Boolean =
+            userDNs.any { uDN -> LDAPCache.userExists(uDN, password) }
 
-    private fun userBoundedInLDAP(username: String, password: String): Boolean =
+    private fun userBoundedInLDAP(userDNs: List<String>, password: String): Boolean =
             LDAPAuthentication.init()
-                    .use { ldap ->
-                        ldap.canUserAuthenticate(username, password)
-                                .also { authenResult ->
-                                    if (authenResult.authenticated) {
-                                        LDAPCache.userAdd(authenResult.userDN, password)
-                                        log.info("Bind cache updated for ${authenResult.userDN}")
-                                    } // no else since all scenarios are covered in LDAPAuthentication
-                                }
-                    }.authenticated
+                    .use { ldap -> ldap.canUserAuthenticate(userDNs, password) }
+                    .map { LDAPCache.userAdd(it.userDN, password) }
+                    .isNotEmpty()
 
     private fun authenticate(username: String, password: String): Boolean =
-        "Authentication Start - $username".let { logTxt ->
-            log.debug(logTxt)
-            when (userInCache(username, password)) {
-                true -> true
-                else -> userBoundedInLDAP(username, password)
-            }.also {
-                if (it)
-                    log.info("Authentication End - successful authentication of $username")
-                else
-                    log.error("Authentication End - authentication failed for $username")
+            LDAPConfig.getByClasspath().toUserDNNodes(username).let { userDNs ->
+                // always check cache before ldap lookup
+                (userInCache(userDNs, password) || userBoundedInLDAP(userDNs, password))
+                        .also { isAuthenticated ->
+                            log.debug("Authentication Start - $username")
+                            if (isAuthenticated) log.info("Authentication End - successful authentication of $username")
+                            else log.error("Authentication End - authentication failed for $username")
+                        }
             }
-        }
 
     override fun configure(
         configs: MutableMap<String, *>?,

diff --git a/src/main/kotlin/no/nav/common/security/authorization/GroupAuthorizer.kt b/src/main/kotlin/no/nav/common/security/authorization/GroupAuthorizer.kt
@@ -6,7 +6,6 @@ import no.nav.common.security.ldap.LDAPCache
 import no.nav.common.security.ldap.LDAPAuthorization
 import no.nav.common.security.ldap.toUserDNNodes
 import org.apache.kafka.common.security.auth.KafkaPrincipal
-import org.slf4j.LoggerFactory
 
 /**
  * A class existing due to test capabilities
@@ -15,37 +14,24 @@ import org.slf4j.LoggerFactory
 
 class GroupAuthorizer(private val uuid: String) : AutoCloseable {
 
-    private fun userGroupMembershipIsCached(groups: List<String>, user: String): Boolean =
-        LDAPConfig.getByClasspath().let { ldapConfig ->
-            val userNodes = ldapConfig.toUserDNNodes(user)
+    private fun userGroupMembershipIsCached(groups: List<String>, userDNs: List<String>): Boolean =
+            userDNs.any { userDN -> groups.any { groupName -> LDAPCache.groupAndUserExists(groupName, userDN, uuid) } }
 
-            groups.fold(false) { res, groupName ->
-                res || userNodes.fold(false) { exists, uDN -> exists || LDAPCache.groupAndUserExists(groupName, uDN) }
-                        .also { if (it) log.debug("[$groupName,$user] is cached ($uuid)") }
-            }
-        }
-
-    private fun userGroupMembershipInLDAP(groups: List<String>, user: String): Boolean =
-        LDAPAuthorization.init(uuid).use { ldap -> ldap.isUserMemberOfAny(user, groups) }
-                .map {
-                    log.info("Group cache updated for [${it.groupName},${it.userDN}] ($uuid)")
-                    LDAPCache.groupAndUserAdd(it.groupName, it.userDN)
-                }
-                .isNotEmpty()
+    private fun userGroupMembershipInLDAP(groups: List<String>, userDNs: List<String>): Boolean =
+            LDAPAuthorization.init(uuid)
+                    .use { ldap -> ldap.isUserMemberOfAny(userDNs, groups) }
+                    .map { LDAPCache.groupAndUserAdd(it.groupName, it.userDN, uuid) }
+                    .isNotEmpty()
 
     fun authorize(principal: KafkaPrincipal, acls: Set<Acl>): Boolean =
-        acls.map { it.principal().name }.let { groups ->
-                when (userGroupMembershipIsCached(groups, principal.name)) {
-                    true -> true
-                    else -> userGroupMembershipInLDAP(groups, principal.name)
-                }
-        }
-
-    override fun close() {
-        // no need for cleanup
-    }
+            LDAPConfig.getByClasspath().toUserDNNodes(principal.name).let { userDNs ->
+                acls
+                        .map { it.principal().name }
+                        .let { groups ->
+                            // always check cache before ldap lookup
+                            userGroupMembershipIsCached(groups, userDNs) || userGroupMembershipInLDAP(groups, userDNs)
+                        }
+            }
 
-    companion object {
-        private val log = LoggerFactory.getLogger(GroupAuthorizer::class.java)
-    }
+    override fun close() {}
 }
diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthentication.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthentication.kt
@@ -11,36 +11,28 @@ import org.slf4j.LoggerFactory
 
 class LDAPAuthentication private constructor(val config: LDAPConfig.Config) : LDAPBase(config) {
 
-    private fun bindOk(user: String, pwd: String): AuthenResult =
+    private fun bindOk(uDN: String, pwd: String): AuthenResult =
             try {
-                if (ldapConnection.bind(user, pwd).resultCode == ResultCode.SUCCESS)
-                    AuthenResult(true, user, "")
+                if (ldapConnection.bind(uDN, pwd).resultCode == ResultCode.SUCCESS)
+                    AuthenResult(true, uDN, "")
                 else {
-                    AuthenResult(false, user, "LDAP bind unsuccessful for $user - unknown situation :-(")
+                    AuthenResult(false, uDN, "LDAP bind unsuccessful for $uDN - unknown situation :-(")
                 }
             } catch (e: LDAPException) {
-                AuthenResult(false, user, "LDAP bind exception for $user - ${e.diagnosticMessage}")
+                AuthenResult(false, uDN, "LDAP bind exception for $uDN - ${e.diagnosticMessage}")
             }
 
-    private fun userNodesBindOk(userNodes: List<String>, pwd: String): AuthenResult =
-        // as long as at least one user DN can authenticate, no error report in log
-        userNodes.map { uDN -> bindOk(uDN, pwd) }.let { result ->
-            if (result.any { it.authenticated })
-                result.first { it.authenticated }
-            else {
-                result.forEach { log.error(it.errMsg) }
-                result.first { !it.authenticated }
-            }
-        }
-
-    override fun canUserAuthenticate(user: String, pwd: String): AuthenResult =
+    override fun canUserAuthenticate(userDNs: List<String>, pwd: String): Set<AuthenResult> =
         if (!ldapConnection.isConnected) {
-            log.error("No LDAP connection, cannot authenticate $user and related password!")
-            AuthenResult(false, "", "")
+            log.error("No LDAP connection, cannot authenticate $userDNs and related password!")
+            emptySet()
         } else {
-            val userNodes = config.toUserDNNodes(user)
-            log.debug("Trying bind for $userNodes and given password")
-            userNodesBindOk(userNodes, pwd)
+            log.debug("Trying bind for $userDNs and given password")
+            userDNs
+                    .map { uDN -> bindOk(uDN, pwd) }
+                    .also { result -> if (result.all { !it.authenticated }) result.forEach { log.error(it.errMsg) } }
+                    .filter { it.authenticated }
+                    .toSet()
         }
 
     companion object {

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthorization.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthorization.kt
@@ -21,17 +21,23 @@ class LDAPAuthorization private constructor(
     // due to no anonymous access allowed for LDAP operations like search, compare, ...
     private val bindDN = config.toUserDN(JAASContext.username)
     private val bindPwd = JAASContext.password
+    private val connectionAndBindIsOk: Boolean
 
     init {
         log.debug("Binding information for authorization fetched from JAAS config file [$bindDN]")
 
-        try {
-            ldapConnection.bind(bindDN, bindPwd)
-            log.debug("Successfully bind to (${config.host},${config.port}) with $bindDN")
-        } catch (e: LDAPException) {
-            log.error("Authorization will fail! " +
-                    "Exception during bind of $bindDN to (${config.host},${config.port}) - ${e.diagnosticMessage}")
-        }
+        connectionAndBindIsOk = if (ldapConnection.isConnected) {
+            try {
+                ldapConnection.bind(bindDN, bindPwd)
+                log.debug("Successfully bind to (${config.host},${config.port}) with $bindDN")
+                true
+            } catch (e: LDAPException) {
+                log.error("Authorization will fail! " +
+                        "Exception during bind of $bindDN to (${config.host},${config.port}) - ${e.diagnosticMessage}")
+                false
+            }
+        } else
+            false
     }
 
     private fun getGroupDN(groupName: String): String =
@@ -66,22 +72,18 @@ class LDAPAuthorization private constructor(
                 emptyList()
             }
 
-    override fun isUserMemberOfAny(user: String, groups: List<String>): Set<AuthorResult> =
-            if (!ldapConnection.isConnected) {
-                log.error("No LDAP connection, cannot verify $user membership in $groups ($uuid)")
+    override fun isUserMemberOfAny(userDNs: List<String>, groups: List<String>): Set<AuthorResult> =
+            if (!connectionAndBindIsOk) {
+                log.error("No LDAP connection, cannot verify $userDNs membership in $groups ($uuid)")
                 emptySet()
-            } else {
-                val userNodes = config.toUserDNNodes(user)
-
+            } else
                 groups.flatMap { groupName ->
                     val members = getGroupMembers(getGroupDN(groupName))
-                    log.debug("Group membership, intersection of $members and $userNodes ($uuid)")
-                    members.intersect(userNodes).map { AuthorResult(groupName, it) }
-                }.let { result ->
-                    log.debug("Intersection result - $result ($uuid)")
-                    result.toSet()
+                    log.debug("Group membership, intersection of $members and $userDNs ($uuid)")
+                    members.intersect(userDNs).map { uDN -> AuthorResult(groupName, uDN) }
                 }
-            }
+                        .also { result -> log.debug("Intersection result - $result ($uuid)") }
+                        .toSet()
 
     companion object {
 

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPBase.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPBase.kt
@@ -8,6 +8,7 @@ import com.unboundid.util.ssl.SSLUtil
 import com.unboundid.util.ssl.TrustAllTrustManager
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
+import kotlin.system.measureTimeMillis
 
 /**
  * A base class for LDAPAuthentication and LDAPAuthorization
@@ -27,8 +28,9 @@ abstract class LDAPBase protected constructor(config: LDAPConfig.Config) : AutoC
     init {
         // initialize LDAP connection
         try {
-            ldapConnection.connect(config.host, config.port)
+            val connTime = measureTimeMillis { ldapConnection.connect(config.host, config.port) }
             log.debug("Successfully connected to (${config.host},${config.port})")
+            log.info("Connection time: $connTime")
         } catch (e: LDAPException) {
             log.error("Authentication and authorization will fail! " +
                     "Exception when connecting to (${config.host},${config.port}) - ${e.diagnosticMessage}")
@@ -45,12 +47,11 @@ abstract class LDAPBase protected constructor(config: LDAPConfig.Config) : AutoC
 
     data class AuthenResult(val authenticated: Boolean, val userDN: String, val errMsg: String)
 
-    open fun canUserAuthenticate(user: String, pwd: String): AuthenResult =
-            AuthenResult(false, "", "")
+    open fun canUserAuthenticate(userDNs: List<String>, pwd: String): Set<AuthenResult> = emptySet()
 
     data class AuthorResult(val groupName: String, val userDN: String)
 
-    open fun isUserMemberOfAny(user: String, groups: List<String>): Set<AuthorResult> = emptySet()
+    open fun isUserMemberOfAny(userDNs: List<String>, groups: List<String>): Set<AuthorResult> = emptySet()
 
     companion object {
 

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPCache.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPCache.kt
@@ -55,27 +55,35 @@ object LDAPCache {
 
     fun userExists(userDN: String, pwd: String): Boolean =
         when (bindCache.getIfPresent(Bind(userDN, pwd))) {
-            is Bind -> true
+            is Bind -> {
+                log.debug("$userDN is cached")
+                true
+            }
             else -> false
         }
 
     fun userAdd(userDN: String, pwd: String) {
         try {
             bindCache.get(Bind(userDN, pwd))
+                    .also { log.info("Bind cache updated for $userDN") }
         } catch (e: java.util.concurrent.ExecutionException) {
             log.error("Exception in userAdd - ${e.cause}")
         }
     }
 
-    fun groupAndUserExists(groupDN: String, userDN: String): Boolean =
-        when (groupCache.getIfPresent(Group(groupDN, userDN))) {
-            is Group -> true
+    fun groupAndUserExists(groupName: String, userDN: String, uuid: String): Boolean =
+        when (groupCache.getIfPresent(Group(groupName, userDN))) {
+            is Group -> {
+                log.debug("[$groupName,$userDN] is cached ($uuid)")
+                true
+            }
             else -> false
         }
 
-    fun groupAndUserAdd(groupDN: String, userDN: String): String =
+    fun groupAndUserAdd(groupName: String, userDN: String, uuid: String): String =
         try {
-            groupCache.get(Group(groupDN, userDN))?.other ?: ""
+            (groupCache.get(Group(groupName, userDN))?.other ?: "")
+                    .also { log.info("Group cache updated for [$groupName,$userDN] ($uuid)") }
         } catch (e: java.util.concurrent.ExecutionException) {
             log.error("Exception in groupAndUserAdd - ${e.cause}")
             ""

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPConfig.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPConfig.kt
@@ -46,10 +46,11 @@ object LDAPConfig {
 
     init {
         cache = try {
-            loadConfig(ClassLoader.getSystemResource("ldapconfig.yaml") ?: URL("")).also {
-                log.info("LDAPConfig for classpath is cached")
-                log.info("ldap configuration values: $it")
-            }
+            loadConfig(ClassLoader.getSystemResource("ldapconfig.yaml") ?: URL(""))
+                    .also {
+                        log.info("LDAPConfig for classpath is cached")
+                        log.info("ldap configuration values: $it")
+                    }
         } catch (e: Exception) {
             log.error("${e.message} - authentication and authorization will fail! ")
             emptyConfig
@@ -113,7 +114,8 @@ object LDAPConfig {
 // A couple of extension functions for Config
 fun LDAPConfig.Config.toUserDN(user: String) = "$usrUid=$user,$usrBaseDN".toLowerCase()
 fun LDAPConfig.Config.toUserDNNodes(user: String) =
+        // assuming most use of Basta generated service accounts
         listOf(
-                "$usrUid=$user,$usrBaseDN".toLowerCase(),
-                "$usrUid=$user,ou=ApplAccounts,$usrBaseDN".toLowerCase()
+                "$usrUid=$user,ou=ApplAccounts,$usrBaseDN".toLowerCase(),
+                "$usrUid=$user,$usrBaseDN".toLowerCase()
         )