oidc: reject storage scopes without path

Fixes commit 094c171

Motivation:
According WLCG Common JWT Profiles document:

  For all  storage.* scopes,  $PATH MUST be specified (but may be /to authorize the entire
  resource associated with the issuer); if not specified for these scopes, the token MUST be
  rejected.

However, dCache doesn't follow this rule, thus, such scopes are accepted
and path is set to '/'.

Modification:
Update WlcgProfileScope.Operation enum to indicate whatever path is
required or optional for a given scope. Require path for all 'storage.*'
scopes.

Result:

spec compatibility and potential security fix.

Ticket: #10523
Acked-by: Paul Millar
Acked-by: Lea Morschel
Acked-by: Dmitry Litvintsev
Target: master, 9.2, 9.1, 9.0, 8.2
Require-book: no
Require-notes: yes
(cherry picked from commit 83d1e77)
Signed-off-by: Tigran Mkrtchyan <[email protected]>

modules/gplazma2-oidc/src/main/java/org/dcache/gplazma/oidc/profiles/WlcgProfileScope.java

@@ -56,7 +56,7 @@ public enum Operation {
          * Read data. Only applies to “online” resources such as disk (as opposed to “nearline” such
          * as tape where the {@literal stage} authorization should be used in addition).
          */
-        READ("storage.read", LIST, READ_METADATA, DOWNLOAD),
+        READ("storage.read", true, LIST, READ_METADATA, DOWNLOAD),
 
         /**
          * Upload data. This includes renaming files if the destination file does not already exist.
@@ -67,47 +67,50 @@ public enum Operation {
          * use case for a separate {@literal storage.create} scope is to enable stage-out of data
          * from jobs on a worker node.
          */
-        CREATE("storage.create", READ_METADATA, UPLOAD, MANAGE, UPDATE_METADATA),
+        CREATE("storage.create", true, READ_METADATA, UPLOAD, MANAGE, UPDATE_METADATA),
 
         /**
          * Change data. This includes renaming files, creating new files, and writing data. This
          * permission includes overwriting or replacing stored data in addition to deleting or
          * truncating data. This is a strict superset of {@literal storage.create}.
          */
-        MODIFY("storage.modify", LIST, READ_METADATA, UPLOAD, MANAGE, DELETE, UPDATE_METADATA),
+        MODIFY("storage.modify", true, LIST, READ_METADATA, UPLOAD, MANAGE, DELETE, UPDATE_METADATA),
 
         /**
          * Read the data, potentially causing data to be staged from a nearline resource to an
          * online resource. This is a superset of {@literal storage.read}.
          */
-        STAGE("storage.stage", LIST, READ_METADATA, DOWNLOAD), // FIXME need to allow staging.
+        STAGE("storage.stage", true, LIST, READ_METADATA, DOWNLOAD), // FIXME need to allow staging.
 
         /**
          * "Read" or query information about job status and attributes.
          */
-        COMPUTE_READ("compute.read"),
+        COMPUTE_READ("compute.read", false),
 
         /**
          * Modify or change the attributes of an existing job.
          */
-        COMPUTE_MODIFY("compute.modify"),
+        COMPUTE_MODIFY("compute.modify", false),
 
         /**
          * Create or submit a new job at the computing resource.
          */
-        COMPUTE_CREATE("compute.create"),
+        COMPUTE_CREATE("compute.create", false),
 
         /**
          * Delete a job from the computing resource, potentially terminating
          * a running job.
          */
-        COMPUTE_CANCEL("compute.cancel");
+        COMPUTE_CANCEL("compute.cancel", false);
 
         private final String label;
         private final EnumSet<Activity> allowedActivities;
 
-        private Operation(String label, Activity... allowedActivities) {
+        private final boolean requirePath;
+
+        private Operation(String label, boolean requirePath, Activity... allowedActivities) {
             this.label = label;
+            this.requirePath = requirePath;
             this.allowedActivities = allowedActivities.length == 0
                     ? EnumSet.noneOf(Activity.class)
                     : EnumSet.copyOf(asList(allowedActivities));
@@ -120,6 +123,10 @@ public String getLabel() {
         public EnumSet<Activity> allowedActivities() {
             return allowedActivities;
         }
+
+        public boolean isPathRequired() {
+            return requirePath;
+        }
     }
 
     private static final Map<String, Operation> OPERATIONS_BY_LABEL;
@@ -146,6 +153,7 @@ public WlcgProfileScope(String scope) {
         checkScopeValid(operation != null, "Unknown operation %s", operationLabel);
 
         if (colon == -1) {
+            checkScopeValid(!operation.isPathRequired(), "Path must be specified");
             path = "/";
         } else {
             String scopePath = scope.substring(colon + 1);

modules/gplazma2-oidc/src/test/java/org/dcache/gplazma/oidc/profiles/WlcgProfileScopeTest.java

@@ -58,6 +58,22 @@ public void shouldNotIdentifyStorageWriteScope() {
         assertFalse(WlcgProfileScope.isWlcgProfileScope("storage.write:/"));
     }
 
+
+    @Test(expected = InvalidScopeException.class)
+    public void shouldRejectStorageCreateScopeWithoutPath() {
+        new WlcgProfileScope("storage.create");
+    }
+
+    @Test(expected = InvalidScopeException.class)
+    public void shouldRejectStorageReadScopeWithoutPath() {
+        new WlcgProfileScope("storage.read");
+    }
+
+    @Test(expected = InvalidScopeException.class)
+    public void shouldRejectStorageModifyScopeWithoutPath() {
+        new WlcgProfileScope("storage.modify");
+    }
+
     @Test
     public void shouldIdentifyComputeReadScope() {
         assertTrue(WlcgProfileScope.isWlcgProfileScope("compute.read"));