bugfix for #552

OpenConext · Jan 31, 2025 · fed9255 · fed9255
1 parent fc49de2
commit fed9255
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 13 deletions.
diff --git a/TIQR.md b/TIQR.md
@@ -0,0 +1,24 @@
+```mermaid
+sequenceDiagram
+    actor User
+    participant Tiqr App
+    participant eduID
+    User->>Tiqr App: Start registration
+    Tiqr App->>eduID: Start enrollment
+    eduID->>Tiqr App: Enrollment data
+    Note right of Tiqr App: EnrollmentKey, metaData URL and qrcode
+    Tiqr App->>eduID: MetaData enrollmentKey
+    eduID->>Tiqr App: MetaData
+    Note right of Tiqr App: Service and Identity (=registrationID)
+    Tiqr App->>eduID: Start authentication
+    eduID->>Tiqr App: Session key and url
+    Note right of Tiqr App: Authentication URL with u=registrationID
+    Tiqr App->>eduID: Finish authentication
+    Note right of Tiqr App: AuthenticationData with userId=registrationID
+    eduID->>eduID: Fetch User with AuthenticationData-userId
+    Note left of eduID: UserNotFoundException
+    eduID->>eduID: Fetch Registration with AuthenticationData-userId
+    eduID->>eduID: Fetch User with Registration-userId
+    eduID->>Tiqr App: OK
+    Tiqr App->>User: 🙏🏻
+```
diff --git a/account-gui/pom.xml b/account-gui/pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.5</version>
+        <version>7.4.6</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>account-gui</artifactId>

diff --git a/myconext-gui/pom.xml b/myconext-gui/pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.5</version>
+        <version>7.4.6</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>myconext-gui</artifactId>

diff --git a/myconext-server/pom.xml b/myconext-server/pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.5</version>
+        <version>7.4.6</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>myconext-server</artifactId>

diff --git a/myconext-server/src/main/java/myconext/tiqr/TiqrController.java b/myconext-server/src/main/java/myconext/tiqr/TiqrController.java
@@ -43,6 +43,7 @@
 import java.time.Instant;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
 
 import static myconext.crypto.HashGenerator.hash;
 import static myconext.log.MDCContext.logWithContext;
@@ -528,22 +529,34 @@ public ResponseEntity<Object> doEnrollment(@ModelAttribute Registration registra
     @PostMapping(value = "/authentication", consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE)
     @Hidden
     public ResponseEntity<Object> doAuthentication(@ModelAttribute AuthenticationData authenticationData) {
-        String userId = authenticationData.getUserId();
-        User user = userRepository.findById(userId).orElseThrow(() -> new UserNotFoundException(userId));
+        String metaDataIdentity = authenticationData.getUserId();
+        /*
+         * This used to be the userID, but in https://github.com/OpenConext/OpenConext-myconext/issues/552 this has
+         * changed to the registrationID. We need to try them both to be backwards compatible
+         */
+        Optional<Registration> optionalRegistration = registrationRepository.findById(metaDataIdentity);
+        Optional<User> optionalUser = optionalRegistration
+                .map(registration -> userRepository.findById(registration.getUserId()))
+                .flatMap(Function.identity());
+        User user = optionalUser
+                .orElseGet(() -> userRepository.findById(metaDataIdentity)
+                        .orElseThrow(() -> new UserNotFoundException("User not found with authenticationData#userId:" + metaDataIdentity)));
+
         if (!rateLimitEnforcer.isUserAllowedTiqrVerification(user)) {
             return ResponseEntity.ok("ERROR");
         }
         try {
             tiqrService.postAuthentication(authenticationData);
 
-            LOG.debug("Successful authentication for user " + userId);
+            LOG.debug(String.format("Successful authentication for user %s, %s" ,user.getEmail(), user.getId()));
 
             rateLimitEnforcer.unsuspendUserAfterTiqrSuccess(user);
             return ResponseEntity.ok("OK");
         } catch (TiqrException | RuntimeException e) {
             //Do not show stacktrace
-            LOG.error(String.format("Exception during authentication for user %s, message %s",
-                    userId,
+            LOG.error(String.format("Exception during authentication for user %s, %s message %s",
+                    user.getEmail(),
+                    user.getId(),
                     e.getMessage()));
             rateLimitEnforcer.suspendUserAfterTiqrFailure(user);
             try {

diff --git a/myconext-server/src/test/java/myconext/tiqr/TiqrControllerTest.java b/myconext-server/src/test/java/myconext/tiqr/TiqrControllerTest.java
@@ -309,7 +309,16 @@ public void fetchRegistration() throws IOException {
     }
 
     @Test
-    public void startAuthentication() throws Exception {
+    public void startAuthenticationWithRegistrationID() throws Exception {
+        doStartAuthentication(true);
+    }
+
+    @Test
+    public void startAuthenticationBackwardCompatibleWithUserID() throws Exception {
+        doStartAuthentication(false);
+    }
+
+    private void doStartAuthentication(boolean useRegistrationId) throws Exception {
         SamlAuthenticationRequest samlAuthenticationRequest = doEnrollmment(true);
 
         Map<String, Object> results = given()
@@ -338,10 +347,11 @@ public void startAuthentication() throws Exception {
         String decryptedSecret = this.decryptRegistrationSecret(registration.getSecret());
         String ocra = OCRA.generateOCRA(decryptedSecret, authentication.getChallenge(), sessionKey);
 
+        String userId = useRegistrationId ? registration.getId() : samlAuthenticationRequest.getUserId();
         given()
                 .contentType(ContentType.URLENC)
                 .formParam("sessionKey", sessionKey)
-                .formParam("userId", samlAuthenticationRequest.getUserId())
+                .formParam("userId", userId)
                 .formParam("response", ocra)
                 .formParam("language", "en")
                 .formParam("operation", "login")

diff --git a/pom.xml b/pom.xml
@@ -3,7 +3,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>org.openconext</groupId>
     <artifactId>myconext</artifactId>
-    <version>7.4.5</version>
+    <version>7.4.6</version>
     <packaging>pom</packaging>
     <name>myconext</name>
     <description>My OpenConext</description>

diff --git a/public-gui/pom.xml b/public-gui/pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.5</version>
+        <version>7.4.6</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>public-gui</artifactId>

diff --git a/tiqr-mock/pom.xml b/tiqr-mock/pom.xml
@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.openconext</groupId>
         <artifactId>myconext</artifactId>
-        <version>7.4.5</version>
+        <version>7.4.6</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
     <artifactId>tiqr-mock</artifactId>