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

feat: extend session lifespan on session refresh (skip=true) #3464

Merged
merged 6 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
}
}

if !session.Remember || session.LoginRequest.Skip {
if !session.Remember || session.LoginRequest.Skip && !session.ExtendSessionLifespan {
// If the user doesn't want to remember the session, we do not store a cookie.
// If login was skipped, it means an authentication cookie was present and
// we don't want to touch it (in order to preserve its original expiry date)
Expand Down
96 changes: 96 additions & 0 deletions consent/strategy_oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,102 @@ func TestStrategyLoginConsentNext(t *testing.T) {
})
})

t.Run("case=should pass if both login and consent are granted and check remember flows with refresh session cookie", func(t *testing.T) {

subject := "subject-1"
c := createDefaultClient(t)
testhelpers.NewLoginConsentUI(t, reg.Config(),
acceptLoginHandler(t, subject, &hydra.AcceptOAuth2LoginRequest{
Remember: pointerx.Bool(true),
}),
acceptConsentHandler(t, &hydra.AcceptOAuth2ConsentRequest{
Remember: pointerx.Bool(true),
GrantScope: []string{"openid"},
Session: &hydra.AcceptOAuth2ConsentRequestSession{
AccessToken: map[string]interface{}{"foo": "bar"},
IdToken: map[string]interface{}{"bar": "baz"},
},
}))

hc := testhelpers.NewEmptyJarClient(t)

followUpHandler := func(extendSessionLifespan bool) {
rememberFor := int64(12345)
testhelpers.NewLoginConsentUI(t, reg.Config(),
checkAndAcceptLoginHandler(t, adminClient, subject, func(t *testing.T, res *hydra.OAuth2LoginRequest, err error) hydra.AcceptOAuth2LoginRequest {
require.NoError(t, err)
assert.True(t, res.Skip)
assert.Equal(t, subject, res.Subject)
assert.Empty(t, res.Client.ClientSecret)
return hydra.AcceptOAuth2LoginRequest{
Subject: subject,
Remember: pointerx.Bool(true),
RememberFor: pointerx.Int64(rememberFor),
ExtendSessionLifespan: pointerx.Bool(extendSessionLifespan),
Context: map[string]interface{}{"foo": "bar"},
}
}),
checkAndAcceptConsentHandler(t, adminClient, func(t *testing.T, res *hydra.OAuth2ConsentRequest, err error) hydra.AcceptOAuth2ConsentRequest {
require.NoError(t, err)
assert.True(t, *res.Skip)
assert.Equal(t, subject, res.Subject)
assert.Empty(t, res.Client.ClientSecret)
return hydra.AcceptOAuth2ConsentRequest{
Remember: pointerx.Bool(true),
GrantScope: []string{"openid"},
Session: &hydra.AcceptOAuth2ConsentRequestSession{
AccessToken: map[string]interface{}{"foo": "bar"},
IdToken: map[string]interface{}{"bar": "baz"},
},
}
}))

hc := &http.Client{
Jar: hc.Jar,
Transport: &http.Transport{},
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
}

_, oauthRes := makeOAuth2Request(t, reg, hc, c, url.Values{"redirect_uri": {c.RedirectURIs[0]}, "scope": {"openid"}})
assert.EqualValues(t, http.StatusFound, oauthRes.StatusCode)
loginChallengeRedirect, err := oauthRes.Location()
require.NoError(t, err)
defer oauthRes.Body.Close()

loginChallengeRes, err := hc.Get(loginChallengeRedirect.String())
require.NoError(t, err)
defer loginChallengeRes.Body.Close()
loginVerifierRedirect, err := loginChallengeRes.Location()

loginVerifierRes, err := hc.Get(loginVerifierRedirect.String())
require.NoError(t, err)
defer loginVerifierRes.Body.Close()

setCookieHeader := loginVerifierRes.Header.Get("set-cookie")
assert.NotNil(t, setCookieHeader)
if extendSessionLifespan {
assert.Regexp(t, fmt.Sprintf("ory_hydra_session_dev=.*; Path=/; Expires=.*Max-Age=%d; HttpOnly; SameSite=Lax", rememberFor), setCookieHeader)
} else {
assert.NotContains(t, setCookieHeader, "ory_hydra_session_dev")
}
}

t.Run("perform first flow", func(t *testing.T) {
makeRequestAndExpectCode(t, hc, c, url.Values{"redirect_uri": {c.RedirectURIs[0]},
"scope": {"openid"}})
})

t.Run("perform follow up flow with extend_session_lifespan=false", func(t *testing.T) {
followUpHandler(false)
})

t.Run("perform follow up flow with extend_session_lifespan=true", func(t *testing.T) {
followUpHandler(true)
})
})

t.Run("case=should pass and check if login context is set properly", func(t *testing.T) {
// This should pass because login was remembered and session id should be set and session context should also work
subject := "aeneas-rekkas"
Expand Down
9 changes: 9 additions & 0 deletions consent/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,15 @@ type HandledLoginRequest struct {
// authorization will be remembered for the duration of the browser session (using a session cookie).
RememberFor int `json:"remember_for"`

// Extend OAuth2 authentication session lifespan
//
// If set to `true`, the OAuth2 authentication cookie lifespan is extended. This is for example useful if you want the user to be able to use `prompt=none` continuously.
//
// This value can only be set to `true` if the user has an authentication, which is the case if the `skip` value is `true`.
//
// required: false
ExtendSessionLifespan bool `json:"extend_session_lifespan"`

// ACR sets the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it
// to express that, for example, a user authenticated using two factor authentication.
ACR string `json:"acr"`
Expand Down
6 changes: 6 additions & 0 deletions flow/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ type Flow struct {
// authorization will be remembered for the duration of the browser session (using a session cookie).
LoginRememberFor int `db:"login_remember_for"`

// LoginExtendSessionLifespan, if set to true, session cookie expiry time will be updated when session is
// refreshed (login skip=true).
LoginExtendSessionLifespan bool `db:"login_extend_session_lifespan"`

// ACR sets the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it
// to express that, for example, a user authenticated using two factor authentication.
ACR string `db:"acr"`
Expand Down Expand Up @@ -288,6 +292,7 @@ func (f *Flow) HandleLoginRequest(h *consent.HandledLoginRequest) error {

f.LoginRemember = h.Remember
f.LoginRememberFor = h.RememberFor
f.LoginExtendSessionLifespan = h.ExtendSessionLifespan
f.ACR = h.ACR
f.AMR = h.AMR
f.Context = h.Context
Expand All @@ -301,6 +306,7 @@ func (f *Flow) GetHandledLoginRequest() consent.HandledLoginRequest {
ID: f.ID,
Remember: f.LoginRemember,
RememberFor: f.LoginRememberFor,
ExtendSessionLifespan: f.LoginExtendSessionLifespan,
ACR: f.ACR,
AMR: f.AMR,
Subject: f.Subject,
Expand Down
1 change: 1 addition & 0 deletions flow/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (f *Flow) setHandledLoginRequest(r *consent.HandledLoginRequest) {
f.ID = r.ID
f.LoginRemember = r.Remember
f.LoginRememberFor = r.RememberFor
f.LoginExtendSessionLifespan = r.ExtendSessionLifespan
f.ACR = r.ACR
f.AMR = r.AMR
f.Subject = r.Subject
Expand Down
7 changes: 7 additions & 0 deletions internal/httpclient/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1876,6 +1876,13 @@ components:
context:
title: "JSONRawMessage represents a json.RawMessage that works well with\
\ JSON, SQL, and Swagger."
extend_session_lifespan:
description: "Extend OAuth2 authentication session lifespan\n\nIf set to\
\ `true`, the OAuth2 authentication cookie lifespan is extended. This\
\ is for example useful if you want the user to be able to use `prompt=none`\
\ continuously.\n\nThis value can only be set to `true` if the user has\
\ an authentication, which is the case if the `skip` value is `true`."
type: boolean
force_subject_identifier:
description: "ForceSubjectIdentifier forces the \"pairwise\" user ID of\
\ the end-user that authenticated. The \"pairwise\" user ID refers to\
Expand Down
26 changes: 26 additions & 0 deletions internal/httpclient/docs/AcceptOAuth2LoginRequest.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Name | Type | Description | Notes
**Acr** | Pointer to **string** | ACR sets the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it to express that, for example, a user authenticated using two factor authentication. | [optional]
**Amr** | Pointer to **[]string** | | [optional]
**Context** | Pointer to **interface{}** | | [optional]
**ExtendSessionLifespan** | Pointer to **bool** | Extend OAuth2 authentication session lifespan If set to `true`, the OAuth2 authentication cookie lifespan is extended. This is for example useful if you want the user to be able to use `prompt=none` continuously. This value can only be set to `true` if the user has an authentication, which is the case if the `skip` value is `true`. | [optional]
**ForceSubjectIdentifier** | Pointer to **string** | ForceSubjectIdentifier forces the \"pairwise\" user ID of the end-user that authenticated. The \"pairwise\" user ID refers to the (Pairwise Identifier Algorithm)[http://openid.net/specs/openid-connect-core-1_0.html#PairwiseAlg] of the OpenID Connect specification. It allows you to set an obfuscated subject (\"user\") identifier that is unique to the client. Please note that this changes the user ID on endpoint /userinfo and sub claim of the ID Token. It does not change the sub claim in the OAuth 2.0 Introspection. Per default, ORY Hydra handles this value with its own algorithm. In case you want to set this yourself you can use this field. Please note that setting this field has no effect if `pairwise` is not configured in ORY Hydra or the OAuth 2.0 Client does not expect a pairwise identifier (set via `subject_type` key in the client's configuration). Please also be aware that ORY Hydra is unable to properly compute this value during authentication. This implies that you have to compute this value on every authentication process (probably depending on the client ID or some other unique value). If you fail to compute the proper value, then authentication processes which have id_token_hint set might fail. | [optional]
**Remember** | Pointer to **bool** | Remember, if set to true, tells ORY Hydra to remember this user by telling the user agent (browser) to store a cookie with authentication data. If the same user performs another OAuth 2.0 Authorization Request, he/she will not be asked to log in again. | [optional]
**RememberFor** | Pointer to **int64** | RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered for the duration of the browser session (using a session cookie). | [optional]
Expand Down Expand Up @@ -116,6 +117,31 @@ HasContext returns a boolean if a field has been set.
`func (o *AcceptOAuth2LoginRequest) UnsetContext()`

UnsetContext ensures that no value is present for Context, not even an explicit nil
### GetExtendSessionLifespan

`func (o *AcceptOAuth2LoginRequest) GetExtendSessionLifespan() bool`

GetExtendSessionLifespan returns the ExtendSessionLifespan field if non-nil, zero value otherwise.

### GetExtendSessionLifespanOk

`func (o *AcceptOAuth2LoginRequest) GetExtendSessionLifespanOk() (*bool, bool)`

GetExtendSessionLifespanOk returns a tuple with the ExtendSessionLifespan field if it's non-nil, zero value otherwise
and a boolean to check if the value has been set.

### SetExtendSessionLifespan

`func (o *AcceptOAuth2LoginRequest) SetExtendSessionLifespan(v bool)`

SetExtendSessionLifespan sets ExtendSessionLifespan field to given value.

### HasExtendSessionLifespan

`func (o *AcceptOAuth2LoginRequest) HasExtendSessionLifespan() bool`

HasExtendSessionLifespan returns a boolean if a field has been set.

### GetForceSubjectIdentifier

`func (o *AcceptOAuth2LoginRequest) GetForceSubjectIdentifier() string`
Expand Down
37 changes: 37 additions & 0 deletions internal/httpclient/model_accept_o_auth2_login_request.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"ID": "auth_session-0016",
"NID": "00000000-0000-0000-0000-000000000000",
"AuthenticatedAt": null,
"Subject": "subject-0016",
"Remember": true
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 1,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0001",
"AMR": [],
"ForceSubjectIdentifier": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 2,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0002",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0002",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 3,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0003",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0003",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 4,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0004",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0004",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 5,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0005",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0005",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 6,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0006",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0006",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 7,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0007",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0007",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 8,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0008",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0008",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 9,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0009",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0009",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 10,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0010",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0010",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 11,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0011",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0011",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 12,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0012",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0012",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 13,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0013",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0013",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"State": 128,
"LoginRemember": true,
"LoginRememberFor": 14,
"LoginExtendSessionLifespan": false,
"ACR": "acr-0014",
"AMR": [],
"ForceSubjectIdentifier": "force_subject_id-0014",
Expand Down
Loading