Skip to content

Conversation

@jportner
Copy link
Contributor

@jportner jportner commented Dec 9, 2019

Small change based on feedback after #49855 was merged.

@jportner jportner force-pushed the session-config-duration branch 2 times, most recently from 700a8b6 to 92fdedc Compare December 9, 2019 18:47
Now these can be formatted in the config file as human-readable
strings.
@jportner jportner force-pushed the session-config-duration branch from 92fdedc to 909ead2 Compare December 9, 2019 19:37
@jportner jportner marked this pull request as ready for review December 9, 2019 20:54
@jportner jportner requested a review from a team as a code owner December 9, 2019 20:54
@jportner jportner assigned azasypkin and unassigned azasypkin Dec 9, 2019
@jportner jportner requested a review from azasypkin December 9, 2019 20:55
@jportner jportner added the release_note:skip Skip the PR/issue when compiling release notes label Dec 9, 2019
@azasypkin
Copy link
Member

ACK: reviewing...

The plugin now uses Duration natively instead of converting these
values to numbers.
Also removed unused vars from legacyCompat.
Even though `session.lifespan` should be `null` by default, the
`session` object is undefined by default. When the config is parsed
we want to make sure that both `session.idleTimeout` and
`session.lifespan` are null by default.
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Looks great, just a couple of nits.

@azasypkin azasypkin added chore Feature:Security/Authentication Platform Security - Authentication Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// labels Dec 13, 2019
@elasticmachine
Copy link
Contributor

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

We had some unnecessary complexity in the config to satisfy test
mocks as they were written. Chagned the tests and simplified said
config.
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported chore Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// v7.6.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants