diff --git a/extensions-core/druid-pac4j/pom.xml b/extensions-core/druid-pac4j/pom.xml index a8cb8b3a08bf..a769a7f12b67 100644 --- a/extensions-core/druid-pac4j/pom.xml +++ b/extensions-core/druid-pac4j/pom.xml @@ -34,12 +34,12 @@ - 3.8.3 + 4.5.7 - + 1.7 - 7.9 - 6.5 + 8.22.1 + 8.22 @@ -145,6 +145,11 @@ easymock test + + org.mockito + mockito-core + test + diff --git a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jFilter.java b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jFilter.java index 4463e43ca29d..0495242835c4 100644 --- a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jFilter.java +++ b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jFilter.java @@ -23,14 +23,14 @@ import org.apache.druid.server.security.AuthConfig; import org.apache.druid.server.security.AuthenticationResult; import org.pac4j.core.config.Config; -import org.pac4j.core.context.J2EContext; +import org.pac4j.core.context.JEEContext; import org.pac4j.core.context.session.SessionStore; import org.pac4j.core.engine.CallbackLogic; import org.pac4j.core.engine.DefaultCallbackLogic; import org.pac4j.core.engine.DefaultSecurityLogic; import org.pac4j.core.engine.SecurityLogic; -import org.pac4j.core.http.adapter.HttpActionAdapter; -import org.pac4j.core.profile.CommonProfile; +import org.pac4j.core.http.adapter.JEEHttpActionAdapter; +import org.pac4j.core.profile.UserProfile; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -47,12 +47,10 @@ public class Pac4jFilter implements Filter { private static final Logger LOGGER = new Logger(Pac4jFilter.class); - private static final HttpActionAdapter NOOP_HTTP_ACTION_ADAPTER = (int code, J2EContext ctx) -> null; - private final Config pac4jConfig; - private final SecurityLogic securityLogic; - private final CallbackLogic callbackLogic; - private final SessionStore sessionStore; + private final SecurityLogic securityLogic; + private final CallbackLogic callbackLogic; + private final SessionStore sessionStore; private final String name; private final String authorizerName; @@ -88,20 +86,20 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo HttpServletRequest httpServletRequest = (HttpServletRequest) servletRequest; HttpServletResponse httpServletResponse = (HttpServletResponse) servletResponse; - J2EContext context = new J2EContext(httpServletRequest, httpServletResponse, sessionStore); + JEEContext context = new JEEContext(httpServletRequest, httpServletResponse, sessionStore); if (Pac4jCallbackResource.SELF_URL.equals(httpServletRequest.getRequestURI())) { callbackLogic.perform( context, pac4jConfig, - NOOP_HTTP_ACTION_ADAPTER, + JEEHttpActionAdapter.INSTANCE, "/", true, false, false, null); } else { - String uid = securityLogic.perform( + Object uid = securityLogic.perform( context, pac4jConfig, - (J2EContext ctx, Collection profiles, Object... parameters) -> { + (JEEContext ctx, Collection profiles, Object... parameters) -> { if (profiles.isEmpty()) { LOGGER.warn("No profiles found after OIDC auth."); return null; @@ -109,11 +107,13 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo return profiles.iterator().next().getId(); } }, - NOOP_HTTP_ACTION_ADAPTER, - null, null, null, null); - + JEEHttpActionAdapter.INSTANCE, + null, "none", null, null); + // Changed the Authorizer from null to "none". + // In the older version, if it is null, it simply grant access and returns authorized. + // But in the newer pac4j version, it uses CsrfAuthorizer as default, And because of this, It was returning 403 in API calls. if (uid != null) { - AuthenticationResult authenticationResult = new AuthenticationResult(uid, authorizerName, name, null); + AuthenticationResult authenticationResult = new AuthenticationResult(uid.toString(), authorizerName, name, null); servletRequest.setAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT, authenticationResult); filterChain.doFilter(servletRequest, servletResponse); } diff --git a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jSessionStore.java b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jSessionStore.java index 02612819d7a8..b0187d5e7293 100644 --- a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jSessionStore.java +++ b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/Pac4jSessionStore.java @@ -25,12 +25,12 @@ import org.apache.druid.java.util.common.logger.Logger; import org.pac4j.core.context.ContextHelper; import org.pac4j.core.context.Cookie; -import org.pac4j.core.context.Pac4jConstants; import org.pac4j.core.context.WebContext; import org.pac4j.core.context.session.SessionStore; import org.pac4j.core.exception.TechnicalException; import org.pac4j.core.profile.CommonProfile; import org.pac4j.core.util.JavaSerializationHelper; +import org.pac4j.core.util.Pac4jConstants; import javax.annotation.Nullable; import java.io.ByteArrayInputStream; @@ -38,6 +38,7 @@ import java.io.IOException; import java.io.Serializable; import java.util.Map; +import java.util.Optional; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; @@ -78,7 +79,7 @@ public String getOrCreateSessionId(WebContext context) @Nullable @Override - public Object get(WebContext context, String key) + public Optional get(WebContext context, String key) { final Cookie cookie = ContextHelper.getCookie(context, PAC4J_SESSION_PREFIX + key); Object value = null; @@ -86,7 +87,7 @@ public Object get(WebContext context, String key) value = uncompressDecryptBase64(cookie.getValue()); } LOGGER.debug("Get from session: [%s] = [%s]", key, value); - return value; + return Optional.ofNullable(value); } @Override @@ -142,7 +143,7 @@ private Serializable uncompressDecryptBase64(final String v) if (v != null && !v.isEmpty()) { byte[] bytes = StringUtils.decodeBase64String(v); if (bytes != null) { - return javaSerializationHelper.unserializeFromBytes(unCompress(cryptoService.decrypt(bytes))); + return javaSerializationHelper.deserializeFromBytes(unCompress(cryptoService.decrypt(bytes))); } } return null; @@ -176,19 +177,19 @@ private Object clearUserProfile(final Object value) { if (value instanceof Map) { final Map profiles = (Map) value; - profiles.forEach((name, profile) -> profile.clearSensitiveData()); + profiles.forEach((name, profile) -> profile.removeLoginData()); return profiles; } else { final CommonProfile profile = (CommonProfile) value; - profile.clearSensitiveData(); + profile.removeLoginData(); return profile; } } @Override - public SessionStore buildFromTrackableSession(WebContext arg0, Object arg1) + public Optional> buildFromTrackableSession(WebContext arg0, Object arg1) { - return null; + return Optional.empty(); } @Override @@ -198,9 +199,9 @@ public boolean destroySession(WebContext arg0) } @Override - public Object getTrackableSession(WebContext arg0) + public Optional getTrackableSession(WebContext arg0) { - return null; + return Optional.empty(); } @Override diff --git a/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jFilterTest.java b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jFilterTest.java new file mode 100644 index 000000000000..0523c970178d --- /dev/null +++ b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jFilterTest.java @@ -0,0 +1,77 @@ +/* + * 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.druid.security.pac4j; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.pac4j.core.context.JEEContext; +import org.pac4j.core.exception.http.ForbiddenAction; +import org.pac4j.core.exception.http.FoundAction; +import org.pac4j.core.exception.http.HttpAction; +import org.pac4j.core.exception.http.WithLocationAction; +import org.pac4j.core.http.adapter.JEEHttpActionAdapter; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.mockito.ArgumentMatchers.any; + +@RunWith(MockitoJUnitRunner.class) +public class Pac4jFilterTest +{ + + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + private JEEContext context; + + @Before + public void setUp() + { + context = new JEEContext(request, response); + } + + @Test + public void testActionAdapterForRedirection() + { + HttpAction httpAction = new FoundAction("testUrl"); + Mockito.doReturn(httpAction.getCode()).when(response).getStatus(); + Mockito.doReturn(((WithLocationAction) httpAction).getLocation()).when(response).getHeader(any()); + JEEHttpActionAdapter.INSTANCE.adapt(httpAction, context); + Assert.assertEquals(response.getStatus(), 302); + Assert.assertEquals(response.getHeader("Location"), "testUrl"); + } + + @Test + public void testActionAdapterForForbidden() + { + HttpAction httpAction = ForbiddenAction.INSTANCE; + Mockito.doReturn(httpAction.getCode()).when(response).getStatus(); + JEEHttpActionAdapter.INSTANCE.adapt(httpAction, context); + Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_FORBIDDEN); + } + +} diff --git a/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jSessionStoreTest.java b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jSessionStoreTest.java index 0349a98a7ccd..772bef7ef6c3 100644 --- a/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jSessionStoreTest.java +++ b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/Pac4jSessionStoreTest.java @@ -25,15 +25,23 @@ import org.junit.Test; import org.pac4j.core.context.Cookie; import org.pac4j.core.context.WebContext; +import org.pac4j.core.profile.CommonProfile; +import org.pac4j.core.profile.definition.CommonProfileDefinition; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; public class Pac4jSessionStoreTest { + private static final String COOKIE_PASSPHRASE = "test-cookie-passphrase"; + @Test public void testSetAndGet() { - Pac4jSessionStore sessionStore = new Pac4jSessionStore("test-cookie-passphrase"); + Pac4jSessionStore sessionStore = new Pac4jSessionStore(COOKIE_PASSPHRASE); WebContext webContext1 = EasyMock.mock(WebContext.class); EasyMock.expect(webContext1.getScheme()).andReturn("https"); @@ -54,7 +62,73 @@ public void testSetAndGet() WebContext webContext2 = EasyMock.mock(WebContext.class); EasyMock.expect(webContext2.getRequestCookies()).andReturn(Collections.singletonList(cookie)); EasyMock.replay(webContext2); + Assert.assertEquals("value", Objects.requireNonNull(sessionStore.get(webContext2, "key")).orElse(null)); + } + + @Test + public void testSetAndGetClearUserProfile() + { + Pac4jSessionStore sessionStore = new Pac4jSessionStore(COOKIE_PASSPHRASE); + + WebContext webContext1 = EasyMock.mock(WebContext.class); + EasyMock.expect(webContext1.getScheme()).andReturn("https"); + Capture cookieCapture = EasyMock.newCapture(); + + webContext1.addResponseCookie(EasyMock.capture(cookieCapture)); + EasyMock.replay(webContext1); + + CommonProfile profile = new CommonProfile(); + profile.addAttribute(CommonProfileDefinition.DISPLAY_NAME, "name"); + sessionStore.set(webContext1, "pac4jUserProfiles", profile); + + Cookie cookie = cookieCapture.getValue(); + Assert.assertTrue(cookie.isSecure()); + Assert.assertTrue(cookie.isHttpOnly()); + Assert.assertTrue(cookie.isSecure()); + Assert.assertEquals(900, cookie.getMaxAge()); + + + WebContext webContext2 = EasyMock.mock(WebContext.class); + EasyMock.expect(webContext2.getRequestCookies()).andReturn(Collections.singletonList(cookie)); + EasyMock.replay(webContext2); + Optional value = sessionStore.get(webContext2, "pac4jUserProfiles"); + Assert.assertTrue(Objects.requireNonNull(value).isPresent()); + Assert.assertEquals("name", ((CommonProfile) value.get()).getAttribute(CommonProfileDefinition.DISPLAY_NAME)); + } + + @Test + public void testSetAndGetClearUserMultipleProfile() + { + Pac4jSessionStore sessionStore = new Pac4jSessionStore(COOKIE_PASSPHRASE); + + WebContext webContext1 = EasyMock.mock(WebContext.class); + EasyMock.expect(webContext1.getScheme()).andReturn("https"); + Capture cookieCapture = EasyMock.newCapture(); + + webContext1.addResponseCookie(EasyMock.capture(cookieCapture)); + EasyMock.replay(webContext1); - Assert.assertEquals("value", sessionStore.get(webContext2, "key")); + CommonProfile profile1 = new CommonProfile(); + profile1.addAttribute(CommonProfileDefinition.DISPLAY_NAME, "name1"); + CommonProfile profile2 = new CommonProfile(); + profile2.addAttribute(CommonProfileDefinition.DISPLAY_NAME, "name2"); + Map profiles = new HashMap<>(); + profiles.put("profile1", profile1); + profiles.put("profile2", profile2); + sessionStore.set(webContext1, "pac4jUserProfiles", profiles); + + Cookie cookie = cookieCapture.getValue(); + Assert.assertTrue(cookie.isSecure()); + Assert.assertTrue(cookie.isHttpOnly()); + Assert.assertTrue(cookie.isSecure()); + Assert.assertEquals(900, cookie.getMaxAge()); + + + WebContext webContext2 = EasyMock.mock(WebContext.class); + EasyMock.expect(webContext2.getRequestCookies()).andReturn(Collections.singletonList(cookie)); + EasyMock.replay(webContext2); + Optional value = sessionStore.get(webContext2, "pac4jUserProfiles"); + Assert.assertTrue(Objects.requireNonNull(value).isPresent()); + Assert.assertEquals(2, ((Map) value.get()).size()); } } diff --git a/licenses.yaml b/licenses.yaml index 1d5fe8c0d0d4..46e9eacdc953 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -776,7 +776,7 @@ name: pac4j-oidc java security library license_category: binary module: extensions/druid-pac4j license_name: Apache License version 2.0 -version: 3.8.3 +version: 4.5.7 libraries: - org.pac4j: pac4j-oidc @@ -786,7 +786,7 @@ name: pac4j-core java security library license_category: binary module: extensions/druid-pac4j license_name: Apache License version 2.0 -version: 3.8.3 +version: 4.5.7 libraries: - org.pac4j: pac4j-core @@ -807,17 +807,27 @@ name: com.nimbusds nimbus-jose-jwt license_category: binary module: extensions/druid-pac4j license_name: Apache License version 2.0 -version: 7.9 +version: 8.22.1 libraries: - com.nimbusds: nimbus-jose-jwt --- +name: com.nimbusds content-type +license_category: binary +module: extensions/druid-pac4j +license_name: Apache License version 2.0 +version: 2.1 +libraries: + - com.nimbusds: content-type + +--- + name: com.nimbusds oauth2-oidc-sdk license_category: binary module: extensions/druid-pac4j license_name: Apache License version 2.0 -version: 6.5 +version: 8.22 libraries: - com.nimbusds: oauth2-oidc-sdk @@ -837,7 +847,7 @@ name: com.sun.mail javax.mail license_category: binary module: extensions/druid-pac4j license_name: CDDL 1.1 -version: 1.6.1 +version: 1.6.2 libraries: - com.sun.mail: javax.mail diff --git a/owasp-dependency-check-suppressions.xml b/owasp-dependency-check-suppressions.xml index 688baaddfcb7..73369dc62c33 100644 --- a/owasp-dependency-check-suppressions.xml +++ b/owasp-dependency-check-suppressions.xml @@ -588,9 +588,11 @@ - + + + CVE-2021-44878