You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by mo...@apache.org on 2021/10/12 16:35:25 UTC

[knox] branch master updated: KNOX-2679 - Remove groups, roles and permissions from SAML profile object for pac4j cookie to save space (#509)

This is an automated email from the ASF dual-hosted git repository.

more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 2c45bbd  KNOX-2679 - Remove groups, roles and permissions from SAML profile object for pac4j cookie to save space (#509)
2c45bbd is described below

commit 2c45bbd37a80a7c7030c5c0c52017171a93a4d73
Author: Sandeep Moré <mo...@gmail.com>
AuthorDate: Tue Oct 12 12:35:21 2021 -0400

    KNOX-2679 - Remove groups, roles and permissions from SAML profile object for pac4j cookie to save space (#509)
---
 .../pac4j/filter/Pac4jDispatcherFilter.java        |  34 ++++-
 .../gateway/pac4j/session/KnoxSessionStore.java    |  39 +++++-
 .../pac4j/session/KnoxSessionStoreTest.java        | 143 +++++++++++++++++++++
 3 files changed, 213 insertions(+), 3 deletions(-)

diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
index 8abd2a3..f715a23 100644
--- a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
+++ b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
@@ -93,6 +93,18 @@ public class Pac4jDispatcherFilter implements Filter {
 
   private static final String PAC4J_SESSION_STORE = "pac4j.session.store";
 
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_GROUPS = "pac4j.session.store.exclude.groups";
+
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_ROLES = "pac4j.session.store.exclude.roles";
+
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS = "pac4j.session.store.exclude.permissions";
+
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT = "true";
+
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT = "true";
+
+  public static final String PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT = "true";
+
   private static final String PAC4J_CLIENT_NAME_PARAM = "clientName";
 
   private static final String PAC4J_OIDC_TYPE = "oidc.type";
@@ -103,6 +115,7 @@ public class Pac4jDispatcherFilter implements Filter {
   private MasterService masterService;
   private KeystoreService keystoreService;
   private AliasService aliasService;
+  private Map<String, String> sessionStoreConfigs = new HashMap();
 
   @Override
   public void init( FilterConfig filterConfig ) throws ServletException {
@@ -186,7 +199,12 @@ public class Pac4jDispatcherFilter implements Filter {
       }
 
       clientName = CommonHelper.isBlank(clientNameParameter) ? clients.get(0).getName() : clientNameParameter;
-
+      /* do we need to exclude groups? */
+      setSessionStoreConfig(filterConfig, PAC4J_SESSION_STORE_EXCLUDE_GROUPS, PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT);
+      /* do we need to exclude roles? */
+      setSessionStoreConfig(filterConfig, PAC4J_SESSION_STORE_EXCLUDE_ROLES, PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT);
+      /* do we need to exclude permissions? */
+      setSessionStoreConfig(filterConfig, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT);
       //decorating client configuration (if needed)
       PAC4J_CLIENT_CONFIGURATION_DECORATOR.decorateClients(clients, properties);
     }
@@ -207,13 +225,25 @@ public class Pac4jDispatcherFilter implements Filter {
     if(!StringUtils.isBlank(sessionStoreVar) && JEESessionStore.class.getName().contains(sessionStoreVar) ) {
       sessionStore = new JEESessionStore();
     } else {
-      sessionStore = new KnoxSessionStore(cryptoService, clusterName, domainSuffix);
+      sessionStore = new KnoxSessionStore(cryptoService, clusterName, domainSuffix, sessionStoreConfigs);
     }
 
     config.setSessionStore(sessionStore);
 
   }
 
+  /**
+   * A helper method to set filter config value
+   * @param filterConfig
+   * @param configName
+   * @param configDefault
+   */
+  private void setSessionStoreConfig(final FilterConfig filterConfig, final String configName, final String configDefault) {
+    final String configValue = filterConfig.getInitParameter(configName);
+    sessionStoreConfigs.put(configName, configValue == null ? configDefault : configValue);
+  }
+
+
   private String resolveAlias(String clusterName, String key, String value) throws ServletException {
     if (value.startsWith(ALIAS_PREFIX) && value.endsWith("}")) {
       String alias = value.substring(ALIAS_PREFIX.length(), value.length() - 1);
diff --git a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
index 7d2cb89..69dffab 100644
--- a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
+++ b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStore.java
@@ -39,11 +39,19 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.Serializable;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_GROUPS;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT;
+
 /**
  * Specific session store where data are saved into cookies (and not in memory).
  * Each data is encrypted and base64 encoded before being saved as a cookie (for security reasons).
@@ -66,11 +74,21 @@ public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {
 
     private final String domainSuffix;
 
+    final Map<String, String> sessionStoreConfigs;
+
     public KnoxSessionStore(final CryptoService cryptoService, final String clusterName, final String domainSuffix) {
+        this(cryptoService, clusterName, domainSuffix, new HashMap());
+    }
+
+    public KnoxSessionStore(final CryptoService cryptoService,
+        final String clusterName,
+        final String domainSuffix,
+        final Map<String, String> sessionStoreConfigs) {
         javaSerializationHelper = new JavaSerializationHelper();
         this.cryptoService = cryptoService;
         this.clusterName = clusterName;
         this.domainSuffix = domainSuffix;
+        this.sessionStoreConfigs = sessionStoreConfigs;
     }
 
 
@@ -212,7 +230,7 @@ public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {
     }
 
     /**
-     * Keep only the fileds that are needed for Pac4J.
+     * Keep only the fields that are needed for Pac4J.
      * Used to reduce the cookie size.
      * @param value profile object
      * @return trimmed profile object
@@ -222,6 +240,25 @@ public class KnoxSessionStore<C extends WebContext> implements SessionStore<C> {
         if(value instanceof Map<?,?>) {
             final Map<String, CommonProfile> profiles = (Map<String, CommonProfile>) value;
             profiles.forEach((name, profile) -> profile.removeLoginData());
+
+            if(sessionStoreConfigs != null) {
+                if(sessionStoreConfigs
+                        .getOrDefault(PAC4J_SESSION_STORE_EXCLUDE_GROUPS, PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT)
+                        .equalsIgnoreCase("true")) {
+                    profiles.forEach((name, profile) -> profile.removeAttribute("groups"));
+                }
+                if(sessionStoreConfigs
+                        .getOrDefault(PAC4J_SESSION_STORE_EXCLUDE_ROLES, PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT)
+                        .equalsIgnoreCase("true")) {
+                    profiles.forEach((name, profile) -> profile.removeAttribute("roles"));
+                }
+                if(sessionStoreConfigs
+                        .getOrDefault(PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT)
+                        .equalsIgnoreCase("true")) {
+                    profiles.forEach((name, profile) -> profile.removeAttribute("permissions"));
+                }
+            }
+
             return profiles;
         } else {
             final CommonProfile profile = (CommonProfile) value;
diff --git a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStoreTest.java b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStoreTest.java
new file mode 100644
index 0000000..5121451
--- /dev/null
+++ b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/session/KnoxSessionStoreTest.java
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.pac4j.session;
+
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.impl.DefaultCryptoService;
+import org.easymock.Capture;
+import org.easymock.EasyMock;
+import org.junit.Assert;
+import org.junit.Test;
+import org.pac4j.core.context.WebContext;
+import org.pac4j.core.profile.CommonProfile;
+import org.pac4j.core.util.Pac4jConstants;
+import org.pac4j.saml.profile.SAML2Profile;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_GROUPS;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES;
+import static org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter.PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT;
+import static org.apache.knox.gateway.pac4j.session.KnoxSessionStore.PAC4J_PASSWORD;
+
+public class KnoxSessionStoreTest {
+  private static final String CLUSTER_NAME = "knox";
+
+  /**
+   * Test exclusion of groups, roles and permissions
+   * from pac4j profile object that is saved as a cookie.
+   * @throws AliasServiceException
+   */
+  @Test
+  public void filterConfigParamsTest()
+      throws AliasServiceException {
+    final AliasService aliasService = EasyMock.createNiceMock(AliasService.class);
+    EasyMock.expect(aliasService.getPasswordFromAliasForCluster(CLUSTER_NAME, PAC4J_PASSWORD, true))
+        .andReturn(PAC4J_PASSWORD.toCharArray()).anyTimes();
+    EasyMock.expect(aliasService.getPasswordFromAliasForCluster(CLUSTER_NAME, PAC4J_PASSWORD))
+        .andReturn(PAC4J_PASSWORD.toCharArray()).anyTimes();
+    EasyMock.replay(aliasService);
+
+    final DefaultCryptoService cryptoService = new DefaultCryptoService();
+    cryptoService.setAliasService(aliasService);
+
+    final Map<String, String> sessionStoreConfigs = new HashMap();
+
+    final Capture<org.pac4j.core.context.Cookie> captureCookieValue = EasyMock.newCapture();
+    final WebContext mockContext = EasyMock.createNiceMock(WebContext.class);
+    EasyMock.expect(mockContext.getFullRequestURL()).andReturn("https://local.com/gateway/knoxsso/").anyTimes();
+    mockContext.addResponseCookie(EasyMock.capture(captureCookieValue));
+
+    EasyMock.replay(mockContext);
+
+
+    final SAML2Profile samlProfile = new SAML2Profile();
+    Set<String> groups = new HashSet<>(Arrays.asList("admin_2", "admin_1", "admin"));
+    Set<String> roles = new HashSet<>(Arrays.asList("roles_2", "roles_1", "roles"));
+    Set<String> permissions = new HashSet<>(Arrays.asList("permissions_2", "permissions_1", "permissions"));
+    Map<String, Object> attributes = new HashMap<>();
+    attributes.put("groups", groups);
+    attributes.put("permissions", permissions);
+    attributes.put("roles", roles);
+    samlProfile.addAttributes(attributes);
+
+    /*
+     * Test the default behavior where groups, roles and permissions are
+     * excluded from the cookie.
+     */
+
+    /* Make sure groups are present */
+    Assert.assertNotNull(samlProfile.getAttribute("groups"));
+    Assert.assertNotNull(samlProfile.getAttribute("roles"));
+    Assert.assertNotNull(samlProfile.getAttribute("permissions"));
+
+
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_GROUPS, PAC4J_SESSION_STORE_EXCLUDE_GROUPS_DEFAULT);
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_ROLES, PAC4J_SESSION_STORE_EXCLUDE_ROLES_DEFAULT);
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS, PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS_DEFAULT);
+
+    final Map<String, CommonProfile> profile = new HashMap<>();
+    profile.put("SAML2Client", samlProfile);
+
+    final KnoxSessionStore sessionStore = new KnoxSessionStore(cryptoService, CLUSTER_NAME, null, sessionStoreConfigs);
+
+    sessionStore.set(mockContext, Pac4jConstants.USER_PROFILES, profile);
+
+    /* Make sure groups are removed */
+    Assert.assertNull(samlProfile.getAttribute("groups"));
+    Assert.assertNull(samlProfile.getAttribute("roles"));
+    Assert.assertNull(samlProfile.getAttribute("permissions"));
+
+
+    /*
+     * Test the override behavior where groups, roles and permissions are
+     * not-excluded from the cookie.
+     */
+    attributes.put("groups", groups);
+    attributes.put("permissions", permissions);
+    attributes.put("roles", roles);
+    samlProfile.addAttributes(attributes);
+
+    /* Make sure groups are present */
+    Assert.assertNotNull(samlProfile.getAttribute("groups"));
+    Assert.assertNotNull(samlProfile.getAttribute("roles"));
+    Assert.assertNotNull(samlProfile.getAttribute("permissions"));
+
+
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_GROUPS, "false");
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_ROLES, "false");
+    sessionStoreConfigs.put(PAC4J_SESSION_STORE_EXCLUDE_PERMISSIONS, "false");
+
+    profile.put("SAML2Client", samlProfile);
+
+    sessionStore.set(mockContext, Pac4jConstants.USER_PROFILES, profile);
+
+    /* Make sure groups are removed */
+    Assert.assertNotNull(samlProfile.getAttribute("groups"));
+    Assert.assertNotNull(samlProfile.getAttribute("roles"));
+    Assert.assertNotNull(samlProfile.getAttribute("permissions"));
+  }
+}