diff --git a/auth/src/main/java/org/gorpipe/gor/auth/utils/CsaApiUtils.java b/auth/src/main/java/org/gorpipe/gor/auth/utils/CsaApiUtils.java index 935fbf07b..0f528615a 100644 --- a/auth/src/main/java/org/gorpipe/gor/auth/utils/CsaApiUtils.java +++ b/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 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); } diff --git a/auth/src/test/java/org/gorpipe/gor/auth/UTestPlatformAuthUsername.java b/auth/src/test/java/org/gorpipe/gor/auth/UTestPlatformAuthUsername.java index 1f9c6c0b9..882754ea0 100644 --- a/auth/src/test/java/org/gorpipe/gor/auth/UTestPlatformAuthUsername.java +++ b/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 --- diff --git a/auth/src/test/java/org/gorpipe/gor/auth/utils/UTestCsaApiUtils.java b/auth/src/test/java/org/gorpipe/gor/auth/utils/UTestCsaApiUtils.java new file mode 100644 index 000000000..7e75703af --- /dev/null +++ b/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 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 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); + } +} diff --git a/util/src/main/java/org/gorpipe/util/Strings.java b/util/src/main/java/org/gorpipe/util/Strings.java index c6a105ca8..a3cd0a39f 100644 --- a/util/src/main/java/org/gorpipe/util/Strings.java +++ b/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; } -} \ No newline at end of file + /** + * 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(); + } + +} diff --git a/util/src/test/java/org/gorpipe/util/UTestStrings.java b/util/src/test/java/org/gorpipe/util/UTestStrings.java index aae4ef22f..050808418 100644 --- a/util/src/test/java/org/gorpipe/util/UTestStrings.java +++ b/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 ")); } -} \ No newline at end of file + + @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")); + } +}