This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new cf239bb Remove GenericPrincipal.getPassword cf239bb is described below commit cf239bb4450927fbc03d6648ea593306380725ba Author: remm <r...@apache.org> AuthorDate: Mon Jan 13 14:23:35 2020 +0100 Remove GenericPrincipal.getPassword The credentials should remain managed by the realm. --- TOMCAT-NEXT.txt | 2 -- .../apache/catalina/realm/GenericPrincipal.java | 24 +++++++++------------- java/org/apache/catalina/realm/MemoryRealm.java | 24 +++++++++++++--------- .../apache/catalina/realm/UserDatabaseRealm.java | 17 ++++++++++++--- .../catalina/realm/TestGenericPrincipal.java | 1 - test/org/apache/catalina/realm/TestJNDIRealm.java | 4 ++-- webapps/docs/changelog.xml | 8 ++++++++ 7 files changed, 48 insertions(+), 32 deletions(-) diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt index 87eadaa..271ae5e 100644 --- a/TOMCAT-NEXT.txt +++ b/TOMCAT-NEXT.txt @@ -63,8 +63,6 @@ New items for 10.0.x onwards: 12. Consider disabling the AJP connector by default. -13. Remove GenericPrincipal.getPassword(). - 14. Remove unused NIO blocking code. 15. Change SocketProperties cache sizes default values. diff --git a/java/org/apache/catalina/realm/GenericPrincipal.java b/java/org/apache/catalina/realm/GenericPrincipal.java index 6a848de..1511b2d 100644 --- a/java/org/apache/catalina/realm/GenericPrincipal.java +++ b/java/org/apache/catalina/realm/GenericPrincipal.java @@ -105,7 +105,6 @@ public class GenericPrincipal implements TomcatPrincipal, Serializable { GSSCredential gssCredential) { super(); this.name = name; - this.password = password; this.userPrincipal = userPrincipal; if (roles == null) { this.roles = new String[0]; @@ -132,25 +131,24 @@ public class GenericPrincipal implements TomcatPrincipal, Serializable { return this.name; } - /** - * The authentication credentials for the user represented by - * this Principal. + * @deprecated Will be removed in Tomcat 10, the password should be accessed + * using RealmBase.getPassword + * @return null */ - protected final String password; - + @Deprecated public String getPassword() { - return this.password; + return null; } /** * The set of roles associated with this user. */ - protected final String roles[]; + protected final String[] roles; public String[] getRoles() { - return this.roles; + return this.roles.clone(); } @@ -242,21 +240,19 @@ public class GenericPrincipal implements TomcatPrincipal, Serializable { // ----------------------------------------------------------- Serialization private Object writeReplace() { - return new SerializablePrincipal(name, password, roles, userPrincipal); + return new SerializablePrincipal(name, roles, userPrincipal); } private static class SerializablePrincipal implements Serializable { private static final long serialVersionUID = 1L; private final String name; - private final String password; private final String[] roles; private final Principal principal; - public SerializablePrincipal(String name, String password, String[] roles, + public SerializablePrincipal(String name, String[] roles, Principal principal) { this.name = name; - this.password = password; this.roles = roles; if (principal instanceof Serializable) { this.principal = principal; @@ -266,7 +262,7 @@ public class GenericPrincipal implements TomcatPrincipal, Serializable { } private Object readResolve() { - return new GenericPrincipal(name, password, Arrays.asList(roles), principal); + return new GenericPrincipal(name, null, Arrays.asList(roles), principal); } } } diff --git a/java/org/apache/catalina/realm/MemoryRealm.java b/java/org/apache/catalina/realm/MemoryRealm.java index e57cd2f..db51794 100644 --- a/java/org/apache/catalina/realm/MemoryRealm.java +++ b/java/org/apache/catalina/realm/MemoryRealm.java @@ -69,6 +69,12 @@ public class MemoryRealm extends RealmBase { private final Map<String,GenericPrincipal> principals = new HashMap<>(); + /** + * The set of credentials for this Realm, keyed by user name. + */ + private final Map<String, String> credentials = new HashMap<>(); + + // ------------------------------------------------------------- Properties /** @@ -118,8 +124,12 @@ public class MemoryRealm extends RealmBase { } GenericPrincipal principal = principals.get(username); + String password = null; + if (principal != null) { + password = this.credentials.get(username); + } - if(principal == null || principal.getPassword() == null) { + if (principal == null || password == null) { // User was not found in the database or the password was null // Waste a bit of time as not to reveal that the user does not exist. getCredentialHandler().mutate(credentials); @@ -129,7 +139,7 @@ public class MemoryRealm extends RealmBase { return null; } - boolean validated = getCredentialHandler().matches(credentials, principal.getPassword()); + boolean validated = getCredentialHandler().matches(credentials, password); if (validated) { if (log.isDebugEnabled()) @@ -171,6 +181,7 @@ public class MemoryRealm extends RealmBase { GenericPrincipal principal = new GenericPrincipal(username, password, list); principals.put(username, principal); + credentials.put(username, password); } @@ -204,14 +215,7 @@ public class MemoryRealm extends RealmBase { */ @Override protected String getPassword(String username) { - - GenericPrincipal principal = principals.get(username); - if (principal != null) { - return principal.getPassword(); - } else { - return null; - } - + return credentials.get(username); } diff --git a/java/org/apache/catalina/realm/UserDatabaseRealm.java b/java/org/apache/catalina/realm/UserDatabaseRealm.java index 64957a9..b001ded 100644 --- a/java/org/apache/catalina/realm/UserDatabaseRealm.java +++ b/java/org/apache/catalina/realm/UserDatabaseRealm.java @@ -102,8 +102,8 @@ public class UserDatabaseRealm extends RealmBase { } if (principal instanceof GenericPrincipal) { GenericPrincipal gp = (GenericPrincipal) principal; - if (gp.getUserPrincipal() instanceof User) { - principal = gp.getUserPrincipal(); + if (gp.getUserPrincipal() instanceof UserDatabasePrincipal) { + principal = database.findUser(gp.getName()); } } if (!(principal instanceof User)) { @@ -185,7 +185,8 @@ public class UserDatabaseRealm extends RealmBase { roles.add(role.getName()); } } - return new GenericPrincipal(username, user.getPassword(), roles, user); + return new GenericPrincipal(username, user.getPassword(), roles, + new UserDatabasePrincipal(username)); } @@ -236,4 +237,14 @@ public class UserDatabaseRealm extends RealmBase { // Release reference to our user database database = null; } + + private class UserDatabasePrincipal implements Principal { + private final String name; + private UserDatabasePrincipal(String name) { + this.name = name; + } + public String getName() { + return name; + } + } } diff --git a/test/org/apache/catalina/realm/TestGenericPrincipal.java b/test/org/apache/catalina/realm/TestGenericPrincipal.java index c1507de..f43fc24 100644 --- a/test/org/apache/catalina/realm/TestGenericPrincipal.java +++ b/test/org/apache/catalina/realm/TestGenericPrincipal.java @@ -63,7 +63,6 @@ public class TestGenericPrincipal { Assert.assertNull(gpOut.getGssCredential()); Assert.assertEquals(gpIn.getName(), gpOut.getName()); - Assert.assertEquals(gpIn.getPassword(), gpOut.getPassword()); Assert.assertArrayEquals(gpIn.getRoles(), gpOut.getRoles()); if (gpIn == gpIn.getUserPrincipal()) { Assert.assertEquals(gpOut, gpOut.getUserPrincipal()); diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java index 35383fd..baf0b14 100644 --- a/test/org/apache/catalina/realm/TestJNDIRealm.java +++ b/test/org/apache/catalina/realm/TestJNDIRealm.java @@ -88,7 +88,7 @@ public class TestJNDIRealm { // THEN Assert.assertTrue(principal instanceof GenericPrincipal); - Assert.assertEquals(PASSWORD, ((GenericPrincipal)principal).getPassword()); + Assert.assertEquals(USER, principal.getName()); } @Test @@ -106,7 +106,7 @@ public class TestJNDIRealm { // THEN Assert.assertTrue(principal instanceof GenericPrincipal); - Assert.assertEquals(ha1(), ((GenericPrincipal)principal).getPassword()); + Assert.assertEquals(USER, principal.getName()); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index df3a222..210194f 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -45,6 +45,14 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 10.0.0.0-M1 (markt)" rtext="in development"> + <subsection name="Catalina"> + <changelog> + <update> + Remove GenericPrincipal.getPassword. The credentials should remain + managed by the realm. (remm) + </update> + </changelog> + </subsection> </section> </body> </document> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org