This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 8efd51f061 Various improvements to the DataSourceRealm and JDBCRealm
8efd51f061 is described below
commit 8efd51f061c026f6339bfa4fe4ef919a04ef130a
Author: Mark Thomas <[email protected]>
AuthorDate: Fri Jul 31 08:49:10 2026 +0100
Various improvements to the DataSourceRealm and JDBCRealm
A failure to connect to the database or an exception during either user
or role lookup will now result in an authentication failure rather than
a partially populated Principal.
For CLIENT-CERT and SPNEGO authentication, the user must exist in the
database for authentication to succeed.
---
.../org/apache/catalina/realm/DataSourceRealm.java | 66 ++++++++++++++++++----
java/org/apache/catalina/realm/JDBCRealm.java | 65 ++++++++++++++++++---
.../apache/catalina/realm/LocalStrings.properties | 1 +
.../apache/catalina/realm/TestDataSourceRealm.java | 9 ++-
webapps/docs/changelog.xml | 8 +++
5 files changed, 129 insertions(+), 20 deletions(-)
diff --git a/java/org/apache/catalina/realm/DataSourceRealm.java
b/java/org/apache/catalina/realm/DataSourceRealm.java
index bfa4572dc0..198260a41e 100644
--- a/java/org/apache/catalina/realm/DataSourceRealm.java
+++ b/java/org/apache/catalina/realm/DataSourceRealm.java
@@ -23,6 +23,7 @@ import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
+import java.util.List;
import javax.naming.Context;
import javax.sql.DataSource;
@@ -317,9 +318,20 @@ public class DataSourceRealm extends RealmBase {
}
// Validate the user's credentials
- boolean validated = getCredentialHandler().matches(credentials,
dbCredentials);
+ boolean authenticationSuccess =
getCredentialHandler().matches(credentials, dbCredentials);
- if (validated) {
+ ArrayList<String> roles = null;
+
+ if (authenticationSuccess) {
+ // Obtain the roles
+ roles = getRoles(dbConnection, username);
+ if (roles == null) {
+ // Role lookup failed so fail authentication
+ authenticationSuccess = false;
+ }
+ }
+
+ if (authenticationSuccess) {
if (containerLog.isTraceEnabled()) {
containerLog.trace(sm.getString("dataSourceRealm.authenticateSuccess",
username));
}
@@ -330,10 +342,8 @@ public class DataSourceRealm extends RealmBase {
return null;
}
- ArrayList<String> list = getRoles(dbConnection, username);
-
// Create and return a suitable Principal for this user
- return new GenericPrincipal(username, credentials, list);
+ return new GenericPrincipal(username, credentials, roles);
}
@@ -447,27 +457,59 @@ public class DataSourceRealm extends RealmBase {
}
+ /**
+ * Confirms if the given user exists in the database.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username Username to check
+ *
+ * @return {@code true} if the user exists, otherwise {@code false}
+ */
+ protected boolean validateUser(Connection dbConnection, String username) {
+ // Use the credentials lookup as a proxy for whether the user exists
+ try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedCredentials)) {
+ stmt.setString(1, username);
+
+ try (ResultSet rs = stmt.executeQuery()) {
+ if (rs.next()) {
+ return true;
+ }
+ }
+ } catch (SQLException e) {
+
containerLog.error(sm.getString("dataSourceRealm.validateUser.exception",
username), e);
+ }
+
+ return false;
+ }
+
+
@Override
protected Principal getPrincipal(String username) {
Connection dbConnection = open();
if (dbConnection == null) {
- return new GenericPrincipal(username, null, null);
+ return null;
}
try {
- return new GenericPrincipal(username, getPassword(dbConnection,
username),
- getRoles(dbConnection, username));
+ if (!validateUser(dbConnection, username)) {
+ return null;
+ }
+ List<String> roles = getRoles(dbConnection, username);
+ if (roles == null) {
+ return null;
+ }
+ return new GenericPrincipal(username, getPassword(dbConnection,
username), roles);
} finally {
close(dbConnection);
}
-
}
+
/**
* Return the roles associated with the given username.
*
* @param username Username for which roles should be retrieved
*
- * @return an array list of the role names
+ * @return an array list of the role names or {@code null} if the lookup
fails
*/
protected ArrayList<String> getRoles(String username) {
@@ -491,14 +533,14 @@ public class DataSourceRealm extends RealmBase {
* @param dbConnection The database connection to be used
* @param username Username for which roles should be retrieved
*
- * @return an array list of the role names
+ * @return an array list of the role names or {@code null} if the lookup
fails
*/
protected ArrayList<String> getRoles(Connection dbConnection, String
username) {
if (allRolesMode != AllRolesMode.STRICT_MODE && !isRoleStoreDefined())
{
// Using an authentication only configuration and no role store has
// been defined so don't spend cycles looking
- return null;
+ return new ArrayList<>(0);
}
try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedRoles)) {
diff --git a/java/org/apache/catalina/realm/JDBCRealm.java
b/java/org/apache/catalina/realm/JDBCRealm.java
index df75fd40c8..ffb5a2bae8 100644
--- a/java/org/apache/catalina/realm/JDBCRealm.java
+++ b/java/org/apache/catalina/realm/JDBCRealm.java
@@ -24,6 +24,7 @@ import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
+import java.util.List;
import java.util.Properties;
import org.apache.catalina.LifecycleException;
@@ -373,9 +374,20 @@ public class JDBCRealm extends RealmBase {
}
// Validate the user's credentials
- boolean validated = getCredentialHandler().matches(credentials,
dbCredentials);
+ boolean authenticationSuccess =
getCredentialHandler().matches(credentials, dbCredentials);
- if (validated) {
+ ArrayList<String> roles = null;
+
+ if (authenticationSuccess) {
+ // Obtain the roles
+ roles = getRoles(username);
+ if (roles == null) {
+ // Role lookup failed so fail authentication
+ authenticationSuccess = false;
+ }
+ }
+
+ if (authenticationSuccess) {
if (containerLog.isTraceEnabled()) {
containerLog.trace(sm.getString("jdbcRealm.authenticateSuccess", username));
}
@@ -386,8 +398,6 @@ public class JDBCRealm extends RealmBase {
return null;
}
- ArrayList<String> roles = getRoles(username);
-
// Create and return a suitable Principal for this user
return new GenericPrincipal(username, credentials, roles);
}
@@ -536,6 +546,39 @@ public class JDBCRealm extends RealmBase {
return null;
}
+
+ /**
+ * Confirms if the given user exists in the database.
+ *
+ * @param username Username to check
+ *
+ * @return {@code true} if the user exists, otherwise {@code false}
+ */
+ protected boolean validateUser(String username) {
+ // Use the credentials lookup as a proxy for whether the user exists
+ try {
+ // Ensure that we have an open database connection
+ open();
+
+ PreparedStatement stmt = credentials(dbConnection, username);
+ try (ResultSet rs = stmt.executeQuery()) {
+ if (rs.next()) {
+ return true;
+ }
+ }
+ } catch (SQLException e) {
+ containerLog.error(sm.getString("jdbcRealm.exception"), e);
+ }
+
+ // Close the connection so that it gets reopened next time
+ if (dbConnection != null) {
+ close(dbConnection);
+ }
+
+ return false;
+ }
+
+
/**
* Get the principal associated with the specified user.
*
@@ -545,9 +588,17 @@ public class JDBCRealm extends RealmBase {
*/
@Override
protected synchronized Principal getPrincipal(String username) {
+ if (!validateUser(username)) {
+ return null;
+ }
+ List<String> roles = getRoles(username);
+ if (roles == null) {
+ return null;
+ }
- return new GenericPrincipal(username, getPassword(username),
getRoles(username));
+ String password = getPassword(username);
+ return new GenericPrincipal(username, password, roles);
}
@@ -556,14 +607,14 @@ public class JDBCRealm extends RealmBase {
*
* @param username The user name
*
- * @return an array list of the role names
+ * @return an array list of the role names or {@code null} if the lookup
fails
*/
protected ArrayList<String> getRoles(String username) {
if (allRolesMode != AllRolesMode.STRICT_MODE && !isRoleStoreDefined())
{
// Using an authentication only configuration and no role store has
// been defined so don't spend cycles looking
- return null;
+ return new ArrayList<>(0);
}
// Number of tries is the number of attempts to connect to the database
diff --git a/java/org/apache/catalina/realm/LocalStrings.properties
b/java/org/apache/catalina/realm/LocalStrings.properties
index 38d68a1003..df97683981 100644
--- a/java/org/apache/catalina/realm/LocalStrings.properties
+++ b/java/org/apache/catalina/realm/LocalStrings.properties
@@ -40,6 +40,7 @@ dataSourceRealm.noUserCredCol=No column was specified for
user credentials
dataSourceRealm.noUserNameCol=No column was specified for user names
dataSourceRealm.noUserTable=No user table was specified
dataSourceRealm.roleConfigMismatch=Role configuration is incomplete: both
userRoleTable and roleNameCol must be specified together
+dataSourceRealm.validateUser.exception=Exception validating that user [{0}]
exists
jaasCallback.username=Returned username [{0}]
diff --git a/test/org/apache/catalina/realm/TestDataSourceRealm.java
b/test/org/apache/catalina/realm/TestDataSourceRealm.java
index 6c09624e1f..cd1471a799 100644
--- a/test/org/apache/catalina/realm/TestDataSourceRealm.java
+++ b/test/org/apache/catalina/realm/TestDataSourceRealm.java
@@ -152,7 +152,14 @@ public class TestDataSourceRealm extends LoggingBaseTest {
List<String> roles = db.getRoles("tomcat");
Assert.assertEquals(2, roles.size());
- db.stop();
+ p = db.getPrincipal("blabla");
+ Assert.assertNull(p);
+
+ p = db.getPrincipal("random");
+ Assert.assertTrue(p instanceof GenericPrincipal);
+ gp = (GenericPrincipal) p;
+ Assert.assertEquals(0, gp.getRoles().length);
+ db.stop();
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b471d6b599..7b24439277 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -202,6 +202,14 @@
<code>doAuthenticate()</code> and/or <code>restoreRequest()</code> will
require modification. (markt)
</fix>
+ <fix>
+ Various improvements to the <code>DataSourceRealm</code>. A failure to
+ connect to the database or an exception during either user or role
+ lookup will now result in an authentication failure rather than a
+ partially populated Principal. For <code>CLIENT-CERT</code> and
+ <code>SPNEGO</code> authentication, the user must exist in the database
+ for authentication to succeed. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
