From 7d89a395ebff4153e9158c514a743900bd16db3f Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 31 Jul 2019 14:55:36 +0300 Subject: [PATCH 1/2] Allow empty token endpoint for implicit flow 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. --- .../authc/oidc/OpenIdConnectRealm.java | 8 +++++++- .../oidc/OpenIdConnectRealmSettingsTests.java | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java index 4267d39cb1f7b..4054452651019 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java @@ -277,9 +277,15 @@ private OpenIdConnectProviderConfiguration buildOpenIdConnectProviderConfigurati // This should never happen as it's already validated in the settings throw new SettingsException("Invalid URI: " + OP_AUTHORIZATION_ENDPOINT.getKey(), e); } + String responseType = require(config, RP_RESPONSE_TYPE); + 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\""); + } URI tokenEndpoint; try { - tokenEndpoint = new URI(require(config, OP_TOKEN_ENDPOINT)); + tokenEndpoint = new URI(tokenEndpointString); } catch (URISyntaxException e) { // This should never happen as it's already validated in the settings throw new SettingsException("Invalid URL: " + OP_TOKEN_ENDPOINT.getKey(), e); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java index 341cf07b0dd7b..e8b02c815c964 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java @@ -86,7 +86,7 @@ public void testInvalidAuthorizationEndpointThrowsError() { Matchers.containsString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_AUTHORIZATION_ENDPOINT))); } - public void testMissingTokenEndpointThrowsError() { + public void testMissingTokenEndpointThrowsErrorInCodeFlow() { final Settings.Builder settingsBuilder = Settings.builder() .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_AUTHORIZATION_ENDPOINT), "https://op.example.com/login") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_ISSUER), "https://op.example.com") @@ -103,6 +103,22 @@ public void testMissingTokenEndpointThrowsError() { Matchers.containsString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_TOKEN_ENDPOINT))); } + public void testMissingTokenEndpointIsAllowedInImplicitFlow() { + final Settings.Builder settingsBuilder = Settings.builder() + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_AUTHORIZATION_ENDPOINT), "https://op.example.com/login") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_ISSUER), "https://op.example.com") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_JWKSET_PATH), "https://op.example.com/jwks.json") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "id_token token"); + settingsBuilder.setSecureSettings(getSecureSettings()); + final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); + assertNotNull(realm); + + } + + public void testInvalidTokenEndpointThrowsError() { final Settings.Builder settingsBuilder = Settings.builder() .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_AUTHORIZATION_ENDPOINT), "https://op.example.com/login") From c75b85f77cfc4fe3944d61bb51a6333c3af7dc43 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Tue, 6 Aug 2019 12:31:17 +0300 Subject: [PATCH 2/2] Return null instead of empty URI --- .../oidc/OpenIdConnectProviderConfiguration.java | 5 +++-- .../security/authc/oidc/OpenIdConnectRealm.java | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectProviderConfiguration.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectProviderConfiguration.java index f5f8c8592b22a..f0eb9bebdf302 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectProviderConfiguration.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectProviderConfiguration.java @@ -23,9 +23,10 @@ public class OpenIdConnectProviderConfiguration { private final String jwkSetPath; public OpenIdConnectProviderConfiguration(Issuer issuer, String jwkSetPath, URI authorizationEndpoint, - URI tokenEndpoint, @Nullable URI userinfoEndpoint, @Nullable URI endsessionEndpoint) { + @Nullable URI tokenEndpoint, @Nullable URI userinfoEndpoint, + @Nullable URI endsessionEndpoint) { this.authorizationEndpoint = Objects.requireNonNull(authorizationEndpoint, "Authorization Endpoint must be provided"); - this.tokenEndpoint = Objects.requireNonNull(tokenEndpoint, "Token Endpoint must be provided"); + this.tokenEndpoint = tokenEndpoint; this.userinfoEndpoint = userinfoEndpoint; this.endsessionEndpoint = endsessionEndpoint; this.issuer = Objects.requireNonNull(issuer, "OP Issuer must be provided"); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java index 4054452651019..836e5cbbf3dc2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java @@ -281,27 +281,27 @@ private OpenIdConnectProviderConfiguration buildOpenIdConnectProviderConfigurati 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\""); + + "] is required when [" + RP_RESPONSE_TYPE.getConcreteSettingForNamespace(name()).getKey() + "] is set to \"code\""); } URI tokenEndpoint; try { - tokenEndpoint = new URI(tokenEndpointString); + tokenEndpoint = tokenEndpointString.isEmpty() ? null : new URI(tokenEndpointString); } catch (URISyntaxException e) { // This should never happen as it's already validated in the settings throw new SettingsException("Invalid URL: " + OP_TOKEN_ENDPOINT.getKey(), e); } URI userinfoEndpoint; try { - userinfoEndpoint = (config.getSetting(OP_USERINFO_ENDPOINT, () -> null) == null) ? null : - new URI(config.getSetting(OP_USERINFO_ENDPOINT, () -> null)); + userinfoEndpoint = (config.getSetting(OP_USERINFO_ENDPOINT).isEmpty()) ? null : + new URI(config.getSetting(OP_USERINFO_ENDPOINT)); } catch (URISyntaxException e) { // This should never happen as it's already validated in the settings throw new SettingsException("Invalid URI: " + OP_USERINFO_ENDPOINT.getKey(), e); } URI endsessionEndpoint; try { - endsessionEndpoint = (config.getSetting(OP_ENDSESSION_ENDPOINT, () -> null) == null) ? null : - new URI(config.getSetting(OP_ENDSESSION_ENDPOINT, () -> null)); + endsessionEndpoint = (config.getSetting(OP_ENDSESSION_ENDPOINT).isEmpty()) ? null : + new URI(config.getSetting(OP_ENDSESSION_ENDPOINT)); } catch (URISyntaxException e) { // This should never happen as it's already validated in the settings throw new SettingsException("Invalid URI: " + OP_ENDSESSION_ENDPOINT.getKey(), e);