OpenID Connect Realm base functionality #37009

jkakavas · 2018-12-28T00:04:54Z

For the benefit of splitting this feature to manageable/reviewable(sic) chuncks, this commit adds

An OpenID Connect Realm definition
Necessary OpenID Connect Realm settings to support Authorization code grant and Implicit grant flows
Rest and Transport Action and Request/Response objects for initiating and completing the authentication flow
Functionality for generating OIDC Authentication Request URIs
Unit tests

Notably missing (to be handled in subsequent PRs):

The actual implementation of the authentication flows
Necessary JW{T,S,E} functionality

Relates: #35339

- Added Realm settings - Added RP/OP configuration classes - Added realm configuration tests - Completed the Prepare Auethentication API

elasticmachine · 2018-12-28T00:04:55Z

Pinging @elastic/es-security

jkakavas · 2018-12-28T07:02:15Z

@elasticmachine test this please

jkakavas · 2018-12-28T11:07:39Z

10:27:39   2> REPRODUCE WITH: ./gradlew :x-pack:plugin:ccr:internalClusterTest -Dtests.seed=8E411DEFD87B60B6 -Dtests.class=org.elasticsearch.xpack.ccr.LocalIndexFollowingIT -Dtests.method="testRemoveRemoteConnection" -Dtests.security.manager=true -Dtests.locale=hi -Dtests.timezone=Asia/Nicosia -Dcompiler.java=11 -Druntime.java=8

@elasticmachine run the gradle build tests 2 please

.../java/org/elasticsearch/xpack/core/security/action/oidc/OpenIdConnectAuthenticateAction.java

jaymode · 2019-01-08T21:47:19Z

...java/org/elasticsearch/xpack/core/security/action/oidc/OpenIdConnectAuthenticateRequest.java

+public class OpenIdConnectAuthenticateRequest extends ActionRequest {
+
+    /**
+     * The URI were the OP redirected the browser after the authentication attempt. This is passed as is from the


s/were/where

...elasticsearch/xpack/core/security/action/oidc/OpenIdConnectPrepareAuthenticationRequest.java

...java/org/elasticsearch/xpack/core/security/action/oidc/OpenIdConnectAuthenticateRequest.java

jaymode · 2019-01-09T15:55:23Z

...ava/org/elasticsearch/xpack/core/security/action/oidc/OpenIdConnectAuthenticateResponse.java

+    }
+
+    @Override
+    public void readFrom(StreamInput in) throws IOException {


I'd love to see us start using Writeable for everything new. That would make this throw a UnsupportedOperationException.

...elasticsearch/xpack/core/security/action/oidc/OpenIdConnectPrepareAuthenticationRequest.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java

...a/org/elasticsearch/xpack/security/action/oidc/TransportOpenIdConnectAuthenticateAction.java

...ugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OPConfiguration.java

...n/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java

jaymode · 2019-01-11T16:02:02Z

@jkakavas when targeting a feature branch, its best to ensure you keep it up to date if you merge master into your PR otherwise it looks like the change you're merging is a lot bigger. I went ahead and updated feature-oidc-realm to be in sync with current master. This changed the PR diff from 8000 lines to 1400 lines.

jaymode

LGTM

albertzaharovits · 2019-01-14T19:20:33Z

...c/main/java/org/elasticsearch/xpack/core/security/authc/oidc/OpenIdConnectRealmSettings.java

+    public static final Setting.AffixSetting<String> OP_TOKEN_ENDPOINT
+        = RealmSettings.simpleString(TYPE, "op.token_endpoint", Setting.Property.NodeScope);
+    public static final Setting.AffixSetting<String> OP_USERINFO_ENDPOINT
+        = RealmSettings.simpleString(TYPE, "op.userinfo_endpoint", Setting.Property.NodeScope);


Can't we get by without the the user_info endpoint. I sense this can be added latter if there are integrations that absolutely require it? Just a thought....

I'll leave it in as it is not a required setting ( so it won't produce any errors/warnings if not configured) and I plan to have the ability to query the user_info endpoint as a configurable part of the authentication flow from the beginning.

albertzaharovits · 2019-01-14T19:21:18Z

...c/main/java/org/elasticsearch/xpack/core/security/authc/oidc/OpenIdConnectRealmSettings.java

+    public static final Setting.AffixSetting<String> RP_REDIRECT_URI
+        = RealmSettings.simpleString(TYPE, "rp.redirect_uri", Setting.Property.NodeScope);
+    public static final Setting.AffixSetting<String> RP_RESPONSE_TYPE
+        = RealmSettings.simpleString(TYPE, "rp.response_type", Setting.Property.NodeScope);


Maybe we can infer this? Keep it to code or id_token?

You are right that it doesn't make much sense now with the allowed values being code and implicit. The original thought was to infer "id_token" from the value implicit, but as also proven by me not adding it, this is probably an unnecessary optimization for the user. I will change implicit to id_token to be in line with the specification

albertzaharovits · 2019-01-14T19:28:53Z

...elasticsearch/xpack/core/security/action/oidc/OpenIdConnectPrepareAuthenticationRequest.java

+
+    private String realmName;
+    private String state;
+    private String nonce;


I think it's easier to reason if we generate the state and nonce. OIDC allows for too much flexibility already....

I went back and forth on this for some time. Agreed, there is no strong argument against allowing Kibana (or any other facilitator ) to determine the nonce and state. I will address this

- Remove the possibility for the facilitator to request a specific nonce or state value and instead generate them always in ES. - Fix the response_type allowed values

jkakavas · 2019-01-15T08:04:51Z

As a though, I think it's better if have specific OP implementation to cater for.

If I'm reading this correctly , I plan on adding support for a subset of the Dynamic Discovery specification (at least the part of consuming the OP metadata

jkakavas · 2019-01-15T08:59:32Z

10:16:34 > Could not resolve all files for configuration ':x-pack:plugin:security:runtime'.
10:16:34    > Could not download xmlsec.jar (org.apache.santuario:xmlsec:2.0.8)
10:16:34       > Could not get resource 'https://jcenter.bintray.com/org/apache/santuario/xmlsec/2.0.8/xmlsec-2.0.8.jar'.
10:16:34          > Connection reset
10:16:34    > Could not download guava.jar (com.google.guava:guava:19.0)
10:16:34       > Could not get resource 'https://jcenter.bintray.com/com/google/guava/guava/19.0/guava-19.0.jar'.
10:16:34          > Connection reset

and

10:16:42 * What went wrong:
10:16:42 Execution failed for task ':client:test:checkstyleMain'.
10:16:42 > Could not resolve all files for configuration ':client:test:checkstyle'.
10:16:42    > Could not download guava.jar (com.google.guava:guava:25.1-jre)
10:16:42       > Could not get resource 'https://jcenter.bintray.com/com/google/guava/guava/25.1-jre/guava-25.1-jre.jar'.
10:16:42          > Could not GET 'https://d29vzk4ow07wi7.cloudfront.net/6db0c3a244c397429c2e362ea2837c3622d5b68bb95105d37c21c36e5bc70abf?response-content-disposition=attachment%3Bfilename%3D%22guava-25.1-jre.jar%22&Policy=eyJTdGF0ZW1lbnQiOiBbeyJSZXNvdXJjZSI6Imh0dHAqOi8vZDI5dnprNG93MDd3aTcuY2xvdWRmcm9udC5uZXQvNmRiMGMzYTI0NGMzOTc0MjljMmUzNjJlYTI4MzdjMzYyMmQ1YjY4YmI5NTEwNWQzN2MyMWMzNmU1YmM3MGFiZj9yZXNwb25zZS1jb250ZW50LWRpc3Bvc2l0aW9uPWF0dGFjaG1lbnQlM0JmaWxlbmFtZSUzRCUyMmd1YXZhLTI1LjEtanJlLmphciUyMiIsIkNvbmRpdGlvbiI6eyJEYXRlTGVzc1RoYW4iOnsiQVdTOkVwb2NoVGltZSI6MTU0NzU0MDUyMH0sIklwQWRkcmVzcyI6eyJBV1M6U291cmNlSXAiOiIwLjAuMC4wLzAifX19XX0_&Signature=Wv5jd2Pqv5oaDdrjCzDaiyvIWVua0a7BOBipWdT29tP2ER3-tJN-10H39d0Tt6uOamzsLfHPC5r1qt7UyBpa9lNMAFpZaJ-7pvTCxGHyncEAb4uCAWLqMBo2hSDo0Cwr~kIrJUIwo-EVlbo1xrfzKrJi1cjpxHIS46SxWOpSnFod3DxIaNz5pNwbapbTbR8FK7W9qp8w2HyoshOhSTk5OZAvYuvKguvzymFjpZeYm9Vphk1HH~mCLyZB-DEh9XMwgKfEwZMzLIhHj7el3wr5L6f9FLEYf9j47t8nCTQs1mdWIt8QIqRW8HMak9NIqox5mRHyW2Mobc-EcXJUJArrjw__&Key-Pair-Id=APKAIFKFWOMXM2UMTSFA'.
10:16:42 :docs:buildRestTests (Thread[Execution worker for ':' Thread 6,5,main]) completed. Took 10.089 secs.
10:16:42             > Connection reset
10:16:42

@elasticmachine I will try and ask you to run the gradle build tests 1 and then also run the gradle build tests 2 if you may

tvernum

Given the approvals from others, I didn't read everything, but I left some comments.
Feel free to address them in a followup if you'd like to get this one merged.

tvernum · 2019-01-15T09:50:08Z

...sticsearch/xpack/security/rest/action/oidc/RestOpenIdConnectPrepareAuthenticationAction.java

+ */
+public class RestOpenIdConnectPrepareAuthenticationAction extends OpenIdConnectBaseRestHandler {
+
+    static final ObjectParser<OpenIdConnectPrepareAuthenticationRequest, Void> PARSER = new ObjectParser<>("oidc_prepare_auithentication",


s/auith/auth/

tvernum · 2019-01-15T10:13:22Z

...java/org/elasticsearch/xpack/core/security/action/oidc/OpenIdConnectAuthenticateRequest.java

+
+    /**
+     * The state value that either we or the facilitator generated for this specific flow and that was stored at the user's session with
+     * the facilitator


These java docs ("the faciliator generated") are no longer true.

tvernum · 2019-01-15T10:18:36Z

...sticsearch/xpack/security/action/oidc/TransportOpenIdConnectPrepareAuthenticationAction.java

+            .filter(r -> r instanceof OpenIdConnectRealm)
+            .map(r -> (OpenIdConnectRealm) r)
+            .filter(r -> r.name().equals(request.getRealmName()))
+            .collect(Collectors.toList());


Why not just call Realms.realm(String) ?

tvernum · 2019-01-15T10:23:41Z

...n/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectToken.java

+    private String nonce;
+
+    /**
+     * @param redirectUri The URI were the OP redirected the browser after the authentication event at the OP. This is passed as is from the


s/were/where/

tvernum · 2019-01-15T10:25:19Z

...n/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectToken.java

+
+    @Override
+    public String principal() {
+        return "<unauthenticated-oidc-user>";


Based on the confusion that users have around unauthenticated-saml-user we used a different pattern on just "<Kerberos Token>" for Kerberos.
My gut feel is to do something similar here "<OIDC Token>" ?

tvernum · 2019-01-15T10:31:07Z

...ity/src/main/java/org/elasticsearch/xpack/security/authc/oidc/RelyingPartyConfiguration.java

+        this.clientId = Objects.requireNonNull(clientId, "RP Client ID must be provided");
+        this.redirectUri = Objects.requireNonNull(redirectUri, "RP Redirect URI must be provided");
+        if (Strings.hasText(responseType) == false) {
+            throw new IllegalArgumentException("Response type must be provided");


I think these error messages need to be clear about the Setting that was incorrect.
Ideally we'd do validation in the Setting itself, but if not, then I think the validation ought to be done in the context of the Realm so that it can reference the setting key.
Otherwise we'll get lots of questions from users about "I got an error saying Response type must be provided but I don't know where to configure it."

Makes sense Tim, I'll address that. I'll do it in the Realm (this is what we do in other realms too I think) but I can move this to the Setting with a validator if need be !

jkakavas · 2019-01-15T12:00:28Z

Thanks for the valuable feedback Tim, I'll go ahead and address this now before merging

jkakavas · 2019-01-16T11:08:15Z

run the gradle build tests 2

jkakavas · 2019-01-17T05:59:18Z

run the gradle build tests 2 and also run the gradle build tests 1

jkakavas · 2019-01-17T06:15:51Z

08:10:54 > Task :x-pack:plugin:ml:cpp-snapshot:downloadMachineLearningSnapshot FAILED
08:10:54 :x-pack:plugin:ml:cpp-snapshot:downloadMachineLearningSnapshot (Thread[Execution worker for ':' Thread 14,5,main]) completed. Took 2 mins 10.741 secs.

jkakavas · 2019-01-17T06:16:42Z

run the gradle build tests 2

jkakavas · 2019-01-17T06:51:11Z

run the gradle build tests 1

jkakavas · 2019-01-17T09:39:41Z

run the gradle build tests 1

jkakavas · 2019-01-18T07:27:42Z

run the gradle build tests 1

jkakavas · 2019-01-18T08:24:02Z

10:10:34 > Task :server:integTest
10:10:34 Tests with failures:
10:10:34   - org.elasticsearch.discovery.MasterDisruptionIT.testFailWithMinimumMasterNodesConfigured
10:10:34   - org.elasticsearch.discovery.zen.ZenDiscoveryIT.testNoShardRelocationsOccurWhenElectedMasterNodeFails

:sigh:

run the gradle build tests 1

jkakavas · 2019-01-18T12:52:15Z

run the gradle build tests 1 just in case we get lucky

jkakavas · 2019-01-18T15:13:51Z

20th time is the charm as an old Greek proverb says, run the gradle build tests 1

This commit adds * An OpenID Connect Realm definition * Necessary OpenID Connect Realm settings to support Authorization code grant and Implicit grant flows * Rest and Transport Action and Request/Response objects for initiating and completing the authentication flow * Functionality for generating OIDC Authentication Request URIs Unit tests Notably missing (to be handled in subsequent PRs): * The actual implementation of the authentication flows * Necessary JW{T,S,E} functionality Relates: elastic#35339

This commit adds an OpenID Connect authentication realm to elasticsearch. Elasticsearch (with the assistance of kibana or another web component) acts as an OpenID Connect Relying Party and supports the Authorization Code Grant and Implicit flows as described in http://ela.st/oidc-spec. It adds support for consuming and verifying signed ID Tokens, both RP initiated and 3rd party initiated Single Sign on and RP initiated signle logout. It also adds an OpenID Connect Provider in the idp-fixture to be used for the associated integration tests. The code in this commit has been tracked in a feature branch and has been previously reviewed and approved in : #37009 #37787 #38474 #38475 #40262

This commit adds an OpenID Connect authentication realm to elasticsearch. Elasticsearch (with the assistance of kibana or another web component) acts as an OpenID Connect Relying Party and supports the Authorization Code Grant and Implicit flows as described in http://ela.st/oidc-spec. It adds support for consuming and verifying signed ID Tokens, both RP initiated and 3rd party initiated Single Sign on and RP initiated signle logout. It also adds an OpenID Connect Provider in the idp-fixture to be used for the associated integration tests. The code in this commit has been tracked in a feature branch and has been previously reviewed and approved in : elastic#37009 elastic#37787 elastic#38474 elastic#38475 elastic#40262

jkakavas added 11 commits December 5, 2018 16:33

Add REST and Transport actions for oidc authentication

5af56b9

support state

c4154f6

Merge remote-tracking branch 'origin/master' into oidc-realm-base

8149339

track current changes

8d54c7c

Ongoing oidc work

cc68f5a

- Added Realm settings - Added RP/OP configuration classes - Added realm configuration tests - Completed the Prepare Auethentication API

Add functionality for building Authentication Requests

72a2dd4

Fix formatting

69fb95c

Fix formatting

12dd9c0

Fix settings

6a4cac6

Correct javadoc

aeacfc5

Add serialiazion and tests for the AuthenticateRequest

845dc36

jkakavas added >feature v7.0.0 :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Dec 28, 2018

jkakavas requested review from tvernum and albertzaharovits December 28, 2018 00:04

Merge remote-tracking branch 'origin/master' into oidc-realm-base

687c109

jaymode reviewed Jan 9, 2019

View reviewed changes

jkakavas mentioned this pull request Jan 9, 2019

[WIP] OIDC Realm JWT+JWS related functionality #37272

Closed

jkakavas added 2 commits January 11, 2019 10:31

Merge remote-tracking branch 'origin/master' into oidc-realm-base

06c26dc

address feedback

4bde32f

jaymode approved these changes Jan 11, 2019

View reviewed changes

address feedback

08be032

albertzaharovits reviewed Jan 14, 2019

View reviewed changes

Address feedback

dcf46fa

- Remove the possibility for the facilitator to request a specific nonce or state value and instead generate them always in ES. - Fix the response_type allowed values

Merge remote-tracking branch 'origin/master' into oidc-realm-base

cb35d6e

tvernum reviewed Jan 15, 2019

View reviewed changes

jkakavas added 2 commits January 15, 2019 22:54

address further feedback

8d9dcf6

Merge remote-tracking branch 'origin/master' into oidc-realm-base

e97097e

Merge remote-tracking branch 'origin/master' into oidc-realm-base

da98413

Merge remote-tracking branch 'origin/master' into oidc-realm-base

03d2db5

jkakavas merged commit 8d54639 into elastic:feature-oidc-realm Jan 18, 2019

jkakavas deleted the oidc-realm-base branch January 18, 2019 17:09

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

jkakavas mentioned this pull request Apr 1, 2019

Add an OpenID Connect authentication realm #40674

Merged

codebrain mentioned this pull request Jul 11, 2019

[meta] 7.1 release elastic/elasticsearch-net#3895

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenID Connect Realm base functionality #37009

OpenID Connect Realm base functionality #37009

jkakavas commented Dec 28, 2018

elasticmachine commented Dec 28, 2018

jkakavas commented Dec 28, 2018

jkakavas commented Dec 28, 2018

jaymode Jan 8, 2019

jaymode Jan 9, 2019

jaymode commented Jan 11, 2019

jaymode left a comment

albertzaharovits Jan 14, 2019

jkakavas Jan 15, 2019

albertzaharovits Jan 14, 2019

jkakavas Jan 15, 2019

albertzaharovits Jan 14, 2019

jkakavas Jan 15, 2019

jkakavas commented Jan 15, 2019

jkakavas commented Jan 15, 2019

tvernum left a comment

tvernum Jan 15, 2019

tvernum Jan 15, 2019

tvernum Jan 15, 2019

tvernum Jan 15, 2019

tvernum Jan 15, 2019

tvernum Jan 15, 2019

jkakavas Jan 15, 2019

jkakavas commented Jan 15, 2019

jkakavas commented Jan 16, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019 •

edited

Loading

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

OpenID Connect Realm base functionality #37009

OpenID Connect Realm base functionality #37009

Conversation

jkakavas commented Dec 28, 2018

elasticmachine commented Dec 28, 2018

jkakavas commented Dec 28, 2018

jkakavas commented Dec 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaymode commented Jan 11, 2019

jaymode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Jan 15, 2019

jkakavas commented Jan 15, 2019

tvernum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Jan 15, 2019

jkakavas commented Jan 16, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019 • edited Loading

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 17, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 18, 2019

jkakavas commented Jan 17, 2019 •

edited

Loading