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

Add an OpenID Connect authentication realm #40674

Merged
merged 30 commits into from
Apr 4, 2019
Merged

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Apr 1, 2019

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.

Documentation will be added in a separate PR as it is not
yet reviewed.
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

jkakavas and others added 24 commits January 18, 2019 19:09
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
Adds implementation for authentication in the OpenID Connect
 realm using the authorization code grant and the implicit flows
 as described in https://openid.net/specs/openid-connect-core-1_0.html
This change introduces an OpenID Connect Provider in idp-fixture
that offers a pre-existing user for testing purposes. 

It also adds tests where a mock facilitator application (HttpClient)
performs authentication using Elasticsearch's oidc realm against
that OpenID Connect Provider, using both the authorization code
and the implicit grant flows.
@jkakavas jkakavas added >feature :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.2.0 labels Apr 1, 2019
@jkakavas jkakavas requested review from bizybot and tvernum April 1, 2019 05:19
@jaymode
Copy link
Member

jaymode commented Apr 3, 2019

Feature branch merges like this one where all of the individual changes have been reviewed, should be ok to merge if CI is happy and there haven't been non trivial changes. That said, @tvernum did find some things that were missed by myself and Albert, so maybe having a "skimmed" review would be worth it prior to merging.

@tvernum
Copy link
Contributor

tvernum commented Apr 3, 2019

I don't know what's the best option is for when merging feature branches.

The key question that needs to be answered at this point is "does the sum of the parts equal a production ready feature".

It's entirely possible that every PR that lead to this point was fine, but the final result is missing something important that means it shouldn't be merged.

It's still a perfectly reasonable opportunity to double check parts that had previously been reviewed, and raise concerns that we missed on the first round, but we're mostly looking for What haven't we done.

How we do that is a separate question, and I think probably better discussed face-to-face.
It's tricky, because in an ideal world, a feature branch would have contribution from many people, and a mix of reviewers, so there's not going to be anyone who is knowledgeable enough about the work to review it well, but independent from the implementation. In those cases, our starting point should be that the team that worked on it are the joint approvers - the question is does the team think this is ready to ship?

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to the extent that I can reasonably review this.

It has tests, feature licensing seems to be correct, dependencies seem to be OK.
I'm happy to rely on the previous reviews having picked up the lower level implementation details.

Copy link
Contributor

@bizybot bizybot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you.

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jkakavas jkakavas merged commit d029a13 into master Apr 4, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 4, 2019
* elastic/master: (25 commits)
  Rollup ignores time_zone on date histogram (elastic#40844)
  HLRC: fix uri encode bug when url path starts with '/' (elastic#34436)
  Mutes GatewayIndexStateIT.testRecoverBrokenIndexMetadata
  Docs: Pin two IDs in the rest client (elastic#40785)
  Adds version 6.7.2
  [DOCS] Remind users to include @ symbol when applying license file (elastic#40688)
  HLRC: Convert xpack methods to client side objects (elastic#40705)
  Allow ILM to stop if indices have nonexistent policies (elastic#40820)
  Add an OpenID Connect authentication realm (elastic#40674)
  [DOCS] Rewrite query def (elastic#40746)
  [ML] Changes default destination index field mapping and adds scripted_metric agg (elastic#40750)
  Docs: Drop inline callouts from painless book (elastic#40805)
  [ML] refactoring start task a bit, removing unused code (elastic#40798)
  [DOCS] Document index.load_fixed_bitset_filters_eagerly (elastic#40780)
  Make remote cluster resolution stricter (elastic#40419)
  [ML] Add created_by info to usage stats (elastic#40518)
  SQL: [Docs] Small fixes for CURRENT_TIMESTAMP docs (elastic#40792)
  convert modules to use testclusters (elastic#40804)
  Replace javax activation with jakarta activation (elastic#40247)
  Document restrictions on fuzzy matching when using synonyms (elastic#40783)
  ...
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 5, 2019
* elastic/master: (36 commits)
  Remove unneded cluster config from test (elastic#40856)
  Make Fuzziness reject illegal values earlier (elastic#33511)
  Remove test-only customisation from TransReplAct (elastic#40863)
  Fix dense/sparse vector limit documentation (elastic#40852)
  Make -try xlint warning disabled by default. (elastic#40833)
  Async Snapshot Repository Deletes (elastic#40144)
  Revert "Replace usages RandomizedTestingTask with built-in Gradle Test (elastic#40564)"
  Init global checkpoint after copy commit in peer recovery (elastic#40823)
  Replace usages RandomizedTestingTask with built-in Gradle Test (elastic#40564)
  [DOCS] Removed redundant (not quite right) information about upgrades.
  Remove string usages of old transport settings (elastic#40818)
  Rollup ignores time_zone on date histogram (elastic#40844)
  HLRC: fix uri encode bug when url path starts with '/' (elastic#34436)
  Mutes GatewayIndexStateIT.testRecoverBrokenIndexMetadata
  Docs: Pin two IDs in the rest client (elastic#40785)
  Adds version 6.7.2
  [DOCS] Remind users to include @ symbol when applying license file (elastic#40688)
  HLRC: Convert xpack methods to client side objects (elastic#40705)
  Allow ILM to stop if indices have nonexistent policies (elastic#40820)
  Add an OpenID Connect authentication realm (elastic#40674)
  ...
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Apr 14, 2019
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 a commit that referenced this pull request Apr 15, 2019
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.

This is a backport of #40674
jkakavas added a commit to jkakavas/kibana that referenced this pull request May 22, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
jkakavas added a commit to elastic/kibana that referenced this pull request May 22, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
legrego pushed a commit to legrego/kibana that referenced this pull request May 22, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
legrego added a commit to elastic/kibana that referenced this pull request May 22, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
legrego pushed a commit to legrego/kibana that referenced this pull request May 23, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
legrego added a commit to elastic/kibana that referenced this pull request May 23, 2019
manage_oidc cluster privilege was added to Elasticsearch in
elastic/elasticsearch#40674
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
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 jkakavas mentioned this pull request Jun 27, 2019
9 tasks
@colings86 colings86 deleted the feature-oidc-realm branch May 27, 2020 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants