Skip to content

Commit

Permalink
[JENKINS-24195] Ignoring irrelevant Active Directory groups
Browse files Browse the repository at this point in the history
Added an option to make Jenkins ignore Active Directory
groups that are not being used by the active
Authorization Strategy. This can significantly improve
performance in environments with a large number of groups
but a small number of corresponding rules defined by the
Authorization Strategy.

Performance tests have been made in an enviroment with
Role Based Authorization Strategy, 7000 jobs and
280 AD groups (where only one of the last groups is
used by the AuthorizationStrategy). The average loading
time of an empty List View went from 6.2 seconds
to 0.2 seconds. An sectioned view with 15 sections
of text went from 36 seconds to 0.6 seconds.

When Matrix Based Authorization was used instead,
loading time of the sectioned view went down from
1.2 seconds to 0.1 seconds.
  • Loading branch information
fredrikpersson committed Aug 11, 2014
1 parent 5679654 commit a256199
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 3 deletions.
16 changes: 16 additions & 0 deletions pom.xml
Expand Up @@ -26,6 +26,10 @@
</developer>
</developers>

<properties>
<powermock.version>1.5.5</powermock.version>
</properties>

<dependencies>
<dependency>
<groupId>org.jvnet.com4j.typelibs</groupId>
Expand All @@ -47,6 +51,18 @@
<artifactId>mailer</artifactId>
<version>1.5</version>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-module-junit4</artifactId>
<version>${powermock.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-api-mockito</artifactId>
<version>${powermock.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<repositories>
Expand Down
Expand Up @@ -34,6 +34,7 @@
import hudson.model.Descriptor;
import hudson.model.Hudson;
import hudson.security.AbstractPasswordBasedSecurityRealm;
import hudson.security.AuthorizationStrategy;
import hudson.security.GroupDetails;
import hudson.security.SecurityRealm;
import hudson.security.TokenBasedRememberMeServices2;
Expand Down Expand Up @@ -134,24 +135,38 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR

private GroupLookupStrategy groupLookupStrategy;

/**
* If true, Jenkins ignores Active Directory groups that are not being used by the active Authorization Strategy.
* This can significantly improve performance in environments with a large number of groups
* but a small number of corresponding rules defined by the Authorization Strategy.
* Groups are considered as used if they are returned by {@link AuthorizationStrategy#getGroups()}.
*/
public final boolean removeIrrelevantGroups;

public ActiveDirectorySecurityRealm(String domain, String site, String bindName, String bindPassword, String server) {
this(domain,site,bindName,bindPassword,server,GroupLookupStrategy.AUTO);
this(domain,site,bindName,bindPassword,server,GroupLookupStrategy.AUTO,false);
}

@DataBoundConstructor
public ActiveDirectorySecurityRealm(String domain, String site, String bindName, String bindPassword, String server, GroupLookupStrategy groupLookupStrategy) {
this(domain,site,bindName,bindPassword,server,groupLookupStrategy,false);
}

@DataBoundConstructor
public ActiveDirectorySecurityRealm(String domain, String site, String bindName,
String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups) {
this.domain = fixEmpty(domain);
this.site = fixEmpty(site);
this.bindName = fixEmpty(bindName);
this.bindPassword = Secret.fromString(fixEmpty(bindPassword));
this.groupLookupStrategy = groupLookupStrategy;
this.removeIrrelevantGroups = removeIrrelevantGroups;

// append default port if not specified
server = fixEmpty(server);
if (server != null) {
if (!server.contains(":")) server += ":3268";
}

this.server = server;
}

Expand Down
Expand Up @@ -24,14 +24,22 @@
package hudson.plugins.active_directory;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;

import hudson.security.SecurityRealm;
import hudson.tasks.Mailer;
import hudson.tasks.Mailer.UserProperty;
import jenkins.model.Jenkins;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.userdetails.User;
import org.acegisecurity.userdetails.UserDetails;
import org.apache.commons.collections.CollectionUtils;

/**
* @author Kohsuke Kawaguchi
Expand Down Expand Up @@ -78,6 +86,29 @@ public String toString() {

@Override
protected void setAuthorities(GrantedAuthority[] authorities) {
SecurityRealm realm = Jenkins.getInstance().getSecurityRealm();
if ((realm instanceof ActiveDirectorySecurityRealm)) {
ActiveDirectorySecurityRealm activeDirectoryRealm = (ActiveDirectorySecurityRealm)realm;
if (activeDirectoryRealm.removeIrrelevantGroups) {
Set<String> referencedGroups = new HashSet<String>();
for (String group : Jenkins.getInstance().getAuthorizationStrategy().getGroups()) {
referencedGroups.add(group.toLowerCase());
}
// We remove irrelevant groups only if the active AuthorizationStrategy has any referenced groups:
if (!referencedGroups.isEmpty()) {
List<GrantedAuthority> relevantGroups = new ArrayList<GrantedAuthority>();

for (GrantedAuthority group : authorities) {
String groupName = group.getAuthority();
if (groupName != null && referencedGroups.contains(groupName.toLowerCase())) {
relevantGroups.add(group);
}
}
authorities = relevantGroups.toArray(new GrantedAuthority[relevantGroups.size()]);
}
}
}

super.setAuthorities(authorities);
StringBuffer sb = new StringBuffer();
sb.append(super.toString()).append(": ");
Expand Down
Expand Up @@ -35,6 +35,9 @@
<f:entry field="groupLookupStrategy" title="${%Group Membership Lookup Strategy}">
<f:select />
</f:entry>
<f:entry field="removeIrrelevantGroups" title="${%Remove irrelevant groups}">
<f:checkbox />
</f:entry>
</f:advanced>
<f:nested>
<f:validateButton with="domain,site,bindName,bindPassword,server" title="${%Test}" method="validate"/>
Expand Down
@@ -0,0 +1,6 @@
<div>
Makes Jenkins ignore every user's Active Directory groups that are not being used by the active Authorization Strategy.
This can significantly improve performance in environments with a large number of groups
but a small number of corresponding rules defined by the Authorization Strategy.
The user is required to re-login in order for the changes to take effect.
</div>
@@ -0,0 +1,140 @@
/*
* The MIT License
*
* Copyright (c) 2014 Sony Mobile Communications Inc. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.plugins.active_directory;

import hudson.security.AuthorizationStrategy;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.GrantedAuthorityImpl;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.jvnet.hudson.test.JenkinsRule;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.modules.junit4.PowerMockRunner;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.powermock.api.mockito.PowerMockito.when;

/**
* Tests for the "Remove Irrelevant Groups" feature.
*
* @author Fredrik Persson &lt;fredrik6.persson@sonymobile.com&gt;
*/
@RunWith(PowerMockRunner.class)
@PowerMockIgnore("javax.crypto.*")
public class RemoveIrrelevantGroupsTest {

@Rule
public JenkinsRule jenkinsRule = new JenkinsRule();

/**
* Sets up Jenkins with an {@link ActiveDirectorySecurityRealm}.
*/
@Before
public void setUp() {
ActiveDirectorySecurityRealm securityRealm = new ActiveDirectorySecurityRealm("domain.com", null, null,
null, null, GroupLookupStrategy.AUTO, true);
jenkinsRule.getInstance().setSecurityRealm(securityRealm);
}

/**
* Makes Jenkins.getInstance().getAuthorizationStrategy.getGroups()
* return argument groups.
*/
private void setUpJenkinsUsedGroups(String... groups) {
Set<String> usedGroups = new HashSet<String>();
for (String group : groups) {
usedGroups.add(group);
}

AuthorizationStrategy authorizationStrategy = PowerMockito.mock(AuthorizationStrategy.class);
when(authorizationStrategy.getGroups()).thenReturn(usedGroups);
jenkinsRule.getInstance().setAuthorizationStrategy(authorizationStrategy);
}

/**
* Verifies that only the relevant user groups are set when calling the
* constructor for {@link ActiveDirectoryUserDetail} and there are
* some relevant groups.
*/
@Test
public void testSomeGroupsAreRelevant() {
setUpJenkinsUsedGroups("UsedGroup-1", "UsedGroup-2");

GrantedAuthority[] userGroups = {
new GrantedAuthorityImpl("UsedGroup-1"),
new GrantedAuthorityImpl("UsedGroup-2"),
new GrantedAuthorityImpl("UnusedGroup")};

ActiveDirectoryUserDetail user = new ActiveDirectoryUserDetail("Username", null,
true, true, true, true, userGroups, null, null, null);

GrantedAuthority[] relevantUserGroups = Arrays.copyOf(userGroups, 2); //The first two are relevant
assertArrayEquals(relevantUserGroups, user.getAuthorities());
}

/**
* Verifies that no user groups are set when calling the
* constructor for {@link ActiveDirectoryUserDetail} and there are no
* relevant groups.
*/
@Test
public void testNoGroupsAreRelevant() {
setUpJenkinsUsedGroups("UsedGroup-1", "UsedGroup-2");

GrantedAuthority[] userGroups = {
new GrantedAuthorityImpl("UnusedGroup")};

ActiveDirectoryUserDetail user = new ActiveDirectoryUserDetail("Username", null,
true, true, true, true, userGroups, null, null, null);

assertEquals(0, user.getAuthorities().length);
}

/**
* Verifies that all argument groups are set when {@link AuthorizationStrategy#getGroups()}
* returns an empty list.
*/
@Test
public void testNoGroupsAreRegistered() {
setUpJenkinsUsedGroups(); //No registered groups by the AuthorizationStrategy.

GrantedAuthority[] userGroups = {
new GrantedAuthorityImpl("UnusedGroup")};

ActiveDirectoryUserDetail user = new ActiveDirectoryUserDetail("Username", null,
true, true, true, true, userGroups, null, null, null);

assertArrayEquals(userGroups, user.getAuthorities());
}

}

0 comments on commit a256199

Please sign in to comment.