honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix refresh remote JWKS logic (#42662) 

This change ensures that:

- We only attempt to refresh the remote JWKS when there is a
signature related error only ( BadJWSException instead of the
geric BadJOSEException )
- We do call OpenIDConnectAuthenticator#getUserClaims upon
successful refresh.
- We test this in OpenIdConnectAuthenticatorTests.

Without this fix, when using the OpenID Connect realm with a remote
JWKSet configured in `op.jwks_path`, the refresh would be triggered
for most configuration errors ( i.e. wrong value for `op.issuer` )
and the kibana wouldn't get a response and timeout since
`getUserClaims` wouldn't be called because
`ReloadableJWKSource#reloadAsync` wouldn't call `onResponse` on the
future.
This commit is contained in:
Ioannis Kakavas 2019-05-30 17:33:06 +03:00
parent 5a76f46ac6
commit 7cabe8acc9
2 changed files with 19 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java
View File

@ -12,6 +12,7 @@ import com.nimbusds.jose.jwk.JWKSelector;
import com.nimbusds.jose.jwk.JWKSet;
import com.nimbusds.jose.jwk.source.JWKSource;
import com.nimbusds.jose.proc.BadJOSEException;
import com.nimbusds.jose.proc.BadJWSException;
import com.nimbusds.jose.proc.JWSVerificationKeySelector;
import com.nimbusds.jose.proc.SecurityContext;
import com.nimbusds.jose.util.IOUtils;
@ -240,7 +241,7 @@ public class OpenIdConnectAuthenticator {
}
claimsListener.onResponse(enrichedVerifiedIdTokenClaims);
}
} catch (BadJOSEException e) {
} catch (BadJWSException e) {
// We only try to update the cached JWK set once if a remote source is used and
// RSA or ECDSA is used for signatures
if (shouldRetry
@ -256,7 +257,7 @@ public class OpenIdConnectAuthenticator {
} else {
claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e));
}
} catch (com.nimbusds.oauth2.sdk.ParseException | ParseException | JOSEException e) {
} catch (com.nimbusds.oauth2.sdk.ParseException | ParseException | BadJOSEException | JOSEException e) {
claimsListener.onFailure(new ElasticsearchSecurityException("Failed to parse or validate the ID Token", e));
}
}
@ -777,6 +778,7 @@ public class OpenIdConnectAuthenticator {
StandardCharsets.UTF_8));
reloadFutureRef.set(null);
LOGGER.trace("Successfully refreshed and cached remote JWKSet");
future.onResponse(null);
} catch (IOException | ParseException e) {
failed(e);
}

15
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java
View File

@ -88,6 +88,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
private Settings globalSettings;
private Environment env;
private ThreadContext threadContext;
private int callsToReloadJwk;
@Before
public void setup() {
@ -95,6 +96,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
.put("xpack.security.authc.realms.oidc.oidc-realm.ssl.verification_mode", "certificate").build();
env = TestEnvironment.newEnvironment(globalSettings);
threadContext = new ThreadContext(globalSettings);
callsToReloadJwk = 0;
}
@After
@ -278,6 +280,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
authenticator.authenticate(token, future);
JWTClaimsSet claimsSet = future.actionGet();
assertThat(claimsSet.getSubject(), equalTo(subject));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsWithExpiredToken() throws Exception {
@ -317,6 +320,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Expired JWT"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsNotYetIssuedToken() throws Exception {
@ -356,6 +360,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("JWT issue time ahead of current time"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsInvalidIssuer() throws Exception {
@ -394,6 +399,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Unexpected JWT issuer"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsInvalidAudience() throws Exception {
@ -432,6 +438,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Unexpected JWT audience"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testAuthenticateImplicitFlowFailsWithForgedRsaIdToken() throws Exception {
@ -456,6 +463,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWSException.class));
assertThat(e.getCause().getMessage(), containsString("Signed JWT rejected: Invalid signature"));
assertThat(callsToReloadJwk, equalTo(1));
}
public void testAuthenticateImplicitFlowFailsWithForgedEcsdsaIdToken() throws Exception {
@ -480,6 +488,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWSException.class));
assertThat(e.getCause().getMessage(), containsString("Signed JWT rejected: Invalid signature"));
assertThat(callsToReloadJwk, equalTo(1));
}
public void testAuthenticateImplicitFlowFailsWithForgedHmacIdToken() throws Exception {
@ -503,6 +512,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWSException.class));
assertThat(e.getCause().getMessage(), containsString("Signed JWT rejected: Invalid signature"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testAuthenticateImplicitFlowFailsWithForgedAccessToken() throws Exception {
@ -532,6 +542,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to verify access token"));
assertThat(e.getCause(), instanceOf(InvalidHashException.class));
assertThat(e.getCause().getMessage(), containsString("Access token hash (at_hash) mismatch"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsWithNoneAlgorithm() throws Exception {
@ -569,6 +580,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJOSEException.class));
assertThat(e.getCause().getMessage(), containsString("Another algorithm expected, or no matching key(s) found"));
assertThat(callsToReloadJwk, equalTo(0));
}
/**
@ -599,6 +611,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJOSEException.class));
assertThat(e.getCause().getMessage(), containsString("Another algorithm expected, or no matching key(s) found"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testImplicitFlowFailsWithUnsignedJwt() throws Exception {
@ -635,6 +648,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
assertThat(e.getMessage(), containsString("Failed to parse or validate the ID Token"));
assertThat(e.getCause(), instanceOf(BadJWTException.class));
assertThat(e.getCause().getMessage(), containsString("Signed ID token expected"));
assertThat(callsToReloadJwk, equalTo(0));
}
public void testJsonObjectMerging() throws Exception {
@ -832,6 +846,7 @@ public class OpenIdConnectAuthenticatorTests extends OpenIdConnectTestCase {
Mockito.doAnswer(invocation -> {
@SuppressWarnings("unchecked")
ActionListener<Void> listener = (ActionListener<Void>) invocation.getArguments()[0];
callsToReloadJwk += 1;
listener.onResponse(null);
return null;
}).when(jwkSource).triggerReload(any(ActionListener.class));