Merge branch 'stable-3.4'
* stable-3.4:
Add missing copyright header to SamlWebFilterIT.java
Remove httpDisplaynameHeader config option
Support display names with non-ISO-8859-1 characters
Change-Id: I80f3619e05364f9bc2eef1df2194e98596b219f3
diff --git a/BUILD b/BUILD
index 00a847d..36550aa 100644
--- a/BUILD
+++ b/BUILD
@@ -10,6 +10,7 @@
resources = glob(["src/main/resources/**"]),
deps = [
"@commons-collections//jar",
+ "@commons-lang//jar",
"@cryptacular//jar",
"@joda-time//jar",
"@opensaml-core//jar",
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index 59f0529..16b7dad 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -15,6 +15,12 @@
)
maven_jar(
+ name = "commons-lang",
+ artifact = "commons-lang:commons-lang:2.6",
+ sha1 = "0ce1edb914c94ebc388f086c6827e8bdeec71ac2",
+ )
+
+ maven_jar(
name = "cryptacular",
artifact = "org.cryptacular:cryptacular:1.2.1",
sha1 = "c470bac7309ac04b0b9529bd7dcb1e0b75954f11",
diff --git a/simplesamlphp/README.md b/simplesamlphp/README.md
index 44646f4..3f15824 100644
--- a/simplesamlphp/README.md
+++ b/simplesamlphp/README.md
@@ -43,7 +43,6 @@
type = HTTP
logoutUrl = http://localhost:8080/simplesaml/saml2/idp/SingleLogoutService.php?ReturnTo=http://localhost:8081
httpHeader = X-SAML-UserName
- httpDisplaynameHeader = X-SAML-DisplayName
httpEmailHeader = X-SAML-EmailHeader
httpExternalIdHeader = X-SAML-ExternalId
autoUpdateAccountActiveStatus = true
diff --git a/src/main/java/com/googlesource/gerrit/plugins/saml/SamlMembership.java b/src/main/java/com/googlesource/gerrit/plugins/saml/SamlMembership.java
index 06940dc..7b91f1e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/saml/SamlMembership.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/saml/SamlMembership.java
@@ -24,9 +24,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.*;
+import com.google.gerrit.server.group.db.GroupDelta;
import com.google.gerrit.server.group.db.GroupsUpdate;
import com.google.gerrit.server.group.db.InternalGroupCreation;
-import com.google.gerrit.server.group.db.InternalGroupUpdate;
import com.google.gerrit.server.notedb.Sequences;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -56,6 +56,7 @@
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<GroupsUpdate> groupsUpdateProvider;
private final Sequences sequences;
+ private final AuthRequest.Factory authRequestFactory;
@Inject
SamlMembership(
@@ -65,7 +66,8 @@
GroupCache groupCache,
IdentifiedUser.GenericFactory userFactory,
@ServerInitiated Provider<GroupsUpdate> groupsUpdateProvider,
- Sequences sequences) {
+ Sequences sequences,
+ AuthRequest.Factory authRequestFactory) {
this.memberAttr = samlConfig.getMemberOfAttr();
this.serverIdent = serverIdent;
this.accountManager = accountManager;
@@ -73,6 +75,7 @@
this.userFactory = userFactory;
this.groupsUpdateProvider = groupsUpdateProvider;
this.sequences = sequences;
+ this.authRequestFactory = authRequestFactory;
}
/**
@@ -84,7 +87,8 @@
public void sync(AuthenticatedUser user, SAML2Profile profile) throws IOException {
Set<AccountGroup.UUID> samlMembership =
Optional.ofNullable((List<?>) profile.getAttribute(memberAttr, List.class))
- .orElse(Collections.emptyList()).stream()
+ .orElse(Collections.emptyList())
+ .stream()
.map(m -> getOrCreateGroup(m.toString()))
.filter(Optional::isPresent)
.map(Optional::get)
@@ -130,9 +134,8 @@
}
private void updateMembers(
- AccountGroup.UUID group, InternalGroupUpdate.MemberModification memberModification) {
- InternalGroupUpdate update =
- InternalGroupUpdate.builder().setMemberModification(memberModification).build();
+ AccountGroup.UUID group, GroupDelta.MemberModification memberModification) {
+ GroupDelta update = GroupDelta.builder().setMemberModification(memberModification).build();
try {
groupsUpdateProvider.get().updateGroup(group, update);
} catch (Exception e) {
@@ -156,8 +159,8 @@
.setNameKey(name)
.setId(groupId)
.build();
- InternalGroupUpdate.Builder groupUpdateBuilder =
- InternalGroupUpdate.builder()
+ GroupDelta.Builder groupUpdateBuilder =
+ GroupDelta.builder()
.setVisibleToAll(false)
.setDescription(samlGroup + " (imported by the SAML plugin)");
return groupsUpdateProvider.get().createGroup(groupCreation, groupUpdateBuilder.build());
@@ -174,7 +177,7 @@
}
private Account.Id getOrCreateAccountId(AuthenticatedUser user) throws IOException {
- AuthRequest authRequest = AuthRequest.forUser(user.getUsername());
+ AuthRequest authRequest = authRequestFactory.createForUser(user.getUsername());
authRequest.setUserName(user.getUsername());
authRequest.setEmailAddress(user.getEmail());
authRequest.setDisplayName(user.getDisplayName());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/saml/SamlWebFilter.java b/src/main/java/com/googlesource/gerrit/plugins/saml/SamlWebFilter.java
index db129f9..5714315 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/saml/SamlWebFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/saml/SamlWebFilter.java
@@ -14,16 +14,20 @@
package com.googlesource.gerrit.plugins.saml;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.Accounts;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.AuthConfig;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
@@ -49,7 +53,6 @@
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
-import org.eclipse.jgit.lib.Config;
import org.pac4j.core.context.J2EContext;
import org.pac4j.core.context.session.SessionStore;
import org.pac4j.core.exception.HttpAction;
@@ -74,12 +77,8 @@
private final SAML2Client saml2Client;
private final SamlConfig samlConfig;
- private final String httpUserNameHeader;
- private final String httpDisplaynameHeader;
- private final String httpEmailHeader;
- private final String httpExternalIdHeader;
+ private final AuthConfig auth;
private final HashSet<String> authHeaders;
- private final boolean userNameToLowerCase;
private final SamlMembership samlMembership;
private final GerritApi gApi;
private final Accounts accounts;
@@ -87,7 +86,8 @@
@Inject
SamlWebFilter(
- @GerritServerConfig Config gerritConfig,
+ AuthConfig auth,
+ @CanonicalWebUrl @Nullable String canonicalUrl,
SitePaths sitePaths,
SamlConfig samlConfig,
SamlMembership samlMembership,
@@ -95,6 +95,7 @@
Accounts accounts,
OneOffRequestContext oneOffRequestContext)
throws IOException {
+ this.auth = auth;
this.samlConfig = samlConfig;
this.samlMembership = samlMembership;
log.debug("Max Authentication Lifetime: " + samlConfig.getMaxAuthLifetimeAttr());
@@ -121,12 +122,11 @@
samlClientConfig.setMaximumAuthenticationLifetime(samlConfig.getMaxAuthLifetimeAttr());
saml2Client = new SAML2Client(samlClientConfig);
- String callbackUrl = gerritConfig.getString("gerrit", null, "canonicalWebUrl") + SAML_CALLBACK;
- httpUserNameHeader = getHeaderFromConfig(gerritConfig, "httpHeader");
- httpDisplaynameHeader = getHeaderFromConfig(gerritConfig, "httpDisplaynameHeader");
- httpEmailHeader = getHeaderFromConfig(gerritConfig, "httpEmailHeader");
- httpExternalIdHeader = getHeaderFromConfig(gerritConfig, "httpExternalIdHeader");
- authHeaders = Sets.newHashSet(httpUserNameHeader, httpEmailHeader, httpExternalIdHeader);
+ authHeaders =
+ Sets.newHashSet(
+ auth.getLoginHttpHeader().toUpperCase(),
+ auth.getHttpEmailHeader().toUpperCase(),
+ auth.getHttpExternalIdHeader().toUpperCase());
if (authHeaders.contains("") || authHeaders.contains(null)) {
throw new RuntimeException("All authentication headers must be set.");
}
@@ -135,9 +135,8 @@
"Unique values for httpUserNameHeader, "
+ "httpEmailHeader and httpExternalIdHeader are required.");
}
- userNameToLowerCase = gerritConfig.getBoolean("auth", "userNameToLowerCase", false);
-
- saml2Client.setCallbackUrl(callbackUrl);
+ checkNotNull(canonicalUrl, "gerrit.canonicalWebUrl must be set in gerrit.config");
+ saml2Client.setCallbackUrl(canonicalUrl + SAML_CALLBACK);
this.gApi = gApi;
this.accounts = accounts;
@@ -245,11 +244,6 @@
saml2Client.redirect(context);
}
- private static String getHeaderFromConfig(Config gerritConfig, String name) {
- String s = gerritConfig.getString("auth", null, name);
- return s == null ? "" : s.toUpperCase();
- }
-
private static boolean isGerritLogin(HttpServletRequest request) {
return request.getRequestURI().indexOf(GERRIT_LOGIN) >= 0;
}
@@ -320,7 +314,7 @@
private String getUserName(SAML2Profile user) {
String username = getAttributeOrElseId(user, samlConfig.getUserNameAttr());
- return userNameToLowerCase ? username.toLowerCase(Locale.US) : username;
+ return auth.isUserNameToLowerCase() ? username.toLowerCase(Locale.US) : username;
}
private static Path ensureExists(Path dataDir) throws IOException {
@@ -348,11 +342,11 @@
@Override
public String getHeader(String name) {
String nameUpperCase = name.toUpperCase();
- if (httpUserNameHeader.equals(nameUpperCase)) {
+ if (auth.getLoginHttpHeader().toUpperCase().equals(nameUpperCase)) {
return user.getUsername();
- } else if (httpEmailHeader.equals(nameUpperCase)) {
+ } else if (auth.getHttpEmailHeader().toUpperCase().equals(nameUpperCase)) {
return user.getEmail();
- } else if (httpExternalIdHeader.equals(nameUpperCase)) {
+ } else if (auth.getHttpExternalIdHeader().toUpperCase().equals(nameUpperCase)) {
return user.getExternalId();
} else {
return super.getHeader(name);
