plataformatec/devise — pull request #2283 by teleological (2013-02-15 18:18:50+00:00)
Title: Require last_request at for timeoutable sessions

Currently, `timeoutable` does not invalidate sessions which do not include a `last_request_at` value. This means that any session issued prior to configuring timeoutable or as a result of a request that sets `devise.skip_trackable` will not ever be timed out.

This change would treat any session that does not include a `last_request_at` value as a timed-out session, so that it will not be possible to bypass timeout restrictions by replaying such sessions.

Prior to this change, `last_request_at` was checked as part of the `after_set_user` hook. This change moves the timeout test to the `after_fetch` hook, which is only fired when the user is set from the session. This is to avoid triggering a session timeout event when authenticating with methods that do not rely on the session while continuing to ensure that the session user is only valid when the session is not timed out.

The `after_set_user` hook is still used to update the value of `last_request_at`, so any authentication method that accompanies a session will continue to refresh that date.

This change highlights but does not correct a latent issue in the interaction of `timeoutable` and `token_authenticatable`. When the `expire_auth_token_on_timeout` is set, the token used by the `token_authenticatable` strategy is invalidated if used along with an expired session.

This behavior is not changed. If an expired session is present, the authentication token will be invalidated by the `after_fetch` hook, which is run before the token authentication strategy is tried.

However, this restriction is easily bypassed by not including a session or session id with the token-authenticated request. Prior to this change, the restriction would have been bypassed because the `after_set_user` hook fired on successful token authentication would have allowed a null `last_request_at`. With this change, the restriction is bypassed because the `after_fetch` hook is never fired.

It might be worth considering deprecating `expire_auth_token_on_timeout` or replacing it with a mechanism like that used by `rememberable`, since it may be providing a false sense of security.