You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2020/08/23 20:23:24 UTC

[qpid-broker-j] branch master updated (370fb35 -> 26aa295)

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

orudyy pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git.


    from 370fb35  QPID-8458 Removed RewriteServlet from HTTP management
     new 6676f22  QPID-8459 AnonymousInteractiveAuthenticator uses request.getRequestDispatcher().forward() instead of parsing request URL
     new 26aa295  QPID-8459: [Broker-J] Reduce code duplication

The 2 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:
 .../management/plugin/HttpManagementUtil.java      |  9 +++
 .../auth/AnonymousInteractiveAuthenticator.java    | 91 ++++++++++------------
 .../auth/OAuth2InteractiveAuthenticator.java       | 35 +++------
 .../SSLClientCertInteractiveAuthenticator.java     |  5 +-
 .../auth/SpnegoInteractiveAuthenticator.java       |  5 +-
 .../auth/UsernamePasswordInteractiveLogin.java     | 17 +---
 .../plugin/servlet/rest/SaslServlet.java           |  4 +-
 .../PreemptiveAuthenticationTest.java              |  4 +-
 8 files changed, 68 insertions(+), 102 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[qpid-broker-j] 01/02: QPID-8459 AnonymousInteractiveAuthenticator uses request.getRequestDispatcher().forward() instead of parsing request URL

Posted by or...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

orudyy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git

commit 6676f224ff7e9149d077bddec1931ac5a9f46546
Author: Tomas Vavricka <to...@deutsche-boerse.com>
AuthorDate: Wed Aug 5 06:13:28 2020 +0000

    QPID-8459 AnonymousInteractiveAuthenticator uses request.getRequestDispatcher().forward() instead of parsing request URL
---
 .../auth/AnonymousInteractiveAuthenticator.java    | 88 ++++++++++------------
 .../auth/UsernamePasswordInteractiveLogin.java     | 17 +----
 .../PreemptiveAuthenticationTest.java              |  4 +-
 3 files changed, 42 insertions(+), 67 deletions(-)

diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
index f165974..c0ad0ab 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
@@ -20,10 +20,13 @@
 
 package org.apache.qpid.server.management.plugin.auth;
 
+import java.io.IOException;
 import java.security.AccessControlException;
 
 import javax.security.auth.Subject;
+import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -54,45 +57,9 @@ public class AnonymousInteractiveAuthenticator implements HttpRequestInteractive
                                                           final HttpManagementConfiguration configuration)
     {
         final Port<?> port = configuration.getPort(request);
-        if(configuration.getAuthenticationProvider(request) instanceof AnonymousAuthenticationManager)
+        if (configuration.getAuthenticationProvider(request) instanceof AnonymousAuthenticationManager)
         {
-            return response ->
-            {
-                AnonymousAuthenticationManager authenticationProvider =
-                        (AnonymousAuthenticationManager) configuration.getAuthenticationProvider(request);
-                AuthenticationResult authenticationResult = authenticationProvider.getAnonymousAuthenticationResult();
-                try
-                {
-                    SubjectAuthenticationResult result = port.getSubjectCreator(request.isSecure(), request.getServerName()).createResultWithGroups(authenticationResult);
-                    Subject original = result.getSubject();
-
-                    if (original == null)
-                    {
-                        throw new SecurityException("Only authenticated users can access the management interface");
-                    }
-                    Subject subject = HttpManagementUtil.createServletConnectionSubject(request, original);
-                    Broker broker = (Broker) authenticationProvider.getParent();
-                    HttpManagementUtil.assertManagementAccess(broker, subject);
-                    HttpManagementUtil.saveAuthorisedSubject(request, subject);
-
-                    String originalRequestUri = getOriginalRequestUri(request);
-                    LOGGER.debug("Successful login. Redirect to original resource {}", originalRequestUri);
-                    response.sendRedirect(originalRequestUri);
-                }
-                catch (SecurityException e)
-                {
-                    if (e instanceof AccessControlException)
-                    {
-                        LOGGER.info("User '{}' is not authorised for management", authenticationResult.getMainPrincipal());
-                        response.sendError(403, "User is not authorised for management");
-                    }
-                    else
-                    {
-                        LOGGER.info("Authentication failed", authenticationResult.getCause());
-                        response.sendError(401);
-                    }
-                }
-            };
+            return response -> getLoginHandler(request, response, configuration, port);
         }
         else
         {
@@ -100,6 +67,39 @@ public class AnonymousInteractiveAuthenticator implements HttpRequestInteractive
         }
     }
 
+    private void getLoginHandler(HttpServletRequest request, HttpServletResponse response,
+                                 HttpManagementConfiguration configuration, Port<?> port) throws ServletException, IOException
+    {
+        final AnonymousAuthenticationManager authenticationProvider =
+                (AnonymousAuthenticationManager) configuration.getAuthenticationProvider(request);
+        final AuthenticationResult authenticationResult = authenticationProvider.getAnonymousAuthenticationResult();
+        try
+        {
+            final SubjectAuthenticationResult result = port.getSubjectCreator(request.isSecure(), request.getServerName()).createResultWithGroups(authenticationResult);
+            final Subject original = result.getSubject();
+
+            if (original == null)
+            {
+                throw new SecurityException("Only authenticated users can access the management interface");
+            }
+            final Subject subject = HttpManagementUtil.createServletConnectionSubject(request, original);
+            final Broker broker = (Broker) authenticationProvider.getParent();
+            HttpManagementUtil.assertManagementAccess(broker, subject);
+            HttpManagementUtil.saveAuthorisedSubject(request, subject);
+            request.getRequestDispatcher(HttpManagement.DEFAULT_LOGIN_URL).forward(request, response);
+        }
+        catch (AccessControlException e)
+        {
+            LOGGER.info("User '{}' is not authorised for management", authenticationResult.getMainPrincipal());
+            response.sendError(HttpServletResponse.SC_FORBIDDEN, "User is not authorised for management");
+        }
+        catch (SecurityException e)
+        {
+            LOGGER.info("Authentication failed", authenticationResult.getCause());
+            response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+        }
+    }
+
     @Override
     public LogoutHandler getLogoutHandler(final HttpServletRequest request,
                                           final HttpManagementConfiguration configuration)
@@ -119,16 +119,4 @@ public class AnonymousInteractiveAuthenticator implements HttpRequestInteractive
     {
         return ANONYMOUS;
     }
-
-    private String getOriginalRequestUri(final HttpServletRequest request)
-    {
-        StringBuffer originalRequestURL = request.getRequestURL();
-        final String queryString = request.getQueryString();
-        if (queryString != null)
-        {
-            originalRequestURL.append("?").append(queryString);
-        }
-        return originalRequestURL.toString();
-    }
-
 }
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/UsernamePasswordInteractiveLogin.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/UsernamePasswordInteractiveLogin.java
index 4f7b98b..541ee43 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/UsernamePasswordInteractiveLogin.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/UsernamePasswordInteractiveLogin.java
@@ -20,11 +20,7 @@
  */
 package org.apache.qpid.server.management.plugin.auth;
 
-import java.io.IOException;
-
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 
 import org.apache.qpid.server.management.plugin.HttpManagement;
 import org.apache.qpid.server.management.plugin.HttpManagementConfiguration;
@@ -35,16 +31,7 @@ import org.apache.qpid.server.security.auth.manager.UsernamePasswordAuthenticati
 @PluggableService
 public class UsernamePasswordInteractiveLogin implements HttpRequestInteractiveAuthenticator
 {
-    private static final String DEFAULT_LOGIN_URL = "/index.html";
-
-    private static  final LogoutHandler LOGOUT_HANDLER = new LogoutHandler()
-    {
-        @Override
-        public void handleLogout(final HttpServletResponse response) throws IOException
-        {
-            response.sendRedirect(HttpManagement.DEFAULT_LOGOUT_URL);
-        }
-    };
+    private static final LogoutHandler LOGOUT_HANDLER = response -> response.sendRedirect(HttpManagement.DEFAULT_LOGOUT_URL);
 
     @Override
     public AuthenticationHandler getAuthenticationHandler(final HttpServletRequest request,
@@ -52,7 +39,7 @@ public class UsernamePasswordInteractiveLogin implements HttpRequestInteractiveA
     {
         if(configuration.getAuthenticationProvider(request) instanceof UsernamePasswordAuthenticationProvider)
         {
-            return response -> request.getRequestDispatcher(DEFAULT_LOGIN_URL).forward(request, response);
+            return response -> request.getRequestDispatcher(HttpManagement.DEFAULT_LOGIN_URL).forward(request, response);
         }
         else
         {
diff --git a/systests/qpid-systests-http-management/src/test/java/org/apache/qpid/tests/http/authentication/PreemptiveAuthenticationTest.java b/systests/qpid-systests-http-management/src/test/java/org/apache/qpid/tests/http/authentication/PreemptiveAuthenticationTest.java
index dd4efb7..14db61d 100644
--- a/systests/qpid-systests-http-management/src/test/java/org/apache/qpid/tests/http/authentication/PreemptiveAuthenticationTest.java
+++ b/systests/qpid-systests-http-management/src/test/java/org/apache/qpid/tests/http/authentication/PreemptiveAuthenticationTest.java
@@ -116,7 +116,7 @@ public class PreemptiveAuthenticationTest extends HttpTestBase
     {
         HttpTestHelper helper = configForClientAuth("CN=foo");
 
-        HttpURLConnection authenticateConnection = helper.openManagementConnection("/index.html", "GET");
+        HttpURLConnection authenticateConnection = helper.openManagementConnection(HttpManagement.DEFAULT_LOGIN_URL, "GET");
         authenticateConnection.setInstanceFollowRedirects(false);
 
         int status = authenticateConnection.getResponseCode();
@@ -125,7 +125,7 @@ public class PreemptiveAuthenticationTest extends HttpTestBase
 
         assertThat(status, is(equalTo(HttpURLConnection.HTTP_MOVED_TEMP)));
 
-        authenticateConnection = helper.openManagementConnection("/index.html", "GET");
+        authenticateConnection = helper.openManagementConnection(HttpManagement.DEFAULT_LOGIN_URL, "GET");
         authenticateConnection.setRequestProperty("Cookie", cookies);
         status = authenticateConnection.getResponseCode();
         authenticateConnection.disconnect();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


[qpid-broker-j] 02/02: QPID-8459: [Broker-J] Reduce code duplication

Posted by or...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

orudyy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-broker-j.git

commit 26aa2957d696f4909e4c7b4ca8893c2280e9e39b
Author: Alex Rudyy <or...@apache.org>
AuthorDate: Sun Aug 23 21:21:05 2020 +0100

    QPID-8459: [Broker-J] Reduce code duplication
    
    This closes #54
---
 .../management/plugin/HttpManagementUtil.java      |  9 ++++++
 .../auth/AnonymousInteractiveAuthenticator.java    | 21 ++++++-------
 .../auth/OAuth2InteractiveAuthenticator.java       | 35 +++++++---------------
 .../SSLClientCertInteractiveAuthenticator.java     |  5 ++--
 .../auth/SpnegoInteractiveAuthenticator.java       |  5 +---
 .../plugin/servlet/rest/SaslServlet.java           |  4 +--
 6 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
index c9f8e04..3c0783c 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/HttpManagementUtil.java
@@ -361,4 +361,13 @@ public class HttpManagementUtil
             // session was invalidated
         }
     }
+
+    public static void createServletConnectionSubjectAssertManagementAccessAndSave(final Broker broker,
+                                                                                   final HttpServletRequest request,
+                                                                                   final Subject original)
+    {
+        final Subject subject = createServletConnectionSubject(request, original);
+        assertManagementAccess(broker, subject);
+        saveAuthorisedSubject(request, subject);
+    }
 }
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
index c0ad0ab..9d25b63 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/AnonymousInteractiveAuthenticator.java
@@ -56,10 +56,12 @@ public class AnonymousInteractiveAuthenticator implements HttpRequestInteractive
     public AuthenticationHandler getAuthenticationHandler(final HttpServletRequest request,
                                                           final HttpManagementConfiguration configuration)
     {
-        final Port<?> port = configuration.getPort(request);
         if (configuration.getAuthenticationProvider(request) instanceof AnonymousAuthenticationManager)
         {
-            return response -> getLoginHandler(request, response, configuration, port);
+            final AnonymousAuthenticationManager authenticationProvider =
+                    (AnonymousAuthenticationManager) configuration.getAuthenticationProvider(request);
+            final Port<?> port = configuration.getPort(request);
+            return response -> getLoginHandler(request, response, authenticationProvider, port);
         }
         else
         {
@@ -67,25 +69,24 @@ public class AnonymousInteractiveAuthenticator implements HttpRequestInteractive
         }
     }
 
-    private void getLoginHandler(HttpServletRequest request, HttpServletResponse response,
-                                 HttpManagementConfiguration configuration, Port<?> port) throws ServletException, IOException
+    private void getLoginHandler(final HttpServletRequest request,
+                                 final HttpServletResponse response,
+                                 final AnonymousAuthenticationManager authenticationProvider,
+                                 final Port<?> port) throws ServletException, IOException
     {
-        final AnonymousAuthenticationManager authenticationProvider =
-                (AnonymousAuthenticationManager) configuration.getAuthenticationProvider(request);
         final AuthenticationResult authenticationResult = authenticationProvider.getAnonymousAuthenticationResult();
         try
         {
-            final SubjectAuthenticationResult result = port.getSubjectCreator(request.isSecure(), request.getServerName()).createResultWithGroups(authenticationResult);
+            final SubjectAuthenticationResult result = port.getSubjectCreator(request.isSecure(), request.getServerName())
+                        .createResultWithGroups(authenticationResult);
             final Subject original = result.getSubject();
 
             if (original == null)
             {
                 throw new SecurityException("Only authenticated users can access the management interface");
             }
-            final Subject subject = HttpManagementUtil.createServletConnectionSubject(request, original);
             final Broker broker = (Broker) authenticationProvider.getParent();
-            HttpManagementUtil.assertManagementAccess(broker, subject);
-            HttpManagementUtil.saveAuthorisedSubject(request, subject);
+            HttpManagementUtil.createServletConnectionSubjectAssertManagementAccessAndSave(broker, request, original);
             request.getRequestDispatcher(HttpManagement.DEFAULT_LOGIN_URL).forward(request, response);
         }
         catch (AccessControlException e)
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java
index d25f54c..3df47a9 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java
@@ -170,9 +170,17 @@ public class OAuth2InteractiveAuthenticator implements HttpRequestInteractiveAut
                         AuthenticationResult authenticationResult = oauth2Provider.authenticateViaAuthorizationCode(authorizationCode, redirectUri, addressSpace);
                         try
                         {
-                            Subject subject = createSubject(authenticationResult);
-                            authoriseManagement(subject);
-                            HttpManagementUtil.saveAuthorisedSubject(request, subject);
+                            SubjectCreator subjectCreator = port.getSubjectCreator(request.isSecure(), request.getServerName());
+                            SubjectAuthenticationResult result = subjectCreator.createResultWithGroups(authenticationResult);
+                            Subject original = result.getSubject();
+
+                            if (original == null)
+                            {
+                                throw new SecurityException("Only authenticated users can access the management interface");
+                            }
+
+                            Broker broker = (Broker) oauth2Provider.getParent();
+                            HttpManagementUtil.createServletConnectionSubjectAssertManagementAccessAndSave(broker, request, original);
 
                             LOGGER.debug("Successful login. Redirect to original resource {}", originalRequestUri);
                             response.sendRedirect(originalRequestUri);
@@ -191,27 +199,6 @@ public class OAuth2InteractiveAuthenticator implements HttpRequestInteractiveAut
                             }
                         }
                     }
-
-                    private Subject createSubject(final AuthenticationResult authenticationResult)
-                    {
-                        SubjectCreator subjectCreator = port.getSubjectCreator(request.isSecure(), request.getServerName());
-                        SubjectAuthenticationResult result = subjectCreator.createResultWithGroups(authenticationResult);
-                        Subject original = result.getSubject();
-
-                        if (original == null)
-                        {
-                            throw new SecurityException("Only authenticated users can access the management interface");
-                        }
-
-                        Subject subject = HttpManagementUtil.createServletConnectionSubject(request, original);
-                        return subject;
-                    }
-
-                    private void authoriseManagement(final Subject subject)
-                    {
-                        Broker broker = (Broker) oauth2Provider.getParent();
-                        HttpManagementUtil.assertManagementAccess(broker, subject);
-                    }
                 };
             }
         }
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SSLClientCertInteractiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SSLClientCertInteractiveAuthenticator.java
index a2ebde0..bfa1a09 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SSLClientCertInteractiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SSLClientCertInteractiveAuthenticator.java
@@ -49,9 +49,8 @@ public class SSLClientCertInteractiveAuthenticator implements HttpRequestInterac
                 final Subject subject = _preemptiveAuthenticator.attemptAuthentication(request, configuration);
                 if (subject != null)
                 {
-                    final Subject servletSubject = HttpManagementUtil.createServletConnectionSubject(request, subject);
-                    HttpManagementUtil.assertManagementAccess((Broker) authenticationProvider.getParent(), servletSubject);
-                    HttpManagementUtil.saveAuthorisedSubject(request, servletSubject);
+                    final Broker broker = (Broker) authenticationProvider.getParent();
+                    HttpManagementUtil.createServletConnectionSubjectAssertManagementAccessAndSave(broker, request, subject);
                     response.sendRedirect("/");
                 }
                 else
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SpnegoInteractiveAuthenticator.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SpnegoInteractiveAuthenticator.java
index 004a8f4..48c8c7e 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SpnegoInteractiveAuthenticator.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/SpnegoInteractiveAuthenticator.java
@@ -74,11 +74,8 @@ public class SpnegoInteractiveAuthenticator implements HttpRequestInteractiveAut
                     final Port<?> port = configuration.getPort(request);
                     final SubjectCreator subjectCreator = port.getSubjectCreator(request.isSecure(), request.getServerName());
                     final SubjectAuthenticationResult result = subjectCreator.createResultWithGroups(authenticationResult);
-                    final Subject subject = HttpManagementUtil.createServletConnectionSubject(request, result.getSubject());
-
                     final Broker broker = (Broker) kerberosProvider.getParent();
-                    HttpManagementUtil.assertManagementAccess(broker, subject);
-                    HttpManagementUtil.saveAuthorisedSubject(request, subject);
+                    HttpManagementUtil.createServletConnectionSubjectAssertManagementAccessAndSave(broker, request, result.getSubject());
                     request.getRequestDispatcher(HttpManagement.DEFAULT_LOGIN_URL).forward(request, response);
                 }
             };
diff --git a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
index 102062d..f0daefb 100644
--- a/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
+++ b/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/SaslServlet.java
@@ -249,9 +249,7 @@ public class SaslServlet extends AbstractServlet
             Broker broker = getBroker();
             try
             {
-                Subject subject = HttpManagementUtil.createServletConnectionSubject(request, original);
-                HttpManagementUtil.assertManagementAccess(broker, subject);
-                HttpManagementUtil.saveAuthorisedSubject(request, subject);
+                HttpManagementUtil.createServletConnectionSubjectAssertManagementAccessAndSave(broker, request, original);
                 if(challenge != null && challenge.length != 0)
                 {
                     outputObject.put("additionalData", Base64.getEncoder().encodeToString(challenge));


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org