hydra icon indicating copy to clipboard operation
hydra copied to clipboard

feat: option to update session cookie expiry time on session refresh

Open aarmam opened this issue 4 years ago • 1 comments

This pull request introduces feature to update session cookie expiry time on session refresh request.

Use case: We want to keep session duration quite short (15 minutes) and force client applications to periodically extend the session by performing authentication requests with prompt=none. Each subsequent authentication request produces a new identity token with lifetime of 15 minutes. But as a security measure we want that browser session cookie would not be kept alive any longer than necessary - therefore browser session cookie duration should be periodically extended, each time by 15 minutes (the same lifetime as each new identity token).

Current situation: Browser session cookie (oauth2_authentication_session) expiration is set from first acceptLoginRequest's remember_for value. When performing subsequent session update requests (authentication requests with prompt=none), then browser session cookie expiration cannot be changed.

Proposed solution: Add refresh_remember_for parameter for PUT /oauth2/auth/requests/login/accept request body. When refresh_remember_for=true, session cookie expiry will be reset.

Related issue(s)

https://github.com/ory/hydra/issues/1690 https://github.com/ory/hydra/issues/1557 https://github.com/ory/hydra/discussions/2246

Checklist

  • [x] I have read the contributing guidelines.
  • [x] I have referenced an issue containing the design document if my change introduces a new feature.
  • [x] I am following the contributing code guidelines.
  • [x] I have read the security policy.
  • [x] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security. vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • [x] I have added tests that prove my fix is effective or that my feature works.
  • [x] I have added or changed the documentation.

Further Comments

Tests and documentation will be commited after inital acceptance of the proposed feature.

aarmam avatar Nov 10 '21 07:11 aarmam

Codecov Report

Merging #2848 (05b2183) into master (3ea014f) will increase coverage by 0.07%. The diff coverage is 100.00%.

:exclamation: Current head 05b2183 differs from pull request most recent head c8f5009. Consider uploading reports for the commit c8f5009 to get more accurate results

@@            Coverage Diff             @@
##           master    #2848      +/-   ##
==========================================
+ Coverage   76.74%   76.81%   +0.07%     
==========================================
  Files         123      123              
  Lines        9022     9071      +49     
==========================================
+ Hits         6924     6968      +44     
- Misses       1657     1660       +3     
- Partials      441      443       +2     
Impacted Files Coverage Δ
consent/types.go 78.12% <ø> (ø)
consent/strategy_default.go 69.59% <100.00%> (ø)
flow/flow.go 92.26% <100.00%> (-0.92%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Nov 24 '21 08:11 codecov[bot]