This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit ce769fafb4491e9d414a86d99ecaf44f7b24e8a7 Author: Alex Heneveld <[email protected]> AuthorDate: Tue Jan 21 12:55:13 2025 +0000 Further tidy-up of login/logout logging --- .../filter/BrooklynSecurityProviderFilterHelper.java | 7 ++++++- .../org/apache/brooklyn/rest/security/LoginLogging.java | 16 +++++++++++++++- .../security/provider/DelegatingSecurityProvider.java | 5 +++++ .../security/provider/ExplicitUsersSecurityProvider.java | 5 +++-- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/BrooklynSecurityProviderFilterHelper.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/BrooklynSecurityProviderFilterHelper.java index 91fd4a3fe3..16ebe737f9 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/BrooklynSecurityProviderFilterHelper.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/BrooklynSecurityProviderFilterHelper.java @@ -165,6 +165,10 @@ public class BrooklynSecurityProviderFilterHelper { if (idxColon >= 0) { user = userpass.substring(0, idxColon); pass = userpass.substring(idxColon + 1); + if ("logout".equals(user) && "logout".equals(pass)) { + // logout:logout sent by UI to clear browser state; force failure here and suppress subsequent error messages in log, after a logout + throw abort("Reauthorization required after logout", provider.requiresUserPass()); + } } else { throw abort("Invalid authorization string (no colon after decoding)", provider.requiresUserPass()); } @@ -192,7 +196,8 @@ public class BrooklynSecurityProviderFilterHelper { } } LoginLogging.logLoginIfNotLogged(preferredSession2, user, - MutableMap.of("origin", webRequest.getRemoteAddr(), "provider", provider.getClass().getName())); + MutableMap.of("origin", webRequest.getRemoteAddr(), "provider", + DelegatingSecurityProvider.getTarget(provider).getClass().getName())); return; } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/LoginLogging.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/LoginLogging.java index f47aedd857..a4506bc0df 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/LoginLogging.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/LoginLogging.java @@ -19,6 +19,7 @@ package org.apache.brooklyn.rest.security; import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,7 +51,20 @@ public class LoginLogging { } public static void logLogout(HttpSession session, String user, Map<String,String> values) { - session.setAttribute(LOGIN_LOGGED, false); + boolean error = false; + try { + session.setAttribute(LOGIN_LOGGED, false); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + // expected + error = true; + } + if (!error) { + log.warn("Expected error clearing logged attribute on session but session did not report an invalidated error: "+session); + values = MutableMap.copyOf(values).add("error", "WARN: unconfirmed; see log above"); + } + // above is just to be safe; normally logout will invalidate session so the above will error with no side-effect + log.debug( "Logout of " + (user==null ? "anonymous user" : "user: "+user) + diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/DelegatingSecurityProvider.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/DelegatingSecurityProvider.java index 318bce0053..29604e5d5a 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/DelegatingSecurityProvider.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/DelegatingSecurityProvider.java @@ -51,6 +51,11 @@ public class DelegatingSecurityProvider implements SecurityProvider { private SecurityProvider delegate; + public static SecurityProvider getTarget(SecurityProvider provider) { + if (provider instanceof DelegatingSecurityProvider) return getTarget( ((DelegatingSecurityProvider) provider).getDelegate() ); + return provider; + } + public synchronized SecurityProvider getDelegate() { if (delegate == null) { delegate = loadDelegate(); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/ExplicitUsersSecurityProvider.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/ExplicitUsersSecurityProvider.java index 0205ec0ebe..7cccee97cc 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/ExplicitUsersSecurityProvider.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/ExplicitUsersSecurityProvider.java @@ -31,6 +31,7 @@ import org.apache.brooklyn.config.StringConfigMap; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.rest.BrooklynWebConfig; import org.apache.brooklyn.rest.security.PasswordHasher; +import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -74,10 +75,10 @@ public class ExplicitUsersSecurityProvider extends AbstractSecurityProvider impl @Override public boolean authenticate(HttpServletRequest request, Supplier<HttpSession> sessionSupplierOnSuccess, String user, String pass) throws SecurityProviderDeniedAuthentication { - if (user==null) return false; + if (Strings.isBlank(user)) return false; if (!allowAnyUserWithValidPass) { if (!allowedUsers.contains(user)) { - LOG.debug("REST rejecting unknown user "+user); + LOG.debug("REST authentication rejecting unknown user '"+user+"'"); return false; } }
