You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2020/01/13 13:23:46 UTC

[tomcat] branch master updated: Remove GenericPrincipal.getPassword

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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org