consent: Don't touch authentication cookie on skipped logins (#1564)

closes #1557

consent/strategy_default.go

@@ -422,18 +422,22 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
 		}
 	}
 
-	if !session.Remember {
-		if !session.LoginRequest.Skip {
-			// If the session should not be remembered (and we're actually not skipping), than the user clearly don't
-			// wants us to store a cookie. So let's bust the authentication session (if one exists).
-			if err := s.revokeAuthenticationSession(w, r); err != nil {
-				return nil, err
-			}
+	if !session.Remember && !session.LoginRequest.Skip {
+		// If the session should not be remembered (and we're actually not skipping), than the user clearly don't
+		// wants us to store a cookie. So let's bust the authentication session (if one exists).
+		if err := s.revokeAuthenticationSession(w, r); err != nil {
+			return nil, err
 		}
+	}
 
+	if !session.Remember || session.LoginRequest.Skip {
+		// 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)
 		return session, nil
 	}
 
+	// Not a skipped login and the user asked to remember its session, store a cookie
 	cookie, _ := s.r.CookieStore().Get(r, CookieAuthenticationName)
 	cookie.Values[CookieAuthenticationSIDName] = sessionID
 	if session.RememberFor >= 0 {

consent/types.go

@@ -143,7 +143,7 @@ type HandledLoginRequest struct {
 	Remember bool `json:"remember"`
 
 	// RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the
-	// authorization will be remembered indefinitely.
+	// authorization will be remembered for the duration of the browser session (using a session cookie).
 	RememberFor int `json:"remember_for"`
 
 	// ACR sets the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it

docs/api.swagger.json

@@ -2062,7 +2062,7 @@
           "x-go-name": "Remember"
         },
         "remember_for": {
-          "description": "RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the\nauthorization will be remembered indefinitely.",
+          "description": "RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the\nauthorization will be remembered for the duration of the browser session (using a session cookie).",
           "type": "integer",
           "format": "int64",
           "x-go-name": "RememberFor"

sdk/java/hydra-client-resttemplate/docs/AcceptLoginRequest.md

@@ -8,7 +8,7 @@ Name | Type | Description | Notes
 **context** | **Map<String, Object>** | Context is an optional object which can hold arbitrary data. The data will be made available when fetching the consent request under the \"context\" field. This is useful in scenarios where login and consent endpoints share data. |  [optional]
 **forceSubjectIdentifier** | **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** | **Boolean** | 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** | **Long** | RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely. |  [optional]
+**rememberFor** | **Long** | 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]
 **subject** | **String** | Subject is the user ID of the end-user that authenticated. | 
 
 

sdk/java/hydra-client-resttemplate/src/main/java/com/github/ory/hydra/model/AcceptLoginRequest.java

@@ -132,10 +132,10 @@ public AcceptLoginRequest rememberFor(Long rememberFor) {
   }
 
    /**
-   * RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely.
+   * 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).
    * @return rememberFor
   **/
-  @ApiModelProperty(value = "RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely.")
+  @ApiModelProperty(value = "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).")
   public Long getRememberFor() {
     return rememberFor;
   }

sdk/js/swagger/docs/AcceptLoginRequest.md

@@ -7,7 +7,7 @@ Name | Type | Description | Notes
 **context** | **{String: Object}** | Context is an optional object which can hold arbitrary data. The data will be made available when fetching the consent request under the \"context\" field. This is useful in scenarios where login and consent endpoints share data. | [optional] 
 **forceSubjectIdentifier** | **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** | **Boolean** | 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** | **Number** | RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely. | [optional] 
+**rememberFor** | **Number** | 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] 
 **subject** | **String** | Subject is the user ID of the end-user that authenticated. | 
 
 

sdk/js/swagger/src/model/AcceptLoginRequest.js

@@ -110,7 +110,7 @@
    */
   exports.prototype['remember'] = undefined;
   /**
-   * RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely.
+   * 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).
    * @member {Number} remember_for
    */
   exports.prototype['remember_for'] = undefined;

sdk/php/swagger/docs/Model/AcceptLoginRequest.md

@@ -7,7 +7,7 @@ Name | Type | Description | Notes
 **context** | **map[string,object]** | Context is an optional object which can hold arbitrary data. The data will be made available when fetching the consent request under the \"context\" field. This is useful in scenarios where login and consent endpoints share data. | [optional] 
 **force_subject_identifier** | **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** | **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] 
-**remember_for** | **int** | RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely. | [optional] 
+**remember_for** | **int** | 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] 
 **subject** | **string** | Subject is the user ID of the end-user that authenticated. | 
 
 [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md)

sdk/php/swagger/lib/Model/AcceptLoginRequest.php

@@ -291,7 +291,7 @@ public function getRememberFor()
 
     /**
      * Sets remember_for
-     * @param int $remember_for RememberFor sets how long the authentication should be remembered for in seconds. If set to `0`, the authorization will be remembered indefinitely.
+     * @param int $remember_for 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).
      * @return $this
      */
     public function setRememberFor($remember_for)