From 3df0adaee4a48c10452fb2064fb8e608b9c985d1 Mon Sep 17 00:00:00 2001 From: Mathieu Baudier Date: Wed, 20 Jan 2021 08:18:24 +0100 Subject: [PATCH] Improve CMS security layer documentation. --- .../argeo/cms/auth/AnonymousLoginModule.java | 4 +--- .../src/org/argeo/cms/auth/CmsAuthUtils.java | 19 +++++---------- .../src/org/argeo/cms/auth/CmsSessionId.java | 4 +--- .../src/org/argeo/cms/auth/CurrentUser.java | 24 ++----------------- .../argeo/cms/auth/HttpRequestCallback.java | 1 + .../cms/auth/HttpSessionLoginModule.java | 5 ++-- .../cms/auth/SingleUserAuthorization.java | 5 ++++ .../argeo/cms/auth/SingleUserLoginModule.java | 16 ++++++++++--- .../argeo/cms/auth/UserAdminLoginModule.java | 11 ++++----- .../org/argeo/cms/auth/UserAdminUtils.java | 7 +++--- .../cms/internal/auth/CmsSessionImpl.java | 1 + .../internal/auth/ConsoleCallbackHandler.java | 10 +++----- .../cms/internal/auth/ImpliedByPrincipal.java | 3 +-- .../cms/internal/http/WebCmsSessionImpl.java | 6 +++-- 14 files changed, 47 insertions(+), 69 deletions(-) diff --git a/org.argeo.cms/src/org/argeo/cms/auth/AnonymousLoginModule.java b/org.argeo.cms/src/org/argeo/cms/auth/AnonymousLoginModule.java index e91fd6033..1d24be7ad 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/AnonymousLoginModule.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/AnonymousLoginModule.java @@ -11,7 +11,6 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.argeo.cms.CmsException; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.osgi.service.useradmin.Authorization; @@ -37,7 +36,7 @@ public class AnonymousLoginModule implements LoginModule { bc = FrameworkUtil.getBundle(AnonymousLoginModule.class).getBundleContext(); assert bc != null; } catch (Exception e) { - throw new CmsException("Cannot initialize login module", e); + throw new IllegalStateException("Cannot initialize login module", e); } } @@ -63,7 +62,6 @@ public class AnonymousLoginModule implements LoginModule { @Override public boolean abort() throws LoginException { - // authorization = null; return true; } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java b/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java index 5e59187e0..e9462c3ad 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/CmsAuthUtils.java @@ -17,10 +17,6 @@ import org.argeo.api.NodeConstants; import org.argeo.api.security.AnonymousPrincipal; import org.argeo.api.security.DataAdminPrincipal; import org.argeo.api.security.NodeSecurityUtils; -//import org.apache.jackrabbit.core.security.AnonymousPrincipal; -//import org.apache.jackrabbit.core.security.SecurityConstants; -//import org.apache.jackrabbit.core.security.principal.AdminPrincipal; -import org.argeo.cms.CmsException; import org.argeo.cms.internal.auth.CmsSessionImpl; import org.argeo.cms.internal.auth.ImpliedByPrincipal; import org.argeo.cms.internal.http.WebCmsSessionImpl; @@ -32,6 +28,7 @@ import org.osgi.framework.ServiceReference; import org.osgi.service.http.HttpContext; import org.osgi.service.useradmin.Authorization; +/** Centrlaises security related registrations. */ class CmsAuthUtils { // Standard final static String SHARED_STATE_NAME = AuthenticatingUser.SHARED_STATE_NAME; @@ -75,8 +72,6 @@ class CmsAuthUtils { NodeSecurityUtils.checkUserName(name); userPrincipal = new X500Principal(name.toString()); principals.add(userPrincipal); - // principals.add(new ImpliedByPrincipal(NodeSecurityUtils.ROLE_USER_NAME, - // userPrincipal)); if (Activator.isSingleUser()) { principals.add(new ImpliedByPrincipal(NodeSecurityUtils.ROLE_ADMIN_NAME, userPrincipal)); @@ -99,10 +94,8 @@ class CmsAuthUtils { } } catch (InvalidNameException e) { - throw new CmsException("Cannot commit", e); + throw new IllegalArgumentException("Cannot commit", e); } - - // registerSessionAuthorization(request, subject, authorization, locale); } private static void checkSubjectEmpty(Subject subject) { @@ -150,7 +143,7 @@ class CmsAuthUtils { cmsSession.close(); cmsSession = null; } else if (!authorization.getName().equals(cmsSession.getAuthorization().getName())) { - throw new CmsException("Inconsistent user " + authorization.getName() + throw new IllegalStateException("Inconsistent user " + authorization.getName() + " for existing CMS session " + cmsSession); } // keyring @@ -175,7 +168,7 @@ class CmsAuthUtils { UUID storedSessionId = subject.getPrivateCredentials(CmsSessionId.class).iterator().next() .getUuid(); // if (storedSessionId.equals(httpSessionId.getValue())) - throw new CmsException( + throw new IllegalStateException( "Subject already logged with session " + storedSessionId + " (not " + nodeSessionId + ")"); } } @@ -191,7 +184,7 @@ class CmsAuthUtils { sr = bc.getServiceReferences(CmsSession.class, "(" + CmsSession.SESSION_LOCAL_ID + "=" + httpSessionId + ")"); } catch (InvalidSyntaxException e) { - throw new CmsException("Cannot get CMS session for id " + httpSessionId, e); + throw new IllegalArgumentException("Cannot get CMS session for id " + httpSessionId, e); } CmsSession cmsSession; if (sr.size() == 1) { @@ -203,7 +196,7 @@ class CmsAuthUtils { } else if (sr.size() == 0) return null; else - throw new CmsException(sr.size() + ">1 web sessions detected for http session " + httpSessionId); + throw new IllegalStateException(sr.size() + ">1 web sessions detected for http session " + httpSessionId); return cmsSession; } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/CmsSessionId.java b/org.argeo.cms/src/org/argeo/cms/auth/CmsSessionId.java index b71eb4fc7..cc435aeb8 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/CmsSessionId.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/CmsSessionId.java @@ -4,8 +4,6 @@ import java.util.UUID; import javax.security.auth.Subject; -import org.argeo.cms.CmsException; - /** * The ID of a {@link CmsSession}, which must be available in the private * credentials of an authenticated {@link Subject}. @@ -15,7 +13,7 @@ public class CmsSessionId { public CmsSessionId(UUID value) { if (value == null) - throw new CmsException("value cannot be null"); + throw new IllegalArgumentException("Value cannot be null"); this.uuid = value; } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/CurrentUser.java b/org.argeo.cms/src/org/argeo/cms/auth/CurrentUser.java index 11dbaa3f7..fa552323c 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/CurrentUser.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/CurrentUser.java @@ -14,7 +14,6 @@ import javax.security.auth.Subject; import javax.security.auth.x500.X500Principal; import org.argeo.api.NodeConstants; -import org.argeo.cms.CmsException; import org.argeo.cms.internal.auth.CmsSessionImpl; import org.argeo.cms.internal.auth.ImpliedByPrincipal; import org.argeo.cms.internal.kernel.Activator; @@ -25,9 +24,6 @@ import org.osgi.service.useradmin.Authorization; * context. */ public final class CurrentUser { - // private final static Log log = LogFactory.getLog(CurrentUser.class); - // private final static BundleContext bc = - // FrameworkUtil.getBundle(CurrentUser.class).getBundleContext(); /* * CURRENT USER API */ @@ -86,7 +82,7 @@ public final class CurrentUser { public final static String getUsername(Subject subject) { if (subject == null) - throw new CmsException("Subject cannot be null"); + throw new IllegalArgumentException("Subject cannot be null"); if (subject.getPrincipals(X500Principal.class).size() != 1) return NodeConstants.ROLE_ANONYMOUS; Principal principal = subject.getPrincipals(X500Principal.class).iterator().next(); @@ -133,32 +129,16 @@ public final class CurrentUser { * HELPERS */ private static Subject currentSubject() { - // CmsAuthenticated cmsView = getNodeAuthenticated(); - // if (cmsView != null) - // return cmsView.getSubject(); Subject subject = getAccessControllerSubject(); if (subject != null) return subject; - throw new CmsException("Cannot find related subject"); + throw new IllegalStateException("Cannot find related subject"); } private static Subject getAccessControllerSubject() { return Subject.getSubject(AccessController.getContext()); } - // public static boolean isAuthenticated() { - // return getAccessControllerSubject() != null; - // } - - /** - * The node authenticated component (typically a CMS view) related to this - * display, or null if none is available from this call. Not API: Only for - * low-level access. - */ - // private static CmsAuthenticated getNodeAuthenticated() { - // return UiContext.getData(CmsAuthenticated.KEY); - // } - private static Authorization getAuthorization(Subject subject) { return subject.getPrivateCredentials(Authorization.class).iterator().next(); } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/HttpRequestCallback.java b/org.argeo.cms/src/org/argeo/cms/auth/HttpRequestCallback.java index 3a3a9f9e0..920f04e34 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/HttpRequestCallback.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/HttpRequestCallback.java @@ -5,6 +5,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +/** Retrieves credentials from an HTTP request. */ public class HttpRequestCallback implements Callback { private HttpServletRequest request; private HttpServletResponse response; diff --git a/org.argeo.cms/src/org/argeo/cms/auth/HttpSessionLoginModule.java b/org.argeo.cms/src/org/argeo/cms/auth/HttpSessionLoginModule.java index acc0ba4e8..8cb524fbe 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/HttpSessionLoginModule.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/HttpSessionLoginModule.java @@ -19,7 +19,6 @@ import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.argeo.cms.CmsException; import org.argeo.cms.internal.kernel.Activator; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; @@ -182,10 +181,10 @@ public class HttpSessionLoginModule implements LoginModule { sharedState.put(CmsAuthUtils.SHARED_STATE_NAME, login); sharedState.put(CmsAuthUtils.SHARED_STATE_PWD, password); } else { - throw new CmsException("Invalid authentication token"); + throw new IllegalStateException("Invalid authentication token"); } } catch (Exception e) { - throw new CmsException("Couldn't retrieve authentication", e); + throw new IllegalStateException("Couldn't retrieve authentication", e); } } else if (basic.equalsIgnoreCase("Negotiate")) { String spnegoToken = st.nextToken(); diff --git a/org.argeo.cms/src/org/argeo/cms/auth/SingleUserAuthorization.java b/org.argeo.cms/src/org/argeo/cms/auth/SingleUserAuthorization.java index c82394850..b4144d30f 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/SingleUserAuthorization.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/SingleUserAuthorization.java @@ -2,6 +2,11 @@ package org.argeo.cms.auth; import org.osgi.service.useradmin.Authorization; +/** + * {@link Authorization} for a single user. + * + * @see SingleUserLoginModule + */ public class SingleUserAuthorization implements Authorization { @Override diff --git a/org.argeo.cms/src/org/argeo/cms/auth/SingleUserLoginModule.java b/org.argeo.cms/src/org/argeo/cms/auth/SingleUserLoginModule.java index 8583bc194..0b163bac3 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/SingleUserLoginModule.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/SingleUserLoginModule.java @@ -3,6 +3,7 @@ package org.argeo.cms.auth; import java.net.InetAddress; import java.net.UnknownHostException; import java.security.Principal; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -13,6 +14,7 @@ import javax.security.auth.kerberos.KerberosPrincipal; import javax.security.auth.login.LoginException; import javax.security.auth.spi.LoginModule; import javax.security.auth.x500.X500Principal; +import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -23,6 +25,7 @@ import org.argeo.naming.LdapAttrs; import org.argeo.osgi.useradmin.IpaUtils; import org.osgi.service.useradmin.Authorization; +/** Login module for when the system is owned by a single user. */ public class SingleUserLoginModule implements LoginModule { private final static Log log = LogFactory.getLog(SingleUserLoginModule.class); @@ -70,9 +73,16 @@ public class SingleUserLoginModule implements LoginModule { principals.add(principal); principals.add(new ImpliedByPrincipal(NodeConstants.ROLE_ADMIN, principal)); principals.add(new DataAdminPrincipal()); - + + HttpServletRequest request = (HttpServletRequest) sharedState.get(CmsAuthUtils.SHARED_STATE_HTTP_REQUEST); + Locale locale = Locale.getDefault(); + if (request != null) + locale = request.getLocale(); + if (locale == null) + locale = Locale.getDefault(); Authorization authorization = new SingleUserAuthorization(); - subject.getPrivateCredentials().add(authorization); + CmsAuthUtils.addAuthorization(subject, authorization); + CmsAuthUtils.registerSessionAuthorization(request, subject, authorization, locale); return true; } @@ -84,7 +94,7 @@ public class SingleUserLoginModule implements LoginModule { @Override public boolean logout() throws LoginException { - // TODO Auto-generated method stub + CmsAuthUtils.cleanUp(subject); return true; } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/UserAdminLoginModule.java b/org.argeo.cms/src/org/argeo/cms/auth/UserAdminLoginModule.java index 54d328cc9..4b72903e2 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/UserAdminLoginModule.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/UserAdminLoginModule.java @@ -29,7 +29,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.argeo.api.NodeConstants; import org.argeo.api.security.CryptoKeyring; -import org.argeo.cms.CmsException; import org.argeo.cms.internal.kernel.Activator; import org.argeo.naming.LdapAttrs; import org.argeo.osgi.useradmin.AuthenticatingUser; @@ -77,7 +76,7 @@ public class UserAdminLoginModule implements LoginModule { this.callbackHandler = callbackHandler; this.sharedState = (Map) sharedState; } catch (Exception e) { - throw new CmsException("Cannot initialize login module", e); + throw new IllegalStateException("Cannot initialize login module", e); } } @@ -117,6 +116,8 @@ public class UserAdminLoginModule implements LoginModule { } else if (singleUser) { username = OsUserUtils.getOsUsername(); password = null; + // TODO retrieve from http session + locale = Locale.getDefault(); } else { // ask for username and password @@ -207,9 +208,7 @@ public class UserAdminLoginModule implements LoginModule { @Override public boolean commit() throws LoginException { - if (locale == null) - subject.getPublicCredentials().add(Locale.getDefault()); - else + if (locale != null) subject.getPublicCredentials().add(locale); if (singleUser) { @@ -299,8 +298,6 @@ public class UserAdminLoginModule implements LoginModule { public boolean logout() throws LoginException { if (log.isTraceEnabled()) log.trace("Logging out from CMS... " + subject); - // boolean httpSessionLogoutOk = CmsAuthUtils.logoutSession(bc, - // subject); CmsAuthUtils.cleanUp(subject); return true; } diff --git a/org.argeo.cms/src/org/argeo/cms/auth/UserAdminUtils.java b/org.argeo.cms/src/org/argeo/cms/auth/UserAdminUtils.java index 3dbc7ad52..8fe042653 100644 --- a/org.argeo.cms/src/org/argeo/cms/auth/UserAdminUtils.java +++ b/org.argeo.cms/src/org/argeo/cms/auth/UserAdminUtils.java @@ -7,7 +7,6 @@ import javax.naming.ldap.LdapName; import javax.naming.ldap.Rdn; import org.argeo.api.NodeConstants; -import org.argeo.cms.CmsException; import org.argeo.naming.LdapAttrs; import org.osgi.service.useradmin.Role; import org.osgi.service.useradmin.User; @@ -59,7 +58,7 @@ public class UserAdminUtils { || last.getType().toLowerCase().equals(LdapAttrs.cn.name())) return (String) last.getValue(); else - throw new CmsException("Cannot retrieve user local id, non valid dn: " + dn); + throw new IllegalArgumentException("Cannot retrieve user local id, non valid dn: " + dn); } /** @@ -129,7 +128,7 @@ public class UserAdminUtils { try { return new LdapName(dn); } catch (InvalidNameException e) { - throw new CmsException("Cannot parse LDAP name " + dn, e); + throw new IllegalArgumentException("Cannot parse LDAP name " + dn, e); } } @@ -158,7 +157,7 @@ public class UserAdminUtils { } return dname; } catch (InvalidNameException e) { - throw new CmsException("Unable to get domain name for " + dn, e); + throw new IllegalArgumentException("Unable to get domain name for " + dn, e); } } diff --git a/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java b/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java index 9ae0fd8d8..b6966765d 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/auth/CmsSessionImpl.java @@ -39,6 +39,7 @@ import org.osgi.framework.ServiceReference; import org.osgi.framework.ServiceRegistration; import org.osgi.service.useradmin.Authorization; +/** Default CMS session implementation. */ public class CmsSessionImpl implements CmsSession { private final static BundleContext bc = FrameworkUtil.getBundle(CmsSessionImpl.class).getBundleContext(); private final static Log log = LogFactory.getLog(CmsSessionImpl.class); diff --git a/org.argeo.cms/src/org/argeo/cms/internal/auth/ConsoleCallbackHandler.java b/org.argeo.cms/src/org/argeo/cms/internal/auth/ConsoleCallbackHandler.java index 4f1d3637b..0979a2157 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/auth/ConsoleCallbackHandler.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/auth/ConsoleCallbackHandler.java @@ -12,17 +12,14 @@ import javax.security.auth.callback.PasswordCallback; import javax.security.auth.callback.TextOutputCallback; import javax.security.auth.callback.UnsupportedCallbackException; -import org.argeo.cms.CmsException; - /** Callback handler to be used with a command line UI. */ public class ConsoleCallbackHandler implements CallbackHandler { @Override - public void handle(Callback[] callbacks) throws IOException, - UnsupportedCallbackException { + public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { Console console = System.console(); if (console == null) - throw new CmsException("No console available"); + throw new IllegalStateException("No console available"); PrintWriter writer = console.writer(); for (int i = 0; i < callbacks.length; i++) { @@ -36,8 +33,7 @@ public class ConsoleCallbackHandler implements CallbackHandler { writer.write(" (" + callback.getDefaultName() + ")"); writer.write(" : "); String answer = console.readLine(); - if (callback.getDefaultName() != null - && answer.trim().equals("")) + if (callback.getDefaultName() != null && answer.trim().equals("")) callback.setName(callback.getDefaultName()); else callback.setName(answer); diff --git a/org.argeo.cms/src/org/argeo/cms/internal/auth/ImpliedByPrincipal.java b/org.argeo.cms/src/org/argeo/cms/internal/auth/ImpliedByPrincipal.java index 5afacf69d..c75360129 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/auth/ImpliedByPrincipal.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/auth/ImpliedByPrincipal.java @@ -10,7 +10,6 @@ import java.util.Set; import javax.naming.InvalidNameException; import javax.naming.ldap.LdapName; -import org.argeo.cms.CmsException; import org.osgi.service.useradmin.Authorization; import org.osgi.service.useradmin.Role; @@ -32,7 +31,7 @@ public final class ImpliedByPrincipal implements Principal, Role { try { this.name = new LdapName(name); } catch (InvalidNameException e) { - throw new CmsException("Badly formatted role name", e); + throw new IllegalArgumentException("Badly formatted role name", e); } if (userPrincipal != null) causes.add(userPrincipal); diff --git a/org.argeo.cms/src/org/argeo/cms/internal/http/WebCmsSessionImpl.java b/org.argeo.cms/src/org/argeo/cms/internal/http/WebCmsSessionImpl.java index 1df7b1760..20f4c032e 100644 --- a/org.argeo.cms/src/org/argeo/cms/internal/http/WebCmsSessionImpl.java +++ b/org.argeo.cms/src/org/argeo/cms/internal/http/WebCmsSessionImpl.java @@ -9,14 +9,16 @@ import javax.servlet.http.HttpSession; import org.argeo.cms.internal.auth.CmsSessionImpl; import org.osgi.service.useradmin.Authorization; +/** CMS session implementation in a web context. */ public class WebCmsSessionImpl extends CmsSessionImpl { // private final static Log log = // LogFactory.getLog(WebCmsSessionImpl.class); private HttpSession httpSession; - public WebCmsSessionImpl(Subject initialSubject, Authorization authorization, Locale locale, HttpServletRequest request) { - super(initialSubject, authorization, locale,request.getSession(false).getId()); + public WebCmsSessionImpl(Subject initialSubject, Authorization authorization, Locale locale, + HttpServletRequest request) { + super(initialSubject, authorization, locale, request.getSession(false).getId()); httpSession = request.getSession(false); } -- 2.30.2