Use regex expression for authorization

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: <ldap_config_path>
+    selfSignKeyPair:
+      privateKeyRsa: <private_key_path>
+      publicKeyRsa: <public_key_path>
 ```
 
 
 ## 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: "<ldap_config_path>"
+  api: 'lb_api'
+  ldapConfigPath: '<ldap_config_path>'
 ```
 
 The LDAP config file should have the following contents:

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;
 
     }

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);
+    }
+}