Skip to content

Commit

Permalink
gplazma: scitoken make ExemptFromNamespaceChecks principal optional
Browse files Browse the repository at this point in the history
Motivation:

A change, introduced with commit d9eb9a9, introduces a new
@AuthenticationOutput principal: ExemptFromNamespaceChecks.  However,
that patch failed to provide a safe-guard that prevents that new
principal being sent to dCache nodes that run a version of dCache that
doesn't support that new principal.

For example, if a Subject (containing ExemptFromNamespaceChecks) is sent
to a pool node that doesn't support the principal then that message will
fail to deserialise.  This means that, for SciToken clients, dCache will
no longer work correctly after upgrading gPlazma.

Modification:

Make the inclusion of the ExemptFromNamespaceChecks principal
configurable, disabled by default.

Add unit tests to verify correct behaviour.

Result:

Avoid an unreleased regression in backwards compatibility.

Target: master
Requires-notes: no
Requires-book: no
Request: 7.2
Request: 7.1
Request: 7.0
Request: 6.2
  • Loading branch information
paulmillar authored and mksahakyan committed Oct 12, 2021
1 parent 14f2019 commit 08b81e3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
Expand Up @@ -70,6 +70,7 @@ public class SciTokenPlugin implements GPlazmaAuthenticationPlugin
private final Set<String> audienceTargets;

private int tokenHistory = 0;
private boolean exemptPrincipalSupported;

public SciTokenPlugin(Properties properties)
{
Expand All @@ -89,6 +90,10 @@ public SciTokenPlugin(Properties properties, HttpClient client)

String targets = properties.getProperty("gplazma.scitoken.audience-targets");
audienceTargets = ImmutableSet.copyOf(Splitter.on(' ').trimResults().split(targets));

// Remove this configuration property once dCache 8.0 is released.
String supportExempt = properties.getProperty("gplazma.scitoken.dcache-supports-exempt-principal");
exemptPrincipalSupported = Boolean.parseBoolean(supportExempt);
}

private boolean isIssuer(Object key)
Expand Down Expand Up @@ -158,7 +163,9 @@ public void authenticate(Set<Object> publicCredentials, Set<Object> privateCrede
Restriction r = buildRestriction(issuer.getPrefix(), scopes);
LOGGER.debug("Authenticated user with restriction: {}", r);
restrictions.add(r);
identifiedPrincipals.add(new ExemptFromNamespaceChecks());
if (exemptPrincipalSupported) {
identifiedPrincipals.add(new ExemptFromNamespaceChecks());
}
} catch (IOException e) {
throw new AuthenticationException(e.getMessage());
}
Expand Down
Expand Up @@ -66,6 +66,7 @@
import diskCacheV111.util.FsPath;

import org.dcache.auth.BearerTokenCredential;
import org.dcache.auth.ExemptFromNamespaceChecks;
import org.dcache.auth.GidPrincipal;
import org.dcache.auth.JwtJtiPrincipal;
import org.dcache.auth.JwtSubPrincipal;
Expand All @@ -78,7 +79,11 @@
import static java.time.temporal.ChronoUnit.MINUTES;
import static java.util.Objects.requireNonNull;
import static org.dcache.auth.attributes.Activity.*;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -1127,6 +1132,40 @@ public void shouldSupportBriansToken() throws Exception
assertTrue(resultingRestriction.isRestricted(DELETE, FsPath.create("/my-file.dat")));
}

@Test
public void shouldIncludeExemptionPricipal() throws Exception
{
given(aSciTokenPlugin()
.withProperty("gplazma.scitoken.issuer!EXAMPLE", "https://example.org/ /prefix uid:1000 gid:1000")
.withProperty("gplazma.scitoken.dcache-supports-exempt-principal", "true"));
givenThat("OP1", isAnIssuer().withURL("https://example.org/").withKey("key1", rsa256Keys()));

whenAuthenticatingWith(aJwtToken()
.withRandomSub()
.withRandomJti()
.withClaim("scope", "openid offline_access storage.read:/ storage.modify:/ wlcg")
.issuedBy("OP1").usingKey("key1"));

assertThat(identifiedPrincipals, hasItem(instanceOf(ExemptFromNamespaceChecks.class)));
}

@Test
public void shouldSuppressExemptionPricipal() throws Exception
{
given(aSciTokenPlugin()
.withProperty("gplazma.scitoken.issuer!EXAMPLE", "https://example.org/ /prefix uid:1000 gid:1000")
.withProperty("gplazma.scitoken.dcache-supports-exempt-principal", "false"));
givenThat("OP1", isAnIssuer().withURL("https://example.org/").withKey("key1", rsa256Keys()));

whenAuthenticatingWith(aJwtToken()
.withRandomSub()
.withRandomJti()
.withClaim("scope", "openid offline_access storage.read:/ storage.modify:/ wlcg")
.issuedBy("OP1").usingKey("key1"));

assertThat(identifiedPrincipals, not(hasItem(instanceOf(ExemptFromNamespaceChecks.class))));
}

private void whenAuthenticatingWith(PrincipalSetMaker maker) throws AuthenticationException
{
identifiedPrincipals.addAll(maker.build());
Expand Down
14 changes: 14 additions & 0 deletions skel/share/defaults/gplazma.properties
Expand Up @@ -558,6 +558,20 @@ gplazma.scitoken.token-history = 0
#
gplazma.scitoken.audience-targets =

# The SciToken plugin can issue a special principal that ensures
# dCache will honour the authorisation statements within the token.
# This requires ALL nodes in the dCache cluster have a sufficiently
# up-to-date version of dCache. This is automatically satisfied if
# all nodes are running the same version of dCache; otherwise, this
# is only satisfied if all nodes support the 'scope' based
# authorisation.
#
# Enable this option ONLY if all nodes are running a "new enough"
# version of dCache.
#
(one-of?true|false)\
gplazma.scitoken.dcache-supports-exempt-principal = false

# ---- Roles plugin
#
# Roles allow dCache to behave differently between two successful
Expand Down

0 comments on commit 08b81e3

Please sign in to comment.