From b44c6b8ce9e19f90badc953892aa5f80196f1de4 Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Wed, 3 Dec 2014 18:52:12 -0800 Subject: [PATCH] CSPACE-6520: Uncommented ReportPostInitHandler block in correct (unified config) tenant bindings prototype file. Moved several database-related utility methods out of ServiceMain and ReportPostInitHandler into JDBC Tools. Now initiate, and close, individual DB connections locally within these utility methods, rather than using passed-in connections, with the goal of improving clarity through isolation. --- .../tenants/tenant-bindings-proto-unified.xml | 17 +- .../tenants/tenant-bindings-proto.xml | 7 - .../services/common/ServiceMain.java | 66 ++----- .../services/common/storage/JDBCTools.java | 167 +++++++++++++++++- .../report/nuxeo/ReportPostInitHandler.java | 78 ++------ 5 files changed, 200 insertions(+), 135 deletions(-) diff --git a/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto-unified.xml b/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto-unified.xml index 1f38e8542..7c2babe7e 100644 --- a/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto-unified.xml +++ b/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto-unified.xml @@ -357,18 +357,21 @@ org.collectionspace.services.report.nuxeo.ReportValidatorHandler - - + + reporterRoleName + reporter + + + + readerRoleName + reader + - --> org.collectionspace.services.report.nuxeo.ReportValidatorHandler org.collectionspace.services.report.nuxeo.ReportPostInitHandler - - - - reporterRoleName - reporter - - readerRoleName diff --git a/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java b/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java index d08e8231a..af5517558 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java +++ b/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java @@ -6,7 +6,6 @@ package org.collectionspace.services.common; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileWriter; import java.io.InputStream; import java.sql.Connection; import java.sql.PreparedStatement; @@ -342,6 +341,12 @@ public class ServiceMain { if (list != null && list.size() > 0) { org.collectionspace.services.config.service.InitHandler handlerType = list.get(0); // REM - 12/2012: We might want to think about supporting multiple post-init handlers String initHandlerClassname = handlerType.getClassname(); + if (Tools.isEmpty(initHandlerClassname)) { + continue; + } + if (logger.isInfoEnabled()) { + logger.info(String.format("Firing post-init handler %s ...", initHandlerClassname)); + } List fields = handlerType.getParams().getField(); @@ -522,7 +527,9 @@ public class ServiceMain { List repoDomainList = tenantBinding.getRepositoryDomain(); for (RepositoryDomainType repoDomain : repoDomainList) { String repoDomainName = repoDomain.getName(); - String dbName = JDBCTools.getDatabaseName(repoDomain.getRepositoryName(), getCspaceInstanceId()); + String repositoryName = repoDomain.getRepositoryName(); + String cspaceInstanceId = getCspaceInstanceId(); + String dbName = JDBCTools.getDatabaseName(repositoryName, cspaceInstanceId); if (nuxeoDBsChecked.contains(dbName)) { if (logger.isDebugEnabled()) { logger.debug("Another user of db: " + dbName + ": Repo: " + repoDomainName @@ -545,9 +552,9 @@ public class ServiceMain { } } else { // Create the user as needed - createUserIfNotExists(conn, dbType, nuxeoUser, nuxeoPW); + JDBCTools.createNewDatabaseUser(JDBCTools.CSADMIN_DATASOURCE_NAME, repositoryName, cspaceInstanceId, dbType, nuxeoUser, nuxeoPW); if (readerUser != null) { - createUserIfNotExists(conn, dbType, readerUser, readerPW); + JDBCTools.createNewDatabaseUser(JDBCTools.CSADMIN_DATASOURCE_NAME, repositoryName, cspaceInstanceId, dbType, readerUser, readerPW); } // Create the database createDatabaseWithRights(conn, dbType, dbName, nuxeoUser, nuxeoPW, readerUser, readerPW); @@ -573,57 +580,6 @@ public class ServiceMain { } - private void createUserIfNotExists(Connection conn, DatabaseProductType dbType, String username, String userPW) - throws Exception { - PreparedStatement pstmt = null; - Statement stmt = null; - final String USER_EXISTS_QUERY_PSQL = "SELECT 1 AS result FROM pg_roles WHERE rolname=?"; - String userExistsQuery; - - if (dbType == DatabaseProductType.POSTGRESQL) { - userExistsQuery = USER_EXISTS_QUERY_PSQL; - } else { - throw new UnsupportedOperationException("CreateUserIfNotExists only supports PSQL - MySQL NYI!"); - } - - try { - pstmt = conn.prepareStatement(userExistsQuery); // create a - // statement - pstmt.setString(1, username); // set dbName param - ResultSet rs = pstmt.executeQuery(); - // extract data from the ResultSet - boolean userExists = rs.next(); - rs.close(); - if (userExists) { - if (logger.isDebugEnabled()) { - logger.debug("User: " + username + " already exists."); - } - } else { - stmt = conn.createStatement(); - String sql = "CREATE ROLE " + username + " WITH PASSWORD '" + userPW + "' LOGIN"; - stmt.executeUpdate(sql); - // Really should do the grants as well. - if (logger.isDebugEnabled()) { - logger.debug("Created Users: '" + username + "' and 'reader'"); - } - } - } catch (Exception e) { - logger.error("createUserIfNotExists failed on exception: " + e.getLocalizedMessage()); - throw e; // propagate - } finally { // close resources - try { - if (pstmt != null) { - pstmt.close(); - } - if (stmt != null) { - stmt.close(); - } - } catch (SQLException se) { - // nothing we can do - } - } - } - private void createDatabaseWithRights(Connection conn, DatabaseProductType dbType, String dbName, String ownerName, String ownerPW, String readerName, String readerPW) throws Exception { Statement stmt = null; diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/JDBCTools.java b/services/common/src/main/java/org/collectionspace/services/common/storage/JDBCTools.java index f47c35b38..55b27395d 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/JDBCTools.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/JDBCTools.java @@ -17,7 +17,6 @@ */ package org.collectionspace.services.common.storage; -import org.collectionspace.services.common.ServiceMain; import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.config.ConfigUtils; import org.slf4j.Logger; @@ -45,6 +44,7 @@ import javax.sql.rowset.RowSetFactory; import javax.sql.rowset.RowSetProvider; import org.apache.tomcat.dbcp.dbcp.BasicDataSource; +import org.collectionspace.services.common.ServiceMain; /** * User: laramie @@ -56,13 +56,15 @@ public class JDBCTools { public static String CSPACE_DATASOURCE_NAME = "CspaceDS"; public static String NUXEO_DATASOURCE_NAME = "NuxeoDS"; // Default database names -// public static String DEFAULT_CSPACE_DATABASE_NAME = ConfigUtils.DEFAULT_CSPACE_DATABASE_NAME; + // public static String DEFAULT_CSPACE_DATABASE_NAME = ConfigUtils.DEFAULT_CSPACE_DATABASE_NAME; public static String DEFAULT_NUXEO_REPOSITORY_NAME = ConfigUtils.DEFAULT_NUXEO_REPOSITORY_NAME; public static String DEFAULT_NUXEO_DATABASE_NAME = ConfigUtils.DEFAULT_NUXEO_DATABASE_NAME; public static String CSADMIN_DATASOURCE_NAME = "CsadminDS"; public static String NUXEO_READER_DATASOURCE_NAME = "NuxeoReaderDS"; public static String NUXEO_USER_NAME = "nuxeo"; public static String SQL_WILDCARD = "%"; + public static String DATABASE_SELECT_PRIVILEGE_NAME = "SELECT"; + // // Private constants @@ -517,6 +519,165 @@ public class JDBCTools { return databaseName; } + + /** + * Grant a specified privilege to a database user. This privilege will + * be applied to all 'public' schema tables within the specified repository. + * + * @param dataSourceName a JDBC datasource name. + * @param repositoryName a repository (e.g. RDBMS database) name. + * @param cspaceInstanceId a CollectionSpace instance identifier. + * @param privilegeName a database privilege (e.g. SELECT) to be granted. + * @param databaseUserName a database user to receive the privilege grant. + */ + public static void grantPrivilegeToDatabaseUser(String dataSourceName, String repositoryName, + String cspaceInstanceId, String privilegeName, String databaseUserName) { + Statement stmt = null; + Connection conn = null; + String sql = String.format("GRANT %s ON ALL TABLES IN SCHEMA public TO %s", privilegeName, databaseUserName); + try { + DatabaseProductType databaseProductType = JDBCTools.getDatabaseProductType(dataSourceName, repositoryName, + cspaceInstanceId); + if (databaseProductType == DatabaseProductType.MYSQL) { + // Nothing to do here: MYSQL already does wildcard grants in init_db.sql + } else if(databaseProductType != DatabaseProductType.POSTGRESQL) { + throw new Exception("Unrecognized database system " + databaseProductType); + } else { + String databaseName = JDBCTools.getDatabaseName(repositoryName, cspaceInstanceId); + // Verify that the database user exists before executing the grant + if (hasDatabaseUser(dataSourceName, repositoryName, cspaceInstanceId, + databaseProductType, databaseUserName)) { + conn = getConnection(dataSourceName, repositoryName, cspaceInstanceId); + stmt = conn.createStatement(); + stmt.execute(sql); + } + } + + } catch (SQLException sqle) { + SQLException tempException = sqle; + // SQLExceptions can be chained. Loop to log all. + while (null != tempException) { + logger.debug("SQL Exception: " + sqle.getLocalizedMessage()); + tempException = tempException.getNextException(); + } + logger.debug("SQL problem in executeQuery: ", sqle); + } catch (Throwable e) { + logger.debug(String.format("Problem granting privileges to database user: %s SQL: %s ERROR: %s", + databaseUserName, sql, e)); + } finally { + try { + if (stmt != null) { + stmt.close(); + } + if (conn != null) { + conn.close(); + } + } catch (SQLException sqle) { + // nothing we can do here except log + logger.warn("SQL Exception when closing statement/connection: " + sqle.getLocalizedMessage()); + } + } + } + + /** + * Create a database user, if that user doesn't already exist. + * + * @param conn a database connection. + * @param dbType a database product type. + * @param username the name of the database user to create. + * @param userPW the initial password for that database user. + */ + public static void createNewDatabaseUser(String dataSourceName, String repositoryName, + String cspaceInstanceId, DatabaseProductType dbType, String username, String userPW) throws Exception { + Statement stmt = null; + Connection conn = null; + if (dbType != DatabaseProductType.POSTGRESQL) { + throw new UnsupportedOperationException("createNewDatabaseUser only supports PostgreSQL"); + } + try { + if (hasDatabaseUser(dataSourceName, repositoryName, cspaceInstanceId, dbType, username)) { + if (logger.isDebugEnabled()) { + logger.debug("User: " + username + " already exists."); + } + } else { + conn = getConnection(dataSourceName, repositoryName, cspaceInstanceId); + stmt = conn.createStatement(); + String sql = "CREATE ROLE " + username + " WITH PASSWORD '" + userPW + "' LOGIN"; + stmt.executeUpdate(sql); + if (logger.isDebugEnabled()) { + logger.debug("Created User: " + username); + } + } + } catch (Exception e) { + logger.error("createNewDatabaseUser failed on exception: " + e.getLocalizedMessage()); + throw e; + } finally { + try { + if (stmt != null) { + stmt.close(); + } + if (conn != null) { + conn.close(); + } + } catch (SQLException sqle) { + // nothing we can do here except log + logger.warn("SQL Exception when closing statement/connection: " + sqle.getLocalizedMessage()); + } + } + } + + /** + * Identify whether a database user already exists. + * + * @param dataSourceName a JDBC datasource name. + * @param repositoryName a repository (e.g. RDBMS database) name. + * @param cspaceInstanceId a CollectionSpace instance identifier. + * @param dbType a database product type. + * @param username the name of the database user to create. + * @param userPW the initial password for that database user. + */ + public static boolean hasDatabaseUser(String dataSourceName, String repositoryName, + String cspaceInstanceId, DatabaseProductType dbType, String username) throws Exception { + PreparedStatement pstmt = null; + Statement stmt = null; + Connection conn = null; + final String USER_EXISTS_QUERY_POSTGRESQL = "SELECT 1 AS result FROM pg_roles WHERE rolname=?"; + if (dbType != DatabaseProductType.POSTGRESQL) { + throw new UnsupportedOperationException("hasDatabaseUser only supports PostgreSQL"); + } + try { + conn = getConnection(dataSourceName, repositoryName, cspaceInstanceId); + pstmt = conn.prepareStatement(USER_EXISTS_QUERY_POSTGRESQL); + pstmt.setString(1, username); + ResultSet rs = pstmt.executeQuery(); + boolean userExists = rs.next(); // Will return a value of 1 if user exists + rs.close(); + return userExists; + } catch (Exception e) { + logger.error("hasDatabaseUser failed on exception: " + e.getLocalizedMessage()); + throw e; + } finally { + try { + if (pstmt != null) { + pstmt.close(); + } + if (stmt != null) { + stmt.close(); + } + if (conn != null) { + conn.close(); + } + } catch (SQLException sqle) { + // nothing we can do here except log + logger.warn("SQL Exception when closing statement/connection: " + sqle.getLocalizedMessage()); + } + } + } + + + // ----------------------------- + // Utility methods for debugging + // ----------------------------- /** * Prints metadata, such as database username and connection URL, @@ -543,7 +704,7 @@ public class JDBCTools { * @param rs a ResultSet. * @throws SQLException */ - public void printResultSetMetaData(ResultSet rs) throws SQLException { + public static void printResultSetMetaData(ResultSet rs) throws SQLException { if (rs == null) { return; } diff --git a/services/report/service/src/main/java/org/collectionspace/services/report/nuxeo/ReportPostInitHandler.java b/services/report/service/src/main/java/org/collectionspace/services/report/nuxeo/ReportPostInitHandler.java index 5a9c5c212..2ec797382 100644 --- a/services/report/service/src/main/java/org/collectionspace/services/report/nuxeo/ReportPostInitHandler.java +++ b/services/report/service/src/main/java/org/collectionspace/services/report/nuxeo/ReportPostInitHandler.java @@ -49,16 +49,15 @@ import org.slf4j.LoggerFactory; public class ReportPostInitHandler extends InitHandler implements IInitHandler { final Logger logger = LoggerFactory.getLogger(ReportPostInitHandler.class); - public static final String DATABASE_SELECT_PRIVILEGE_NAME = "SELECT"; +// public static final String REPORTER_ROLE_NAME_KEY = "reporterRoleName"; +// public static final String DEFAULT_REPORTER_ROLE_NAME = "reporter" + ServiceMain.getInstance().getCspaceInstanceId(); +// private String reporterRoleName = DEFAULT_REPORTER_ROLE_NAME; + // Currently retained for backward compatibility public static final String READER_ROLE_NAME_KEY = "readerRoleName"; public static final String DEFAULT_READER_ROLE_NAME = "reader" + ServiceMain.getInstance().getCspaceInstanceId(); private String readerRoleName = DEFAULT_READER_ROLE_NAME; - - public static final String REPORTER_ROLE_NAME_KEY = "reporterRoleName"; - public static final String DEFAULT_REPORTER_ROLE_NAME = "reporter" + ServiceMain.getInstance().getCspaceInstanceId(); - private String reporterRoleName = DEFAULT_REPORTER_ROLE_NAME; /** See the class javadoc for this class: it shows the syntax supported in the configuration params. */ @@ -71,14 +70,14 @@ public class ReportPostInitHandler extends InitHandler implements IInitHandler { List propertyList) throws Exception { //Check for existing privileges, and if not there, grant them for(Property prop : propertyList) { - if(REPORTER_ROLE_NAME_KEY.equals(prop.getKey())) { - String value = prop.getValue(); - if(Tools.notEmpty(value) && !DEFAULT_REPORTER_ROLE_NAME.equals(value)){ - reporterRoleName = value + ServiceMain.getInstance().getCspaceInstanceId(); - logger.debug("ReportPostInitHandler: overriding reporterRoleName default value to use: " - + value); - } - } +// if(REPORTER_ROLE_NAME_KEY.equals(prop.getKey())) { +// String value = prop.getValue(); +// if(Tools.notEmpty(value) && !DEFAULT_REPORTER_ROLE_NAME.equals(value)){ +// reporterRoleName = value + ServiceMain.getInstance().getCspaceInstanceId(); +// logger.debug("ReportPostInitHandler: overriding reporterRoleName default value to use: " +// + value); +// } +// } // FIXME: Currently retained for backward compatibility; remove this block when appropriate if(READER_ROLE_NAME_KEY.equals(prop.getKey())) { String value = prop.getValue(); @@ -89,57 +88,10 @@ public class ReportPostInitHandler extends InitHandler implements IInitHandler { } } } - String privilegeName = DATABASE_SELECT_PRIVILEGE_NAME; - grantPrivilegeToDatabaseRole(dataSourceName, repositoryName, cspaceInstanceId, privilegeName, reporterRoleName); + String privilegeName = JDBCTools.DATABASE_SELECT_PRIVILEGE_NAME; +// JDBCTools.grantPrivilegeToDatabaseUser(dataSourceName, repositoryName, cspaceInstanceId, privilegeName, reporterRoleName); // FIXME: Currently retained for backward compatibility; remove the following line when appropriate - grantPrivilegeToDatabaseRole(dataSourceName, repositoryName, cspaceInstanceId, privilegeName, readerRoleName); - } - - // FIXME: This method might be refactorable / movable to the - // org.collectionspace.services.common.storage.JDBCTools class. - // If so, any database privilege constants here should be moved with it. - private void grantPrivilegeToDatabaseRole(String dataSourceName, String repositoryName, String cspaceInstanceId, - String privilegeName, String roleName) { - Connection conn = null; - Statement stmt = null; - String sql = ""; - try { - DatabaseProductType databaseProductType = JDBCTools.getDatabaseProductType(dataSourceName, repositoryName, - cspaceInstanceId); - if (databaseProductType == DatabaseProductType.MYSQL) { - // Nothing to do: MYSQL already does wildcard grants in init_db.sql - } else if(databaseProductType != DatabaseProductType.POSTGRESQL) { - throw new Exception("Unrecognized database system " + databaseProductType); - } else { - String databaseName = JDBCTools.getDatabaseName(repositoryName, cspaceInstanceId); - conn = JDBCTools.getConnection(dataSourceName, databaseName); - stmt = conn.createStatement(); - // FIXME: Check first that role exists before executing the grant - sql = String.format("GRANT %s ON ALL TABLES IN SCHEMA public TO %s", privilegeName, roleName); - stmt.execute(sql); - } - - } catch (SQLException sqle) { - SQLException tempException = sqle; - while (null != tempException) { // SQLExceptions can be chained. Loop to log all. - logger.debug("SQL Exception: " + sqle.getLocalizedMessage()); - tempException = tempException.getNextException(); - } - logger.debug("ReportPostInitHandler: SQL problem in executeQuery: ", sqle); - } catch (Throwable e) { - logger.debug("ReportPostInitHandler: problem checking/adding grant for reader: "+readerRoleName+") SQL: "+sql+" ERROR: "+e); - } finally { - try { - if (conn != null) { - conn.close(); - } - if (stmt != null) { - stmt.close(); - } - } catch (SQLException sqle) { - logger.debug("SQL Exception closing statement/connection in executeQuery: " + sqle.getLocalizedMessage()); - } - } + JDBCTools.grantPrivilegeToDatabaseUser(dataSourceName, repositoryName, cspaceInstanceId, privilegeName, readerRoleName); } -- 2.47.3