Issue #5272 Clean up LoginServices and LoginModules (#5641)

* Issue #5272 Clean up LoginServices and LoginModules

Signed-off-by: Jan Bartel <janb@webtide.com>

demos/demo-jaas-webapp/src/main/config/modules/demo-jaas.mod

@@ -14,14 +14,13 @@ jdbc
 jsp
 annotations
 ext
-demo-realm
 
 [files]
 basehome:modules/demo.d/demo-jaas.xml|webapps/demo-jaas.xml
 basehome:modules/demo.d/demo-login.conf|etc/demo-login.conf
 basehome:modules/demo.d/demo-login.properties|etc/demo-login.properties
 maven://org.eclipse.jetty.demos/demo-jaas-webapp/${jetty.version}/war|webapps/demo-jaas.war
 
-[ini-template]
+[ini]
 # Enable security via jaas, and configure it
-jetty.jaas.login.conf=etc/demo-login.conf
+jetty.jaas.login.conf?=etc/demo-login.conf

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java

@@ -231,12 +231,14 @@ public UserIdentity login(final String username, final Object credentials, final
         }
         catch (Exception e)
         {
-            LOG.trace("IGNORED", e);
+            if (LOG.isDebugEnabled())
+                LOG.debug("Login error", e);
         }
         finally
         {
             INSTANCE.remove();
         }
+
         return null;
     }
 

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASUserPrincipal.java

@@ -26,7 +26,7 @@
  * JAASUserPrincipal
  * <p>
  * Implements the JAAS version of the
- * org.eclipse.jetty.http.UserPrincipal interface.
+ * org.eclipse.jetty.security.UserPrincipal interface.
  */
 public class JAASUserPrincipal implements Principal
 {

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractDatabaseLoginModule.java

@@ -27,6 +27,7 @@
 import javax.security.auth.Subject;
 import javax.security.auth.callback.CallbackHandler;
 
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -57,11 +58,11 @@ public abstract class AbstractDatabaseLoginModule extends AbstractLoginModule
      */
     public abstract Connection getConnection() throws Exception;
 
-    public class JDBCUserInfo extends UserInfo
+    public class JDBCUser extends JAASUser
     {
-        public JDBCUserInfo(String userName, Credential credential)
+        public JDBCUser(UserPrincipal user)
         {
-            super(userName, credential);
+            super(user);
         }
 
         @Override
@@ -79,7 +80,7 @@ public List<String> doFetchRoles()
      * @throws Exception if unable to get the user info
      */
     @Override
-    public UserInfo getUserInfo(String userName)
+    public JAASUser getUser(String userName)
         throws Exception
     {
         try (Connection connection = getConnection())
@@ -100,11 +101,9 @@ public UserInfo getUserInfo(String userName)
             }
 
             if (dbCredential == null)
-            {
                 return null;
-            }
 
-            return new JDBCUserInfo(userName, Credential.getCredential(dbCredential));
+            return new JDBCUser(new UserPrincipal(userName, Credential.getCredential(dbCredential)));
         }
     }
 

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/AbstractLoginModule.java

@@ -19,11 +19,11 @@
 package org.eclipse.jetty.jaas.spi;
 
 import java.io.IOException;
-import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.security.auth.Subject;
 import javax.security.auth.callback.Callback;
 import javax.security.auth.callback.CallbackHandler;
@@ -34,9 +34,10 @@
 import javax.security.auth.login.LoginException;
 import javax.security.auth.spi.LoginModule;
 
-import org.eclipse.jetty.jaas.JAASPrincipal;
 import org.eclipse.jetty.jaas.JAASRole;
 import org.eclipse.jetty.jaas.callback.ObjectCallback;
+import org.eclipse.jetty.security.UserPrincipal;
+import org.eclipse.jetty.util.thread.AutoLock;
 
 /**
  * AbstractLoginModule
@@ -50,84 +51,65 @@ public abstract class AbstractLoginModule implements LoginModule
 
     private boolean authState = false;
     private boolean commitState = false;
-    private JAASUserInfo currentUser;
+    private JAASUser currentUser;
     private Subject subject;
 
-    /**
-     * JAASUserInfo
-     *
-     * This class unites the UserInfo data with jaas concepts
-     * such as Subject and Principals
-     */
-    public class JAASUserInfo
+    public abstract static class JAASUser
     {
-        private UserInfo user;
-        private Principal principal;
-        private List<JAASRole> roles;
-
-        public JAASUserInfo(UserInfo u)
+        private final UserPrincipal _user;
+        private List<JAASRole> _roles;
+
+        public JAASUser(UserPrincipal u)
         {
-            this.user = u;
-            this.principal = new JAASPrincipal(u.getUserName());
+            _user = u;
         }
 
         public String getUserName()
         {
-            return this.user.getUserName();
-        }
-
-        public Principal getPrincipal()
-        {
-            return this.principal;
+            return _user.getName();
         }
 
         /**
          * @param subject The subject
          */
         public void setJAASInfo(Subject subject)
         {
-            subject.getPrincipals().add(this.principal);
-            if (this.user.getCredential() != null)
-            {
-                subject.getPrivateCredentials().add(this.user.getCredential());
-            }
-            subject.getPrincipals().addAll(roles);
+            if (_user == null)
+                return;
+
+            _user.configureSubject(subject);
+            if (_roles != null)
+                subject.getPrincipals().addAll(_roles);
         }
 
         /**
          * @param subject The subject
          */
         public void unsetJAASInfo(Subject subject)
         {
-            subject.getPrincipals().remove(this.principal);
-            if (this.user.getCredential() != null)
-            {
-                subject.getPrivateCredentials().remove(this.user.getCredential());
-            }
-            subject.getPrincipals().removeAll(this.roles);
+            if (_user == null)
+                return;
+            _user.deconfigureSubject(subject);
+            if (_roles != null)
+                subject.getPrincipals().removeAll(_roles);
         }
 
         public boolean checkCredential(Object suppliedCredential)
         {
-            return this.user.checkCredential(suppliedCredential);
+            return _user.authenticate(suppliedCredential);
         }
 
         public void fetchRoles() throws Exception
         {
-            this.user.fetchRoles();
-            this.roles = new ArrayList<JAASRole>();
-            if (this.user.getRoleNames() != null)
-            {
-                Iterator<String> itor = this.user.getRoleNames().iterator();
-                while (itor.hasNext())
-                {
-                    this.roles.add(new JAASRole((String)itor.next()));
-                }
-            }
+            List<String> rolenames = doFetchRoles();
+            if (rolenames != null)
+                _roles = rolenames.stream().map(JAASRole::new).collect(Collectors.toList());
         }
+
+        public abstract List<String> doFetchRoles() throws Exception;
     }
 
-    public abstract UserInfo getUserInfo(String username) throws Exception;
+    public abstract JAASUser getUser(String username) throws Exception;
 
     public Subject getSubject()
     {
@@ -139,12 +121,12 @@ public void setSubject(Subject s)
         this.subject = s;
     }
 
-    public JAASUserInfo getCurrentUser()
+    public JAASUser getCurrentUser()
     {
         return this.currentUser;
     }
 
-    public void setCurrentUser(JAASUserInfo u)
+    public void setCurrentUser(JAASUser u)
     {
         this.currentUser = u;
     }
@@ -252,15 +234,15 @@ public boolean login() throws LoginException
                 throw new FailedLoginException();
             }
 
-            UserInfo userInfo = getUserInfo(webUserName);
+            JAASUser user = getUser(webUserName);
 
-            if (userInfo == null)
+            if (user == null)
             {
                 setAuthenticated(false);
                 throw new FailedLoginException();
             }
 
-            currentUser = new JAASUserInfo(userInfo);
+            currentUser = user;
             setAuthenticated(currentUser.checkCredential(webCredential));
 
             if (isAuthenticated())

jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/LdapLoginModule.java

@@ -45,6 +45,7 @@
 import javax.security.auth.login.LoginException;
 
 import org.eclipse.jetty.jaas.callback.ObjectCallback;
+import org.eclipse.jetty.security.UserPrincipal;
 import org.eclipse.jetty.util.TypeUtil;
 import org.eclipse.jetty.util.security.Credential;
 import org.slf4j.Logger;
@@ -179,18 +180,13 @@ public class LdapLoginModule extends AbstractLoginModule
 
     private DirContext _rootContext;
 
-    public class LDAPUserInfo extends UserInfo
+    public class LDAPUser extends JAASUser
     {
         Attributes attributes;
 
-        /**
-         * @param userName the user name
-         * @param credential the credential
-         * @param attributes the user {@link Attributes}
-         */
-        public LDAPUserInfo(String userName, Credential credential, Attributes attributes)
+        public LDAPUser(UserPrincipal user, Attributes attributes)
         {
-            super(userName, credential);
+            super(user);
             this.attributes = attributes;
         }
 
@@ -201,6 +197,25 @@ public List<String> doFetchRoles() throws Exception
         }
     }
 
+    public class LDAPBindingUser extends JAASUser
+    {   
+        DirContext _context;
+        String _userDn;
+
+        public LDAPBindingUser(UserPrincipal user, DirContext context, String userDn)
+        {
+            super(user);
+            _context = context;
+            _userDn = userDn;
+        }
+
+        @Override
+        public List<String> doFetchRoles() throws Exception
+        {
+            return getUserRolesByDn(_context, _userDn);
+        }
+    }
+
     /**
      * get the available information about the user
      * <p>
@@ -214,19 +229,17 @@ public List<String> doFetchRoles() throws Exception
      * @throws Exception if unable to get the user info
      */
     @Override
-    public UserInfo getUserInfo(String username) throws Exception
+    public JAASUser getUser(String username) throws Exception
     {
         Attributes attributes = getUserAttributes(username);
         String pwdCredential = getUserCredentials(attributes);
 
         if (pwdCredential == null)
-        {
             return null;
-        }
 
         pwdCredential = convertCredentialLdapToJetty(pwdCredential);
         Credential credential = Credential.getCredential(pwdCredential);
-        return new LDAPUserInfo(username, credential, attributes);
+        return new LDAPUser(new UserPrincipal(username, credential), attributes);
     }
 
     protected String doRFC2254Encoding(String inputString)
@@ -421,15 +434,15 @@ public boolean login() throws LoginException
             else
             {
                 // This sets read and the credential
-                UserInfo userInfo = getUserInfo(webUserName);
+                JAASUser userInfo = getUser(webUserName);
 
                 if (userInfo == null)
                 {
                     setAuthenticated(false);
                     return false;
                 }
 
-                setCurrentUser(new JAASUserInfo(userInfo));
+                setCurrentUser(userInfo);
 
                 if (webCredential instanceof String)
                     authed = credentialLogin(Credential.getCredential((String)webCredential));
@@ -520,12 +533,8 @@ public boolean bindingLogin(String username, Object password) throws LoginExcept
         try
         {
             DirContext dirContext = new InitialDirContext(environment);
-            List<String> roles = getUserRolesByDn(dirContext, userDn);
-
-            UserInfo userInfo = new UserInfo(username, null, roles);
-            setCurrentUser(new JAASUserInfo(userInfo));
+            setCurrentUser(new LDAPBindingUser(new UserPrincipal(username, null), dirContext, userDn));
             setAuthenticated(true);
-
             return true;
         }
         catch (javax.naming.AuthenticationException e)