NFC-47 Codereview findings. Readme update, tests and filter.

Author: SanderKondratjevNortal

README.md

@@ -48,7 +48,7 @@ Implement the session-backed challenge nonce store as follows:
 import org.springframework.beans.factory.ObjectFactory;
 import eu.webeid.security.challenge.ChallengeNonce;
 import eu.webeid.security.challenge.ChallengeNonceStore;
-import javax.servlet.http.HttpSession;
+import jakarta.servlet.http.HttpSession;
 
 public class SessionBackedChallengeNonceStore implements ChallengeNonceStore {
 
@@ -134,36 +134,102 @@ import eu.webeid.security.validator.AuthTokenValidatorBuilder;
 ...
 ```
 
-## 6. Add a REST endpoint for issuing challenge nonces
+## 6. Add a filter for issuing challenge nonces
 
-A REST endpoint that issues challenge nonces is required for authentication. The endpoint must support `GET` requests.
+A REST endpoint that issues challenge nonces is required for authentication.  
+Since this step is part of the authentication flow, it is implemented as a Spring Security filter instead of a regular controller. The filter must support `POST` requests.
 
-In the following example, we are using the [Spring RESTful Web Services framework](https://spring.io/guides/gs/rest-service/) to implement the endpoint, see also the full implementation [here](example/blob/main/src/main/java/eu/webeid/example/web/rest/ChallengeController.java).
+The `WebEidChallengeNonceFilter` handles `/auth/challenge` requests and issues a new nonce.  
+See the full implementation [here](example/src/main/java/eu/webeid/example/security/WebEidChallengeNonceFilter.java).
 
 ```java
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.RestController;
-import eu.webeid.security.challenge.ChallengeNonceGenerator;
-...
+public final class WebEidChallengeNonceFilter extends OncePerRequestFilter {
+    private static final ObjectWriter OBJECT_WRITER = new ObjectMapper().writer();
+    private final RequestMatcher requestMatcher;
+    private final ChallengeNonceGenerator nonceGenerator;
+
+    public WebEidChallengeNonceFilter(String path, ChallengeNonceGenerator nonceGenerator) {
+        this.requestMatcher = PathPatternRequestMatcher.withDefaults().matcher(HttpMethod.POST, path);
+        this.nonceGenerator = nonceGenerator;
+    }
+
+    @Override
+    protected void doFilterInternal(
+        @NonNull HttpServletRequest request,
+        @NonNull HttpServletResponse response,
+        @NonNull FilterChain chain
+    ) throws ServletException, IOException {
+        if (!requestMatcher.matches(request)) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        var dto = new ChallengeDTO(nonceGenerator.generateAndStoreNonce().getBase64EncodedNonce());
+
+        response.setContentType(MediaType.APPLICATION_JSON_VALUE);
+        OBJECT_WRITER.writeValue(response.getWriter(), dto);
+    }
+
+    public record ChallengeDTO(String nonce) {}
+}
+```
 
-@RestController
-@RequestMapping("auth")
-public class ChallengeController {
+Similarly, the `WebEidMobileAuthInitFilter` handles `/auth/mobile/init` requests and issues a deep link for mobile authentication.  
+See the full implementation [here](example/src/main/java/eu/webeid/example/security/WebEidMobileAuthInitFilter.java).
 
-    @Autowired // for brevity, prefer constructor dependency injection
-    private ChallengeNonceGenerator nonceGenerator;
+```java
+public final class WebEidMobileAuthInitFilter extends OncePerRequestFilter {
+    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+    private final RequestMatcher requestMatcher;
+    private final ChallengeNonceGenerator nonceGenerator;
+    private final String loginPath;
+
+    public WebEidMobileAuthInitFilter(String path, String loginPath, ChallengeNonceGenerator nonceGenerator) {
+        this.requestMatcher = PathPatternRequestMatcher.withDefaults().matcher(HttpMethod.POST, path);
+        this.nonceGenerator = nonceGenerator;
+        this.loginPath = loginPath;
+    }
 
-    @GetMapping("challenge")
-    public ChallengeDTO challenge() {
-        // a simple DTO with a single 'nonce' field
-        final ChallengeDTO challenge = new ChallengeDTO();
-        challenge.setNonce(nonceGenerator.generateAndStoreNonce().getBase64EncodedNonce());
-        return challenge;
+    @Override
+    protected void doFilterInternal(
+        @NonNull HttpServletRequest request,
+        @NonNull HttpServletResponse response,
+        @NonNull FilterChain chain
+    ) throws IOException, ServletException {
+        if (!requestMatcher.matches(request)) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        var challenge = nonceGenerator.generateAndStoreNonce();
+
+        String loginUri = ServletUriComponentsBuilder.fromCurrentContextPath()
+            .path(loginPath).build().toUriString();
+
+        String payloadJson = OBJECT_MAPPER.writeValueAsString(
+            new AuthPayload(challenge.getBase64EncodedNonce(), loginUri)
+        );
+        String encoded = Base64.getEncoder().encodeToString(payloadJson.getBytes(StandardCharsets.UTF_8));
+        String eidAuthUri = "web-eid-mobile://auth#" + encoded;
+
+        response.setContentType(MediaType.APPLICATION_JSON_VALUE);
+        OBJECT_MAPPER.writeValue(response.getWriter(), new AuthUri(eidAuthUri));
     }
+
+    record AuthPayload(String challenge, @JsonProperty("login_uri") String loginUri) {}
+    record AuthUri(@JsonProperty("auth_uri") String authUri) {}
 }
 ```
 
+Both filters are registered in the Spring Security filter chain in ApplicationConfiguration:
+```java
+http
+  .addFilterBefore(new WebEidMobileAuthInitFilter("/auth/mobile/init", "/auth/mobile/login", challengeNonceGenerator),
+      UsernamePasswordAuthenticationFilter.class)
+  .addFilterBefore(new WebEidChallengeNonceFilter("/auth/challenge", challengeNonceGenerator),
+      UsernamePasswordAuthenticationFilter.class);
+```
+
 Also, see general guidelines for implementing secure authentication services [here](https://github.com/SK-EID/smart-id-documentation/wiki/Secure-Implementation-Guide).
 
 ## 7. Implement authentication

example/README.md

@@ -114,12 +114,13 @@ The source code folder `src` contains the application source code and resources
 The `src/main/java/eu/webeid/example` directory contains the Spring Boot application Java class and the following subdirectories:
 
 -   `config`: Spring and HTTP security configuration, Web eID authentication token validation library configuration, trusted CA certificates loading etc,
--   `security`: Web eID authentication token validation library integration with Spring Security via an `AuthenticationProvider` and `AuthenticationProcessingFilter`,
+-   `security`: Web eID authentication token validation library integration with Spring Security
+    -   `AuthenticationProvider` and `AuthenticationProcessingFilter` for handling Web eID authentication tokens,
+    -   `WebEidChallengeNonceFilter` for issuing the challenge nonce required by the authentication flow,
+    -   `WebEidMobileAuthInitFilter` for generating the deep link (`auth_uri`) used in mobile login,
+    -   `WebEidAjaxLoginProcessingFilter` and `WebEidLoginPageGeneratingFilter` for handling login requests.
 -   `service`: Web eID signing service implementation that uses DigiDoc4j, and DigiDoc4j runtime configuration,
--   `web`: Spring Web MVC controller for the welcome page and Spring Web REST controllers that provide endpoints
-    -   for getting the challenge nonce used by the authentication token validation library,
-    -   for digital signing.
-    -   for initializing and handling the mobile login and digital signing flow
+-   `web`: Spring Web MVC controller for the welcome page and Spring Web REST controller that provides a digital signing endpoint.
 
 The `src/resources` directory contains the resources used by the application:
 

example/src/main/java/eu/webeid/example/security/ajax/AjaxAuthenticationSuccessHandler.java

@@ -50,15 +50,15 @@ public class AjaxAuthenticationSuccessHandler extends SimpleUrlAuthenticationSuc
 
     @Override
     public void onAuthenticationSuccess(
-            HttpServletRequest request,
-            HttpServletResponse response,
-            Authentication authentication
+        HttpServletRequest request,
+        HttpServletResponse response,
+        Authentication authentication
     )
-            throws IOException {
+        throws IOException {
         LOG.info("onAuthenticationSuccess(): {}", authentication);
 
         response.setStatus(HttpServletResponse.SC_OK);
-        response.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE + ";charset=UTF-8");
+        response.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE);
 
         response.getWriter().write(AuthSuccessDTO.asJson(authentication));
     }

example/src/main/java/eu/webeid/example/security/ui/WebEidLoginPageGeneratingFilter.java

@@ -71,7 +71,7 @@ public WebEidLoginPageGeneratingFilter(String path) {
 
               if (payload["error"]) {
                 showErrorMessage({
-                  code: "MOPP_ERROR",
+                  code: payload["code"] ?? "MOPP_ERROR",
                   message: payload["message"] ?? "Authentication failed in mobile app"
                 });
                 return;
@@ -118,7 +118,7 @@ protected void doFilterInternal(@NonNull HttpServletRequest request, @NonNull Ht
 
         String html = generateHtml(csrf);
 
-        response.setContentType(MediaType.TEXT_HTML_VALUE + ";charset=UTF-8");
+        response.setContentType(MediaType.TEXT_HTML_VALUE);
         response.getWriter().write(html);
     }
 

example/src/test/java/eu/webeid/example/WebApplicationTest.java

@@ -22,6 +22,7 @@
 
 package eu.webeid.example;
 
+import eu.webeid.example.security.dto.AuthTokenDTO;
 import eu.webeid.example.testutil.Dates;
 import eu.webeid.example.testutil.HttpHelper;
 import eu.webeid.example.testutil.ObjectMother;
@@ -31,6 +32,9 @@
 import org.digidoc4j.impl.asic.xades.XadesSignature;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.http.HttpStatus;
@@ -45,12 +49,14 @@
 import eu.webeid.security.challenge.ChallengeNonce;
 import eu.webeid.security.util.DateAndTime;
 
+import java.util.stream.Stream;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 
 @SpringBootTest
 @WebAppConfiguration
-public class WebApplicationTest {
+class WebApplicationTest {
 
     @Autowired
     private WebApplicationContext context;
@@ -61,12 +67,12 @@ public class WebApplicationTest {
     private static DefaultMockMvcBuilder mvcBuilder;
 
     @BeforeEach
-    public void setup() {
+    void setup() {
         mvcBuilder = MockMvcBuilders.webAppContextSetup(context).addFilters(springSecurityFilterChain);
     }
 
     @Test
-    public void testRoot() throws Exception {
+    void testRoot() throws Exception {
         // @formatter:off
         MockHttpServletResponse response = mvcBuilder
             .build()
@@ -78,8 +84,9 @@ public void testRoot() throws Exception {
         System.out.println(response.getContentAsString());
     }
 
-    @Test
-    public void testHappyFlow_LoginPrepareSignDownload() throws Exception {
+    @ParameterizedTest
+    @MethodSource("provideAuthToken")
+    void testHappyFlow_LoginPrepareSignDownload(AuthTokenDTO authToken) throws Exception {
         new MockUp<AsicSignatureFinalizer>() {
             @Mock
             public void validateOcspResponse(XadesSignature xadesSignature) {
@@ -95,7 +102,7 @@ public void validateOcspResponse(XadesSignature xadesSignature) {
         // Act and assert
         mvcBuilder.build().perform(get("/auth/challenge"));
 
-        MvcResult result = HttpHelper.login(mvcBuilder, session, ObjectMother.mockAuthToken());
+        MvcResult result = HttpHelper.login(mvcBuilder, session, authToken);
         session = (MockHttpSession) result.getRequest().getSession();
         MockHttpServletResponse response = result.getResponse();
         assertEquals("{\"sub\":\"JAAK-KRISTJAN JÕEORG\",\"auth\":\"[ROLE_USER]\"}", response.getContentAsString());
@@ -121,21 +128,10 @@ public static MockMultipartFile mockMultipartFile() {
         assertEquals("attachment; filename=example-for-signing.asice", response.getHeader("Content-Disposition"));
     }
 
-    @Test
-    public void testHappyFlow_V11LoginAuthToken() throws Exception {
-        new MockUp<AsicSignatureFinalizer>() {
-            @Mock
-            public void validateOcspResponse(XadesSignature xadesSignature) {
-                // Do not call real OCSP service in tests.
-            }
-        };
-
-        MockHttpSession session = new MockHttpSession();
-        session.setAttribute("challenge-nonce", new ChallengeNonce(ObjectMother.VALID_CHALLENGE_NONCE, DateAndTime.utcNow().plusMinutes(1)));
-
-        MvcResult result = HttpHelper.login(mvcBuilder, session, ObjectMother.mockV11AuthToken());
-        MockHttpServletResponse response = result.getResponse();
-        assertEquals("{\"sub\":\"JAAK-KRISTJAN JÕEORG\",\"auth\":\"[ROLE_USER]\"}", response.getContentAsString());
+    static Stream<Arguments> provideAuthToken() {
+        return Stream.of(
+            Arguments.of(ObjectMother.mockAuthToken()),
+            Arguments.of(ObjectMother.mockV11AuthToken())
+        );
     }
-
 }

example/src/test/java/eu/webeid/example/security/WebEidChallengeNonceFilterTest.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2020-2025 Estonian Information System Authority
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package eu.webeid.example.security;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.http.MediaType;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.MvcResult;
+
+import java.util.Base64;
+
+import static eu.webeid.security.challenge.ChallengeNonceGenerator.NONCE_LENGTH;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+@SpringBootTest
+@AutoConfigureMockMvc
+class WebEidChallengeNonceFilterTest {
+
+    @Autowired
+    private ObjectMapper mapper;
+
+    @Autowired
+    private MockMvc mvc;
+
+    @Test
+    void challengeReturnsNonceWithExpectedBase64Length() throws Exception {
+        MvcResult result = mvc.perform(post("/auth/challenge")
+                .with(csrf())
+                .accept(MediaType.APPLICATION_JSON))
+            .andExpect(status().isOk())
+            .andExpect(content().contentTypeCompatibleWith(MediaType.APPLICATION_JSON))
+            .andReturn();
+
+        JsonNode json = mapper.readTree(result.getResponse().getContentAsByteArray());
+        String nonce = json.get("nonce").asText();
+
+        assertThat(nonce)
+            .isNotBlank()
+            .hasSize(expectedBase64Len());
+    }
+
+    private static int expectedBase64Len() {
+        return Base64.getEncoder().encodeToString(new byte[NONCE_LENGTH]).length();
+    }
+}