feat: option to update session cookie expiry time on session refresh#2848
Closed
aarmam wants to merge 2 commits intoory:masterfrom
Closed
feat: option to update session cookie expiry time on session refresh#2848aarmam wants to merge 2 commits intoory:masterfrom
aarmam wants to merge 2 commits intoory:masterfrom
Conversation
aarmam
force-pushed
the
feature/refresh-remember-for
branch
from
829dace to
219d381
Compare
Codecov Report
@@ 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
aarmam
force-pushed
the
feature/refresh-remember-for
branch
from
219d381 to
a3899d7
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
2 times, most recently
from
c26c329 to
a59aa69
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
2 times, most recently
from
737f3b8 to
2e15430
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
2 times, most recently
from
03cd8ce to
b50aba5
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
2 times, most recently
from
d9a22b2 to
657981b
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
from
657981b to
a584f09
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
4 times, most recently
from
c708b57 to
0d931c4
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
2 times, most recently
from
70a1117 to
8f1b39c
Compare
aarmam
force-pushed
the
feature/refresh-remember-for
branch
from
7a0b09a to
0c146ea
Compare
sgal
reviewed
Feb 23, 2023
|
|
||
| // RefreshRememberFor, if set to true, session cookie expiry time will be updated when session is | ||
| // refreshed (login skip=true). | ||
| RefreshRememberFor bool `json:"refresh_remember_for"` |
Contributor
There was a problem hiding this comment.
To be consistent with the existing values, it is better to name this property refresh_remember since it is a boolean value.
Existing props are following this format:
remember- booleanremember_for- number
Member
There was a problem hiding this comment.
Agree, maybe even clearer extend_session_lifespan. Could also be a time value alternatively (although it would be conflicting with remember_for)
Contributor
Author
There was a problem hiding this comment.
I'm on vacation and will come back to this in few weeks! :)
aeneasr
force-pushed
the
feature/refresh-remember-for
branch
from
0c146ea to
c8f5009
Compare
7 tasks
harnash
pushed a commit
to Wikia/ory-hydra
that referenced
this pull request
Apr 12, 2023
It is now possible to extend session lifespans when accepting login challenges. Closes ory#1690 Closes ory#1557 Closes ory#2246 Closes ory#2848 Co-authored-by: Mart Aarma <[email protected]> Co-authored-by: Henning Perl <[email protected]> Co-authored-by: ory-bot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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'sremember_forvalue.When performing subsequent session update requests (authentication requests with
prompt=none), then browser session cookie expiration cannot be changed.Proposed solution: Add
refresh_remember_forparameter for PUT /oauth2/auth/requests/login/accept request body. Whenrefresh_remember_for=true, session cookie expiry will be reset.Related issue(s)
#1690
#1557
#2246
Checklist
introduces a new feature.
contributing code guidelines.
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.
works.
Further Comments
Tests and documentation will be commited after inital acceptance of the proposed feature.