From 756f028782667f2a4040fa184a3a453c8df1f09e Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Mon, 21 Oct 2019 15:52:58 +1100 Subject: [PATCH 1/7] Remove unnecessary details logged for OIDC This commit removes unnecessary details logged for OIDC. --- .../authc/oidc/OpenIdConnectAuthenticator.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index a3218fc90552f..6257027ed16c7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -514,12 +514,12 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener Date: Wed, 23 Oct 2019 08:50:17 +1100 Subject: [PATCH 2/7] address review comment - truncate the string keeping first and last 2 chars. - the `OIDCTokenResponse` did not parse the error scenario correctly. - checks for the response status and then on error prints the response message as warning --- .../oidc/OpenIdConnectAuthenticator.java | 53 +++++++++++++------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 6257027ed16c7..9af09beb99141 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -14,6 +14,7 @@ import com.nimbusds.jose.proc.BadJOSEException; import com.nimbusds.jose.proc.JWSVerificationKeySelector; import com.nimbusds.jose.proc.SecurityContext; +import com.nimbusds.jose.util.Base64URL; import com.nimbusds.jose.util.IOUtils; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; @@ -80,6 +81,7 @@ import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.concurrent.ListenableFuture; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.watcher.FileChangesListener; import org.elasticsearch.watcher.FileWatcher; import org.elasticsearch.watcher.ResourceWatcherService; @@ -514,29 +516,28 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener(accessToken, idToken)); @@ -548,6 +549,28 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener 0) { + truncated += "."; + } + truncated += truncateBetweenFirstAndLast2Chars(input[i].toString()); + } + return truncated; + } + + private static String truncateBetweenFirstAndLast2Chars(String input) { + if (Strings.hasText(input) == false || input.length() <= 4) { + return input; + } + return input.substring(0, 2) + "***" + input.substring(input.length() - 2); + } + /** * Creates a {@link CloseableHttpAsyncClient} that uses a {@link PoolingNHttpClientConnectionManager} */ From e653f6f96c5deda76a11d661c78550e118933ca7 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Thu, 24 Oct 2019 17:02:49 +1100 Subject: [PATCH 3/7] Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java Co-Authored-By: Ioannis Kakavas --- .../xpack/security/authc/oidc/OpenIdConnectAuthenticator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 9af09beb99141..de22f3c9f2d7d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -531,7 +531,7 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener Date: Thu, 24 Oct 2019 17:09:27 +1100 Subject: [PATCH 4/7] address review comment --- .../security/authc/oidc/OpenIdConnectAuthenticator.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index de22f3c9f2d7d..d137b0e965ffe 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -532,12 +532,11 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener(accessToken, idToken)); @@ -559,12 +558,12 @@ private static String truncateJWT(JWT jwt) { if (i > 0) { truncated += "."; } - truncated += truncateBetweenFirstAndLast2Chars(input[i].toString()); + truncated += truncateToken(input[i].toString()); } return truncated; } - private static String truncateBetweenFirstAndLast2Chars(String input) { + private static String truncateToken(String input) { if (Strings.hasText(input) == false || input.length() <= 4) { return input; } From 5d96c771b1f3eed753044bd80201818145c69f3c Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Mon, 28 Oct 2019 14:00:03 +1100 Subject: [PATCH 5/7] its okay to log Id token, address review comment --- .../authc/oidc/OpenIdConnectAuthenticator.java | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index d137b0e965ffe..73271055aebe2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -531,7 +531,7 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener 0) { - truncated += "."; - } - truncated += truncateToken(input[i].toString()); - } - return truncated; - } - private static String truncateToken(String input) { if (Strings.hasText(input) == false || input.length() <= 4) { return input; From c8ded849c3c7525a18d473a96fd884024f1824cc Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Mon, 28 Oct 2019 15:55:22 +1100 Subject: [PATCH 6/7] remove unused import --- .../xpack/security/authc/oidc/OpenIdConnectAuthenticator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 73271055aebe2..0f68393d0e688 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -14,7 +14,6 @@ import com.nimbusds.jose.proc.BadJOSEException; import com.nimbusds.jose.proc.JWSVerificationKeySelector; import com.nimbusds.jose.proc.SecurityContext; -import com.nimbusds.jose.util.Base64URL; import com.nimbusds.jose.util.IOUtils; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; @@ -97,7 +96,6 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; - import java.net.URLEncoder; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -113,8 +111,8 @@ import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.ALLOWED_CLOCK_SKEW; -import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECTION_READ_TIMEOUT; +import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_CONNECT_TIMEOUT; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_CONNECTIONS; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_MAX_ENDPOINT_CONNECTIONS; import static org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings.HTTP_SOCKET_TIMEOUT; From 4e2a0d6322cf5443d10d7a3d65b3efed4ae4b948 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Thu, 31 Oct 2019 09:54:35 +0200 Subject: [PATCH 7/7] Parse the response as a token error response, only when the status is 400 as indicated by the standard --- .../authc/oidc/OpenIdConnectAuthenticator.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 0f68393d0e688..12d054015cb05 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -518,10 +518,14 @@ private void handleTokenResponse(HttpResponse httpResponse, ActionListener