Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. by azasypkin · Pull Request #82817 · elastic/kibana

azasypkin · 2020-11-06T10:49:15Z

See the detailed description of the problem in #78260. But in short the idea is following:

PKI and Kerberos should always try to initiate a new session for a non-API requests that require authentication.
PKI and Kerberos should NOT try to initiate a new session for an API requests unless there is a session associated with them.

When we re-initiate session because of expired access/refresh token it doesn't matter if it's an API request or not since the user-facing session is still valid and the fact that we're internally using tokens is just an implementation detail. Same for requests that don't require authentication, if we have a valid Kibana session and we can re-acquire tokens without user involvement then we should do it.

Fixes: #78260

Release note: previously API request could create a new PKI or Kerberos session without user even realizing it. With this fix we use a bit more sophisticated heuristic to know when a new session is really desired.

azasypkin · 2020-11-06T10:59:00Z

.github/CODEOWNERS

note: just continue moving the remaining tests whenever I have chance.

azasypkin · 2020-11-06T14:02:28Z

x-pack/plugins/security/server/authentication/providers/kerberos.ts

note: I'm not sure why we weren't checking the canStartNewSession here before, but I cannot find any reason why we shouldn't do it now. And we also should give a higher priority to the current session as well.

azasypkin · 2020-11-06T14:05:12Z

x-pack/plugins/security/server/authentication/providers/pki.test.ts

note: I'll remove this stupid setup in the scope of #80952 soon.

azasypkin · 2020-11-06T14:08:07Z

x-pack/test/security_api_integration/tests/kerberos/kerberos_login.ts

note: recently we started to treat all /internal/.... as API requests (canRedirectRequest === false).

elasticmachine · 2020-11-06T15:46:27Z

Pinging @elastic/kibana-security (Team:Security)

…unauthenticated XHR requests.

azasypkin · 2020-11-06T16:33:54Z

Took the liberty of force-pushing to this branch to resolve conflicts with the most recent master as likely no one is reviewing this yet, but that was the last force-push in this PR 🙈

legrego · 2020-11-06T17:10:08Z

ACK: will review on Monday or Tuesday next week

legrego

LGTM!

legrego · 2020-11-09T14:02:16Z

x-pack/plugins/security/server/authentication/providers/kerberos.ts

    }

-    // If refresh token is no longer valid, then we should clear session and renegotiate using SPNEGO.
+    // If refresh token is no longer valid, let's check if the request is allowed to start a new session, and if so try


Is this comment still valid? It seems like we don't have to check if the request is allowed to start a new session anymore, since authentication via refresh token is extending an existing session.

You're absolutely right! This looks like a leftover from one of my initial WIPs, will fix, thanks!

kibanamachine · 2020-11-09T16:31:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0670897

Metrics [docs]

✅ unchanged

History

💚 Build #86665 succeeded 1a7f5a1
💚 Build #86452 succeeded 0b6369e
💚 Build #86395 succeeded 1291abb8c52b9d4d0cbe2f60184609e74388bd39
💔 Build #86357 failed 9c6a51c0ecde7a92e9e9be1c037936fbeb7ff74d
💔 Build #86356 failed 78c878f2ee4ca2b8bc06ae9600a4afeef27b0aae

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…unauthenticated XHR/API requests. (elastic#82817) * Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR requests. * Review#1: fix comment. # Conflicts: # .github/CODEOWNERS # x-pack/scripts/functional_tests.js

azasypkin · 2020-11-09T18:26:02Z

7.x/7.11.0: 129f4df

* master: (39 commits) Fix ilm navigation (elastic#81664) [Lens] Distinct icons for XY and pie chart value labels toolbar (elastic#82927) [data.search.aggs] Throw an error when trying to create an agg type that doesn't exist. (elastic#81509) Index patterns api - load field list on server (elastic#82629) New events resolver (elastic#82170) [App Search] Misc naming tech debt (elastic#82770) load empty_kibana in test to have clean starting point (elastic#82772) Remove data <--> expressions circular dependencies. (elastic#82685) Update 8.0 breaking change template to gather information on how to programmatically detect it. (elastic#82905) Add alerting as codeowners to related documentation folder (elastic#82777) Add captions to user and space grid pages (elastic#82713) add alternate path for x-pack/Cloud test for Lens (elastic#82634) Uses asCurrentUser in getClusterUuid (elastic#82908) [Alerting][Connectors] Add new executor subaction to get 3rd party case fields (elastic#82519) Fix test import objects (elastic#82767) [ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate (elastic#81662) Update alert type selection layout to rows instead of grid (elastic#73665) Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (elastic#82817) Update grunt and related packages (elastic#79327) Allow the repository to search across all namespaces (elastic#82863) ...

azasypkin added release_note:fix Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// Feature:Security/Authentication Platform Security - Authentication v7.11.0 labels Nov 6, 2020

azasypkin commented Nov 6, 2020

View reviewed changes

azasypkin force-pushed the issue-78260-new-session-pki-kerberos branch 3 times, most recently from 9430012 to 1291abb Compare November 6, 2020 13:53

azasypkin commented Nov 6, 2020

View reviewed changes

azasypkin marked this pull request as ready for review November 6, 2020 15:46

azasypkin requested a review from a team as a code owner November 6, 2020 15:46

Prevent Kerberos and PKI providers from initiating a new session for …

0b6369e

…unauthenticated XHR requests.

azasypkin force-pushed the issue-78260-new-session-pki-kerberos branch from 1291abb to 0b6369e Compare November 6, 2020 16:30

legrego self-requested a review November 6, 2020 17:10

Merge branch 'master' into issue-78260-new-session-pki-kerberos

1a7f5a1

legrego approved these changes Nov 9, 2020

View reviewed changes

azasypkin added 2 commits November 9, 2020 15:27

Merge branch 'master' into issue-78260-new-session-pki-kerberos

cb9b05c

Review#1: fix comment.

0670897

azasypkin merged commit 45ddd69 into elastic:master Nov 9, 2020

azasypkin deleted the issue-78260-new-session-pki-kerberos branch November 9, 2020 16:34

azasypkin added the v8.0.0 label Nov 9, 2020

azasypkin mentioned this pull request Nov 9, 2020

[7.x] Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (#82817) #82977

Merged

azasypkin added the backported label Nov 9, 2020

n0othing mentioned this pull request Mar 3, 2021

Support PKI and Kerberos auth against Kibana APIs #93452

Open

mbarretta mentioned this pull request Apr 7, 2021

[Docs] Update Metricbeat and Filebeat documentation to remove mentions of PKI authentication for Kibana APIs elastic/beats#24980

Closed

EamonnTP mentioned this pull request Aug 17, 2021

[DOCS] Remove PKI mentions elastic/beats#27422

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests.#82817

Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests.#82817
azasypkin merged 4 commits intoelastic:masterfrom
azasypkin:issue-78260-new-session-pki-kerberos

azasypkin commented Nov 6, 2020 •

edited

Loading

Uh oh!

azasypkin Nov 6, 2020

Uh oh!

azasypkin Nov 6, 2020 •

edited

Loading

Uh oh!

azasypkin Nov 6, 2020

Uh oh!

azasypkin Nov 6, 2020

Uh oh!

elasticmachine commented Nov 6, 2020

Uh oh!

azasypkin commented Nov 6, 2020

Uh oh!

legrego commented Nov 6, 2020

Uh oh!

legrego left a comment

Uh oh!

legrego Nov 9, 2020

Uh oh!

azasypkin Nov 9, 2020

Uh oh!

kibanamachine commented Nov 9, 2020

Uh oh!

azasypkin commented Nov 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

azasypkin commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azasypkin Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

azasypkin Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azasypkin Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

azasypkin Nov 6, 2020

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 6, 2020

Uh oh!

azasypkin commented Nov 6, 2020

Uh oh!

legrego commented Nov 6, 2020

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

legrego Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

azasypkin Nov 9, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Nov 9, 2020

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

azasypkin commented Nov 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

azasypkin commented Nov 6, 2020 •

edited

Loading

azasypkin Nov 6, 2020 •

edited

Loading