fix(ENGKNOW-3998): only look up email usernames in CSA

auth/src/main/java/org/gorpipe/gor/auth/utils/CsaApiUtils.java

@@ -1,6 +1,6 @@
 package org.gorpipe.gor.auth.utils;
 
-import com.google.common.base.Strings;
+import org.gorpipe.util.Strings;
 import org.gorpipe.gor.auth.GeneralAuthInfo;
 import org.gorpipe.gor.auth.GorAuthInfo;
 import org.gorpipe.security.cred.CsaApiService;
@@ -37,11 +37,12 @@ public static GorAuthInfo updateWithCsaApi(CsaApiService csaApiService, GorAuthI
             organizationId = updateOrganizationId(projectId, projectMap);
         }
 
-        if (Strings.isNullOrEmpty(userId) && !Strings.isNullOrEmpty(userName)) {
+        // CSA looks users up by email, so skip non-email usernames (e.g. service accounts) that it can't know.
+        if (Strings.isNullOrEmpty(userId) && Strings.isEmail(userName)) {
             Map<String, Object> userMap = getUserMapByEmail(csaApiService, userName);
             userId = updateUserId(userId, userMap);
 
-            if (userRoles.isEmpty() && !Strings.isNullOrEmpty(project) && !Strings.isNullOrEmpty(userName)) {
+            if (userRoles.isEmpty() && !Strings.isNullOrEmpty(project)) {
                 List csaUserRoles = getUserRoleList(csaApiService, project, userName);
                 updateUserRoles(userRoles, csaUserRoles);
             }

auth/src/test/java/org/gorpipe/gor/auth/UTestPlatformAuthUsername.java

@@ -29,6 +29,9 @@
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 /**
  * Username resolution for platform JWTs. Service-account tokens (Keycloak client_credentials) carry no email claim,
@@ -139,8 +142,6 @@ public void jwtAuthHandlesMissingRealmAccess() throws Exception {
     @Test
     public void jwtAuthCachesServiceAccountsSeparately() throws Exception {
         CsaApiService csaApiService = mock(CsaApiService.class);
-        doReturn(Collections.singletonMap("id", 11)).when(csaApiService).getUserByEmail("service-account-a");
-        doReturn(Collections.singletonMap("id", 22)).when(csaApiService).getUserByEmail("service-account-b");
         doReturn(null).when(csaApiService).getProject(anyString());
         doReturn("CSA").when(config).updateAuthInfoPolicy();
 
@@ -149,9 +150,10 @@ public void jwtAuthCachesServiceAccountsSeparately() throws Exception {
         GorAuthInfo b = auth.getGorAuthInfo(PROJECT, parse(token().withClaim("preferred_username", "service-account-b")));
 
         Assert.assertEquals("service-account-a", a.getUsername());
-        Assert.assertEquals("11", a.getUserId());
         Assert.assertEquals("service-account-b", b.getUsername());
-        Assert.assertEquals("22", b.getUserId());
+        // One cache miss (and CSA project lookup) per service account; no CSA user lookup for non-email usernames.
+        verify(csaApiService, times(2)).getProject(PROJECT);
+        verify(csaApiService, never()).getUserByEmail(anyString());
     }
 
     // --- PlatformAuth ---

auth/src/test/java/org/gorpipe/gor/auth/utils/UTestCsaApiUtils.java

@@ -0,0 +1,67 @@
+package org.gorpipe.gor.auth.utils;
+
+import org.gorpipe.gor.auth.GeneralAuthInfo;
+import org.gorpipe.gor.auth.GorAuthInfo;
+import org.gorpipe.security.cred.CsaApiService;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.*;
+
+/**
+ * CSA looks users up by email. Usernames that aren't emails (e.g. service accounts such as sequenceminer) must not be
+ * looked up, as CSA answers 404 for them and every request logged a warning.
+ */
+public class UTestCsaApiUtils {
+
+    private static final String PROJECT = "test-proj";
+
+    private CsaApiService csaApiService;
+
+    @Before
+    public void setUp() throws IOException {
+        csaApiService = mock(CsaApiService.class);
+        Map<String, Object> projectMap = new LinkedHashMap<>();
+        projectMap.put("id", 5);
+        projectMap.put("organization_id", 7);
+        doReturn(projectMap).when(csaApiService).getProject(PROJECT);
+    }
+
+    @Test
+    public void nonEmailUserIsNotLookedUpInCsa() throws IOException {
+        GorAuthInfo info = CsaApiUtils.updateWithCsaApi(csaApiService, authInfo("sequenceminer"));
+
+        verify(csaApiService, never()).getUserByEmail(anyString());
+        verify(csaApiService, never()).getUserRoleList(anyString(), anyString());
+        Assert.assertEquals(5, info.getProjectId());
+        Assert.assertEquals("sequenceminer", info.getUsername());
+        Assert.assertEquals("", info.getUserId());
+        Assert.assertTrue(info.getUserRoles().isEmpty());
+    }
+
+    @Test
+    public void emailUserGetsIdAndRolesFromCsa() throws IOException {
+        String user = "user@email.com";
+        doReturn(Collections.singletonMap("id", 10)).when(csaApiService).getUserByEmail(user);
+        Map<String, Object> role = new LinkedHashMap<>();
+        role.put("role", "researcher");
+        doReturn(List.of(role)).when(csaApiService).getUserRoleList(PROJECT, user);
+
+        GorAuthInfo info = CsaApiUtils.updateWithCsaApi(csaApiService, authInfo(user));
+
+        Assert.assertEquals("10", info.getUserId());
+        Assert.assertEquals(List.of("researcher"), info.getUserRoles());
+    }
+
+    private static GorAuthInfo authInfo(String username) {
+        return new GeneralAuthInfo(0, PROJECT, username, "", null, 0, Long.MAX_VALUE);
+    }
+}

util/src/main/java/org/gorpipe/util/Strings.java

@@ -1,7 +1,12 @@
 package org.gorpipe.util;
 
+import java.util.regex.Pattern;
+
 public class Strings {
 
+    // One '@' with a non-empty local part and domain, no whitespace or '/' (the value may end up in a URL path).
+    private static final Pattern EMAIL = Pattern.compile("[^\\s@/]+@[^\\s@/]+");
+
     /**
      * @param s string to check
      * @return returns true if the String is null or blank after trimming, otherwise returns false.
@@ -29,4 +34,14 @@ public static String blankNull(String s) {
         return s;
     }
 
-}
+    /**
+     * Loose check: a single '@' between a non-empty local part and domain, without whitespace or '/'.
+     *
+     * @param s string to check
+     * @return returns true if the String looks like an email address, otherwise returns false.
+     */
+    public static boolean isEmail(String s) {
+        return s != null && EMAIL.matcher(s).matches();
+    }
+
+}

util/src/test/java/org/gorpipe/util/UTestStrings.java

@@ -33,4 +33,20 @@ public void testBlankNull() {
         Assert.assertEquals("abc", Strings.blankNull("abc"));
         Assert.assertNotSame("abc", Strings.blankNull("abc "));
     }
-}
+
+    @Test
+    public void testIsEmail() {
+        Assert.assertTrue(Strings.isEmail("user@email.com"));
+        Assert.assertFalse(Strings.isEmail(null));
+        Assert.assertFalse(Strings.isEmail(""));
+        Assert.assertFalse(Strings.isEmail("sequenceminer"));
+        Assert.assertFalse(Strings.isEmail("service-account-sequenceminer"));
+        Assert.assertFalse(Strings.isEmail("@email.com"));
+        Assert.assertFalse(Strings.isEmail("user@"));
+        Assert.assertFalse(Strings.isEmail(" user@email.com "));
+        Assert.assertFalse(Strings.isEmail("user name@email.com"));
+        Assert.assertFalse(Strings.isEmail("a@b@c"));
+        Assert.assertFalse(Strings.isEmail("../../x@y"));
+        Assert.assertFalse(Strings.isEmail("user@email.com/x"));
+    }
+}