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

[FEATURE] Add support for JWT exp claim for cookie expiry #1784

Closed
derek-ho opened this issue Feb 15, 2024 · 3 comments · Fixed by #1773
Closed

[FEATURE] Add support for JWT exp claim for cookie expiry #1784

derek-ho opened this issue Feb 15, 2024 · 3 comments · Fixed by #1773
Assignees
Labels
enhancement New feature or request triaged

Comments

@derek-ho
Copy link
Collaborator

derek-ho commented Feb 15, 2024

Is your feature request related to a problem?
The current implementation of JWT does not look at the exp. of the JWT and instead always sets the expiry of the cookie based on the dashboard setting opensearch_security.session.ttl. Code link. Instead, we should parse the JWT and see if the JWT has information on when it expires. If it does, we should use that value, if it is less than the TTL. If it doesn't, we should default to the TTL.

What solution would you like?
A session/cookie should only be valid for the lesser of the TTL and the jwt's expiry claim (if it exists).

What alternatives have you considered?
None

Do you have any additional context?
Tested and verified that backend is not returning data unexpectedly as it is today, but sessions should be scoped to the expiry of the JWT.

@peternied
Copy link
Member

@derek-ho Please update this issue so there is no default text from the feature template.

@cwperks
Copy link
Member

cwperks commented Feb 16, 2024

@derek-ho I think there are 2 (arguably 3) cases to consider:

  1. The JWT would expire before the length of the session
  2. The JWT expiry is longer than the session length
  3. The JWT does not expire

In scenario 1, I think the expiry from the JWT should override the session length otherwise the user would start getting errors when making requests during their active OSD session since the token is expired, but their session is active.

For scenario 2 and 3 I think we should use the session length configured, especially for scenario 3 where we should not be giving a user a session indefinitely. Similar to username and password, if a user wants to login again they can present their credentials but just because they knew the username and password once does not mean that they should get a session forever.

@derek-ho
Copy link
Collaborator Author

@derek-ho I think there are 2 (arguably 3) cases to consider:

  1. The JWT would expire before the length of the session
  2. The JWT expiry is longer than the session length
  3. The JWT does not expire

In scenario 1, I think the expiry from the JWT should override the session length otherwise the user would start getting errors when making requests during their active OSD session since the token is expired, but their session is active.

For scenario 2 and 3 I think we should use the session length configured, especially for scenario 3 where we should not be giving a user a session indefinitely. Similar to username and password, if a user wants to login again they can present their credentials but just because they knew the username and password once does not mean that they should get a session forever.

Agree. I may have worded the issue confusingly at first. This is the logic I applied in my PR #1773

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged
Projects
None yet
3 participants