Merge pull request #139 from companieshouse/feature/gci-2310_change_get_item_authorisation

GCI-2310 Change authenticaton/authorisaton for get order item

Author: cgregoryCH

src/main/java/uk/gov/companieshouse/orders/api/interceptor/UserAuthenticationInterceptor.java

@@ -56,8 +56,9 @@ private boolean checkAuthenticated(HttpServletRequest request, HttpServletRespon
                 return hasSignedInUser(request, response);
             case GET_PAYMENT_DETAILS:
             case GET_ORDER:
-            case GET_ORDER_ITEM:
                 return hasAuthenticatedClient(request, response);
+            case GET_ORDER_ITEM:
+                return securityManager.checkIdentity() || hasAuthenticatedClient(request, response);
             case GET_CHECKOUT:
             case SEARCH:
                 return securityManager.checkIdentity();

src/main/java/uk/gov/companieshouse/orders/api/interceptor/UserAuthorisationInterceptor.java

@@ -87,8 +87,9 @@ private boolean checkAuthorised(HttpServletRequest request, HttpServletResponse
             case GET_CHECKOUT:
                 return securityManager.checkPermission() || getRequestClientIsAuthorised(request, response, this::getCheckoutUserIsResourceOwner);
             case GET_ORDER:
-            case GET_ORDER_ITEM:
                 return getRequestClientIsAuthorised(request, response, this::getOrderUserIsResourceOwner);
+            case GET_ORDER_ITEM:
+                return securityManager.checkPermission() || getRequestClientIsAuthorised(request, response, this::getOrderUserIsResourceOwner);
             case SEARCH:
                 return securityManager.checkPermission();
             case PATCH_PAYMENT_DETAILS:

src/test/java/uk/gov/companieshouse/orders/api/interceptor/UserAuthenticationInterceptorTests.java

@@ -258,26 +258,15 @@ void preHandleRejectsBlankIdentityValue() {
         thenRequestIsRejected();
     }
 
-    @DisplayName("Authentication for orders/search endpoint succeeds if caller identity is valid")
-    @Test
-    void ordersSearchValidIdentity() {
-        when(securityManager.checkIdentity()).thenReturn(true);
-        givenRequest(GET, "/checkouts/search");
-
-        boolean actual = interceptorUnderTest.preHandle(request, response, handler);
-
-        assertThat(actual, is(true));
-    }
-
-    @DisplayName("Authentication for orders/search endpoint false if caller identity is invalid")
-    @Test
-    void ordersSearchInvalidIdentity() {
-        when(securityManager.checkIdentity()).thenReturn(false);
-        givenRequest(GET, "/checkouts/search");
+    @ParameterizedTest(name = "{index}: {0}")
+    @MethodSource("hasAdminAuthenticationFixtures")
+    void ordersSearchValidIdentity(String displayName, String endpoint, boolean isValid) {
+        when(securityManager.checkIdentity()).thenReturn(isValid);
+        givenRequest(GET, endpoint);
 
         boolean actual = interceptorUnderTest.preHandle(request, response, handler);
 
-        assertThat(actual, is(false));
+        assertThat(actual, is(isValid));
     }
 
     @Test
@@ -428,4 +417,11 @@ private static Stream<Arguments> signedInGetRequestFixtures() {
                 arguments("preHandle accepts get order item request that has signed in user headers", "/orders/1234/items/5678"),
                 arguments("preHandle accepts get basket links request from a user with OAuth2 authentication", "/basket/links"));
     }
+
+    private static Stream<Arguments> hasAdminAuthenticationFixtures() {
+        return Stream.of(arguments("Authentication for orders/search endpoint succeeds if caller identity is valid", "/checkouts/search", true),
+                arguments("Authentication for orders/search endpoint fails if caller identity is invalid", "/checkouts/search", false),
+                arguments("Authentication for get order item endpoint succeeds if caller identity is valid", "/orders/1234/items/5678", true),
+                arguments("Authentication for get order item endpoint fails if caller identity is invalid", "/orders/1234/items/5678", false));
+    }
 }

src/test/java/uk/gov/companieshouse/orders/api/interceptor/UserAuthorisationInterceptorTests.java

@@ -245,6 +245,37 @@ void ordersSearchInvalidAuthorisation() {
         assertThat(actual, is(false));
     }
 
+    @Test
+    @DisplayName("Authorisation for get order item endpoint succeeds if caller has admin permissions")
+    void getOrderItemValidAuthorisation() {
+        // given
+        givenRequest(GET, "/orders/1234/items/5678");
+        givenRequestHasSignedInUser(ERIC_IDENTITY_VALUE);
+        givenPathVariable(ORDER_ID_PATH_VARIABLE, "1");
+        when(orderRepository.findById("1")).thenReturn(Optional.of(order));
+        when(securityManager.checkPermission()).thenReturn(true);
+
+        // when
+        boolean actual = interceptorUnderTest.preHandle(request, response, handler);
+
+        // then
+        assertThat(actual, is(true));
+    }
+
+    @DisplayName("Authorisation for get order item endpoint false if caller has admin permissions")
+    @Test
+    void getOrderItemSearchInvalidAuthorisation() {
+        givenRequest(GET, "/orders/1234/items/5678");
+        givenRequestHasSignedInUser(ERIC_IDENTITY_VALUE);
+        givenPathVariable(ORDER_ID_PATH_VARIABLE, "1");
+        when(orderRepository.findById("1")).thenReturn(Optional.of(order));
+        when(securityManager.checkPermission()).thenReturn(false);
+
+        boolean actual = interceptorUnderTest.preHandle(request, response, handler);
+
+        assertThat(actual, is(false));
+    }
+
     @ParameterizedTest(name = "{index}: {0}")
     @MethodSource("apiGetRequestFixtures")
     void preHandleAcceptsAuthorisedInternalApiGetRequest(final String displayName, final String uri) {