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.

Sign up for GitHub

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

Jump to bottom

[Authc] Security authentication config #205367

Merged
merged 4 commits into from
Jan 6, 2025
Merged

[Authc] Security authentication config #205367

merged 4 commits into from
Jan 6, 2025

Conversation

elena-shostak
Copy link
Contributor

@elena-shostak elena-shostak commented Jan 2, 2025 •
edited by kibanamachine
Loading

Summary

We cannot support security.authc evolvement for versioned routes, since authentication is passed down to hapi during route registration and it is tight up with the authentication strategy defined. Adjusted the code to pass auth option correctly.

kibana/packages/core/http/core-http-server-internal/src/http_server.ts

Lines 378 to 393 in e5cf28b

private getAuthOption(
authRequired: RouteConfigOptions<any>['authRequired'] = true
): undefined | false | { mode: 'required' | 'try' } {
if (this.authRegistered === false) return undefined;
if (authRequired === true) {
return { mode: 'required' };
}
if (authRequired === 'optional') {
// we want to use HAPI `try` mode and not `optional` to not throw unauthorized errors when the user
// has invalid or expired credentials
return { mode: 'try' };
}
if (authRequired === false) {
return false;
}

Checklist

Fixes: #205360

@elena-shostak elena-shostak added Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jan 2, 2025
@elena-shostak elena-shostak changed the title Security authc config [Authc] Security authentication config Jan 2, 2025
@elena-shostak elena-shostak marked this pull request as ready for review January 2, 2025 16:06
@elena-shostak elena-shostak requested review from a team as code owners January 2, 2025 16:06
@elasticmachine
Copy link
Contributor

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

@kc13greiner kc13greiner self-requested a review January 2, 2025 16:29
kc13greiner
kc13greiner approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@kc13greiner kc13greiner left a comment

Choose a reason for hiding this comment

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

LGTM!

@elena-shostak elena-shostak force-pushed the security-authc-config branch from 883da44 to b682613 Compare January 6, 2025 10:03
jloleysens
jloleysens approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Overall looks good and makes sense to me. Left a non blocker comment, but I'd like to get your thoughts before merging!

src/core/packages/http/server-internal/src/http_server.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #7 / serverless observability UI Dataset Quality Dataset quality summary shows poor, degraded and good count as 0 and all dataset as healthy

The CI Stats report is too large to be displayed here, check out the CI build annotation for this information.

History

@elena-shostak elena-shostak merged commit 26cc597 into elastic:main Jan 6, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12635845595

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 6, 2025
@elena-shostak
[Authc] Security authentication config (elastic#205367)
15b4b84 
## Summary

We cannot support `security.authc` evolvement for versioned routes,
since authentication is passed down to hapi during route registration
and it is tight up with the authentication strategy defined. Adjusted
the code to pass `auth` option correctly.

https://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

__Fixes: https://github.com/elastic/kibana/issues/205360__

(cherry picked from commit 26cc597)
@kibanamachine kibanamachine mentioned this pull request Jan 6, 2025
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 6, 2025
@kibanamachine @elena-shostak
[8.x] [Authc] Security authentication config (#205367) (#205625)
b8ea6b4 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Authc] Security authentication config
(#205367)](#205367)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Elena
Shostak","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-06T16:02:21Z","message":"[Authc]
Security authentication config (#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","Feature:Security/Authentication","release_note:skip","v9.0.0","backport:prev-minor"],"title":"[Authc]
Security authentication
config","number":205367,"url":"https://github.com/elastic/kibana/pull/205367","mergeCommit":{"message":"[Authc]
Security authentication config (#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205367","number":205367,"mergeCommit":{"message":"[Authc]
Security authentication config (#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881"}}]}]
BACKPORT-->

Co-authored-by: Elena Shostak <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
@elena-shostak @kowalczyk-krzysztof
[Authc] Security authentication config (elastic#205367)
eafba2c 
## Summary

We cannot support `security.authc` evolvement for versioned routes,
since authentication is passed down to hapi during route registration
and it is tight up with the authentication strategy defined. Adjusted
the code to pass `auth` option correctly.


https://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393


### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

__Fixes: https://github.com/elastic/kibana/issues/205360__
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
@elena-shostak @CAWilson94
[Authc] Security authentication config (elastic#205367)
2717274 
## Summary

We cannot support `security.authc` evolvement for versioned routes,
since authentication is passed down to hapi during route registration
and it is tight up with the authentication strategy defined. Adjusted
the code to pass `auth` option correctly.


https://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393


### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

__Fixes: https://github.com/elastic/kibana/issues/205360__
viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025
@elena-shostak @viduni94
[Authc] Security authentication config (elastic#205367)
4085fee 
## Summary

We cannot support `security.authc` evolvement for versioned routes,
since authentication is passed down to hapi during route registration
and it is tight up with the authentication strategy defined. Adjusted
the code to pass `auth` option correctly.


https://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393


### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

__Fixes: https://github.com/elastic/kibana/issues/205360__
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kc13greiner kc13greiner kc13greiner approved these changes

@jloleysens jloleysens jloleysens approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Authentication] Issue with security.authc
5 participants
@elena-shostak @elasticmachine @kibanamachine @jloleysens @kc13greiner