From d5e0fba7f22b9fd9f2c5578ffd2d8ab249aede60 Mon Sep 17 00:00:00 2001
From: Andrew Alexander <adrw@squareup.com>
Date: Mon, 12 Aug 2019 14:12:54 -0400
Subject: [PATCH] Remove duplicate authentication logic in WebTab

https://github.com/cashapp/misk/pull/1142#discussion_r312961702
---
 misk/src/main/kotlin/misk/MiskCaller.kt       | 12 +++++++++++
 .../misk/security/authz/AccessInterceptor.kt  | 13 +-----------
 misk/src/main/kotlin/misk/web/WebTab.kt       | 20 +------------------
 .../web/actions/AdminDashboardTabAction.kt    |  7 ++++---
 4 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/misk/src/main/kotlin/misk/MiskCaller.kt b/misk/src/main/kotlin/misk/MiskCaller.kt
index 78f01828afa..c55a1d706f0 100644
--- a/misk/src/main/kotlin/misk/MiskCaller.kt
+++ b/misk/src/main/kotlin/misk/MiskCaller.kt
@@ -27,4 +27,16 @@ data class MiskCaller(
       "service=$service"
     }
   }
+
+  /** Determine based on allowed capabilities/services if the caller is permitted */
+  fun isAllowed(allowedCapabilities: Set<String>, allowedServices: Set<String>): Boolean {
+    // Allow if we don't have any requirements on service or capability
+    if (allowedServices.isEmpty() && allowedCapabilities.isEmpty()) return true
+
+    // Allow if the caller has provided an allowed service
+    if (service != null && allowedServices.contains(service)) return true
+
+    // Allow if the caller has provided an allowed capability
+    return capabilities.any { allowedCapabilities.contains(it) }
+  }
 }
diff --git a/misk/src/main/kotlin/misk/security/authz/AccessInterceptor.kt b/misk/src/main/kotlin/misk/security/authz/AccessInterceptor.kt
index d3622eeaddb..800399c87ae 100644
--- a/misk/src/main/kotlin/misk/security/authz/AccessInterceptor.kt
+++ b/misk/src/main/kotlin/misk/security/authz/AccessInterceptor.kt
@@ -19,7 +19,7 @@ class AccessInterceptor private constructor(
 
   override fun intercept(chain: Chain): Any {
     val caller = caller.get() ?: throw UnauthenticatedException()
-    if (!isAllowed(caller)) {
+    if (!caller.isAllowed(allowedCapabilities, allowedServices)) {
       logger.info { "$caller is not allowed to access ${chain.action}" }
       throw UnauthorizedException()
     }
@@ -27,17 +27,6 @@ class AccessInterceptor private constructor(
     return chain.proceed(chain.args)
   }
 
-  private fun isAllowed(caller: MiskCaller): Boolean {
-    // Allow if we don't have any requirements on service or capability
-    if (allowedServices.isEmpty() && allowedCapabilities.isEmpty()) return true
-
-    // Allow if the caller has provided an allowed service
-    if (caller.service != null && allowedServices.contains(caller.service)) return true
-
-    // Allow if the caller has provided an allowed capability
-    return caller.capabilities.any { allowedCapabilities.contains(it) }
-  }
-
   internal class Factory @Inject internal constructor(
     private val caller: @JvmSuppressWildcards ActionScoped<MiskCaller?>,
     private val registeredEntries: List<AccessAnnotationEntry>
diff --git a/misk/src/main/kotlin/misk/web/WebTab.kt b/misk/src/main/kotlin/misk/web/WebTab.kt
index 8bb5c94c10a..3e138aad8e7 100644
--- a/misk/src/main/kotlin/misk/web/WebTab.kt
+++ b/misk/src/main/kotlin/misk/web/WebTab.kt
@@ -1,7 +1,5 @@
 package misk.web
 
-import misk.MiskCaller
-
 abstract class WebTab(
   slug: String,
   url_path_prefix: String,
@@ -9,20 +7,4 @@ abstract class WebTab(
   // it does not deal with any other permissions such as static resource access or otherwise
   val capabilities: Set<String> = setOf(),
   val services: Set<String> = setOf()
-) : ValidWebEntry(slug = slug, url_path_prefix = url_path_prefix) {
-  fun isAuthenticated(caller: MiskCaller?): Boolean = when {
-    // no capabilities/service requirement => unauthenticated and null caller requests allowed
-    capabilities.isEmpty() && services.isEmpty() -> true
-
-    // capability/service requirement present but caller null => assume authentication broken
-    caller == null -> false
-
-    // matching capability
-    capabilities.any { caller.capabilities.contains(it) } -> true
-
-    // matching service
-    services.any { caller.service == it } -> true
-
-    else -> false
-  }
-}
+) : ValidWebEntry(slug = slug, url_path_prefix = url_path_prefix)
\ No newline at end of file
diff --git a/misk/src/main/kotlin/misk/web/actions/AdminDashboardTabAction.kt b/misk/src/main/kotlin/misk/web/actions/AdminDashboardTabAction.kt
index fbfbd0f299d..74f1e2cf364 100644
--- a/misk/src/main/kotlin/misk/web/actions/AdminDashboardTabAction.kt
+++ b/misk/src/main/kotlin/misk/web/actions/AdminDashboardTabAction.kt
@@ -32,12 +32,13 @@ class AdminDashboardTabAction @Inject constructor() : WebAction {
   @ResponseContentType(MediaTypes.APPLICATION_JSON)
   @Unauthenticated
   fun getAll(): Response {
-    val caller = callerProvider.get()
-    val authorizedAdminDashboardTabs = adminDashboardTabs.filter { it.isAuthenticated(caller) }
+    val caller = callerProvider.get() ?: return Response()
+    val authorizedAdminDashboardTabs =
+      adminDashboardTabs.filter { caller.isAllowed(it.capabilities, it.services) }
     return Response(adminDashboardTabs = authorizedAdminDashboardTabs)
   }
 
-  data class Response(val adminDashboardTabs: List<DashboardTab>)
+  data class Response(val adminDashboardTabs: List<DashboardTab> = listOf())
 }
 
 @Qualifier