Fix the settings of roles_seperator (#1618)

* Fix the settings of roles_seperator Signed-off-by: Peter Nied <[email protected]>
opensearch-project · Feb 16, 2022 · 5b994af · 5b994af
1 parent f916e04
commit 5b994af
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 6 deletions.
diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
@@ -95,7 +95,8 @@ class AuthTokenProcessorHandler {
 
         this.samlRolesKey = settings.get("roles_key");
         this.samlSubjectKey = settings.get("subject_key");
-        String samlRolesSeparator = settings.get("roles_seperator");
+        // Originally release with a typo, prioritize correct spelling over typo'ed version
+        String samlRolesSeparator = settings.get("roles_separator", settings.get("roles_seperator"));
         this.kibanaRootUrl = settings.get("kibana_url");
         if (samlRolesSeparator != null) {
             this.samlRolesSeparatorPattern = Pattern.compile(samlRolesSeparator);

diff --git a/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java
@@ -567,20 +567,30 @@ public void idpEndpointWithQueryStringTest() throws Exception {
         Assert.assertEquals("horst", jwt.getClaim("sub"));
     }
 
-    @SuppressWarnings("unchecked")
     @Test
     public void commaSeparatedRolesTest() throws Exception {
+        final Settings.Builder settingsBuilder = Settings.builder().put("roles_seperator", ";").put("roles_separator", ",");
+        commaSeparatedRoles("a,b", settingsBuilder);
+    }
+
+    @Test
+    public void legacyCommaSeparatedRolesTest() throws Exception {
+        final Settings.Builder settingsBuilder = Settings.builder().put("roles_seperator", ";");
+        commaSeparatedRoles("a;b", settingsBuilder);
+    }
+
+    @SuppressWarnings("unchecked")
+    private void commaSeparatedRoles(final String rolesAsString, final Settings.Builder settingsBuilder) throws Exception {
         mockSamlIdpServer.setAuthenticateUser("horst");
         mockSamlIdpServer.setSignResponses(true);
         mockSamlIdpServer.loadSigningKeys("saml/kirk-keystore.jks", "kirk");
-        mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList("a,b"));
+        mockSamlIdpServer.setAuthenticateUserRoles(Arrays.asList(rolesAsString));
         mockSamlIdpServer.setEndpointQueryString(null);
 
-        Settings settings = Settings.builder().put(IDP_METADATA_URL, mockSamlIdpServer.getMetadataUri())
+        Settings settings = settingsBuilder.put(IDP_METADATA_URL, mockSamlIdpServer.getMetadataUri())
                 .put("kibana_url", "http://wherever").put("idp.entity_id", mockSamlIdpServer.getIdpEntityId())
-                .put("exchange_key", "abc").put("roles_key", "roles").put("roles_seperator", ",").put("path.home", ".")
+                .put("exchange_key", "abc").put("roles_key", "roles").put("path.home", ".")
                 .build();
-
         HTTPSamlAuthenticator samlAuthenticator = new HTTPSamlAuthenticator(settings, null);
 
         AuthenticateHeaders authenticateHeaders = getAutenticateHeaders(samlAuthenticator);