-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Allow empty token endpoint for implicit flow #45038
Conversation
When using the implicit flow in OpenID Connect, the op.token_endpoint_url should not be mandatory as there is no need to contact the token endpoint of the OP.
Pinging @elastic/es-security |
String tokenEndpointString = config.getSetting(OP_TOKEN_ENDPOINT); | ||
if (responseType.equals("code") && tokenEndpointString.isEmpty()) { | ||
throw new SettingsException("The configuration setting [" + OP_TOKEN_ENDPOINT.getConcreteSettingForNamespace(name()).getKey() | ||
+ "] is required when [" + RP_RESPONSE_TYPE.getConcreteSettingForNamespace(name()).getKey() + " is set to \"code\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+ "] is required when [" + RP_RESPONSE_TYPE.getConcreteSettingForNamespace(name()).getKey() + " is set to \"code\""); | |
+ "] is required when [" + RP_RESPONSE_TYPE.getConcreteSettingForNamespace(name()).getKey() + "] is set to \"code\""); |
URI tokenEndpoint; | ||
try { | ||
tokenEndpoint = new URI(require(config, OP_TOKEN_ENDPOINT)); | ||
tokenEndpoint = new URI(tokenEndpointString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes more sense for tokenEndpoint
to be null (rather than an empty URI) if tokenEndpointString
is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine test this please |
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/1 |
When using the implicit flow in OpenID Connect, the op.token_endpoint_url should not be mandatory as there is no need to contact the token endpoint of the OP.
When using the implicit flow in OpenID Connect, the
op.token_endpoint_url should not be mandatory as there is no need
to contact the token endpoint of the OP.