Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP: Improve username case sensitivity support (assist #4821). #3078

Merged
merged 20 commits into from Oct 9, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions components/common/src/ome/system/Principal.java
@@ -1,7 +1,5 @@
/*
* ome.system.Principal
*
* Copyright 2006 University of Dundee. All rights reserved.
* Copyright 2006-2014 University of Dundee. All rights reserved.
* Use is subject to license terms supplied in LICENSE.txt
*/

Expand Down Expand Up @@ -36,6 +34,8 @@ public class Principal implements java.security.Principal, Serializable {

protected String type;

private final PreferenceContext preferences = new PreferenceContext();

/**
* Creates a Principal with null group and event type. These must be taken
* from the session.
Expand All @@ -47,6 +47,10 @@ public Principal(String name) {
}

public Principal(String name, String group, String eventType) {
if (Boolean.parseBoolean(preferences
.getProperty("omero.security.ignore_case"))) {
name = name.toLowerCase();
}
this.name = name;
this.group = group;
this.type = eventType;
Expand Down
Expand Up @@ -114,6 +114,7 @@
</bean>

<bean id="chainedPasswordProvider" class="ome.security.auth.PasswordProviders">
<constructor-arg ref="atomicIgnoreCase"/>
<constructor-arg>
<list>
<ref bean="ldapPasswordProvider"/>
Expand All @@ -122,9 +123,15 @@
</constructor-arg>
</bean>

<bean id="atomicIgnoreCase"
class="java.util.concurrent.atomic.AtomicBoolean">
<constructor-arg value="${omero.security.ignore_case}"/>
</bean>

<bean id="roleProvider" class="ome.security.auth.SimpleRoleProvider">
<constructor-arg ref="securitySystem"/>
<constructor-arg ref="omeroSessionFactory"/>
<constructor-arg ref="atomicIgnoreCase"/>
</bean>

<bean id="passwordUtil" class="ome.security.auth.PasswordUtil">
Expand Down
9 changes: 6 additions & 3 deletions components/server/src/ome/logic/AdminImpl.java
Expand Up @@ -133,7 +133,7 @@ public void setApplicationContext(ApplicationContext ctx)
throws BeansException {
this.context = (OmeroContext) ctx;
}

public AdminImpl(SqlAction sql, SessionFactory osf,
MailSender mailSender, SimpleMailMessage templateMessage,
ACLVoter aclVoter, PasswordProvider passwordProvider,
Expand Down Expand Up @@ -176,7 +176,8 @@ public Experimenter userProxy(final String omeName) {
}

Experimenter e = iQuery.findByString(Experimenter.class, "omeName",
omeName);
roleProvider.isIgnoreCaseLookup() ? omeName.toLowerCase()
: omeName);

if (e == null) {
throw new ApiUsageException("No such experimenter: " + omeName);
Expand Down Expand Up @@ -308,7 +309,9 @@ public Experimenter getExperimenter(final long id) {
@RolesAllowed("user")
public Experimenter lookupExperimenter(final String omeName) {
Experimenter e = iQuery.execute(new UserQ(new Parameters().addString(
"name", omeName)));
"name",
roleProvider.isIgnoreCaseLookup() ? omeName.toLowerCase()
: omeName)));

if (e == null) {
throw new ApiUsageException("No such experimenter: " + omeName);
Expand Down
13 changes: 11 additions & 2 deletions components/server/src/ome/logic/LdapImpl.java
Expand Up @@ -187,8 +187,14 @@ private Experimenter mapUserName(String username, PersonContextMapper mapper) {

if (p.size() == 1 && p.get(0) != null) {
Experimenter e = p.get(0);
if (e.getOmeName().equals(username)) {
return p.get(0);
if (provider.isIgnoreCaseLookup()) {
if (e.getOmeName().equalsIgnoreCase(username)) {
return p.get(0);
}
} else {
if (e.getOmeName().equals(username)) {
return p.get(0);
}
}
}
throw new ApiUsageException(
Expand Down Expand Up @@ -417,6 +423,9 @@ public Experimenter createUser(String username, String password) {
*/
public Experimenter createUser(String username, String password,
boolean checkPassword) {
if (provider.isIgnoreCaseLookup()) {
username = username.toLowerCase();
}
if (iQuery.findByString(Experimenter.class, "omeName", username) != null) {
throw new ValidationException("User already exists: " + username);
}
Expand Down
19 changes: 14 additions & 5 deletions components/server/src/ome/security/auth/PasswordProviders.java
@@ -1,12 +1,12 @@
/*
* $Id$
*
* Copyright 2009 Glencoe Software, Inc. All rights reserved.
* Copyright 2009-2014 Glencoe Software, Inc. All rights reserved.
* Use is subject to license terms supplied in LICENSE.txt
*/

package ome.security.auth;

import java.util.concurrent.atomic.AtomicBoolean;

import org.springframework.util.Assert;

/**
Expand All @@ -20,14 +20,23 @@ public class PasswordProviders implements PasswordProvider {

final private PasswordProvider[] providers;

private AtomicBoolean ignoreCaseLookup;

public PasswordProviders(PasswordProvider... providers) {
this(new AtomicBoolean(false), providers);
}

public PasswordProviders(AtomicBoolean ignoreCaseLookup,
PasswordProvider... providers) {
Assert.notNull(providers);
int l = providers.length;
this.providers = new PasswordProvider[l];
System.arraycopy(providers, 0, this.providers, 0, l);
this.ignoreCaseLookup = ignoreCaseLookup;
}

public boolean hasPassword(String user) {
user = ignoreCaseLookup.get() ? user.toLowerCase() : user;
for (PasswordProvider provider : providers) {
boolean hasPassword = provider.hasPassword(user);
if (hasPassword) {
Expand All @@ -38,6 +47,7 @@ public boolean hasPassword(String user) {
}

public Boolean checkPassword(String user, String password, boolean readOnly) {
user = ignoreCaseLookup.get() ? user.toLowerCase() : user;
for (PasswordProvider provider : providers) {
Boolean rv = provider.checkPassword(user, password, readOnly);
if (rv != null) {
Expand All @@ -49,7 +59,7 @@ public Boolean checkPassword(String user, String password, boolean readOnly) {

public void changePassword(String user, String password)
throws PasswordChangeException {

user = ignoreCaseLookup.get() ? user.toLowerCase() : user;
for (PasswordProvider provider : providers) {
boolean hasPassword = provider.hasPassword(user);
if (hasPassword) {
Expand All @@ -72,5 +82,4 @@ public void changeDistinguisedName(String user, String dn)
}
throw new PasswordChangeException("No provider found:" + user);
}

}
2 changes: 2 additions & 0 deletions components/server/src/ome/security/auth/RoleProvider.java
Expand Up @@ -40,4 +40,6 @@ long createExperimenter(Experimenter experimenter,

void removeGroups(final Experimenter user,
final ExperimenterGroup... groups);

boolean isIgnoreCaseLookup();
}
32 changes: 23 additions & 9 deletions components/server/src/ome/security/auth/SimpleRoleProvider.java
Expand Up @@ -10,13 +10,12 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import ome.conditions.ApiUsageException;
import ome.conditions.ValidationException;
import ome.model.IObject;
import ome.model.internal.Permissions;
import ome.model.internal.Permissions.Right;
import ome.model.internal.Permissions.Role;
import ome.model.meta.Experimenter;
import ome.model.meta.ExperimenterGroup;
import ome.model.meta.GroupExperimenterMap;
Expand All @@ -26,17 +25,17 @@
import ome.tools.hibernate.SecureMerge;
import ome.tools.hibernate.SessionFactory;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.hibernate.Query;
import org.hibernate.Session;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Implements {@link RoleProvider}.
*
*
* Note: All implementations were originally copied from AdminImpl for
* ticket:1226.
*
*
* @author Josh Moore, josh at glencoesoftware.com
* @since 4.0
*/
Expand All @@ -49,9 +48,17 @@ public class SimpleRoleProvider implements RoleProvider {

final protected SessionFactory sf;

private AtomicBoolean ignoreCaseLookup;

public SimpleRoleProvider(SecuritySystem sec, SessionFactory sf) {
this(sec, sf, new AtomicBoolean(false));
}

public SimpleRoleProvider(SecuritySystem sec, SessionFactory sf,
AtomicBoolean ignoreCaseLookup) {
this.sec = sec;
this.sf = sf;
this.ignoreCaseLookup = ignoreCaseLookup;
}

public String nameById(long id) {
Expand Down Expand Up @@ -101,11 +108,14 @@ public long createExperimenter(Experimenter experimenter,
SecureAction action = new SecureMerge(session);

Experimenter e = copyUser(experimenter);
if (isIgnoreCaseLookup()) {
e.setOmeName(e.getOmeName().toLowerCase());
}
e.getDetails().copy(sec.newTransientDetails(e));
e = sec.doAction(action, e);
session.flush();

GroupExperimenterMap link = linkGroupAndUser(defaultGroup, e, false);
linkGroupAndUser(defaultGroup, e, false);
if (null != otherGroups) {
for (ExperimenterGroup group : otherGroups) {
linkGroupAndUser(group, e, false);
Expand Down Expand Up @@ -205,6 +215,10 @@ public <T extends IObject> T updateObject(T... objs) {
session.flush();
}

public boolean isIgnoreCaseLookup() {
return ignoreCaseLookup.get();
}

// ~ Helpers
// =========================================================================

Expand Down Expand Up @@ -232,8 +246,8 @@ protected GroupExperimenterMap linkGroupAndUser(ExperimenterGroup group,
link.getDetails().copy(sec.newTransientDetails(link));

Session session = sf.getSession();
sec.doAction(new SecureMerge(session), userById(e.getId(), session),
link);
sec.<IObject> doAction(new SecureMerge(session),
userById(e.getId(), session), link);
session.flush();
return link;
}
Expand Down
11 changes: 11 additions & 0 deletions components/server/test/ome/services/ldap/LdapIntegrationTest.java
Expand Up @@ -8,6 +8,7 @@
import java.io.File;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;

import ome.api.local.LocalQuery;
import ome.api.local.LocalUpdate;
Expand All @@ -17,6 +18,7 @@
import ome.model.meta.Session;
import ome.security.auth.LdapConfig;
import ome.security.auth.LdapPasswordProvider;
import ome.security.auth.PasswordProviders;
import ome.security.auth.PasswordUtil;
import ome.security.auth.RoleProvider;
import ome.services.sessions.SessionManager;
Expand All @@ -31,6 +33,7 @@
import ome.util.SqlAction;

import org.springframework.aop.target.HotSwappableTargetSource;
import org.springframework.beans.BeansException;
import org.springframework.context.support.FileSystemXmlApplicationContext;
import org.springframework.ldap.core.LdapTemplate;
import org.springframework.ldap.core.support.LdapContextSource;
Expand Down Expand Up @@ -275,6 +278,14 @@ public Object execute(Work work) {
fixture.applicationContext = this.mCtx;
fixture.template = (LdapTemplate) mCtx.getBean("ldapTemplate");
fixture.template.setContextSource(source);
try {
fixture.ignoreCaseLookup = fixture.ctx.getBean("testIgnoreCase",
Boolean.class);
fixture.applicationContext.getBean("atomicIgnoreCase",
AtomicBoolean.class).set(fixture.ignoreCaseLookup);
} catch (BeansException be) {
// skip this fixture
}

InternalServiceFactory isf = new InternalServiceFactory(mCtx);
SqlAction sql = (SqlAction) mCtx.getBean("simpleSqlAction");
Expand Down
19 changes: 16 additions & 3 deletions components/server/test/ome/services/ldap/LdapTest.java
Expand Up @@ -10,6 +10,7 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.naming.NamingException;

Expand All @@ -21,6 +22,8 @@
import ome.model.meta.Experimenter;
import ome.security.auth.LdapConfig;
import ome.security.auth.LdapPasswordProvider;
import ome.security.auth.PasswordProvider;
import ome.security.auth.PasswordProviders;
import ome.security.auth.PasswordUtil;
import ome.security.auth.RoleProvider;
import ome.services.util.Executor;
Expand Down Expand Up @@ -67,6 +70,7 @@ public class Fixture {
LdapPasswordProvider provider;
public LdapTemplate template;
public OmeroContext applicationContext;
boolean ignoreCaseLookup;

public void createUserWithGroup(LdapTest t, final String dn,
String group) {
Expand Down Expand Up @@ -107,6 +111,11 @@ public Object execute(Executor.Work work) {
}

void close() {
if (ignoreCaseLookup) {
applicationContext.getBean("atomicIgnoreCase",
AtomicBoolean.class).set(false);
ignoreCaseLookup = false;
}
ctx.close();
}
}
Expand Down Expand Up @@ -190,6 +199,7 @@ protected Fixture createFixture(File ctxFile) throws Exception {

fixture.provider = new LdapPasswordProvider(new PasswordUtil(sql),
fixture.ldap);

return fixture;
}

Expand All @@ -210,10 +220,12 @@ protected void assertPasses(Fixture fixture, Map<String, List<String>> users)
}

assertNotNull(dn);
assertEquals(user, ldap.findExperimenter(user).getOmeName());
assertEquals(fixture.ignoreCaseLookup ? user.toLowerCase() : user,
ldap.findExperimenter(user).getOmeName());
fixture.createUserWithGroup(this, dn, users.get(user).get(0));
assertNotNull(fixture.createUser(user, "password", true));
fixture.login(user, users.get(user).get(0), "password");
fixture.login(fixture.ignoreCaseLookup ? user.toLowerCase() : user,
users.get(user).get(0), "password");
}
}

Expand Down Expand Up @@ -262,7 +274,8 @@ protected void assertCreateUserFromLdap(Fixture fixture,
}

assertNotNull(dn);
assertEquals(user, ldap.findExperimenter(user).getOmeName());
assertEquals(fixture.ignoreCaseLookup ? user.toLowerCase() : user,
ldap.findExperimenter(user).getOmeName());
fixture.createUserWithGroup(this, dn, users.get(user).get(0));
assertNotNull(fixture.createUser(user));
try {
Expand Down
16 changes: 16 additions & 0 deletions components/server/test/ome/services/ldap/caseInsensitive/test.ldif
@@ -0,0 +1,16 @@
dn: ou=site,dc=ci,dc=eg
objectclass: organizationalUnit
objectClass: top
ou: site

dn: ou=People,ou=site,dc=ci,dc=eg
objectclass: organizationalUnit
objectClass: top
ou: People

dn: cn=caseinsensitive,ou=People,ou=site,dc=ci,dc=eg
cn: caseinsensitive
givenName: Testy
sn: Tester
objectClass: person
userPassword: password