Skip to content

Commit

Permalink
Merge pull request #22 from rsandell/JENKINS-45431
Browse files Browse the repository at this point in the history 
[Fix JENKINS-45431] Moved the config snippet to the correct jelly
  • Loading branch information
@rsandell
rsandell committed Sep 13, 2017
2 parents fae415f + 6103253 commit d181e7f
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 17 deletions.
2 changes: 2 additions & 0 deletions src/main/java/hudson/security/LDAPSecurityRealm.java
View file
Expand Up @@ -1193,6 +1193,7 @@ public LDAPUserDetailsService(WebApplicationContext appContext,
this(findBean(LdapUserSearch.class, appContext), findBean(LdapAuthoritiesPopulator.class, appContext), groupMembershipStrategy, configurationId);
}

@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification = "Only on newer core versions") //TODO remove when core is bumped
public LdapUserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
username = fixUsername(username);
try {
Expand Down Expand Up @@ -1287,6 +1288,7 @@ public LdapUserDetails loadUserByUsername(String username) throws UsernameNotFou
*/
@Extension
public static final class MailAdressResolverImpl extends MailAddressResolver {
@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification = "Only on newer core versions") //TODO remove when core is bumped
public String findMailAddressFor(User u) {
final Jenkins jenkins = Jenkins.getInstance();
if (jenkins == null) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/jenkins/security/plugins/ldap/LDAPConfiguration.java
View file
Expand Up @@ -25,6 +25,7 @@
*/
package jenkins.security.plugins.ldap;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import groovy.lang.Binding;
import hudson.DescriptorExtensionList;
import hudson.Extension;
Expand Down Expand Up @@ -389,6 +390,7 @@ public boolean noCustomBindScript() {
}

// note that this works better in 1.528+ (JENKINS-19124)
@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification = "Only on newer core versions") //TODO remove when core is bumped
public FormValidation doCheckServer(@QueryParameter String value, @QueryParameter String managerDN, @QueryParameter Secret managerPasswordSecret) {
String server = value;
String managerPassword = Secret.toString(managerPasswordSecret);
Expand Down Expand Up @@ -442,6 +444,7 @@ public FormValidation doCheckServer(@QueryParameter String value, @QueryParamete
}
}

@SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification = "Only on newer core versions") //TODO remove when core is bumped
public DescriptorExtensionList<LDAPGroupMembershipStrategy, Descriptor<LDAPGroupMembershipStrategy>> getGroupMembershipStrategies() {
final Jenkins jenkins = Jenkins.getInstance();
if (jenkins != null) {
Expand Down
10 changes: 0 additions & 10 deletions src/main/resources/hudson/security/LDAPSecurityRealm/config.jelly
View file
Expand Up @@ -72,15 +72,5 @@ THE SOFTWARE.
<f:checkbox default="true" title="${%Disable Backward Compatibility for Roles}"/>
</f:entry>
<f:optionalProperty field="cache" title="${%Enable cache}"/>
<f:entry field="environmentProperties" title="${%Environment Properties}">
<!-- TODO JENKINS-22910 must repeat the field attr: -->
<f:repeatableProperty field="environmentProperties">
<f:entry title="">
<div align="right">
<f:repeatableDeleteButton/>
</div>
</f:entry>
</f:repeatableProperty>
</f:entry>
</f:advanced>
</j:jelly>
10 changes: 10 additions & 0 deletions src/main/resources/jenkins/security/plugins/ldap/LDAPConfiguration/config.jelly
View file
Expand Up @@ -37,5 +37,15 @@
<f:entry field="mailAddressAttributeName" title="${%Email Address LDAP attribute}">
<f:textbox default="${descriptor.DEFAULT_MAILADDRESS_ATTRIBUTE_NAME}"/>
</f:entry>
<f:entry field="environmentProperties" title="${%Environment Properties}">
<!-- TODO JENKINS-22910 must repeat the field attr: -->
<f:repeatableProperty field="environmentProperties">
<f:entry title="">
<div align="right">
<f:repeatableDeleteButton/>
</div>
</f:entry>
</f:repeatableProperty>
</f:entry>
</f:advanced>
</j:jelly>
41 changes: 36 additions & 5 deletions src/test/java/hudson/security/LDAPEmbeddedTest.java
View file
Expand Up @@ -24,19 +24,19 @@

package hudson.security;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import hudson.model.User;
import hudson.tasks.MailAddressResolver;
import hudson.tasks.Mailer;
import hudson.util.FormValidation;
import hudson.util.Secret;

import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import jenkins.model.IdStrategy;
import jenkins.security.plugins.ldap.FromGroupSearchLDAPGroupMembershipStrategy;
import jenkins.security.plugins.ldap.FromUserRecordLDAPGroupMembershipStrategy;
import jenkins.security.plugins.ldap.LDAPTestConfiguration;
import jenkins.security.plugins.ldap.LDAPRule;
import jenkins.security.plugins.ldap.LDAPSchema;
import jenkins.security.plugins.ldap.*;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.userdetails.UserDetails;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
Expand All @@ -55,6 +55,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

@LDAPTestConfiguration
public class LDAPEmbeddedTest {
Expand Down Expand Up @@ -488,4 +489,34 @@ public void validate() throws Exception {
assertThat("Always report outer kind as OK", validation.kind, is(FormValidation.Kind.OK));
}

@Test
@LDAPSchema(ldif = "planetexpress", id = "planetexpress", dn = "dc=planetexpress,dc=com")
public void usingEnvironmentProperties() throws Exception {
LDAPConfiguration c = new LDAPConfiguration(ads.getUrl(), "", false, "uid=admin,ou=system", Secret.fromString("pass"));

LDAPSecurityRealm.EnvironmentProperty[] environmentProperties = {new LDAPSecurityRealm.EnvironmentProperty("java.naming.ldap.typesOnly", "true")};
c.setEnvironmentProperties(environmentProperties);

List<LDAPConfiguration> configurations = new ArrayList<LDAPConfiguration>();
configurations.add(c);
LDAPSecurityRealm realm = new LDAPSecurityRealm(
configurations,
false,
new LDAPSecurityRealm.CacheConfiguration(100, 1000),
IdStrategy.CASE_INSENSITIVE,
IdStrategy.CASE_INSENSITIVE
);

r.jenkins.setSecurityRealm(realm);
r.submit(r.createWebClient().goTo("configureSecurity").getFormByName("config"));

try {
r.createWebClient().login("fry", "fry");
fail("Should not be able to login");
} catch (FailingHttpStatusCodeException e) {
System.out.println("Got a bad login==good");
}
}


}
51 changes: 49 additions & 2 deletions src/test/java/hudson/security/LDAPSecurityRealmTest.java
View file
Expand Up @@ -417,7 +417,7 @@ public void customBeanBindingHindersMultiServerConfig() throws IOException, SAXE
//Smoke test
assertThat(form.getCheckedRadioButton("realm"), new LDAPSelectionMatcher());
DomNodeList<HtmlElement> buttons = form.getElementsByTagName("button");
assertThat(buttons, hasItem(new RepeatableDeleteButtonMatcher()));
//assertThat(buttons, hasItem(new RepeatableDeleteButtonMatcher()));
assertThat(buttons, hasItem(new AddServerButtonMatcher()));

//Verify with custom
Expand All @@ -427,12 +427,59 @@ public void customBeanBindingHindersMultiServerConfig() throws IOException, SAXE

assertThat(form.getCheckedRadioButton("realm"), new LDAPSelectionMatcher());
buttons = form.getElementsByTagName("button");
assertThat(buttons, not(hasItem(new RepeatableDeleteButtonMatcher())));
//assertThat(buttons, not(hasItem(new RepeatableDeleteButtonMatcher())));
assertThat(buttons, not(hasItem(new AddServerButtonMatcher())));
assertThat(form.getTextContent(), containsString("Ability to make multiple server configurations turned off due to the presence of custom LDAPBindSecurityRealm.groovy"));

}

@Test
public void configRoundTripEnvironmentProperties() throws Exception {
final String server = "ldap.itd.umich.edu";
final String rootDN = "ou=umich,ou.edu";
final String userSearchBase = "cn=users,ou=umich,ou.edu";
final String managerDN = "cn=admin,ou=umich,ou.edu";
final String managerSecret = "secret";

LDAPConfiguration c = new LDAPConfiguration(server, rootDN, false, managerDN, Secret.fromString(managerSecret));

LDAPSecurityRealm.EnvironmentProperty[] environmentProperties = {new LDAPSecurityRealm.EnvironmentProperty("java.naming.ldap.typesOnly", "true")};
c.setEnvironmentProperties(environmentProperties);
c.setUserSearchBase(userSearchBase);

List<LDAPConfiguration> configurations = new ArrayList<LDAPConfiguration>();
configurations.add(c);
LDAPSecurityRealm realm = new LDAPSecurityRealm(
configurations,
false,
null,
IdStrategy.CASE_INSENSITIVE,
IdStrategy.CASE_INSENSITIVE
);

r.jenkins.setSecurityRealm(realm);

final JenkinsRule.WebClient client = r.createWebClient();
r.submit(client.goTo("configureSecurity").getFormByName("config"));

LDAPSecurityRealm newRealm = (LDAPSecurityRealm) r.jenkins.getSecurityRealm();
assertNotSame(realm, newRealm);
LDAPConfiguration newConfig = newRealm.getConfigurations().get(0);
assertEquals(server, newConfig.getServer());
assertEquals(rootDN, newConfig.getRootDN());
assertEquals(userSearchBase, newConfig.getUserSearchBase());
assertEquals(managerDN, newConfig.getManagerDN());
assertEquals(managerSecret, newConfig.getManagerPassword());
assertThat(newRealm.getUserIdStrategy(), instanceOf(IdStrategy.CaseInsensitive.class));
assertEquals(LDAPSecurityRealm.DescriptorImpl.DEFAULT_USER_SEARCH, newConfig.getUserSearch());
assertEquals(LDAPSecurityRealm.DescriptorImpl.DEFAULT_DISPLAYNAME_ATTRIBUTE_NAME, newConfig.getDisplayNameAttributeName());
assertEquals(LDAPSecurityRealm.DescriptorImpl.DEFAULT_MAILADDRESS_ATTRIBUTE_NAME, newConfig.getMailAddressAttributeName());
assertTrue(newConfig.getEnvironmentProperties().length > 0);
assertEquals(newConfig.getEnvironmentProperties()[0].getName(), c.getEnvironmentProperties()[0].getName());
assertEquals(newConfig.getEnvironmentProperties()[0].getValue(), c.getEnvironmentProperties()[0].getValue());
}


private static class AddServerButtonMatcher extends BaseButtonMatcher {
protected AddServerButtonMatcher() {
super("Add Server");
Expand Down

0 comments on commit d181e7f

Please sign in to comment.