Revert "Build OAuth redirect URL when X-Forwarded-Host is present"
This reverts commit 9398c7877ab279303a29cb01120d681433d30a6b.
Reason for revert: The fix is on the wrong component, as the calculation of the proper canonical web url should have been fixed with Change 386616
Change-Id: I6e41ef307274f7f8afdf1af51bd34da5b067d80c
diff --git a/README.md b/README.md
index 812657c..5a95019 100644
--- a/README.md
+++ b/README.md
@@ -156,49 +156,6 @@
* Add the webhook secret as `webhookSecret` entry in `github` section of
`etc/secure.config`.
-### Enhancing GitHub Sign-In Redirection in a Gerrit Multi-Site setup
-
-To ensure the success of the "Sign in flow," two critical aspects must be
-addressed:
-
-1. **GitHub OAuth Application Registration**:
-When registering Gerrit as a [GitHub OAuth application](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/creating-an-oauth-app),
-it is imperative that the property "Authorization callback URL" includes the
-`primary domain` as the host, i.e `https://my-domain.org/oauth`. This is
-essential to ensure that Gerrit sites like `review-1.my-domain.org` or
-`review-2.my-domain.org` are correctly redirected once authorization has been
-granted by GitHub. This requirement aligns with the documentation on
-[redirect_uri](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#redirect-urls):
- ```
- The redirect_uri parameter is optional. If omitted, GitHub will redirect
- users to the callback URL configured in the OAuth app settings. If
- provided, the redirect URL's host (excluding sub-domains) and port must
- exactly match the callback URL. The redirect URL's path must reference
- a subdirectory of the callback URL.
-```
-
-For a callback URL like `https://my-domain.org/oauth`:
-- Valid `redirect_uri` examples:
-```
-1. https://my-domain.org/oauth
-2. https://my-domain.org/oauth/subdir/other
-3. https://review-1.my-domain.org/oauth
-4. https://review-2.my-domain.org/oauth
-5. https://review-1.my-domain.org/oauth/subdir/other
-```
-- Invalid `redirect_uri` examples:
-```
-1. https://my-domain.org/bar
-2. https://my-domain.org/
-3. https://my-domain.org:8080/oauth
-4. https://review-1.my-domain.org:8080/oauth
-5. https://my-domain.com
-```
-
-2. **Propagation of the X-Forwarded-Host Header**:
-It is essential to ensure the propagation from the upstream proxy of the
-header [X-Forwarded-Host](https://www.rfc-editor.org/rfc/rfc7239.html).
-
### Contributing to the GitHub plugin
The GitHub plugin uses the lombok library, which provides a set of
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
index 6c47ea7..7ef81d1 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
@@ -19,7 +19,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
-import com.google.common.net.HttpHeaders;
import com.google.gerrit.extensions.client.AuthType;
import com.google.gerrit.httpd.CanonicalWebUrl;
import com.google.gerrit.server.config.ConfigUtil;
@@ -29,19 +28,15 @@
import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
import java.io.FileInputStream;
import java.io.IOException;
-import java.net.MalformedURLException;
-import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
-import java.util.stream.Stream;
import javax.servlet.http.HttpServletRequest;
import lombok.Getter;
import org.eclipse.jgit.lib.Config;
@@ -154,32 +149,9 @@
}
public String getOAuthFinalRedirectUrl(HttpServletRequest req) {
- if (req == null) {
- return GERRIT_OAUTH_FINAL;
- }
-
- String canonicalWebUrlAsString = canonicalWebUrl.get(req);
- String forwardedHost = req.getHeader(HttpHeaders.X_FORWARDED_HOST);
- Optional<String> clientHost = extractClientHost(forwardedHost);
- try {
- if (clientHost.isPresent()) {
- URL canonicalWebUrlAsURL = new URL(canonicalWebUrlAsString);
- return new URL(
- canonicalWebUrlAsURL.getProtocol(),
- clientHost.get(),
- canonicalWebUrlAsURL.getPort(),
- GERRIT_OAUTH_FINAL)
- .toString();
- }
-
- return trimTrailingSlash(canonicalWebUrlAsString) + GERRIT_OAUTH_FINAL;
- } catch (MalformedURLException ex) {
- throw new IllegalStateException(
- String.format(
- "Error building the OAuth final redirect url from canonical url: %s and X-Forwarded-Host header: %s",
- canonicalWebUrlAsString, forwardedHost),
- ex);
- }
+ return req == null
+ ? GERRIT_OAUTH_FINAL
+ : trimTrailingSlash(canonicalWebUrl.get(req)) + GERRIT_OAUTH_FINAL;
}
public String getScopeSelectionUrl(HttpServletRequest req) {
@@ -332,19 +304,4 @@
return passwordDevice;
}
-
- /**
- * This method extracts the client host.
- *
- * @param hosts String that represents a list of hosts, i.e "test.example.io,
- * subdomain1.example.io" where the first host from the leftmost is the client host while the
- * rest are the private/internal hosts.
- * @return return Optional with the value client host if exists otherwise Optional empty.
- */
- private Optional<String> extractClientHost(String hosts) {
- if (Strings.isNullOrEmpty(hosts)) {
- return Optional.empty();
- }
- return Stream.of(hosts.split(",")).map(String::trim).filter(h -> !h.isEmpty()).findFirst();
- }
}