From e772a0be87b8b581eb41e4f7598b3a9d0ee09dd5 Mon Sep 17 00:00:00 2001 From: Jaeho Yoo Date: Mon, 11 Sep 2023 18:43:12 +0900 Subject: [PATCH] Use regex expression for lb authorization --- docs/security.md | 20 ++- .../gateway/ha/security/LbAuthorizer.java | 21 +-- .../gateway/ha/security/TestLbAuthorizer.java | 127 ++++++++++++++++++ 3 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java diff --git a/docs/security.md b/docs/security.md index 004ab8361..fbec36e6c 100644 --- a/docs/security.md +++ b/docs/security.md @@ -96,35 +96,43 @@ authentication: ### Form/LDAP +LDAP requires both random key pair and config path for LDAP + ``` authentication: defaultType: "form" form: ldapConfigPath: + selfSignKeyPair: + privateKeyRsa: + publicKeyRsa: ``` ## Authorization -Trino Gateway supports the following roles: +Trino Gateway supports the following roles in regex string format: - admin : Allows access to the Editor tab, which can be used to configure the backends - user : Allows access to the rest of the website -- api : Allows access to to rest apis to configure the backends +- api : Allows access to rest apis to configure the backends Users with attributes next to the role will be giving those privileges the -users. User attributes from LDAP is supported or you can use the preset users -defined in the yaml file. Authorization is supported via LDAP user attributes +users. You can use the preset users defined in the yaml file. +LDAP Authorization is also supported by adding user attribute configs in file. + +- Check out [LDAPTestConfig.yml](https://github.com/trinodb/trino-gateway/blob/main/gateway-ha/src/test/resources/auth/ldapTestConfig.yml) file for config details ``` +# Roles should be in regex format authorization: admin: 'lb_admin' user: 'lb_user' - api: "lb_api" - ldapConfigPath: "" + api: 'lb_api' + ldapConfigPath: '' ``` The LDAP config file should have the following contents: diff --git a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbAuthorizer.java b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbAuthorizer.java index 4e213e06f..de178e918 100644 --- a/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbAuthorizer.java +++ b/gateway-ha/src/main/java/io/trino/gateway/ha/security/LbAuthorizer.java @@ -17,25 +17,26 @@ public LbAuthorizer(AuthorizationConfiguration configuration) { public boolean authorize(LbPrincipal principal, String role) { switch (role) { case "ADMIN": - log.info("User {} identified as ADMIN", principal.getName()); + log.info("User '{}' with memberOf({}) was identified as ADMIN({})", + principal.getName(), principal.getMemberOf(), configuration.getAdmin()); return principal.getMemberOf() - .filter(m -> m.contains(configuration.getAdmin())) + .filter(m -> m.matches(configuration.getAdmin())) .isPresent(); case "USER": - log.info("User {} identified as USER", principal.getName()); + log.info("User '{}' with memberOf({}) identified as USER({})", + principal.getName(), principal.getMemberOf(), configuration.getUser()); return principal.getMemberOf() - .filter(m -> m.contains(configuration.getUser())) + .filter(m -> m.matches(configuration.getUser())) .isPresent(); case "API": - log.info("User {} identified as USER", principal.getName()); + log.info("User '{}' with memberOf({}) identified as API({})", + principal.getName(), principal.getMemberOf(), configuration.getApi()); return principal.getMemberOf() - .filter(m -> m.contains(configuration.getApi())) + .filter(m -> m.matches(configuration.getApi())) .isPresent(); default: - log.warn("User {} is neither member of {} or of {} based on ldap search", - principal.getName(), - configuration.getAdmin(), - configuration.getUser()); + log.warn("User '{}' with role {} has no regex match based on ldap search", + principal.getName(), role); return false; } diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java new file mode 100644 index 000000000..c373931e1 --- /dev/null +++ b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbAuthorizer.java @@ -0,0 +1,127 @@ +package io.trino.gateway.ha.security; + +import io.trino.gateway.ha.config.AuthorizationConfiguration; +import lombok.extern.slf4j.Slf4j; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.util.Optional; +import java.util.regex.PatternSyntaxException; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@Slf4j +public class TestLbAuthorizer { + private static final String USER = "username"; + private static LbPrincipal principal; + private static LbAuthorizer authorizer; + private static AuthorizationConfiguration configuration; + private static final String ADMIN_ROLE = "ADMIN"; + private static final String USER_ROLE = "USER"; + private static final String API_ROLE = "API"; + private static final String UNKNOWN_ROLE = "UNKNOWN"; + + private static final String PVFX_DATA_31 = "PVFX_DATA_31"; + + @BeforeAll + public static void setup() { + configuration = new AuthorizationConfiguration(); + principal = new LbPrincipal(USER, Optional.of(PVFX_DATA_31)); + } + + static void configureRole(String regex, String role) { + if (role.equalsIgnoreCase(ADMIN_ROLE)){ + configuration.setAdmin(regex); + authorizer = new LbAuthorizer(configuration); + } + if (role.equalsIgnoreCase(USER_ROLE)) { + configuration.setUser(regex); + authorizer = new LbAuthorizer(configuration); + } + if (role.equalsIgnoreCase(API_ROLE)) { + configuration.setApi(regex); + authorizer = new LbAuthorizer(configuration); + } + } + + static void assertMatch(String role) { + assertTrue(authorizer.authorize(principal, role)); + } + + static void assertNotMatch(String role) { + assertFalse(authorizer.authorize(principal, role)); + } + + static void assertBadPattern(String role) { + log.info("Configured bad regex pattern for role [{}]", role); + try{ + assertNotMatch(role); + } catch (PatternSyntaxException e) { + log.info("Failed to compile ==> OKAY"); + } + } + + @Test + public void testBasic() { + configureRole(PVFX_DATA_31, ADMIN_ROLE); + assertMatch(ADMIN_ROLE); + + configureRole(PVFX_DATA_31, UNKNOWN_ROLE); + assertNotMatch(UNKNOWN_ROLE); // UNKNOWN ROLE should always return FALSE + + configureRole("PVFX", USER_ROLE); + assertNotMatch(USER_ROLE); + + configureRole("DATA", API_ROLE); + assertNotMatch(API_ROLE); + + configureRole("31", ADMIN_ROLE); + assertNotMatch(ADMIN_ROLE); + } + + @Test + public void testZeroOrMoreCharacters() { + configureRole("PVFX(.*)", ADMIN_ROLE); + assertMatch(ADMIN_ROLE); + + configureRole("(?i)pvfx(.*)", USER_ROLE); + assertMatch(USER_ROLE); + + configureRole("(.*)", API_ROLE); + assertMatch(API_ROLE); + + configureRole("PVFX_DATA_31(.*)", ADMIN_ROLE); + assertMatch(ADMIN_ROLE); + + configureRole("(.*)_31", USER_ROLE); + assertMatch(USER_ROLE); + + configureRole("(.*)DATA(.*)", API_ROLE); + assertMatch(API_ROLE); + + configureRole("^.+$", ADMIN_ROLE); + assertMatch(ADMIN_ROLE); + + configureRole("^.+$", UNKNOWN_ROLE); + assertNotMatch(UNKNOWN_ROLE); // UNKNOWN ROLE should always return FALSE + + configureRole("(.*)DATA", USER_ROLE); + assertNotMatch(USER_ROLE); + + configureRole("PVFX__(.*)", API_ROLE); + assertNotMatch(API_ROLE); + } + + @Test + public void testBadPatterns() throws Exception { + configureRole("^[a-zA--Z0-9_]+$", ADMIN_ROLE); // bad range + assertBadPattern(ADMIN_ROLE); + + configureRole("^[a-zA-Z0-9_*$", USER_ROLE); // missing ] + assertBadPattern(USER_ROLE); + + configureRole("^[a-zA-Z0-9_]+$\\", API_ROLE); // nothing to escape + assertBadPattern(API_ROLE); + } +}