Cleanup Authorisation Filter to only call doFilter in a single place (#769)

* Add test for current covered case of Limited access.

* Catch other case of limited access requests being made.

* Move different authorisation methods to own functions.

Author: ben-madley-mt

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/AuthorisationFilter.kt

@@ -43,20 +43,30 @@ class AuthorisationFilter(
     val authoriseConsumerService = AuthoriseConsumerService()
     val requestedPath = req.requestURI
 
-    val includesResult = authoriseConsumerService.doesConsumerHaveIncludesAccess(authorisationConfig.consumers[subjectDistinguishedName], requestedPath)
-    if (includesResult) {
+    if (authorisedThroughIncludes(authoriseConsumerService, subjectDistinguishedName, requestedPath) ||
+      authorisedThroughRole(authoriseConsumerService, subjectDistinguishedName, requestedPath)
+    ) {
       try {
         chain.doFilter(request, response)
       } catch (e: Throwable) {
-        if (e.cause is LimitedAccessException) {
-          res.sendError(HttpServletResponse.SC_FORBIDDEN, e.message)
+        val cause = e.cause
+        if (cause is LimitedAccessException) {
+          res.sendError(HttpServletResponse.SC_FORBIDDEN, cause.message)
         } else {
           throw e
         }
       }
       return
+    } else {
+      res.sendError(HttpServletResponse.SC_FORBIDDEN, "Unable to authorise $requestedPath for $subjectDistinguishedName")
     }
+  }
 
+  private fun authorisedThroughRole(
+    authoriseConsumerService: AuthoriseConsumerService,
+    subjectDistinguishedName: String?,
+    requestedPath: String,
+  ): Boolean {
     val consumerConfig: ConsumerConfig? = authorisationConfig.consumers[subjectDistinguishedName]
     val consumersRoles = consumerConfig?.roles
     val rolesInclude =
@@ -67,11 +77,12 @@ class AuthorisationFilter(
       }
     val roleResult =
       authoriseConsumerService.doesConsumerHaveRoleAccess(rolesInclude, requestedPath)
-    if (!roleResult) {
-      res.sendError(HttpServletResponse.SC_FORBIDDEN, "Unable to authorise $requestedPath for $subjectDistinguishedName")
-      return
-    }
-
-    chain.doFilter(request, response)
+    return roleResult
   }
+
+  private fun authorisedThroughIncludes(
+    authoriseConsumerService: AuthoriseConsumerService,
+    subjectDistinguishedName: String?,
+    requestedPath: String,
+  ) = authoriseConsumerService.doesConsumerHaveIncludesAccess(authorisationConfig.consumers[subjectDistinguishedName], requestedPath)
 }

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/extensions/AuthorisationFilterTest.kt

@@ -1,6 +1,7 @@
 package uk.gov.justice.digital.hmpps.hmppsintegrationapi.extensions
 
 import jakarta.servlet.FilterChain
+import jakarta.servlet.ServletException
 import jakarta.servlet.http.HttpServletRequest
 import jakarta.servlet.http.HttpServletResponse
 import org.junit.jupiter.api.BeforeEach
@@ -12,6 +13,7 @@ import org.mockito.kotlin.reset
 import org.mockito.kotlin.whenever
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.config.AuthorisationConfig
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.config.GlobalsConfig
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.exception.LimitedAccessException
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerConfig
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.Role
@@ -91,4 +93,29 @@ class AuthorisationFilterTest {
 
     verify(mockResponse, times(1)).sendError(403, "No subject-distinguished-name header provided for authorisation")
   }
+
+  @Test
+  fun `Forbidden if limited access caused by error for path not found in roles, but found in includes`() {
+    val authorisationConfig = AuthorisationConfig()
+    authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = listOf(examplePath), filters = ConsumerFilters(prisons = null), roles = listOf()))
+    val invalidRoleConfig = GlobalsConfig(roles = mapOf(roleName to Role(include = emptyList())))
+    val authorisationFilter = AuthorisationFilter(authorisationConfig, invalidRoleConfig)
+    whenever(mockChain.doFilter(mockRequest, mockResponse)).thenThrow(ServletException(LimitedAccessException()))
+
+    authorisationFilter.doFilter(mockRequest, mockResponse, mockChain)
+
+    verify(mockResponse, times(1)).sendError(403, "Attempt to access a limited access case")
+  }
+
+  @Test
+  fun `Forbidden if limited access caused by error for path found in roles (but not in includes)`() {
+    val authorisationConfig = AuthorisationConfig()
+    authorisationConfig.consumers = mapOf(exampleConsumer to ConsumerConfig(include = emptyList(), filters = ConsumerFilters(prisons = null), roles = exampleRoles))
+    val authorisationFilter = AuthorisationFilter(authorisationConfig, exampleGlobalsConfig)
+    whenever(mockChain.doFilter(mockRequest, mockResponse)).thenThrow(ServletException(LimitedAccessException()))
+
+    authorisationFilter.doFilter(mockRequest, mockResponse, mockChain)
+
+    verify(mockResponse, times(1)).sendError(403, "Attempt to access a limited access case")
+  }
 }