Skip to content

Commit

Permalink
gplazma: oidc update explicit AuthZ parsing
Browse files Browse the repository at this point in the history
Motivation:

The WLCG JWT Profile describes how JWTs may include explicit
authorisation statements in the 'scope' claim (e.g.,
`storage.read:/foo`).

When processing a token, the oidc gPlazma plugin decides whether the
token carries explicit AuthZ statements.  If it does, then the resulting
login will suspend namespace-based authorisation and fully honour the
authorisation from the token.

There are a few issues with how dCache currently parses these explicit
AuthZ statements.

  1.  The profile describes how the resource identifier is optional
      (e.g., `storage.read` is valid), but dCache scope parsing
      currently rejects explicit AuthZ if the resource identifier is
      missing.

  2.  The profile describes several explicit AuthZ statements (e.g.,
      `compute.read`).  Currently, dCache completely ignores these
      statements.  However, this is not complete correct because such
      compute explicit AuthZ statements indicate the token (in general)
      is carrying explicit AuthZ.  Therefore, the presence of compute
      explicit AuthZ statements and a lack of any storage explicit AuthZ
      statements should result in dCache rejecting requests by that
      token.

Modification:

Update scope parsing components to relax the requirement on having a
resource identifier.  An explicit AuthZ statement without a resource
identifier is equivalent to adding `:/` to the statement (e.g.,
`storage.read` interpreted as `storage.read:/`).

Add extra valid values for the "compute.\*" explicit AuthZ statements.
This are ignored by the rest of dCache, but their presence will result
in dCache considering the token as one with explicit AuthZ.

Result:

dCache will now accept non-targeted explicit AuthZ statements in the
scope claim (e.g., `storage.read`).  dCache will consider tokens
containing compute explicit AuthZ statements but without any storage
explicit AuthZ statements as tokens with explicit authorisation; the
lack of any storage explicit authorisation statements will result in all
requests with that token being denied.

Target: master
Requires-notes: yes
Requires-book: no
Request: 9.2
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/13996/
Acked-by: Dmitry Litvintsev
  • Loading branch information
paulmillar committed Oct 4, 2023
1 parent 49661e6 commit 094c171
Show file tree
Hide file tree
Showing 3 changed files with 182 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2020-2022 Deutsches Elektronen-Synchrotron
* Copyright (C) 2020-2023 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
Expand Down Expand Up @@ -80,14 +80,37 @@ public enum Operation {
* 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", LIST, READ_METADATA, DOWNLOAD), // FIXME need to allow staging.

/**
* "Read" or query information about job status and attributes.
*/
COMPUTE_READ("compute.read"),

/**
* Modify or change the attributes of an existing job.
*/
COMPUTE_MODIFY("compute.modify"),

/**
* Create or submit a new job at the computing resource.
*/
COMPUTE_CREATE("compute.create"),

/**
* Delete a job from the computing resource, potentially terminating
* a running job.
*/
COMPUTE_CANCEL("compute.cancel");

private final String label;
private final EnumSet<Activity> allowedActivities;

private Operation(String label, Activity... allowedActivities) {
this.label = label;
this.allowedActivities = EnumSet.copyOf(asList(allowedActivities));
this.allowedActivities = allowedActivities.length == 0
? EnumSet.noneOf(Activity.class)
: EnumSet.copyOf(asList(allowedActivities));
}

public String getLabel() {
Expand Down Expand Up @@ -116,29 +139,38 @@ public EnumSet<Activity> allowedActivities() {

public WlcgProfileScope(String scope) {
int colon = scope.indexOf(':');
checkScopeValid(colon != -1, "Missing ':' in scope");

String operationLabel = scope.substring(0, colon);
String operationLabel = colon == -1 ? scope : scope.substring(0, colon);

operation = OPERATIONS_BY_LABEL.get(operationLabel);
checkScopeValid(operation != null, "Unknown operation %s", operationLabel);

String scopePath = scope.substring(colon + 1);
checkScopeValid(scopePath.startsWith("/"), "Path does not start with /");
if (colon == -1) {
path = "/";
} else {
String scopePath = scope.substring(colon + 1);
checkScopeValid(scopePath.startsWith("/"), "Path does not start with /");

path = URI.create(scopePath).getPath();
path = URI.create(scopePath).getPath();
}

LOGGER.debug("WlcgProfileScope created from scope \"{}\": op={} path={}",
scope, operation, path);
}

public static boolean isWlcgProfileScope(String scope) {
int colon = scope.indexOf(':');
return colon >= MINIMUM_LABEL_SIZE
&& OPERATIONS_BY_LABEL.keySet().contains(scope.substring(0, colon));
String authz = colon == -1 ? scope : scope.substring(0, colon);
return authz.length() >= MINIMUM_LABEL_SIZE
&& OPERATIONS_BY_LABEL.keySet().contains(authz);
}

@Override
public Optional<MultiTargetedRestriction.Authorisation> authorisation(FsPath prefix) {
if (operation.allowedActivities.isEmpty()) {
return Optional.empty();
}

FsPath absPath = prefix.resolve(path.substring(1));
LOGGER.debug("WlcgProfileScope authorising {} with prefix \"{}\" to path {}",
prefix, operation.allowedActivities, absPath);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2020-2022 Deutsches Elektronen-Synchrotron
* Copyright (C) 2020-2023 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
Expand All @@ -17,8 +17,6 @@
*/
package org.dcache.gplazma.oidc.profiles;

import org.dcache.gplazma.oidc.profiles.WlcgProfileScope;

import static org.dcache.auth.attributes.Activity.DOWNLOAD;
import static org.dcache.auth.attributes.Activity.LIST;
import static org.dcache.auth.attributes.Activity.READ_METADATA;
Expand All @@ -35,6 +33,11 @@

public class WlcgProfileScopeTest {

@Test(expected=InvalidScopeException.class)
public void shouldRejectUnknownScope() {
new WlcgProfileScope("unknown-value");
}

@Test
public void shouldIdentifyStorageReadScope() {
assertTrue(WlcgProfileScope.isWlcgProfileScope("storage.read:/"));
Expand All @@ -56,7 +59,41 @@ public void shouldNotIdentifyStorageWriteScope() {
}

@Test
public void shouldParseReadScope() {
public void shouldIdentifyComputeReadScope() {
assertTrue(WlcgProfileScope.isWlcgProfileScope("compute.read"));
}

@Test
public void shouldIdentifyComputeModifyScope() {
assertTrue(WlcgProfileScope.isWlcgProfileScope("compute.modify"));
}

@Test
public void shouldIdentifyComputeCreateScope() {
assertTrue(WlcgProfileScope.isWlcgProfileScope("compute.create"));
}

@Test
public void shouldIdentifyComputeCancelScope() {
assertTrue(WlcgProfileScope.isWlcgProfileScope("compute.cancel"));
}

@Test
public void shouldParseReadScopeWithoutResourcePath() {
WlcgProfileScope scope = new WlcgProfileScope("storage.read");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isPresent());

Authorisation auth = maybeAuth.get();

assertThat(auth.getPath(), equalTo(FsPath.create("/VOs/wlcg")));
assertThat(auth.getActivity(), containsInAnyOrder(LIST, READ_METADATA, DOWNLOAD));
}

@Test
public void shouldParseReadScopeWithRootResourcePath() {
WlcgProfileScope scope = new WlcgProfileScope("storage.read:/");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));
Expand All @@ -68,4 +105,59 @@ public void shouldParseReadScope() {
assertThat(auth.getPath(), equalTo(FsPath.create("/VOs/wlcg")));
assertThat(auth.getActivity(), containsInAnyOrder(LIST, READ_METADATA, DOWNLOAD));
}

@Test
public void shouldParseReadScopeWithNonRootResourcePath() {
WlcgProfileScope scope = new WlcgProfileScope("storage.read:/foo");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isPresent());

Authorisation auth = maybeAuth.get();

assertThat(auth.getPath(), equalTo(FsPath.create("/VOs/wlcg/foo")));
assertThat(auth.getActivity(), containsInAnyOrder(LIST, READ_METADATA, DOWNLOAD));
}

@Test(expected=InvalidScopeException.class)
public void shouldRejectReadScopeWithRelativeResourcePath() {
new WlcgProfileScope("storage.read:foo");
}

@Test
public void shouldParseComputeReadScope() {
WlcgProfileScope scope = new WlcgProfileScope("compute.read");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isEmpty());
}

@Test
public void shouldParseComputeModifyScope() {
WlcgProfileScope scope = new WlcgProfileScope("compute.modify");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isEmpty());
}

@Test
public void shouldParseComputeCreateScope() {
WlcgProfileScope scope = new WlcgProfileScope("compute.create");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isEmpty());
}

@Test
public void shouldParseComputeCancelScope() {
WlcgProfileScope scope = new WlcgProfileScope("compute.cancel");

Optional<Authorisation> maybeAuth = scope.authorisation(FsPath.create("/VOs/wlcg"));

assertTrue(maybeAuth.isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,36 @@ public void shouldAcceptRootReadAndNonRootModifyWlcgScope() throws Exception {
assertFalse(r.isRestricted(Activity.UPDATE_METADATA, FsPath.create("/prefix/write-target/my-file")));
}

@Test
public void shouldAcceptComputeReadScopeWithDenyAllAuthz() throws Exception {
given(aWlcgProfile().withPrefix("/prefix"));

when(invoked().withIdP(anIp("MY-OP"))
.withStringClaim("wlcg.ver", "1.0")
.withStringClaim("scope", "openid compute.read"));

assertThat(principals, hasItem(any(ExemptFromNamespaceChecks.class)));
assertThat(restriction, isPresent());
Restriction r = restriction.get();
assertTrue(r.isRestricted(Activity.UPLOAD, FsPath.create("/my-file")));
assertTrue(r.isRestricted(Activity.MANAGE, FsPath.create("/my-file")));
assertTrue(r.isRestricted(Activity.DELETE, FsPath.create("/my-file")));
assertTrue(r.isRestricted(Activity.UPDATE_METADATA, FsPath.create("/my-file")));
assertTrue(r.isRestricted(Activity.DOWNLOAD, FsPath.create("/my-file")));

assertTrue(r.isRestricted(Activity.UPLOAD, FsPath.create("/other/my-file")));
assertTrue(r.isRestricted(Activity.MANAGE, FsPath.create("/other/my-file")));
assertTrue(r.isRestricted(Activity.DELETE, FsPath.create("/other/my-file")));
assertTrue(r.isRestricted(Activity.UPDATE_METADATA, FsPath.create("/other/my-file")));
assertTrue(r.isRestricted(Activity.DOWNLOAD, FsPath.create("/other/my-file")));

assertTrue(r.isRestricted(Activity.UPLOAD, FsPath.create("/prefix/my-file")));
assertTrue(r.isRestricted(Activity.MANAGE, FsPath.create("/prefix/my-file")));
assertTrue(r.isRestricted(Activity.DELETE, FsPath.create("/prefix/my-file")));
assertTrue(r.isRestricted(Activity.UPDATE_METADATA, FsPath.create("/prefix/my-file")));
assertTrue(r.isRestricted(Activity.DOWNLOAD, FsPath.create("/prefix/my-file")));
}

@Test
public void shouldIncludeAuthzIdentity() throws Exception {
given(aWlcgProfile().withPrefix("/prefix")
Expand All @@ -497,6 +527,20 @@ public void shouldIncludeAuthzIdentity() throws Exception {
assertThat(principals, not(hasItem(new GroupNamePrincipal("non-authz-group"))));
}

@Test
public void shouldIncludeAuthzIdentityWhenAcceptingComputeReadScope() throws Exception {
given(aWlcgProfile().withPrefix("/prefix")
.withAuthzIdentity(aSetOfPrincipals().withGroupname("authz-group"))
.withNonAuthzIdentity(aSetOfPrincipals().withGroupname("non-authz-group")));

when(invoked().withIdP(anIp("MY-OP"))
.withStringClaim("wlcg.ver", "1.0")
.withStringClaim("scope", "openid compute.read"));

assertThat(principals, hasItem(new GroupNamePrincipal("authz-group")));
assertThat(principals, not(hasItem(new GroupNamePrincipal("non-authz-group"))));
}

private void given(WlcgProfileBuilder builder) {

profile = builder.build();
Expand Down

0 comments on commit 094c171

Please sign in to comment.