Skip to content

Commit

Permalink
Fixing LDAP sync / hardening LDAPUtils.saveUser()
Browse files Browse the repository at this point in the history 
This commit aims to address 2 different problems:

- If an error occurs during LDAP sync or user's connection, the whole
  SQL transaction is aborted, so a first error, the whole process is
  discarded. Adding some checkpoints to be able to recover after an
  error (concerning user's connection, the error is related to adding a
  group into DB ; for the LDAP sync job, the error is related to
  removing a user who is still owning some metadatas).

- When a user connects a call is made to LDAPUtils.saveUser(), which
  tries to add groups if not existing. But groupId was potentially null,
  resulting of an Exception being thrown, and the user was not able to
  connect.

Runtime tested on a complete local copy of Aquitaine PIGMA production
environment.
  • Loading branch information
@pmauduit
pmauduit committed Sep 5, 2014
1 parent 9fd0426 commit 6a7e692
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 101 deletions.
25 changes: 18 additions & 7 deletions jeeves/src/main/java/jeeves/resources/dbms/Dbms.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,19 @@

import jeeves.constants.Jeeves;
import jeeves.utils.Log;

import org.jdom.Element;

import javax.sql.DataSource;

import java.io.StringReader;
import java.sql.Connection;
import java.sql.Date;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.sql.Savepoint;
import java.sql.Time;
import java.sql.Timestamp;
import java.sql.Types;
Expand Down Expand Up @@ -73,7 +76,7 @@ public Dbms(DataSource dataSource, String url) throws ClassNotFoundException
this.dataSource = dataSource;
this.url = url;
}

//--------------------------------------------------------------------------
//---
//--- Connection methods
Expand Down Expand Up @@ -215,7 +218,7 @@ public Element selectFull(String query, Map<String, String> formats, Object... a
}
long start = System.currentTimeMillis();
resultSet = stmt.executeQuery();

Element result = buildResponse(resultSet, formats);
long end = System.currentTimeMillis();

Expand Down Expand Up @@ -290,7 +293,7 @@ public int execute(String query, Object... args) throws SQLException
}
finally
{
if(stmt != null) {
if(stmt != null) {
stmt.close();
}
}
Expand Down Expand Up @@ -446,7 +449,7 @@ private String stripIllegalChars(String input) {
output = output.replaceAll(c, "");
}
}

return output;
}

Expand Down Expand Up @@ -482,16 +485,24 @@ private String getArgs(Object[] args)

return sb.toString();
}

/**
* In case DBMS connection was not closed
* due to some error, close connection on
* due to some error, close connection on
* finalize.
*/
protected void finalize() {
disconnect();
}


public Savepoint setSavePoint() throws SQLException {
return conn.setSavepoint();
}

public void rollbackToSavepoint(Savepoint sp) throws SQLException {
conn.rollback(sp);
}

}

//=============================================================================
Expand Down
73 changes: 41 additions & 32 deletions web/src/main/java/org/fao/geonet/kernel/security/ldap/LDAPSynchronizerJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import jeeves.server.resources.ResourceManager;
import jeeves.utils.Log;
import jeeves.utils.SerialFactory;

import org.fao.geonet.constants.Geonet;
import org.jdom.Element;
import org.quartz.JobDataMap;
Expand All @@ -39,71 +40,73 @@
import javax.naming.NamingException;
import javax.naming.directory.DirContext;
import javax.naming.directory.SearchResult;

import java.sql.SQLException;
import java.sql.Savepoint;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class LDAPSynchronizerJob extends QuartzJobBean {

private ApplicationContext applicationContext;

private DefaultSpringSecurityContextSource contextSource;

@Override
protected void executeInternal(JobExecutionContext jobExecContext)
throws JobExecutionException {
try {
if (Log.isDebugEnabled(Geonet.LDAP)) {
Log.debug(Geonet.LDAP, "LDAPSynchronizerJob starting ...");
}

// Retrieve application context. A defautl SpringBeanJobFactory
// will not provide the application context to the job. Use
// AutowiringSpringBeanJobFactory.
applicationContext = (ApplicationContext) jobExecContext
.getJobDetail().getJobDataMap().get("applicationContext");


if (applicationContext == null) {
Log.error(
Geonet.LDAP,
" Application context is null. Be sure to configure SchedulerFactoryBean job factory property with AutowiringSpringBeanJobFactory.");
}

// Get LDAP information defining which users to sync
final JobDataMap jdm = jobExecContext.getJobDetail()
.getJobDataMap();
contextSource = (DefaultSpringSecurityContextSource) jdm
.get("contextSource");

String ldapUserSearchFilter = (String) jdm
.get("ldapUserSearchFilter");
String ldapUserSearchBase = (String) jdm.get("ldapUserSearchBase");
String ldapUserSearchAttribute = (String) jdm
.get("ldapUserSearchAttribute");

DirContext dc = contextSource.getReadOnlyContext();

// Get database
ResourceManager resourceManager = applicationContext
.getBean(ResourceManager.class);
Dbms dbms = null;

try {
dbms = (Dbms) resourceManager.openDirect(Geonet.Res.MAIN_DB);

// Users
synchronizeUser(ldapUserSearchFilter, ldapUserSearchBase,
ldapUserSearchAttribute, dc, dbms);

// And optionaly groups
String createNonExistingLdapGroup = (String) jdm
.get("createNonExistingLdapGroup");

if ("true".equals(createNonExistingLdapGroup)) {
SerialFactory serialFactory = applicationContext
.getBean(SerialFactory.class);

String ldapGroupSearchFilter = (String) jdm
.get("ldapGroupSearchFilter");
String ldapGroupSearchBase = (String) jdm
Expand All @@ -112,7 +115,7 @@ protected void executeInternal(JobExecutionContext jobExecContext)
.get("ldapGroupSearchAttribute");
String ldapGroupSearchPattern = (String) jdm
.get("ldapGroupSearchPattern");

synchronizeGroup(ldapGroupSearchFilter,
ldapGroupSearchBase, ldapGroupSearchAttribute,
ldapGroupSearchPattern, dc, dbms, serialFactory);
Expand Down Expand Up @@ -154,21 +157,21 @@ protected void executeInternal(JobExecutionContext jobExecContext)
e);
e.printStackTrace();
}

if (Log.isDebugEnabled(Geonet.LDAP)) {
Log.debug(Geonet.LDAP, "LDAPSynchronizerJob done.");
}
}


private void synchronizeUser(String ldapUserSearchFilter,
String ldapUserSearchBase, String ldapUserSearchAttribute,
DirContext dc, Dbms dbms) throws NamingException, SQLException {
// Do something for LDAP users ? Currently user is updated on log
// in only.
NamingEnumeration<?> userList = dc.search(ldapUserSearchBase,
ldapUserSearchFilter, null);

// Build a list of LDAP users
StringBuffer usernames = new StringBuffer();
while (userList.hasMore()) {
Expand All @@ -178,7 +181,7 @@ private void synchronizeUser(String ldapUserSearchFilter,
.get());
usernames.append("', ");
}

// Remove LDAP user available in db and not in LDAP if not linked to
// metadata
String query = "SELECT id FROM Users WHERE authtype=? AND username NOT IN ("
Expand All @@ -188,57 +191,63 @@ private void synchronizeUser(String ldapUserSearchFilter,
Element r = (Element) record;
int userId = Integer.valueOf(r.getChildText("id"));
Log.debug(Geonet.LDAP, " - Removing user: " + userId);
Savepoint sp = null;
try {
sp = dbms.setSavePoint();
dbms.execute("DELETE FROM UserGroups WHERE userId=?", userId);
dbms.execute("DELETE FROM Users WHERE authtype=? AND id=?",
LDAPConstants.LDAP_FLAG, userId);

} catch (Exception ex) {
Log.error(Geonet.LDAP, "Failed to remove LDAP user with id "
+ userId
+ " in database. User is probably a metadata owner."
+ " Transfer owner first.", ex);
if (sp != null) {
dbms.rollbackToSavepoint(sp);
}
}
}
}


private void synchronizeGroup(String ldapGroupSearchFilter,
String ldapGroupSearchBase, String ldapGroupSearchAttribute,
String ldapGroupSearchPattern, DirContext dc, Dbms dbms,
SerialFactory serialFactory) throws NamingException, SQLException {

NamingEnumeration<?> groupList = dc.search(ldapGroupSearchBase,
ldapGroupSearchFilter, null);
Pattern ldapGroupSearchPatternCompiled = null;
if (!"".equals(ldapGroupSearchPattern)) {
ldapGroupSearchPatternCompiled = Pattern
.compile(ldapGroupSearchPattern);
}

while (groupList.hasMore()) {
SearchResult sr = (SearchResult) groupList.next();

// TODO : should we retrieve LDAP group id and do an update of group
// name
// This will require to store in local db the remote id
String groupName = (String) sr.getAttributes()
.get(ldapGroupSearchAttribute).get();

if (!"".equals(ldapGroupSearchPattern)) {
Matcher m = ldapGroupSearchPatternCompiled.matcher(groupName);
boolean b = m.matches();
if (b) {
groupName = m.group(1);
}
}

Element groupIdRequest = dbms.select(
"SELECT id FROM Groups WHERE name = ?", groupName);
Element groupRecord = groupIdRequest.getChild("record");
String groupId = null;

if (groupRecord == null) {
LDAPUtils.createIfNotExist(groupName, groupId, dbms, serialFactory);
groupId = LDAPUtils.createIfNotExist(groupName, groupId, dbms, serialFactory);

} else {
groupId = groupRecord.getChildText("id");
Expand All @@ -248,11 +257,11 @@ private void synchronizeGroup(String ldapGroupSearchFilter,
}
}
}

public DefaultSpringSecurityContextSource getContextSource() {
return contextSource;
}

public void setContextSource(
DefaultSpringSecurityContextSource contextSource) {
this.contextSource = contextSource;
Expand Down
Loading

0 comments on commit 6a7e692

Please sign in to comment.