You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by lq...@apache.org on 2016/02/10 18:04:16 UTC

svn commit: r1729657 - in /qpid/java/trunk: broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/ broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/ broker-core/src/main/java/org/apache/qpid...

Author: lquack
Date: Wed Feb 10 17:04:16 2016
New Revision: 1729657

URL: http://svn.apache.org/viewvc?rev=1729657&view=rev
Log:
QPID-7028: [Java Broker] Improve OAuth2

* don't send Content-Type header if not needed
* handle authorization endpoints with query strings in their URL
* send the servletRoot URL as redirect_uri to play nice with authorization endpoints with strict uri matching (e.g., google)
* improve logging of failure cases

Modified:
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/OAuth2AuthenticationProviderImpl.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/FacebookIdentityResolverService.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/github/GitHubOAuth2IdentityResolverService.java
    qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/google/GoogleOAuth2IdentityResolverService.java
    qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/OAuth2AuthenticationProviderImpl.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/OAuth2AuthenticationProviderImpl.java?rev=1729657&r1=1729656&r2=1729657&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/OAuth2AuthenticationProviderImpl.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/OAuth2AuthenticationProviderImpl.java Wed Feb 10 17:04:16 2016
@@ -260,16 +260,11 @@ public class OAuth2AuthenticationProvide
             requestBody.put("response_type", "token");
             body = OAuth2Utils.buildRequestQuery(requestBody).getBytes(UTF8);
             connection.connect();
-        }
-        catch (IOException e)
-        {
-            return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, e);
-        }
 
-        try (OutputStream output = connection.getOutputStream())
-        {
-            output.write(body);
-            output.close();
+            try (OutputStream output = connection.getOutputStream())
+            {
+                output.write(body);
+            }
 
             try (InputStream input = OAuth2Utils.getResponseStream(connection))
             {
@@ -283,6 +278,7 @@ public class OAuth2AuthenticationProvide
                                                                                       responseCode,
                                                                                       responseMap.get("error"),
                                                                                       responseMap.get("error_description")));
+                    LOGGER.error("Call to token endpoint failed", e);
                     return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, e);
                 }
 
@@ -290,6 +286,7 @@ public class OAuth2AuthenticationProvide
                 if (accessTokenObject == null)
                 {
                     IllegalStateException e = new IllegalStateException("Token endpoint response did not include 'access_token'");
+                    LOGGER.error("Unexpected token endpoint response", e);
                     return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, e);
                 }
                 String accessToken = String.valueOf(accessTokenObject);
@@ -299,13 +296,14 @@ public class OAuth2AuthenticationProvide
             catch (JsonProcessingException e)
             {
                 IllegalStateException ise = new IllegalStateException(String.format("Token endpoint '%s' did not return json",
-                                                                                    tokenEndpoint),
-                                                                      e);
+                                                                                    tokenEndpoint), e);
+                LOGGER.error("Unexpected token endpoint response", e);
                 return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, ise);
             }
         }
         catch (IOException e)
         {
+            LOGGER.error("Call to token endpoint failed", e);
             return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, e);
         }
     }
@@ -321,6 +319,7 @@ public class OAuth2AuthenticationProvide
         }
         catch (IOException | IdentityResolverException e)
         {
+            LOGGER.error("Call to identity resolver failed", e);
             return new AuthenticationResult(AuthenticationResult.AuthenticationStatus.ERROR, e);
         }
     }

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/FacebookIdentityResolverService.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/FacebookIdentityResolverService.java?rev=1729657&r1=1729656&r2=1729657&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/FacebookIdentityResolverService.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/facebook/FacebookIdentityResolverService.java Wed Feb 10 17:04:16 2016
@@ -115,7 +115,6 @@ public class FacebookIdentityResolverSer
         HttpURLConnection connection = connectionBuilder.build();
 
         connection.setRequestProperty("Accept-Charset", UTF8);
-        connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=" + UTF8);
         connection.setRequestProperty("Accept", "application/json");
         connection.setRequestProperty("Authorization", "Bearer " + accessToken);
 

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/github/GitHubOAuth2IdentityResolverService.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/github/GitHubOAuth2IdentityResolverService.java?rev=1729657&r1=1729656&r2=1729657&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/github/GitHubOAuth2IdentityResolverService.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/github/GitHubOAuth2IdentityResolverService.java Wed Feb 10 17:04:16 2016
@@ -121,7 +121,6 @@ public class GitHubOAuth2IdentityResolve
         HttpURLConnection connection = connectionBuilder.build();
 
         connection.setRequestProperty("Accept-Charset", UTF8);
-        connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=" + UTF8);
         connection.setRequestProperty("Accept", "application/vnd.github.v3+json");
         connection.setRequestProperty("Authorization", "token " + accessToken);
 

Modified: qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/google/GoogleOAuth2IdentityResolverService.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/google/GoogleOAuth2IdentityResolverService.java?rev=1729657&r1=1729656&r2=1729657&view=diff
==============================================================================
--- qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/google/GoogleOAuth2IdentityResolverService.java (original)
+++ qpid/java/trunk/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/oauth2/google/GoogleOAuth2IdentityResolverService.java Wed Feb 10 17:04:16 2016
@@ -125,7 +125,6 @@ public class GoogleOAuth2IdentityResolve
         HttpURLConnection connection = connectionBuilder.build();
 
         connection.setRequestProperty("Accept-Charset", UTF8);
-        connection.setRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=" + UTF8);
         connection.setRequestProperty("Accept", "application/json");
         connection.setRequestProperty("Authorization", "Bearer " + accessToken);
 

Modified: qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java
URL: http://svn.apache.org/viewvc/qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java?rev=1729657&r1=1729656&r2=1729657&view=diff
==============================================================================
--- qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java (original)
+++ qpid/java/trunk/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/auth/OAuth2InteractiveAuthenticator.java Wed Feb 10 17:04:16 2016
@@ -20,6 +20,8 @@
 package org.apache.qpid.server.management.plugin.auth;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.security.SecureRandom;
 import java.util.Enumeration;
 import java.util.HashMap;
@@ -53,6 +55,8 @@ public class OAuth2InteractiveAuthentica
     private static final int STATE_NONCE_BIT_SIZE = 256;
     private static final String STATE_NAME = "stateNonce";
     private static final String TYPE = "OAuth2";
+    private static final String ORIGINAL_REQUEST_URI_SESSION_ATTRIBUTE = "originalRequestURI";
+    private static final String REDIRECT_URI_SESSION_ATTRIBUTE = "redirectURI";
 
     private SecureRandom _random = new SecureRandom();
 
@@ -110,7 +114,8 @@ public class OAuth2InteractiveAuthentica
                     LOGGER.warn("Deny login attempt with wrong state: {}", state);
                     return new FailedAuthenticationHandler(401, "Received request with wrong state: " + state);
                 }
-                final String redirectUri = (String) httpSession.getAttribute("redirectUri");
+                final String redirectUri = (String) httpSession.getAttribute(REDIRECT_URI_SESSION_ATTRIBUTE);
+                final String originalRequestUri = (String) httpSession.getAttribute(ORIGINAL_REQUEST_URI_SESSION_ATTRIBUTE);
                 return new AuthenticationHandler()
                 {
                     @Override
@@ -119,8 +124,8 @@ public class OAuth2InteractiveAuthentica
                         AuthenticationResult authenticationResult = oauth2Provider.authenticateViaAuthorizationCode(authorizationCode, redirectUri);
                         createSubject(authenticationResult);
 
-                        LOGGER.debug("Successful login. Redirect to original resource {}", redirectUri);
-                        response.sendRedirect(redirectUri);
+                        LOGGER.debug("Successful login. Redirect to original resource {}", originalRequestUri);
+                        response.sendRedirect(originalRequestUri);
                     }
 
                     private void createSubject(final AuthenticationResult authenticationResult)
@@ -161,9 +166,11 @@ public class OAuth2InteractiveAuthentica
                                                  final OAuth2AuthenticationProvider oauth2Provider)
     {
         final String redirectUri = getRedirectUri(request);
+        final String originalRequestUri = getOriginalRequestUri(request);
         final String authorizationEndpoint = oauth2Provider.getAuthorizationEndpointURI().toString();
         final HttpSession httpSession = request.getSession();
-        httpSession.setAttribute("redirectUri", redirectUri);
+        httpSession.setAttribute(REDIRECT_URI_SESSION_ATTRIBUTE, redirectUri);
+        httpSession.setAttribute(ORIGINAL_REQUEST_URI_SESSION_ATTRIBUTE, originalRequestUri);
 
         Map<String, String> queryArgs = new HashMap<>();
         queryArgs.put("client_id", oauth2Provider.getClientId());
@@ -175,14 +182,32 @@ public class OAuth2InteractiveAuthentica
             queryArgs.put("scope", oauth2Provider.getScope());
         }
 
-        // TODO: currently we assume, but don't check, that the authorizationEndpointURI does not contain a query string
         StringBuilder urlBuilder = new StringBuilder(authorizationEndpoint);
-        urlBuilder.append("?");
+        String query = oauth2Provider.getAuthorizationEndpointURI().getQuery();
+        if (query == null)
+        {
+            urlBuilder.append("?");
+        }
+        else if (query.length() > 0)
+        {
+            urlBuilder.append("&");
+        }
         urlBuilder.append(OAuth2Utils.buildRequestQuery(queryArgs));
 
         return urlBuilder.toString();
     }
 
+    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();
+    }
+
     private Map<String, String> getRequestParameters(final HttpServletRequest request)
     {
         Map<String, String> requestParameters = new HashMap<>();
@@ -208,13 +233,26 @@ public class OAuth2InteractiveAuthentica
 
     private String getRedirectUri(final HttpServletRequest request)
     {
-        StringBuffer redirectUri = request.getRequestURL();
-        final String queryString = request.getQueryString();
-        if (queryString != null)
+        String servletPath = request.getServletPath() != null ? request.getServletPath() : "";
+        String pathInfo = request.getPathInfo() != null ? request.getPathInfo() : "";
+        final String requestURL = request.getRequestURL().toString();
+        try
+        {
+            URI redirectURI = new URI(requestURL);
+            String redirectString = redirectURI.normalize().toString();
+            if (!redirectString.endsWith(servletPath + pathInfo))
+            {
+                throw new IllegalStateException(String.format("RequestURL has unexpected format '%s'", redirectString));
+            }
+            redirectString = redirectString.substring(0, redirectString.length() - (servletPath.length() + pathInfo.length()));
+
+            return redirectString;
+        }
+        catch (URISyntaxException e)
         {
-            redirectUri.append(queryString);
+            throw new IllegalStateException(String.format("RequestURL has unexpected format '%s'",
+                                                          requestURL), e);
         }
-        return redirectUri.toString();
     }
 
     private String createState(HttpSession session)



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