Skip to content

Commit

Permalink
Merge pull request #10 from fredrikpersson/removegroups
Browse files Browse the repository at this point in the history
[JENKINS-24195] Ignoring irrelevant Active Directory groups
  • Loading branch information
rsandell committed Aug 27, 2014
2 parents 5679654 + 962f2c9 commit 90c0688
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 4 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 @@ -11,7 +11,10 @@
</f:entry>
<f:entry title="${%Domain controller}" field="server">
<f:textbox />
</f:entry>
</f:entry>
<f:entry field="removeIrrelevantGroups" title="${%Remove irrelevant groups}">
<f:checkbox />
</f:entry>
</f:advanced>
</j:when>
<j:otherwise>
Expand All @@ -35,6 +38,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 90c0688

Please sign in to comment.