- general refactoring

- collected log messages into enum Monitoring, due to monitoring in Kibana
instaclustr · Dec 6, 2018 · c40090c · c40090c
1 parent fac0469
commit c40090c
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 33 deletions.
diff --git a/src/main/kotlin/no/nav/common/security/Monitoring.kt b/src/main/kotlin/no/nav/common/security/Monitoring.kt
@@ -0,0 +1,30 @@
+package no.nav.common.security
+
+/**
+ * Monitoring reflects utilization of log messages in kibana, in visualize components
+ * and dashboard. All visualize components and dashboard are found by searching for 'kafka'
+ *
+ * Thus, change of text in Monitoring require corresponding change in kibana
+ */
+
+enum class Monitoring(val txt: String) {
+
+    LDAP_BASE_TIME("LDAP connection time"),
+    LDAP_BASE_FAILURE("Authentication and authorization will fail! Exception when connecting to"),
+
+    AUTHENTICATION_CACHE_UPDATED("Bind cache updated"),
+    AUTHENTICATION_CACHE_UPDATE_FAILED("Exception in userAdd"),
+    AUTHENTICATION_LDAP_FAILURE("No LDAP connection, cannot authenticate"),
+    AUTHENTICATION_FAILED("Authentication End - authentication failed"),
+    AUTHENTICATION_SUCCESS("Authentication End - successful authentication"),
+
+    AUTHORIZATION_CACHE_UPDATED("Group cache updated"),
+    AUTHORIZATION_CACHE_UPDATE_FAILED("Exception in groupAndUserAdd"),
+    AUTHORIZATION_LDAP_FAILURE("No LDAP connection, cannot verify"),
+    AUTHORIZATION_BIND_FAILED("Authorization will fail! Exception during bind of"),
+    AUTHORIZATION_BIND_TIME("LDAP bind time"),
+    AUTHORIZATION_SEARCH_MISS("LDAP search couldn't resolve group DN for"),
+    AUTHORIZATION_SEARCH_FAILURE("Cannot resolve group DN for"),
+    AUTHORIZATION_GROUP_FAILURE("Cannot get group members"),
+    AUTHORIZATION_FAILED("Authorization End")
+}
diff --git a/src/main/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthentication.kt b/src/main/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthentication.kt
@@ -1,5 +1,6 @@
 package no.nav.common.security.authentication
 
+import no.nav.common.security.Monitoring
 import no.nav.common.security.ldap.LDAPAuthentication
 import no.nav.common.security.ldap.LDAPCache
 import no.nav.common.security.ldap.LDAPConfig
@@ -64,8 +65,8 @@ class SimpleLDAPAuthentication : AuthenticateCallbackHandler {
                 (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")
+                            if (isAuthenticated) log.info("${Monitoring.AUTHENTICATION_SUCCESS.txt} of $username")
+                            else log.error("${Monitoring.AUTHENTICATION_FAILED.txt} for $username")
                         }
             }
 

diff --git a/src/main/kotlin/no/nav/common/security/authorization/SimpleLDAPAuthorizer.kt b/src/main/kotlin/no/nav/common/security/authorization/SimpleLDAPAuthorizer.kt
@@ -5,6 +5,7 @@ import kafka.security.auth.Acl
 import kafka.security.auth.Operation
 import kafka.security.auth.Resource
 import kafka.security.auth.SimpleAclAuthorizer
+import no.nav.common.security.Monitoring
 import org.apache.kafka.common.acl.AclPermissionType
 import org.apache.kafka.common.resource.ResourceType
 import org.apache.kafka.common.security.auth.KafkaPrincipal
@@ -59,7 +60,7 @@ class SimpleLDAPAuthorizer : SimpleAclAuthorizer() {
 
         // nothing to do if empty acl set
         if (acls.isEmpty()) {
-            log.error("Authorization End - $authContext - empty ALLOW ACL for [$lResource,$lOperation], is not authorized ($uuid)")
+            log.error("${Monitoring.AUTHORIZATION_FAILED.txt} - $authContext - empty ALLOW ACL for [$lResource,$lOperation], is not authorized ($uuid)")
             return false
         }
 
@@ -69,7 +70,7 @@ class SimpleLDAPAuthorizer : SimpleAclAuthorizer() {
 
         when (isAuthorized) {
             true -> log.debug("Authorization End - $authContext is authorized!")
-            false -> log.error("Authorization End - $authContext is not authorized!")
+            false -> log.error("${Monitoring.AUTHORIZATION_FAILED.txt} - $authContext is not authorized!")
         }
 
         return isAuthorized

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthentication.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthentication.kt
@@ -2,6 +2,7 @@ package no.nav.common.security.ldap
 
 import com.unboundid.ldap.sdk.LDAPException
 import com.unboundid.ldap.sdk.ResultCode
+import no.nav.common.security.Monitoring
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
@@ -24,7 +25,7 @@ class LDAPAuthentication private constructor(val config: LDAPConfig.Config) : LD
 
     override fun canUserAuthenticate(userDNs: List<String>, pwd: String): Set<AuthenResult> =
         if (!ldapConnection.isConnected) {
-            log.error("No LDAP connection, cannot authenticate $userDNs and related password!")
+            log.error("${Monitoring.AUTHENTICATION_LDAP_FAILURE.txt} $userDNs and related password!")
             emptySet()
         } else {
             log.debug("Trying bind for $userDNs and given password")

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthorization.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPAuthorization.kt
@@ -5,8 +5,10 @@ import com.unboundid.ldap.sdk.Filter
 import com.unboundid.ldap.sdk.SearchRequest
 import com.unboundid.ldap.sdk.SearchScope
 import com.unboundid.ldap.sdk.LDAPSearchException
+import no.nav.common.security.Monitoring
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
+import kotlin.system.measureTimeMillis
 
 /**
  * A class verifying group membership with LDAP compare-matched
@@ -28,12 +30,12 @@ class LDAPAuthorization private constructor(
 
         connectionAndBindIsOk = if (ldapConnection.isConnected) {
             try {
-                ldapConnection.bind(bindDN, bindPwd)
+                val connTime = measureTimeMillis { ldapConnection.bind(bindDN, bindPwd) }
                 log.debug("Successfully bind to (${config.host},${config.port}) with $bindDN")
+                log.info("${Monitoring.AUTHORIZATION_BIND_TIME.txt} $connTime")
                 true
             } catch (e: LDAPException) {
-                log.error("Authorization will fail! " +
-                        "Exception during bind of $bindDN to (${config.host},${config.port}) - ${e.diagnosticMessage}")
+                log.error("${Monitoring.AUTHORIZATION_BIND_FAILED.txt} $bindDN to (${config.host},${config.port}) - ${e.diagnosticMessage}")
                 false
             }
         } else
@@ -50,12 +52,12 @@ class LDAPAuthorization private constructor(
                             if (it.entryCount == 1)
                                 it.searchEntries[0].dn
                             else {
-                                log.error("LDAP search couldn't resolve group DN for $groupName under ${config.grpBaseDN} ($uuid)")
+                                log.error("${Monitoring.AUTHORIZATION_SEARCH_MISS.txt} $groupName under ${config.grpBaseDN} ($uuid)")
                                 ""
                             }
                         }
             } catch (e: LDAPSearchException) {
-                log.error("Cannot resolve group DN for $groupName under ${config.grpBaseDN} ($uuid)")
+                log.error("${Monitoring.AUTHORIZATION_SEARCH_FAILURE.txt} $groupName under ${config.grpBaseDN} ($uuid)")
                 ""
             }
 
@@ -68,13 +70,13 @@ class LDAPAuthorization private constructor(
                 else
                     emptyList()
             } catch (e: LDAPException) {
-                log.error("Cannot get group members - ${config.grpAttrName} - for $groupDN ($uuid)")
+                log.error("${Monitoring.AUTHORIZATION_GROUP_FAILURE.txt} - ${config.grpAttrName} - for $groupDN ($uuid)")
                 emptyList()
             }
 
     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)")
+                log.error("${Monitoring.AUTHORIZATION_LDAP_FAILURE.txt} $userDNs membership in $groups ($uuid)")
                 emptySet()
             } else
                 groups.flatMap { groupName ->

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPBase.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPBase.kt
@@ -6,6 +6,7 @@ import com.unboundid.ldap.sdk.LDAPException
 import com.unboundid.ldap.sdk.DisconnectType
 import com.unboundid.util.ssl.SSLUtil
 import com.unboundid.util.ssl.TrustAllTrustManager
+import no.nav.common.security.Monitoring
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import kotlin.system.measureTimeMillis
@@ -30,10 +31,9 @@ abstract class LDAPBase protected constructor(config: LDAPConfig.Config) : AutoC
         try {
             val connTime = measureTimeMillis { ldapConnection.connect(config.host, config.port) }
             log.debug("Successfully connected to (${config.host},${config.port})")
-            log.info("Connection time: $connTime")
+            log.info("${Monitoring.LDAP_BASE_TIME.txt} $connTime")
         } catch (e: LDAPException) {
-            log.error("Authentication and authorization will fail! " +
-                    "Exception when connecting to (${config.host},${config.port}) - ${e.diagnosticMessage}")
+            log.error("${Monitoring.LDAP_BASE_FAILURE.txt} (${config.host},${config.port}) - ${e.diagnosticMessage}")
             ldapConnection.setDisconnectInfo(
                     DisconnectType.IO_ERROR,
                     "Exception when connecting to LDAP(${config.host},${config.port})", e)

diff --git a/src/main/kotlin/no/nav/common/security/ldap/LDAPCache.kt b/src/main/kotlin/no/nav/common/security/ldap/LDAPCache.kt
@@ -3,6 +3,7 @@ package no.nav.common.security.ldap
 import com.github.benmanes.caffeine.cache.CacheLoader
 import com.github.benmanes.caffeine.cache.Caffeine
 import com.github.benmanes.caffeine.cache.LoadingCache
+import no.nav.common.security.Monitoring
 import org.slf4j.LoggerFactory
 import java.util.concurrent.TimeUnit
 
@@ -62,14 +63,14 @@ object LDAPCache {
             else -> false
         }
 
-    fun userAdd(userDN: String, pwd: String) {
+    fun userAdd(userDN: String, pwd: String): String =
         try {
-            bindCache.get(Bind(userDN, pwd))
-                    .also { log.info("Bind cache updated for $userDN") }
+            (bindCache.get(Bind(userDN, pwd))?.other ?: "")
+                    .also { log.info("${Monitoring.AUTHENTICATION_CACHE_UPDATED.txt} for $userDN") }
         } catch (e: java.util.concurrent.ExecutionException) {
-            log.error("Exception in userAdd - ${e.cause}")
+            log.error("${Monitoring.AUTHENTICATION_CACHE_UPDATE_FAILED.txt} - ${e.cause}")
+            ""
         }
-    }
 
     fun groupAndUserExists(groupName: String, userDN: String, uuid: String): Boolean =
         when (groupCache.getIfPresent(Group(groupName, userDN))) {
@@ -83,9 +84,9 @@ object LDAPCache {
     fun groupAndUserAdd(groupName: String, userDN: String, uuid: String): String =
         try {
             (groupCache.get(Group(groupName, userDN))?.other ?: "")
-                    .also { log.info("Group cache updated for [$groupName,$userDN] ($uuid)") }
+                    .also { log.info("${Monitoring.AUTHORIZATION_CACHE_UPDATED.txt} for [$groupName,$userDN] ($uuid)") }
         } catch (e: java.util.concurrent.ExecutionException) {
-            log.error("Exception in groupAndUserAdd - ${e.cause}")
+            log.error("${Monitoring.AUTHORIZATION_CACHE_UPDATE_FAILED.txt} - ${e.cause}")
             ""
         }
 

diff --git a/src/test/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthenticationSpec.kt b/src/test/kotlin/no/nav/common/security/authentication/SimpleLDAPAuthenticationSpec.kt
@@ -17,7 +17,7 @@ object SimpleLDAPAuthenticationSpec : Spek({
             LDAPCache.invalidateAllBinds()
         }
 
-        describe("authentication should work correctly") {
+        context("authentication should work correctly") {
 
             // kind of misuse of the prompt field in NameCallback... Ok in test context
             val tests = mapOf(

diff --git a/src/test/kotlin/no/nav/common/security/authorization/GroupAuthorizerSpec.kt b/src/test/kotlin/no/nav/common/security/authorization/GroupAuthorizerSpec.kt
@@ -101,7 +101,7 @@ object GroupAuthorizerSpec : Spek({
                 Triple("srvc02", "KC-tpc-03", "tpc-03") to false
                 )
 
-        describe("describe allowance") {
+        context("describe allowance") {
 
             refUserDescribeACL.forEach { tr, result ->
 
@@ -116,7 +116,7 @@ object GroupAuthorizerSpec : Spek({
             }
         }
 
-        describe("write allowance") {
+        context("write allowance") {
 
             refUserWriteACL.forEach { tr, result ->
 
@@ -131,7 +131,7 @@ object GroupAuthorizerSpec : Spek({
             }
         }
 
-        describe("read allowance") {
+        context("read allowance") {
 
             refUserReadACL.forEach { tr, result ->
 

diff --git a/src/test/kotlin/no/nav/common/security/ldap/LDAPAuthenticationSpec.kt b/src/test/kotlin/no/nav/common/security/ldap/LDAPAuthenticationSpec.kt
@@ -38,7 +38,7 @@ object LDAPAuthenticationSpec : Spek({
                 Pair("invalid", "srvc01") to false
         )
 
-        describe("correct path to default YAML config") {
+        context("correct path to default YAML config") {
 
             refUsers.forEach { user, result ->
 
@@ -53,7 +53,7 @@ object LDAPAuthenticationSpec : Spek({
             }
         }
 
-        describe("classpath to YAML config") {
+        context("classpath to YAML config") {
 
             refUsers.forEach { user, result ->
 

diff --git a/src/test/kotlin/no/nav/common/security/ldap/LDAPAuthorizationSpec.kt b/src/test/kotlin/no/nav/common/security/ldap/LDAPAuthorizationSpec.kt
@@ -42,7 +42,7 @@ object LDAPAuthorizationSpec : Spek({
                 Pair("srvc02", listOf("KC-tpc-02", "rmy-02", "KP-tpc-03")) to 2
         )
 
-        describe("correct path to default YAML config") {
+        context("correct path to default YAML config") {
 
             refUserGroup.forEach { usrGrp, size ->
 
@@ -59,7 +59,7 @@ object LDAPAuthorizationSpec : Spek({
             }
         }
 
-        describe("classpath to  YAML config") {
+        context("classpath to  YAML config") {
 
             refUserGroup.forEach { usrGrp, size ->
 

diff --git a/src/test/kotlin/no/nav/common/security/ldap/LDAPConfigSpec.kt b/src/test/kotlin/no/nav/common/security/ldap/LDAPConfigSpec.kt
@@ -20,7 +20,7 @@ object LDAPConfigSpec : Spek({
                 2,
                 4)
 
-        describe("getBySource - correct path to different YAML configs") {
+        context("getBySource - correct path to different YAML configs") {
 
             val refLDAPConfigOther = LDAPConfig.Config(
                     "host",
@@ -47,14 +47,14 @@ object LDAPConfigSpec : Spek({
             }
         }
 
-        describe("getBySource - incorrect path to YAML config") {
+        context("getBySource - incorrect path to YAML config") {
 
             it("should return empty config") {
                 LDAPConfig.getBySource("invalid.yaml") shouldEqual LDAPConfig.emptyConfig
             }
         }
 
-        describe("getByClasspath - load of default yaml config") {
+        context("getByClasspath - load of default yaml config") {
 
             it("should return default yaml config") {
                 // will find ldapconfig.yaml resource under build/resources/ldapconfig.yaml...