OAuth: Respect servlet context path in URL for login token
Due to a limitation in Jetty [1] we cannot rely on getPathInfo() from web filter and need to strip the context path manually. [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=28323 GitHub-Bug: https://github.com/davido/gerrit-oauth-provider/issues/6 Change-Id: Ie5e82abfc1b03b5be72769e05665ecd6099d2897
This commit is contained in:
		 David Ostrovsky
					David Ostrovsky
				
			
				
					committed by
					
						 David Pursehouse
						David Pursehouse
					
				
			
			
				
	
			
			
			 David Pursehouse
						David Pursehouse
					
				
			
						parent
						
							5fa42e8d93
						
					
				
				
					commit
					87d15d972d
				
			| @@ -22,6 +22,8 @@ import com.google.gerrit.extensions.auth.oauth.OAuthToken; | |||||||
| import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo; | import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo; | ||||||
| import com.google.gerrit.extensions.auth.oauth.OAuthVerifier; | import com.google.gerrit.extensions.auth.oauth.OAuthVerifier; | ||||||
| import com.google.gerrit.extensions.registration.DynamicItem; | import com.google.gerrit.extensions.registration.DynamicItem; | ||||||
|  | import com.google.gerrit.extensions.restapi.Url; | ||||||
|  | import com.google.gerrit.httpd.CanonicalWebUrl; | ||||||
| import com.google.gerrit.httpd.WebSession; | import com.google.gerrit.httpd.WebSession; | ||||||
| import com.google.gerrit.reviewdb.client.Account; | import com.google.gerrit.reviewdb.client.Account; | ||||||
| import com.google.gerrit.server.account.AccountException; | import com.google.gerrit.server.account.AccountException; | ||||||
| @@ -36,8 +38,6 @@ import org.slf4j.Logger; | |||||||
| import org.slf4j.LoggerFactory; | import org.slf4j.LoggerFactory; | ||||||
|  |  | ||||||
| import java.io.IOException; | import java.io.IOException; | ||||||
| import java.net.URLDecoder; |  | ||||||
| import java.nio.charset.StandardCharsets; |  | ||||||
| import java.security.NoSuchAlgorithmException; | import java.security.NoSuchAlgorithmException; | ||||||
| import java.security.SecureRandom; | import java.security.SecureRandom; | ||||||
|  |  | ||||||
| @@ -53,17 +53,20 @@ class OAuthSession { | |||||||
|   private final String state; |   private final String state; | ||||||
|   private final DynamicItem<WebSession> webSession; |   private final DynamicItem<WebSession> webSession; | ||||||
|   private final AccountManager accountManager; |   private final AccountManager accountManager; | ||||||
|  |   private final CanonicalWebUrl urlProvider; | ||||||
|   private OAuthServiceProvider serviceProvider; |   private OAuthServiceProvider serviceProvider; | ||||||
|   private OAuthToken token; |   private OAuthToken token; | ||||||
|   private OAuthUserInfo user; |   private OAuthUserInfo user; | ||||||
|   private String redirectUrl; |   private String redirectToken; | ||||||
|  |  | ||||||
|   @Inject |   @Inject | ||||||
|   OAuthSession(DynamicItem<WebSession> webSession, |   OAuthSession(DynamicItem<WebSession> webSession, | ||||||
|       AccountManager accountManager) { |       AccountManager accountManager, | ||||||
|  |       CanonicalWebUrl urlProvider) { | ||||||
|     this.state = generateRandomState(); |     this.state = generateRandomState(); | ||||||
|     this.webSession = webSession; |     this.webSession = webSession; | ||||||
|     this.accountManager = accountManager; |     this.accountManager = accountManager; | ||||||
|  |     this.urlProvider = urlProvider; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   boolean isLoggedIn() { |   boolean isLoggedIn() { | ||||||
| @@ -95,7 +98,7 @@ class OAuthSession { | |||||||
|  |  | ||||||
|       if (isLoggedIn()) { |       if (isLoggedIn()) { | ||||||
|         log.debug("Login-SUCCESS " + this); |         log.debug("Login-SUCCESS " + this); | ||||||
|         authenticateAndRedirect(response); |         authenticateAndRedirect(request, response); | ||||||
|         return true; |         return true; | ||||||
|       } else { |       } else { | ||||||
|         response.sendError(SC_UNAUTHORIZED); |         response.sendError(SC_UNAUTHORIZED); | ||||||
| @@ -103,15 +106,22 @@ class OAuthSession { | |||||||
|       } |       } | ||||||
|     } else { |     } else { | ||||||
|       log.debug("Login-PHASE1 " + this); |       log.debug("Login-PHASE1 " + this); | ||||||
|       redirectUrl = request.getRequestURI(); |       redirectToken = request.getRequestURI(); | ||||||
|  |       // We are here in content of filter. | ||||||
|  |       // Due to this Jetty limitation: | ||||||
|  |       // https://bz.apache.org/bugzilla/show_bug.cgi?id=28323 | ||||||
|  |       // we cannot use LoginUrlToken.getToken() method, | ||||||
|  |       // because it relies on getPathInfo() and it is always null here. | ||||||
|  |       redirectToken = redirectToken.substring( | ||||||
|  |           request.getContextPath().length()); | ||||||
|       response.sendRedirect(oauth.getAuthorizationUrl() + |       response.sendRedirect(oauth.getAuthorizationUrl() + | ||||||
|           "&state=" + state); |           "&state=" + state); | ||||||
|       return false; |       return false; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   private void authenticateAndRedirect(HttpServletResponse rsp) |   private void authenticateAndRedirect(HttpServletRequest req, | ||||||
|       throws IOException { |       HttpServletResponse rsp) throws IOException { | ||||||
|     com.google.gerrit.server.account.AuthRequest areq = |     com.google.gerrit.server.account.AuthRequest areq = | ||||||
|         new com.google.gerrit.server.account.AuthRequest(user.getExternalId()); |         new com.google.gerrit.server.account.AuthRequest(user.getExternalId()); | ||||||
|     AuthResult arsp; |     AuthResult arsp; | ||||||
| @@ -164,16 +174,17 @@ class OAuthSession { | |||||||
|     } |     } | ||||||
|  |  | ||||||
|     webSession.get().login(arsp, true); |     webSession.get().login(arsp, true); | ||||||
|     String suffix = redirectUrl.substring( |     String suffix = redirectToken.substring( | ||||||
|         OAuthWebFilter.GERRIT_LOGIN.length() + 1); |         OAuthWebFilter.GERRIT_LOGIN.length() + 1); | ||||||
|     suffix = URLDecoder.decode(suffix, StandardCharsets.UTF_8.name()); |     StringBuilder rdr = new StringBuilder(urlProvider.get(req)); | ||||||
|     rsp.sendRedirect(suffix); |     rdr.append(Url.decode(suffix)); | ||||||
|  |     rsp.sendRedirect(rdr.toString()); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   void logout() { |   void logout() { | ||||||
|     token = null; |     token = null; | ||||||
|     user = null; |     user = null; | ||||||
|     redirectUrl = null; |     redirectToken = null; | ||||||
|     serviceProvider = null; |     serviceProvider = null; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user