dcache-frontend,dcache-bulk,dcache-qos: regularize observance of the …

…admin role

Motivation:

Frontend currently does some special things
in terms of admin permissions by changing
the Subject to ROOT.  But this is actually
an improper subject to pass along to the
Bulk or QoS services, particularly because
it may result in root ownership of the
request.

Now that we have an admin role Principal,
the most consistent idea would be to allow
each service in the message chain to
make its own decision as to whether to
promote the subject to ROOT capabilities
or not based on the RolePrincipal.

Modification:

Make the necessary changes to regularize
the Admin role semantics.

Result:

Correct behavior in Bulk and QoS
with respect to admin privileges.

Target: master
Patch: https://rb.dcache.org/r/14090/
Requires-notes: no
Acked-by: Tigran

modules/common/src/main/java/org/dcache/auth/Subjects.java

@@ -23,6 +23,7 @@
 import javax.annotation.Nullable;
 import javax.security.auth.Subject;
 import javax.security.auth.kerberos.KerberosPrincipal;
+import org.dcache.auth.RolePrincipal.Role;
 import org.dcache.util.PrincipalSetMaker;
 import org.globus.gsi.gssapi.jaas.GlobusPrincipal;
 import org.slf4j.Logger;
@@ -70,6 +71,15 @@ public static boolean isRoot(Subject subject) {
         return hasUid(subject, 0);
     }
 
+    public static boolean hasAdminRole(Subject subject) {
+        return hasRole(subject, Role.ADMIN);
+    }
+
+    public static boolean hasRole(Subject subject, Role role) {
+        return subject.getPrincipals().stream().filter(p -> p instanceof RolePrincipal)
+              .anyMatch(p -> ((RolePrincipal) p).hasRole(role));
+    }
+
     /**
      * Return true if the subject is root or has the special ExemptFromNamespaceChecks principal.
      *

modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/BulkActivityFactory.java

@@ -112,7 +112,6 @@ public final class BulkActivityFactory implements CellMessageSender, Environment
     private CellStub poolManager;
     private CellStub qosEngine;
     private PoolMonitor poolMonitor;
-    private PnfsHandler pnfsHandler;
     private QoSResponseReceiver qoSResponseReceiver;
     private CellEndpoint endpoint;
 
@@ -134,11 +133,13 @@ public BulkActivity createActivity(BulkRequest request, Subject subject,
                   "cannot create " + activity + "; no such activity.");
         }
 
-        LOGGER.debug("creating instance of activity {} for request {}.", activity, request.getUid());
+        LOGGER.debug("creating instance of activity {} for request {}.", activity,
+              request.getUid());
 
         BulkActivity bulkActivity = provider.createActivity();
         bulkActivity.setSubject(subject);
         bulkActivity.setRestriction(restriction);
+
         bulkActivity.setActivityExecutor(activityExecutors.get(activity));
         bulkActivity.setCallbackExecutor(callbackExecutors.get(activity));
         BulkTargetRetryPolicy retryPolicy = retryPolicies.get(activity);
@@ -164,9 +165,6 @@ public void initialize() {
             provider.configure(environment);
             providers.put(provider.getActivity(), provider);
         }
-        pnfsHandler = new PnfsHandler(pnfsManager);
-        pnfsHandler.setRestriction(Restrictions.none());
-        pnfsHandler.setSubject(Subjects.ROOT);
     }
 
     /**
@@ -240,8 +238,15 @@ public void setEnvironment(Map<String, Object> environment) {
     private void configureEndpoints(BulkActivity activity) {
         if (activity instanceof NamespaceHandlerAware) {
             PnfsHandler pnfsHandler = new PnfsHandler(pnfsManager);
-            pnfsHandler.setRestriction(activity.getRestriction());
-            pnfsHandler.setSubject(activity.getSubject());
+            Subject subject = activity.getSubject();
+            Restriction restriction = activity.getRestriction();
+            if (Subjects.hasAdminRole(subject)) {
+                pnfsHandler.setSubject(Subjects.ROOT);
+                pnfsHandler.setRestriction(Restrictions.none());
+            } else {
+                pnfsHandler.setSubject(subject);
+                pnfsHandler.setRestriction(restriction);
+            }
             ((NamespaceHandlerAware) activity).setNamespaceHandler(pnfsHandler);
         }
 

modules/dcache-bulk/src/main/java/org/dcache/services/bulk/activity/plugin/delete/DeleteActivity.java

@@ -91,6 +91,7 @@ public DeleteActivity(String name, TargetType targetType) {
     public ListenableFuture<PnfsDeleteEntryMessage> perform(String rid, long tid, FsPath path,
           FileAttributes attributes) {
         PnfsDeleteEntryMessage msg = new PnfsDeleteEntryMessage(path.toString());
+        msg.setSubject(subject);
         if (attributes != null && attributes.getFileType() == FileType.DIR && skipDirs) {
             msg.setSucceeded();
             return Futures.immediateFuture(msg);

modules/dcache-bulk/src/main/java/org/dcache/services/bulk/store/jdbc/request/JdbcBulkRequestStore.java

@@ -325,8 +325,8 @@ public BulkArchivedRequestInfo getArchivedInfo(Subject subject, String rid)
 
         BulkArchivedRequestInfo info = list.get(0);
 
-        if (!Subjects.isRoot(subject) && !BulkRequestStore.uidGidKey(subject)
-              .equals(info.getOwner())) {
+        if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject) &&
+              !BulkRequestStore.uidGidKey(subject).equals(info.getOwner())) {
             throw new BulkPermissionDeniedException("Subject does not have "
                   + "permission to read archived request " + rid);
         }
@@ -468,7 +468,7 @@ public boolean isRequestSubject(Subject subject, String uid)
         if (requestSubject.isEmpty()) {
             return false;
         }
-        return Subjects.isRoot(subject) ||
+        return Subjects.isRoot(subject) || Subjects.hasAdminRole(subject) ||
               BulkRequestStore.uidGidKey(subject)
                     .equals(BulkRequestStore.uidGidKey(requestSubject.get()));
     }
@@ -773,7 +773,8 @@ private String checkRequestPermissions(Subject subject, String uid)
                   + "request has no subject.", uid);
         }
 
-        if (!Subjects.isRoot(subject) && !key.equals(BulkRequestStore.uidGidKey(requestSubject))) {
+        if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject) &&
+              !key.equals(BulkRequestStore.uidGidKey(requestSubject))) {
             throw new BulkPermissionDeniedException(uid);
         }
 

modules/dcache-frontend/src/main/java/org/dcache/restful/resources/bulk/BulkResources.java

@@ -59,11 +59,15 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
  */
 package org.dcache.restful.resources.bulk;
 
+import static org.dcache.restful.util.HttpServletRequests.getUserRootAwareTargetPrefix;
+import static org.dcache.restful.util.JSONUtils.newBadRequestException;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.gson.Gson;
 import com.google.gson.JsonParseException;
+import diskCacheV111.util.PnfsHandler;
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiParam;
@@ -72,10 +76,14 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import io.swagger.annotations.Authorization;
 import io.swagger.annotations.Example;
 import io.swagger.annotations.ExampleProperty;
-import org.json.JSONArray;
-import org.json.JSONObject;
-import org.springframework.stereotype.Component;
-
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.inject.Named;
 import javax.security.auth.Subject;
@@ -95,19 +103,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.stream.Collectors;
-
-import diskCacheV111.util.PnfsHandler;
-
-import org.dcache.auth.Subjects;
 import org.dcache.auth.attributes.Restriction;
 import org.dcache.auth.attributes.Restrictions;
 import org.dcache.cells.CellStub;
@@ -129,9 +124,9 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import org.dcache.services.bulk.BulkRequestStatus;
 import org.dcache.services.bulk.BulkRequestStatusMessage;
 import org.dcache.services.bulk.BulkRequestSummary;
-
-import static org.dcache.restful.util.HttpServletRequests.getUserRootAwareTargetPrefix;
-import static org.dcache.restful.util.JSONUtils.newBadRequestException;
+import org.json.JSONArray;
+import org.json.JSONObject;
+import org.springframework.stereotype.Component;
 
 /**
  * <p>RESTful API to the BulkService.</p>
@@ -483,10 +478,6 @@ public static Subject getSubject() {
             throw new NotAuthorizedException("User cannot be anonymous.");
         }
 
-        if (RequestUser.isAdmin()) {
-            return Subjects.ROOT;
-        }
-
         return RequestUser.getSubject();
     }
 

modules/dcache-frontend/src/main/java/org/dcache/restful/resources/namespace/FileResources.java

@@ -446,7 +446,7 @@ public Response cmrResources(
                     String targetQos = reqPayload.getString("target");
                     String qosPolicy = reqPayload.getString("policy");
                     Integer qosState = (Integer)reqPayload.get("state");
-                    Subject subject = RequestUser.isAdmin() ? Subjects.ROOT : RequestUser.getSubject();
+                    Subject subject = RequestUser.getSubject();
                     if (!useQosService) {
                         new QoSTransitionEngine(poolmanager,
                               poolMonitor,

modules/dcache-frontend/src/main/java/org/dcache/restful/util/HttpServletRequests.java

@@ -27,8 +27,6 @@
 import javax.security.auth.Subject;
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.BadRequestException;
-import org.dcache.auth.RolePrincipal;
-import org.dcache.auth.RolePrincipal.Role;
 import org.dcache.auth.Subjects;
 import org.dcache.auth.attributes.LoginAttribute;
 import org.dcache.auth.attributes.Restriction;
@@ -56,9 +54,7 @@ public static Set<LoginAttribute> getLoginAttributes(HttpServletRequest request)
     }
 
     public static boolean isAdmin(HttpServletRequest request) {
-        return RequestUser.getSubject().getPrincipals().stream()
-              .filter(p -> p instanceof RolePrincipal)
-              .anyMatch(p -> ((RolePrincipal) p).hasRole(Role.ADMIN));
+        return Subjects.hasAdminRole(RequestUser.getSubject());
     }
 
     public static Subject roleAwareSubject(HttpServletRequest request) {

modules/dcache-qos/src/main/java/org/dcache/qos/remote/clients/RemoteQoSVerificationClient.java

@@ -100,7 +100,14 @@ public void fileQoSAdjustmentCompleted(QoSAdjustmentResponse adjustmentResponse)
     @Override
     public void fileQoSVerificationCancelled(PnfsId pnfsId, Subject subject) throws QoSException {
         QoSVerificationCancelledMessage msg = new QoSVerificationCancelledMessage(pnfsId);
-        msg.setSubject(subject);
+        /*
+         *  The only cancellation that can currently take place is either
+         *  through the Bulk service or the admin interface.
+         *  In the former case, a pre-authorization check of the subject is done;
+         *  in the latter, the subject has the admin role.
+         *  Hence no further authorization is necessary here and no futher checks of
+         *  the subject are done.
+         */
         verificationService.send(msg);
     }
 

modules/dcache-qos/src/main/java/org/dcache/qos/services/engine/util/QoSPolicyCache.java

@@ -70,6 +70,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import javax.security.auth.Subject;
 import org.dcache.auth.Subjects;
 import org.dcache.cells.CellStub;
 import org.dcache.qos.QoSPolicy;
@@ -155,7 +156,8 @@ public PnfsManagerGetQoSPolicyMessage messageArrived(PnfsManagerGetQoSPolicyMess
     }
 
     public PnfsManagerRmQoSPolicyMessage messageArrived(PnfsManagerRmQoSPolicyMessage message) throws CacheException {
-        if (!Subjects.isRoot(message.getSubject())) {
+        Subject subject = message.getSubject();
+        if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject)) {
             throw new PermissionDeniedCacheException("not authorized to remove policies.");
         }
         removePolicy(message.getPolicyName());
@@ -169,7 +171,8 @@ public PnfsManagerRmQoSPolicyMessage messageArrived(PnfsManagerRmQoSPolicyMessag
      */
     public PnfsManagerAddQoSPolicyMessage messageArrived(PnfsManagerAddQoSPolicyMessage message)
           throws CacheException {
-        if (!Subjects.isRoot(message.getSubject())) {
+        Subject subject = message.getSubject();
+        if (!Subjects.isRoot(subject) && !Subjects.hasAdminRole(subject)) {
             throw new PermissionDeniedCacheException("not authorized to update policies.");
         }
 

modules/dcache/src/main/java/org/dcache/pinmanager/DefaultAuthorizationPolicy.java

@@ -7,7 +7,7 @@
 public class DefaultAuthorizationPolicy implements AuthorizationPolicy {
 
     private boolean isAuthorized(Subject subject, Pin pin) {
-        return (Subjects.isRoot(subject) ||
+        return (Subjects.isRoot(subject) || Subjects.hasAdminRole(subject) ||
               Subjects.hasUid(subject, pin.getUid()) ||
               Subjects.hasGid(subject, pin.getGid()));
     }