Skip to content

Commit

Permalink
gplazma: oidc allow suppression of offline verification
Browse files Browse the repository at this point in the history
Motivation:

Some OPs (e.g., EGI-CheckIn) provide more information about the user via
the userinfo endpoint that is encoded in the JWT.  If that additional
information is important then dCache must use the userinfo endpoint.

Modification:

When attempting to verify a JWT, check whether the OP is configured to
suppress offline verification.  If so, fail the offline verification and
fall back to querying the userinfo endpoint.

Result:

An OP declaration now supports the `-suppress=offline` argument to force
dCache to query the userinfo endpoint, even if the access token is a
JWT.  A warning is logged on startup if offline verification is
suppressed.

Target: master
Request: 9.0
Request: 8.2
Requires-notes: yes
Requires-book: no
Patch: https://rb.dcache.org/r/13902/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Mar 14, 2023
1 parent f13812e commit 1d41db5
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class Issuer {
private final Queue<String> previousJtis;
private final IdentityProvider provider;
private final HttpClient client;
private final boolean offlineSuppressed;

private final Supplier<Map<String, PublicKey>> keys = MemoizeMapWithExpiry.memorize(this::readJwksDocument)
.whenEmptyFor(Duration.ofMinutes(1))
Expand All @@ -70,6 +71,17 @@ public Issuer(HttpClient client, IdentityProvider provider, int tokenHistory) {
this.provider = requireNonNull(provider);
this.client = requireNonNull(client);
previousJtis = tokenHistory > 0 ? EvictingQueue.create(tokenHistory) : null;
offlineSuppressed = provider.isSuppressed("offline");
if (offlineSuppressed) {
LOGGER.warn("Offline verification of JWT access tokens issued by OP {} has been "
+ "suppressed. This may cause various problems, including dCache being slow to "
+ "process requests with such tokens and dCache generating high load for the OP.",
provider.getName());
}
}

public boolean isOfflineSuppressed() {
return offlineSuppressed;
}

public IdentityProvider getIdentityProvider() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public ExtractResult extract(String token) throws AuthenticationException, Unabl

var issuer = issuerOf(jwt);

if (issuer.isOfflineSuppressed()) {
throw new UnableToProcess("offline suppressed");
}

return new ExtractResult(issuer.getIdentityProvider(), jwt.getPayloadMap());
} catch (IOException e) {
throw new UnableToProcess(e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ public MockIdentityProviderBuilder withDiscovery(String json) {
return this;
}

public MockIdentityProviderBuilder withSuppress(String keyword) {
BDDMockito.given(provider.isSuppressed(keyword)).willReturn(true);
return this;
}

public IdentityProvider build() {
return provider;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,24 @@ public void shouldReturnEndpoint() throws Exception {
assertThat(endpoint, is(equalTo("https://oidc.example.org/")));
}

@Test
public void shouldNotShowSuppressedByDefault() throws Exception {
given(aClient());
given(anIp("EXAMPLE").withEndpoint("https://oidc.example.org/"));
given(anIssuer().withoutHistory());

assertThat(issuer.isOfflineSuppressed(), is(equalTo(false)));
}

@Test
public void shouldShowOfflineSuppressedWhenConfigured() throws Exception {
given(aClient());
given(anIp("EXAMPLE").withEndpoint("https://oidc.example.org/").withSuppress("offline"));
given(anIssuer().withoutHistory());

assertThat(issuer.isOfflineSuppressed(), is(equalTo(true)));
}

@Test
public void shouldAcceptJwtWithoutKidWhenJkwsNoKid() throws Exception {
given(aClient().onGet("https://oidc.example.org/jwks").responds()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,19 @@ public void shouldNotProcessNonJwt() throws Exception {
verification.extract("token-that-is-not-a-jwt");
}

@Test(expected=UnableToProcess.class)
public void shouldNotProcessValidJwtWhenOfflineSuppressed() throws Exception {
given(anIp("EXAMPLE").withEndpoint("https://oidc.example.org/"));
given(anOfflineJwtVerification()
.withEmptyAudienceTargetProperty()
.withIssuer(anIssuer().withIp(identityProvider).withOfflineSuppressed()));
given(aJwt()
.withPayloadClaim("iss", "https://oidc.example.org/")
.withPayloadClaim("sub", "paul"));

verification.extract(jwt);
}

@Test(expected=AuthenticationException.class)
public void shouldRejectTokenWithoutIss() throws Exception {
given(anIp("EXAMPLE").withEndpoint("https://oidc.example.org/"));
Expand Down Expand Up @@ -271,6 +284,11 @@ public MockIssuerBuilder withIp(IdentityProvider ip) {
return this;
}

public MockIssuerBuilder withOfflineSuppressed() {
BDDMockito.given(issuer.isOfflineSuppressed()).willReturn(true);
return this;
}

public Issuer build() {
checkState(hasIp);
return issuer;
Expand Down
18 changes: 18 additions & 0 deletions skel/share/defaults/gplazma.properties
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,24 @@ gplazma.htpasswd.file.cache-period.unit = SECONDS
#
# The following suppress keywords are supported:
#
# offline Suppress offline validation. By default, dCache will
# attempt to validate a JWT using the OP's public keys.
# If the token is valid then the claims stored within
# the JWT are used by dCache. This option suppresses
# this behaviour, resulting in dCache making an HTTP
# request to the OP's userinfo endpoint to check the
# validity of the token and obtain the claims that
# describe the user.
#
# In general, offline verification is preferred as it
# is faster and avoids making an HTTP request (one per
# token) against the OP.
#
# Suppressing offline validation is needed if OP
# includes less information in the JWT than is
# available from the userinfo endpoint and that
# additional information is needed (e.g., group
# membership).
#
(prefix)gplazma.oidc.provider = ${dcache.oidc.provider}

Expand Down

0 comments on commit 1d41db5

Please sign in to comment.