Skip to content

Commit

Permalink
Handle unexpected/unchecked exceptions correctly (elastic#49080)
Browse files Browse the repository at this point in the history
Ensures that methods that are called from different threads ( i.e.
from the callbacks of org.apache.http.concurrent.FutureCallback )
catch `Exception` instead of only the expected checked exceptions.

This resolves a bug where OpenIdConnectAuthenticator#mergeObjects
would throw an IllegalStateException that was never caught causing
the thread to hang and the listener to never be called. This would
in turn cause Kibana requests to authenticate with OpenID Connect
to timeout and fail without even logging anything relevant.

This also guards against unexpected Exceptions that might be thrown
by invoked library methods while performing the necessary operations
in these callbacks.
  • Loading branch information
jkakavas authored Nov 15, 2019
1 parent 83c1ebf commit 5cf112e
Showing 1 changed file with 5 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLEncoder;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -434,7 +433,7 @@ private void handleUserinfoResponse(HttpResponse httpResponse, JWTClaimsSet veri
httpResponse.getStatusLine().getReasonPhrase()));
}
}
} catch (IOException | com.nimbusds.oauth2.sdk.ParseException | ParseException e) {
} catch (Exception e) {
claimsListener.onFailure(new ElasticsearchSecurityException("Failed to get user information from the UserInfo endpoint.",
e));
}
Expand Down Expand Up @@ -544,7 +543,7 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener<Tuple
}
tokensListener.onResponse(new Tuple<>(accessToken, idToken));
}
} catch (IOException | com.nimbusds.oauth2.sdk.ParseException e) {
} catch (Exception e) {
tokensListener.onFailure(
new ElasticsearchSecurityException("Failed to exchange code for Id Token using the Token Endpoint. " +
"Unable to parse Token Response", e));
Expand Down Expand Up @@ -748,7 +747,7 @@ public void onFileChanged(Path file) {
/**
* Remote JSON Web Key source specified by a JWKSet URL. The retrieved JWK set is cached to
* avoid unnecessary http requests. A single attempt to update the cached set is made
* (with {@ling ReloadableJWKSource#triggerReload}) when the {@link IDTokenValidator} fails
* (with {@link ReloadableJWKSource#triggerReload}) when the {@link IDTokenValidator} fails
* to validate an ID Token (because of an unknown key) as this might mean that the OpenID
* Connect Provider has rotated the signing keys.
*/
Expand Down Expand Up @@ -795,7 +794,7 @@ public void completed(HttpResponse result) {
reloadFutureRef.set(null);
LOGGER.trace("Successfully refreshed and cached remote JWKSet");
future.onResponse(null);
} catch (IOException | ParseException e) {
} catch (Exception e) {
failed(e);
}
}
Expand All @@ -815,7 +814,7 @@ public void cancelled() {
});
return null;
});
} catch (URISyntaxException e) {
} catch (Exception e) {
future.onFailure(e);
reloadFutureRef.set(null);
}
Expand Down

0 comments on commit 5cf112e

Please sign in to comment.