From b2551987ad032be1e7e4176d0304e2f47843b2e3 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Wed, 15 Aug 2018 16:09:05 -0700 Subject: [PATCH 1/4] No need to manually force port 80 when X-Forwarded-Port is not specified. This results in a bug when having X-Forwarded-Proto as https and X-Forwarded-Port not specified. It will produce a URL with https and port 80 appended at the end. This bug is prevelant on the most current App Engine Java Flexible environment. It specified the scheme, but no port which causes the code to fail and redirect to a non-existant url. --- .../social/security/provider/OAuth2AuthenticationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java index e679b2e09..e281e78b7 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java @@ -138,7 +138,7 @@ protected StringBuffer getProxyHeaderAwareRequestURL(HttpServletRequest request) String schemeHeader = request.getHeader("X-Forwarded-Proto"); String portHeader = request.getHeader("X-Forwarded-Port"); String scheme = StringUtils.isEmpty(schemeHeader) ? "http" : schemeHeader; - String port = StringUtils.isEmpty(portHeader) ? "80" : portHeader; + String port = StringUtils.isEmpty(portHeader) ? "" : portHeader; if (scheme.equals("http") && port.equals("80")){ port = ""; } From 197cd1e18ce88545094611fd040bb9443103ba0c Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Sat, 13 Oct 2018 15:03:36 -0700 Subject: [PATCH 2/4] If there is no forwarded scheme, use the existing scheme. Previously, if you were using non-proxied https, it would force you to use http. Now it will default to whatever scheme you currently are using. --- .../social/security/provider/OAuth2AuthenticationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java index e281e78b7..e8096d6ec 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java @@ -137,7 +137,7 @@ protected StringBuffer getProxyHeaderAwareRequestURL(HttpServletRequest request) StringBuffer sb = new StringBuffer(); String schemeHeader = request.getHeader("X-Forwarded-Proto"); String portHeader = request.getHeader("X-Forwarded-Port"); - String scheme = StringUtils.isEmpty(schemeHeader) ? "http" : schemeHeader; + String scheme = StringUtils.isEmpty(schemeHeader) ? request.getScheme() : schemeHeader; String port = StringUtils.isEmpty(portHeader) ? "" : portHeader; if (scheme.equals("http") && port.equals("80")){ port = ""; From d7d31f1b0a5699cbdf523935bc66664b30d8f0f2 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Sun, 14 Oct 2018 23:48:13 -0700 Subject: [PATCH 3/4] Move OAuth#AuthenticationService common elements to parent AbstractSocialAuthenticationService. Add Oath2 proxy fix to Oath1 implementation. Fixes #261. --- .../AbstractSocialAuthenticationService.java | 62 +++++++++++++++++++ .../provider/OAuth1AuthenticationService.java | 37 ----------- .../provider/OAuth2AuthenticationService.java | 59 ------------------ 3 files changed, 62 insertions(+), 96 deletions(-) diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java index 961090203..f80932c0a 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java @@ -15,10 +15,15 @@ */ package org.springframework.social.security.provider; +import java.util.HashSet; +import java.util.Set; + import javax.servlet.http.HttpServletRequest; import org.springframework.beans.factory.InitializingBean; import org.springframework.social.connect.Connection; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * @author Stefan Fussennegger @@ -30,6 +35,8 @@ public abstract class AbstractSocialAuthenticationService implements SocialAu private String connectionAddedRedirectUrl; + private Set returnToUrlParameters; + public void afterPropertiesSet() throws Exception { } @@ -52,4 +59,59 @@ public void setConnectionAddedRedirectUrl(String connectionAddedRedirectUrl) { this.connectionAddedRedirectUrl = connectionAddedRedirectUrl; } + public void setReturnToUrlParameters(Set returnToUrlParameters) { + Assert.notNull(returnToUrlParameters, "returnToUrlParameters cannot be null"); + this.returnToUrlParameters = returnToUrlParameters; + } + + public Set getReturnToUrlParameters() { + if (returnToUrlParameters == null) { + returnToUrlParameters = new HashSet(); + } + return returnToUrlParameters; + } + + protected String buildReturnToUrl(HttpServletRequest request) { + StringBuffer sb = getProxyHeaderAwareRequestURL(request); + sb.append("?"); + for (String name : getReturnToUrlParameters()) { + // Assume for simplicity that there is only one value + String value = request.getParameter(name); + + if (value == null) { + continue; + } + sb.append(name).append("=").append(value).append("&"); + } + sb.setLength(sb.length() - 1); // strip trailing ? or & + return sb.toString(); + } + + protected StringBuffer getProxyHeaderAwareRequestURL(HttpServletRequest request) { + String host = request.getHeader("Host"); + if (StringUtils.isEmpty(host)) { + return request.getRequestURL(); + } + StringBuffer sb = new StringBuffer(); + String schemeHeader = request.getHeader("X-Forwarded-Proto"); + String portHeader = request.getHeader("X-Forwarded-Port"); + String scheme = StringUtils.isEmpty(schemeHeader) ? "http" : schemeHeader; + String port = StringUtils.isEmpty(portHeader) ? "" : portHeader; + if (scheme.equals("http") && port.equals("80")) { + port = ""; + } + if (scheme.equals("https") && port.equals("443")) { + port = ""; + } + sb.append(scheme); + sb.append("://"); + sb.append(host); + if (StringUtils.hasLength(port)) { + sb.append(":"); + sb.append(port); + } + sb.append(request.getRequestURI()); + return sb; + } + } diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth1AuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth1AuthenticationService.java index 6527785e3..51476a644 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth1AuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth1AuthenticationService.java @@ -15,9 +15,6 @@ */ package org.springframework.social.security.provider; -import java.util.HashSet; -import java.util.Set; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -45,8 +42,6 @@ public class OAuth1AuthenticationService extends AbstractSocialAuthentication private final Log logger = LogFactory.getLog(getClass()); private static final String OAUTH_TOKEN_ATTRIBUTE = "oauthToken"; - - private Set returnToUrlParameters; private OAuth1ConnectionFactory connectionFactory; @@ -62,18 +57,6 @@ public void setConnectionFactory(OAuth1ConnectionFactory connectionFactory) { this.connectionFactory = connectionFactory; } - public void setReturnToUrlParameters(Set returnToUrlParameters) { - Assert.notNull(returnToUrlParameters, "returnToUrlParameters cannot be null"); - this.returnToUrlParameters = returnToUrlParameters; - } - - public Set getReturnToUrlParameters() { - if (returnToUrlParameters == null) { - returnToUrlParameters = new HashSet(); - } - return returnToUrlParameters; - } - public void afterPropertiesSet() throws Exception { super.afterPropertiesSet(); Assert.notNull(getConnectionFactory(), "connectionFactory"); @@ -114,26 +97,6 @@ public SocialAuthenticationToken getAuthToken(HttpServletRequest request, HttpSe } } - protected String buildReturnToUrl(HttpServletRequest request) { - StringBuffer sb = request.getRequestURL(); - sb.append("?"); - - for (String name : getReturnToUrlParameters()) { - // Assume for simplicity that there is only one value - String value = request.getParameter(name); - - if (value == null) { - continue; - } - sb.append(name).append("=").append(value).append("&"); - - } - - sb.setLength(sb.length() - 1); // strip trailing ? or & - - return sb.toString(); - } - private OAuthToken extractCachedRequestToken(HttpServletRequest request) { OAuthToken requestToken = (OAuthToken) request.getSession().getAttribute(OAUTH_TOKEN_ATTRIBUTE); request.getSession().removeAttribute(OAUTH_TOKEN_ATTRIBUTE); diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java index e8096d6ec..f93a224a3 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/OAuth2AuthenticationService.java @@ -15,9 +15,6 @@ */ package org.springframework.social.security.provider; -import java.util.HashSet; -import java.util.Set; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -42,8 +39,6 @@ public class OAuth2AuthenticationService extends AbstractSocialAuthentication protected final Log logger = LogFactory.getLog(getClass()); private OAuth2ConnectionFactory connectionFactory; - - private Set returnToUrlParameters; private String defaultScope = ""; @@ -59,18 +54,6 @@ public void setConnectionFactory(OAuth2ConnectionFactory connectionFactory) { this.connectionFactory = connectionFactory; } - public void setReturnToUrlParameters(Set returnToUrlParameters) { - Assert.notNull(returnToUrlParameters, "returnToUrlParameters cannot be null"); - this.returnToUrlParameters = returnToUrlParameters; - } - - public Set getReturnToUrlParameters() { - if (returnToUrlParameters == null) { - returnToUrlParameters = new HashSet(); - } - return returnToUrlParameters; - } - /** * @param defaultScope OAuth scope to use, i.e. requested permissions */ @@ -113,48 +96,6 @@ private String generateState(OAuth2ConnectionFactory connectionFactory, HttpS return (state != null) ? state : connectionFactory.generateState(); } - protected String buildReturnToUrl(HttpServletRequest request) { - StringBuffer sb = getProxyHeaderAwareRequestURL(request); - sb.append("?"); - for (String name : getReturnToUrlParameters()) { - // Assume for simplicity that there is only one value - String value = request.getParameter(name); - - if (value == null) { - continue; - } - sb.append(name).append("=").append(value).append("&"); - } - sb.setLength(sb.length() - 1); // strip trailing ? or & - return sb.toString(); - } - - protected StringBuffer getProxyHeaderAwareRequestURL(HttpServletRequest request) { - String host = request.getHeader("Host"); - if (StringUtils.isEmpty(host)) { - return request.getRequestURL(); - } - StringBuffer sb = new StringBuffer(); - String schemeHeader = request.getHeader("X-Forwarded-Proto"); - String portHeader = request.getHeader("X-Forwarded-Port"); - String scheme = StringUtils.isEmpty(schemeHeader) ? request.getScheme() : schemeHeader; - String port = StringUtils.isEmpty(portHeader) ? "" : portHeader; - if (scheme.equals("http") && port.equals("80")){ - port = ""; - } - if (scheme.equals("https") && port.equals("443")){ - port = ""; - } - sb.append(scheme); - sb.append("://"); - sb.append(host); - if (StringUtils.hasLength(port)){ - sb.append(":"); - sb.append(port); - } - sb.append(request.getRequestURI()); - return sb; - } private void setScope(HttpServletRequest request, OAuth2Parameters params) { String requestedScope = request.getParameter("scope"); if (StringUtils.hasLength(requestedScope)) { From 6596ffaee7f09735070f1dfc49c7eeb085a8b933 Mon Sep 17 00:00:00 2001 From: Scott Murphy Date: Sun, 21 Oct 2018 11:46:18 -0400 Subject: [PATCH 4/4] If there is no forwarded scheme, use the existing scheme. Previously If you were using non-proxied https, it would force you to use http. Now it will default to whatever scheme you currently are using. --- .../security/provider/AbstractSocialAuthenticationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java b/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java index f80932c0a..894c989b3 100644 --- a/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java +++ b/spring-social-security/src/main/java/org/springframework/social/security/provider/AbstractSocialAuthenticationService.java @@ -95,7 +95,7 @@ protected StringBuffer getProxyHeaderAwareRequestURL(HttpServletRequest request) StringBuffer sb = new StringBuffer(); String schemeHeader = request.getHeader("X-Forwarded-Proto"); String portHeader = request.getHeader("X-Forwarded-Port"); - String scheme = StringUtils.isEmpty(schemeHeader) ? "http" : schemeHeader; + String scheme = StringUtils.isEmpty(schemeHeader) ? request.getScheme() : schemeHeader; String port = StringUtils.isEmpty(portHeader) ? "" : portHeader; if (scheme.equals("http") && port.equals("80")) { port = "";