You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2021/05/18 14:56:22 UTC
[sling-org-apache-sling-auth-core] branch master updated (a9280cc
-> c0e04d8)
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git.
from a9280cc Update README.md
new 6ced9aa SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
new 47eee11 SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
new c0e04d8 SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
pom.xml | 6 ++
.../sling/auth/core/impl/FailureCodesMapper.java | 79 ++++++++++++++++
.../sling/auth/core/impl/SlingAuthenticator.java | 65 +++++--------
.../sling/auth/core/spi/AuthenticationHandler.java | 10 +-
.../apache/sling/auth/core/spi/package-info.java | 4 +-
.../auth/core/impl/FailureCodesMapperTest.java | 101 +++++++++++++++++++++
6 files changed, 218 insertions(+), 47 deletions(-)
create mode 100644 src/main/java/org/apache/sling/auth/core/impl/FailureCodesMapper.java
create mode 100644 src/test/java/org/apache/sling/auth/core/impl/FailureCodesMapperTest.java
[sling-org-apache-sling-auth-core] 01/03: SLING-10383 - Do not
check for redirect loops when a login fails due to an expired token
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git
commit 6ced9aa1437ca4f90f2d478253ba67cb76e96191
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Mon May 17 16:15:12 2021 +0200
SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
Minor code cleanups
---
.../java/org/apache/sling/auth/core/impl/SlingAuthenticator.java | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
index d075e61..e7143b1 100644
--- a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
+++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
@@ -370,7 +370,7 @@ public class SlingAuthenticator implements Authenticator,
/**
* Get the configuration for the http auth
* @param config The configuration
- * @return The http auth
+ * @return The http auth
*/
public static String getHttpAuth(final Config config) {
final String http;
@@ -1177,7 +1177,7 @@ public class SlingAuthenticator implements Authenticator,
* and who is now impersonating as <i>user</i>.
*/
private void sendSudoCookie(
- HttpServletRequest request,
+ HttpServletRequest request,
HttpServletResponse response,
final String user, final int maxAge, final String path,
final String owner) {
@@ -1405,7 +1405,7 @@ public class SlingAuthenticator implements Authenticator,
}
private void postLoginEvent(final AuthenticationInfo authInfo) {
- final Dictionary<String, Object> properties = new Hashtable<String, Object>();
+ final Dictionary<String, Object> properties = new Hashtable<>();
properties.put(SlingConstants.PROPERTY_USERID, authInfo.getUser());
properties.put(AuthenticationInfo.AUTH_TYPE, authInfo.getAuthType());
@@ -1424,7 +1424,7 @@ public class SlingAuthenticator implements Authenticator,
AuthenticationHandler.FAILURE_REASON_CODES reason_code = getFailureReasonFromException(authInfo, reason);
//if reason_code is null, it is problem some non-login related failure, so don't send the event
if (reason_code != null) {
- final Dictionary<String, Object> properties = new Hashtable<String, Object>();
+ final Dictionary<String, Object> properties = new Hashtable<>();
if (authInfo.getUser() != null) {
properties.put(SlingConstants.PROPERTY_USERID, authInfo.getUser());
}
[sling-org-apache-sling-auth-core] 02/03: SLING-10383 - Do not
check for redirect loops when a login fails due to an expired token
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git
commit 47eee11b0a05bdd3d20fcbfba81b2077f503f058
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Mon May 17 16:22:12 2021 +0200
SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
Expose a new failure reason for expired tokens.
---
pom.xml | 6 ++++++
.../org/apache/sling/auth/core/spi/AuthenticationHandler.java | 10 +++++++++-
src/main/java/org/apache/sling/auth/core/spi/package-info.java | 4 ++--
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/pom.xml b/pom.xml
index c8bd88b..59ed2c6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -73,6 +73,12 @@
<artifactId>org.osgi.service.component</artifactId>
</dependency>
<dependency>
+ <groupId>biz.aQute.bnd</groupId>
+ <artifactId>biz.aQute.bnd.annotation</artifactId>
+ <version>5.3.0</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
<version>2.20.0</version>
diff --git a/src/main/java/org/apache/sling/auth/core/spi/AuthenticationHandler.java b/src/main/java/org/apache/sling/auth/core/spi/AuthenticationHandler.java
index 35a5ab4..2816110 100644
--- a/src/main/java/org/apache/sling/auth/core/spi/AuthenticationHandler.java
+++ b/src/main/java/org/apache/sling/auth/core/spi/AuthenticationHandler.java
@@ -25,6 +25,8 @@ import javax.servlet.http.HttpServletResponse;
import org.osgi.annotation.versioning.ConsumerType;
+import aQute.bnd.annotation.baseline.BaselineIgnore;
+
/**
* The <code>AuthenticationHandler</code> interface defines the service API used
* by the authentication implementation to support plugin various ways of
@@ -116,16 +118,22 @@ public interface AuthenticationHandler {
* change initial password is enabled</li>
* <li><code>account_locked</code>: the account was disabled or locked</li>
* <li><code>account_not_found</code>: the account was not found (not the same as username password mismatch)</li>
+ * <li><code>expired_token</code>: the token credentials used have expired</li>
* </ul>
* @since 1.1.0
*/
+ // When adding a new field to the enum bnd will require a minor version bump
+ // That's unfortunately too much for an SPI package and should really have no impact
+ // on implementors since the enum values are not exposed from any public API
+ @BaselineIgnore("1.2.3")
enum FAILURE_REASON_CODES {
INVALID_LOGIN,
PASSWORD_EXPIRED,
PASSWORD_EXPIRED_AND_NEW_PASSWORD_IN_HISTORY,
UNKNOWN,
ACCOUNT_LOCKED,
- ACCOUNT_NOT_FOUND;
+ ACCOUNT_NOT_FOUND,
+ EXPIRED_TOKEN;
@Override
public String toString() {
diff --git a/src/main/java/org/apache/sling/auth/core/spi/package-info.java b/src/main/java/org/apache/sling/auth/core/spi/package-info.java
index b6428ba..7171d7f 100755
--- a/src/main/java/org/apache/sling/auth/core/spi/package-info.java
+++ b/src/main/java/org/apache/sling/auth/core/spi/package-info.java
@@ -26,9 +26,9 @@
* being an abstract base implementation from which concrete
* implementations may inherit.
*
- * @version 1.2.2
+ * @version 1.2.3
*/
-@org.osgi.annotation.versioning.Version("1.2.2")
+@org.osgi.annotation.versioning.Version("1.2.3")
package org.apache.sling.auth.core.spi;
[sling-org-apache-sling-auth-core] 03/03: SLING-10383 - Do not
check for redirect loops when a login fails due to an expired token
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-auth-core.git
commit c0e04d82129e501aee91068fe130c294ad5f4a02
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Mon May 17 16:41:35 2021 +0200
SLING-10383 - Do not check for redirect loops when a login fails due to an expired token
- do not try and break login loops for expired tokens
- extract failure code mapping logic to a the FailureCodesMapper class and add tests for it.
---
.../sling/auth/core/impl/FailureCodesMapper.java | 79 ++++++++++++++++
.../sling/auth/core/impl/SlingAuthenticator.java | 57 ++++--------
.../auth/core/impl/FailureCodesMapperTest.java | 101 +++++++++++++++++++++
3 files changed, 197 insertions(+), 40 deletions(-)
diff --git a/src/main/java/org/apache/sling/auth/core/impl/FailureCodesMapper.java b/src/main/java/org/apache/sling/auth/core/impl/FailureCodesMapper.java
new file mode 100644
index 0000000..f3699b6
--- /dev/null
+++ b/src/main/java/org/apache/sling/auth/core/impl/FailureCodesMapper.java
@@ -0,0 +1,79 @@
+/*
+ * 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.sling.auth.core.impl;
+
+import javax.jcr.SimpleCredentials;
+import javax.security.auth.login.AccountLockedException;
+import javax.security.auth.login.AccountNotFoundException;
+import javax.security.auth.login.CredentialExpiredException;
+
+import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.auth.core.spi.AuthenticationHandler.FAILURE_REASON_CODES;
+import org.apache.sling.auth.core.spi.AuthenticationInfo;
+import org.jetbrains.annotations.NotNull;
+
+public final class FailureCodesMapper {
+
+ /**
+ * Determine the failure reason from the thrown exception
+ *
+ * @param authInfo The authentication info
+ * @param reason The exception
+ *
+ * @return The failure code, possibly <tt>unknown</tt> if no mapping could be found
+ */
+ public static @NotNull FAILURE_REASON_CODES getFailureReason(final AuthenticationInfo authInfo, Exception reason) {
+
+ FAILURE_REASON_CODES code = FAILURE_REASON_CODES.UNKNOWN;
+ if (reason instanceof LoginException) {
+ if (reason.getCause() instanceof CredentialExpiredException) {
+ // force failure attribute to be set so handlers can
+ // react to this special circumstance
+ Object creds = authInfo.get("user.jcr.credentials");
+ if (creds instanceof SimpleCredentials && ((SimpleCredentials) creds).getAttribute("PasswordHistoryException") != null) {
+ code = FAILURE_REASON_CODES.PASSWORD_EXPIRED_AND_NEW_PASSWORD_IN_HISTORY;
+ } else {
+ code = FAILURE_REASON_CODES.PASSWORD_EXPIRED;
+ }
+ } else if (reason.getCause() instanceof AccountLockedException) {
+ code = FAILURE_REASON_CODES.ACCOUNT_LOCKED;
+ } else if (reason.getCause() instanceof AccountNotFoundException) {
+ code = FAILURE_REASON_CODES.ACCOUNT_NOT_FOUND;
+ } else if (isTokenCredentialsExpiredException(reason)) {
+ code = FAILURE_REASON_CODES.EXPIRED_TOKEN;
+ } else {
+ // default to invalid login as the reason
+ code = FAILURE_REASON_CODES.INVALID_LOGIN;
+ }
+ }
+
+ return code;
+ }
+
+ private static boolean isTokenCredentialsExpiredException(Exception reason) {
+ // we don't want to strongly bind to Oak class names, so we use the String form here
+ // requires Oak 1.40+ ( https://issues.apache.org/jira/browse/OAK-9433 )
+ return reason.getCause() != null
+ && reason.getCause().getClass().getSimpleName().equals("TokenCredentialsExpiredException"); // NOSONAR
+ }
+
+ private FailureCodesMapper() {
+ // prevent instantiation
+ }
+}
diff --git a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
index e7143b1..5ea4289 100644
--- a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
+++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
@@ -28,10 +28,6 @@ import java.util.Dictionary;
import java.util.Hashtable;
import java.util.List;
-import javax.jcr.SimpleCredentials;
-import javax.security.auth.login.AccountLockedException;
-import javax.security.auth.login.AccountNotFoundException;
-import javax.security.auth.login.CredentialExpiredException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletRequestEvent;
import javax.servlet.ServletRequestListener;
@@ -50,6 +46,7 @@ import org.apache.sling.auth.core.AuthUtil;
import org.apache.sling.auth.core.AuthenticationSupport;
import org.apache.sling.auth.core.spi.AuthenticationFeedbackHandler;
import org.apache.sling.auth.core.spi.AuthenticationHandler;
+import org.apache.sling.auth.core.spi.AuthenticationHandler.FAILURE_REASON_CODES;
import org.apache.sling.auth.core.spi.AuthenticationInfo;
import org.apache.sling.auth.core.spi.AuthenticationInfoPostProcessor;
import org.apache.sling.auth.core.spi.DefaultAuthenticationFeedbackHandler;
@@ -902,7 +899,7 @@ public class SlingAuthenticator implements Authenticator,
// request authentication information and send 403 (Forbidden)
// if no handler can request authentication information.
- AuthenticationHandler.FAILURE_REASON_CODES code = getFailureReasonFromException(authInfo, reason);
+ FAILURE_REASON_CODES code = FailureCodesMapper.getFailureReason(authInfo, reason);
String message = null;
switch (code) {
case ACCOUNT_LOCKED:
@@ -917,6 +914,9 @@ public class SlingAuthenticator implements Authenticator,
case PASSWORD_EXPIRED_AND_NEW_PASSWORD_IN_HISTORY:
message = "Password expired and new password found in password history";
break;
+ case EXPIRED_TOKEN:
+ message = "Expired authentication token";
+ break;
case UNKNOWN:
case INVALID_LOGIN:
default:
@@ -951,38 +951,7 @@ public class SlingAuthenticator implements Authenticator,
}
- /**
- * Try to determine the failure reason from the thrown exception
- */
- private AuthenticationHandler.FAILURE_REASON_CODES getFailureReasonFromException(final AuthenticationInfo authInfo, Exception reason) {
- AuthenticationHandler.FAILURE_REASON_CODES code = null;
- if (reason.getClass().getName().contains("TooManySessionsException")) {
- // not a login failure just unavailable service
- code = null;
- } else if (reason instanceof LoginException) {
- if (reason.getCause() instanceof CredentialExpiredException) {
- // force failure attribute to be set so handlers can
- // react to this special circumstance
- Object creds = authInfo.get("user.jcr.credentials");
- if (creds instanceof SimpleCredentials && ((SimpleCredentials) creds).getAttribute("PasswordHistoryException") != null) {
- code = AuthenticationHandler.FAILURE_REASON_CODES.PASSWORD_EXPIRED_AND_NEW_PASSWORD_IN_HISTORY;
- } else {
- code = AuthenticationHandler.FAILURE_REASON_CODES.PASSWORD_EXPIRED;
- }
- } else if (reason.getCause() instanceof AccountLockedException) {
- code = AuthenticationHandler.FAILURE_REASON_CODES.ACCOUNT_LOCKED;
- } else if (reason.getCause() instanceof AccountNotFoundException) {
- code = AuthenticationHandler.FAILURE_REASON_CODES.ACCOUNT_NOT_FOUND;
- }
- if (code == null) {
- // default to invalid login as the reason
- code = AuthenticationHandler.FAILURE_REASON_CODES.INVALID_LOGIN;
- }
- }
-
- return code;
- }
/**
* Tries to request credentials from the client. The following mechanisms
@@ -1093,6 +1062,10 @@ public class SlingAuthenticator implements Authenticator,
AuthUtil.sendInvalid(request, response);
}
+ private boolean isExpiredToken(HttpServletRequest request) {
+ return FAILURE_REASON_CODES.EXPIRED_TOKEN == request.getAttribute(AuthenticationHandler.FAILURE_REASON_CODE);
+ }
+
/**
* Returns <code>true</code> if the current request was referred to by the
* same URL as the current request has. This is assumed to be caused by a
@@ -1104,6 +1077,10 @@ public class SlingAuthenticator implements Authenticator,
* <code>false</code> otherwise
*/
private boolean isLoginLoop(final HttpServletRequest request) {
+
+ if (isExpiredToken(request))
+ return false;
+
String referer = request.getHeader("Referer");
if (referer != null) {
StringBuffer sb = request.getRequestURL();
@@ -1421,9 +1398,9 @@ public class SlingAuthenticator implements Authenticator,
*/
private void postLoginFailedEvent(final HttpServletRequest request, final AuthenticationInfo authInfo, Exception reason) {
// The reason for the failure may be useful to downstream subscribers.
- AuthenticationHandler.FAILURE_REASON_CODES reason_code = getFailureReasonFromException(authInfo, reason);
- //if reason_code is null, it is problem some non-login related failure, so don't send the event
- if (reason_code != null) {
+ FAILURE_REASON_CODES reasonCode = FailureCodesMapper.getFailureReason(authInfo, reason);
+ //if reason code is unknowm, it is problem some non-login related failure, so don't send the event
+ if (reasonCode != FAILURE_REASON_CODES.UNKNOWN) {
final Dictionary<String, Object> properties = new Hashtable<>();
if (authInfo.getUser() != null) {
properties.put(SlingConstants.PROPERTY_USERID, authInfo.getUser());
@@ -1431,7 +1408,7 @@ public class SlingAuthenticator implements Authenticator,
if (authInfo.getAuthType() != null) {
properties.put(AuthenticationInfo.AUTH_TYPE, authInfo.getAuthType());
}
- properties.put("reason_code", reason_code.name());
+ properties.put("reason_code", reasonCode.name());
EventAdmin localEA = this.eventAdmin;
if (localEA != null) {
diff --git a/src/test/java/org/apache/sling/auth/core/impl/FailureCodesMapperTest.java b/src/test/java/org/apache/sling/auth/core/impl/FailureCodesMapperTest.java
new file mode 100644
index 0000000..ecc765e
--- /dev/null
+++ b/src/test/java/org/apache/sling/auth/core/impl/FailureCodesMapperTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.sling.auth.core.impl;
+
+import static org.apache.sling.auth.core.impl.FailureCodesMapper.getFailureReason;
+
+import javax.jcr.SimpleCredentials;
+import javax.security.auth.login.AccountLockedException;
+import javax.security.auth.login.AccountNotFoundException;
+import javax.security.auth.login.CredentialExpiredException;
+
+import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.auth.core.spi.AuthenticationHandler.FAILURE_REASON_CODES;
+import org.apache.sling.auth.core.spi.AuthenticationInfo;
+import org.hamcrest.CoreMatchers;
+import org.hamcrest.MatcherAssert;
+import org.junit.Test;
+
+public class FailureCodesMapperTest {
+
+ private final AuthenticationInfo dummyAuthInfo = new AuthenticationInfo("dummy");
+
+ @Test
+ public void unknown() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new RuntimeException()),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.UNKNOWN));
+ }
+
+ @Test
+ public void loginExceptionWithNoCause() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new LoginException("Something went wrong")),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.INVALID_LOGIN));
+ }
+
+ @Test
+ public void passwordExpired() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new LoginException(new CredentialExpiredException())),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.PASSWORD_EXPIRED));
+ }
+
+ @Test
+ public void accountLocked() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new LoginException(new AccountLockedException())),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.ACCOUNT_LOCKED));
+ }
+
+ @Test
+ public void accountNotFound() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new LoginException(new AccountNotFoundException())),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.ACCOUNT_NOT_FOUND));
+ }
+
+ @Test
+ public void expiredToken() {
+ MatcherAssert.assertThat(
+ getFailureReason(dummyAuthInfo, new LoginException(new TokenCredentialsExpiredException())),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.EXPIRED_TOKEN));
+ }
+
+ @Test
+ public void passwordExpiredAndNewPasswordInHistory() {
+
+ AuthenticationInfo info = new AuthenticationInfo("dummy");
+ SimpleCredentials credentials = new SimpleCredentials("ignored", "ignored".toCharArray());
+ credentials.setAttribute("PasswordHistoryException", new Object()); // value is not checked
+ info.put("user.jcr.credentials", credentials);
+
+ MatcherAssert.assertThat(
+ getFailureReason(info, new LoginException(new CredentialExpiredException())),
+ CoreMatchers.equalTo(FAILURE_REASON_CODES.PASSWORD_EXPIRED_AND_NEW_PASSWORD_IN_HISTORY));
+ }
+
+ // doubles for an Oak class
+ static class TokenCredentialsExpiredException extends Exception {
+
+ private static final long serialVersionUID = 1L;
+
+ }
+
+}