Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data-plane]: Make OIDCDiscoveryConfig more resistant against invalid OIDC discovery config #3674

Closed
creydr opened this issue Feb 6, 2024 · 0 comments · Fixed by #3707
Closed
Assignees

Comments

@creydr
Copy link
Contributor

creydr commented Feb 6, 2024

Currently the loading of the OIDC config from the discovery endpoint is not very robust and fails on an invalid OIDC config (e.g. when the issuer was configured in the k8s config without https and thus the query results in a 404):

return webClient
.getAbs("https://kubernetes.default.svc/.well-known/openid-configuration")
.bearerTokenAuthentication(kubeConfig.getAutoOAuthToken())
.send()
.compose(res -> {
logger.debug("Got raw OIDC discovery info: " + res.bodyAsString());
try {
ObjectMapper mapper = new ObjectMapper();
OIDCInfo oidcInfo = mapper.readValue(res.bodyAsString(), OIDCInfo.class);
oidcDiscoveryConfig.issuer = oidcInfo.getIssuer();
return webClient
.getAbs(oidcInfo.getJwks().toString())
.bearerTokenAuthentication(kubeConfig.getAutoOAuthToken())
.send();
} catch (JsonProcessingException e) {
logger.error("Failed to parse OIDC discovery info", e);
return Future.failedFuture(e);
}
})
.compose(res -> {

This should be changed (e.g. status code checking) and the OIDCDiscoveryConfig fail the returned future correctly in such a case (e.g. with an additional exception, which can then be handled by the caller).

@creydr creydr changed the title Make OIDCDiscoveryConfig more resistant against invalid OIDC discovery config [data-plane]: Make OIDCDiscoveryConfig more resistant against invalid OIDC discovery config Feb 6, 2024
@creydr creydr linked a pull request Feb 20, 2024 that will close this issue
@creydr creydr self-assigned this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant