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);