From 9e3c2da6faadea2946eb0ed92caaaa3d827b2856 Mon Sep 17 00:00:00 2001 From: Patrick Dowler Date: Mon, 12 Feb 2024 13:41:58 -0800 Subject: [PATCH] cadc-log: switch to IvoaGroupClient and remove cruft --- cadc-log/README.md | 13 +- cadc-log/build.gradle | 2 +- .../ca/nrc/cadc/log/LogControlServlet.java | 192 +++--------------- 3 files changed, 39 insertions(+), 168 deletions(-) diff --git a/cadc-log/README.md b/cadc-log/README.md index 2c0fd32a..b735c195 100644 --- a/cadc-log/README.md +++ b/cadc-log/README.md @@ -13,14 +13,21 @@ endpoint are configured with a `cadc-log.properties` file at runtime. This file can be added to service config to grant perrmission to use the LogControlServlet at runtime. ```properties user = {X509 distinguished name} -user = {X509 distinguished name} +user = ... + +username = {network username} +username = ... group = {IVOA GMS group identifier} -group = {IVOA GMS group identifier} +group = ... ``` -Both the `user` and `group` properties are optional and support multiple values. The specified +All of the `user`, `username`, and `group` properties are optional and support multiple values. The specified users are granted permission to view (GET) and change (POST) log levels in the running service. +For the X509 distinguished name, the log control endpoint can successfully authorise the user even when the +associated AAI system is unavailable, so this mechanism is slightly more robust when trying to diagnose AAI +related issues. + ## log control REST API This is a very simple explanation; TODO: document with OpenAI so it can be included in service API docs. diff --git a/cadc-log/build.gradle b/cadc-log/build.gradle index b8e4b60a..6c766a89 100644 --- a/cadc-log/build.gradle +++ b/cadc-log/build.gradle @@ -14,7 +14,7 @@ sourceCompatibility = 1.8 group = 'org.opencadc' -version = '1.2.0' +version = '1.2.1' description = 'OpenCADC Logging Init server library' def git_url = 'https://github.com/opencadc/core' diff --git a/cadc-log/src/main/java/ca/nrc/cadc/log/LogControlServlet.java b/cadc-log/src/main/java/ca/nrc/cadc/log/LogControlServlet.java index 12dcf1d1..03743f32 100644 --- a/cadc-log/src/main/java/ca/nrc/cadc/log/LogControlServlet.java +++ b/cadc-log/src/main/java/ca/nrc/cadc/log/LogControlServlet.java @@ -72,6 +72,7 @@ import ca.nrc.cadc.auth.AuthenticationUtil; import ca.nrc.cadc.auth.Authorizer; import ca.nrc.cadc.auth.HttpPrincipal; +import ca.nrc.cadc.auth.IdentityManager; import ca.nrc.cadc.cred.client.CredUtil; import ca.nrc.cadc.net.TransientException; import ca.nrc.cadc.util.Log4jInit; @@ -102,9 +103,8 @@ import javax.servlet.http.HttpServletResponse; import org.apache.log4j.Level; import org.apache.log4j.Logger; -import org.opencadc.gms.GroupClient; import org.opencadc.gms.GroupURI; -import org.opencadc.gms.GroupUtil; +import org.opencadc.gms.IvoaGroupClient; /** * Sets up log4j for whichever webapp contains this @@ -163,7 +163,7 @@ public class LogControlServlet extends HttpServlet { private static final String PACKAGES_PARAM = "logLevelPackages"; private static final String LOG_CONTROL_CONFIG = "cadc-log.properties"; - static final String USER_DNS_PROPERTY = "user"; + static final String USER_X509_PROPERTY = "user"; static final String GROUP_URIS_PROPERTY = "group"; static final String USERNAME_PROPERTY = "username"; static final String SECRET_PROPERTY = "secret"; @@ -171,10 +171,6 @@ public class LogControlServlet extends HttpServlet { private Level level = null; private List packages; - private String authorizerClassName; - private String accessGroup; - private String logControlProperties; - /** * Initialize the logging. This method should only get * executed once and, if properly configured, it should @@ -215,7 +211,6 @@ public void init(final ServletConfig config) throws ServletException { String thisPkg = LogControlServlet.class.getPackage().getName(); Log4jInit.setLevel(webapp, thisPkg, Level.WARN); - packages.add(thisPkg); logger.warn("log level: " + thisPkg + " = " + Level.WARN); String packageParamValues = config.getInitParameter(PACKAGES_PARAM); @@ -379,105 +374,57 @@ private void authorize(HttpServletRequest request, boolean readOnly) // Check the secret first. if (isAuthorizedSecret(request, mvp)) { - logger.debug("Secret authorized."); + logger.warn("Secret authorized."); return; } - // Get the calling subject. If possible, augment the subject, - // but if augmenting fails, use the subject provided only. + // ugh: this should happen earlier and all code should already be inside + // a Subject.doAs(...) but this allows us to fall back to lazy augment + // if normal augment fails Subject subject; try { - subject = AuthenticationUtil.getSubject(request); + subject = AuthenticationUtil.getSubject(request, true); } catch (Exception e) { logger.error("Augment subject failed, using non-augmented subject: " + e.getMessage()); subject = AuthenticationUtil.getSubject(request, false); } - logger.debug(subject.toString()); + logger.debug("caller: " + subject); + IdentityManager im = AuthenticationUtil.getIdentityManager(); + String userDisplay = im.toDisplayString(subject); - // first check if request user matches authorized config file users Set authorizedUsers = getAuthorizedUserPrincipals(mvp); if (isAuthorizedUser(subject, authorizedUsers)) { - logger.debug(subject.getPrincipals(X500Principal.class) + " is an authorized user"); + logger.warn(userDisplay + " is an authorized user"); return; } - // Check for groups configured in servlet init or properties file. Set groupUris = getAuthorizedGroupUris(mvp); - - if (groupUris.isEmpty() && accessGroup == null) { - // early return to avoid checkCredentials below - throw new AccessControlException("permission denied"); - } - - // Check if calling user is a member of a properties file group. - try { - if (CredUtil.checkCredentials(subject)) { - URI serviceID = null; - GroupClient groupClient = null; - for (GroupURI groupUri : groupUris) { - if (!groupUri.getServiceID().equals(serviceID)) { - serviceID = groupUri.getServiceID(); - groupClient = GroupUtil.getGroupClient(serviceID); - } - GroupMemberAction memberCheck = new GroupMemberAction(groupClient, groupUri); - if (isAuthorizedGroup(memberCheck, subject)) { - logger.info(subject.getPrincipals(X500Principal.class) + " is a member of " + groupUri); + if (!groupUris.isEmpty()) { + try { + if (CredUtil.checkCredentials(subject)) { + IvoaGroupClient groupClient = new IvoaGroupClient(); + Set mem = Subject.doAs(subject, + (PrivilegedExceptionAction>) () -> groupClient.getMemberships(groupUris)); + if (!mem.isEmpty()) { + StringBuilder sb = new StringBuilder(); + sb.append(userDisplay).append(" is a member of:"); + for (GroupURI g : mem) { + sb.append(" ").append(g.getURI().toASCIIString()); + } + logger.warn(sb.toString()); return; } } - } - } catch (Exception e) { - throw new AccessControlException("permission denied, reason: credential check failed: " + e.getMessage()); - } - - // Check if calling user is a member of a servlet init group. - Authorizer authorizer = getAuthorizer(accessGroup); - if (authorizer != null) { - GroupAuthorizationAction groupCheck = new GroupAuthorizationAction(authorizer, readOnly); - if (isAuthorizedGroup(groupCheck, subject)) { - logger.debug(subject.getPrincipals(X500Principal.class) + " is member of " + accessGroup); - return; + } catch (Exception e) { + throw new AccessControlException("permission denied, reason: credential check failed: " + e.getMessage()); } } throw new AccessControlException("permission denied"); } - /** - * Get a Group authorizer for the given group URI. - * - * @param groupURI groupURI string value. - * @return A Group authorizer. - */ - private Authorizer getAuthorizer(String groupURI) { - Authorizer authorizer = null; - if (authorizerClassName != null) { - try { - Class authClass = Class.forName(authorizerClassName); - if (groupURI != null) { - try { - Constructor ctor = authClass.getConstructor(String.class); - Object o = ctor.newInstance(groupURI); - authorizer = (Authorizer) o; - } catch (NoSuchMethodException ex) { - logger.warn("authorizer " + authorizerClassName + " has no constructor(String), ignoring groupURI=" + groupURI); - Object o = authClass.newInstance(); - authorizer = (Authorizer) o; - } - } else { - // no-arg constructor - Object o = authClass.newInstance(); - authorizer = (Authorizer) o; - } - } catch (Exception e) { - logger.error("Could not load group authorizer for groupURI=" + groupURI, e); - } - } - return authorizer; - } - /** * Supported secret submission is from a query parameter (secret=) or a header (X-CADC-LOGCONTROL). * @param request The HTTP Servlet request. @@ -520,38 +467,6 @@ private boolean isAuthorizedUser(Subject subject, Set authorizedUsers return false; } - /** - * Check that the subject can perform the action to authorize access. - * - * @param action The PrivilegedExceptionAction - * @param subject The Subject to check - * @return true if the calling user is a member of an authorized group, false otherwise. - */ - private boolean isAuthorizedGroup(PrivilegedExceptionAction action, Subject subject) - throws TransientException { - try { - if (subject == null) { - action.run(); - } else { - try { - Subject.doAs(subject, action); - } catch (PrivilegedActionException e) { - throw e.getException(); - } - } - return true; - } catch (Exception e) { - if (e instanceof AccessControlException) { - logger.debug("Group authorization failed: " + e.getMessage()); - } else if (e instanceof TransientException) { - throw (TransientException) e; - } else { - throw new IllegalStateException(e); - } - } - return false; - } - /** * Get a Set of X500Principal's from the logControl properties. Return * an empty set if the properties do not exist or can't be read. @@ -562,7 +477,7 @@ Set getAuthorizedUserPrincipals(MultiValuedProperties mvp) { Set principals = new HashSet<>(); if (mvp != null) { try { - List properties = mvp.getProperty(USER_DNS_PROPERTY); + List properties = mvp.getProperty(USER_X509_PROPERTY); if (properties != null) { for (String property : properties) { if (!property.isEmpty()) { @@ -626,55 +541,4 @@ private MultiValuedProperties getLogControlProperties() { // empty return new MultiValuedProperties(); } - - static class GroupAuthorizationAction implements PrivilegedExceptionAction { - - private Authorizer authorizer; - private boolean readOnly; - - GroupAuthorizationAction(Authorizer authorizer, boolean readOnly) { - this.authorizer = authorizer; - this.readOnly = readOnly; - } - - @Override - public Object run() throws Exception { - try { - if (readOnly) { - authorizer.getReadPermission(null); - } else { - authorizer.getWritePermission(null); - } - } catch (FileNotFoundException e) { - throw new IllegalStateException("UnexpectedException", e); - } - - return null; - } - } - - static class GroupMemberAction implements PrivilegedExceptionAction { - - private GroupClient groupClient; - private GroupURI groupURI; - - GroupMemberAction(GroupClient groupClient, GroupURI groupURI) { - this.groupClient = groupClient; - this.groupURI = groupURI; - } - - @Override - public Object run() throws Exception { - // GMSClient can throw a RuntimeException for an auth failure. - try { - if (!groupClient.isMember(groupURI)) { - throw new AccessControlException("not a member of " + groupURI); - } - } catch (RuntimeException e) { - throw new AccessControlException(e.getMessage()); - } - return null; - } - } - }