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

Add support for "session"-duration secret cookies #1506

Closed
ecton opened this issue Jan 5, 2021 · 3 comments
Closed

Add support for "session"-duration secret cookies #1506

ecton opened this issue Jan 5, 2021 · 3 comments
Labels
accepted An accepted request or suggestion request Request for new functionality

Comments

@ecton
Copy link

ecton commented Jan 5, 2021

I'm currently using the master branch to develop a new project against, and in the process of setting up the Remember Me checkbox for sign in, I noticed I wasn't able to get the cookie to be stored with a 'Session' duration. This is done by not emitting an Expires attribute. While that page also notes a warning about session restore causing issues, I still for my site want my cookies to be able to be tied to a Session so that Remember Me emits a session-length cookie when unchecked and a long expiration time when Remember Me is checked.

I discovered that this isn't currently possible. CookieJar::add_private calls CookieJar::set_private_defaults, which has this chunk:

if cookie.expires().is_none() {
  cookie.set_expires(time::OffsetDateTime::now_utc() + time::Duration::weeks(1));
}

This prevents inserting a private cookie with no expiration date. I actually completely agree that setting a duration by default is a good security tactic, but in my case, I want to explicitly ensure this cookie has no Expires attribute.

For SameSite this is done by offering a SameSite::None in addition to allowing None when setting the same_site attribute. I would suggest offering some sort of similar value that can represent an explicitly omitted Expires attribute.

@jebrosen jebrosen added the request Request for new functionality label Jan 5, 2021
@SergioBenitez
Copy link
Member

I think we should keep these defaults but also provide a kind of escape hatch. One idea is to add a couple more methods to the effect of add_session_cookie and add_private_session_cookie. Another is to have a specific setting in Cookie itself to make it a "session" cookie, abstracting away what makes it a session cookie. Here, we'd hope something like the following would have your desired effect:

let cookie = Cookie::build("foo", "bar").session();
cookies.add_private(cookie);

There is precedent for this given the existence of the permanent builder method.

@jebrosen What do you think?

@SergioBenitez SergioBenitez added the accepted An accepted request or suggestion label Jan 12, 2021
@jebrosen
Copy link
Collaborator

It seems like one of the underlying problems here is that expires is an Option<OffsetDateTime>, so there's no way to distinguish between "not set explicitly" and "set to None / session duration". This seems fine for Cookie in isolation; it can say that the default is None / session duration, but for setting private cookie defaults we would ideally not override an explicitly-set None. Most of the other values where rocket_http overrides the defaults do have a way to distinguish between "unset" and "set-to-None".

Another is to have a specific setting in Cookie itself to make it a "session" cookie, abstracting away what makes it a session

This sounds reasonable on the face of it, but I worry that it would lead to two APIs ("session()" and "expires()") that inspect/mutate the same source of truth. I think I could be okay with something like that, as long as the interactions between those two and potential gotchas are made very clear.

@arnodb
Copy link

arnodb commented Jan 13, 2021

Hi Rocketers,

That is something I faced with at work and here is the current state of what we did:

  • With version 0.4 we just reimplemented the function which adds private cookies and removed the default value, such a fork is acceptable as its size is very limited.
  • With version 0.5 the API does now allow such a fork and the problem remains "to fix". So I'll keep looking at that particular issue updates. Whatever you do to allow session private cookies, I'll take it :).

Side note: our migration to 0.5 is on standby due to many things moving on your side but all in all I did not spot anything that would stop us doing that.

SergioBenitez added a commit that referenced this issue Feb 25, 2021
SergioBenitez added a commit that referenced this issue Feb 26, 2021
Routing:
  * Unicode characters are accepted anywhere in route paths. (#998)
  * Dyanmic query values can (and must) be any `FromForm` type. The `Form` type
    is no longer useable in any query parameter type.

Capped
  * A new `Capped` type is used to indicate when data has been truncated due to
    incoming data limits. It allows checking whether data is complete or
    truncated. `DataStream` methods returns `Capped` types.
  * Several `Capped<T>` types implement `FromData`, `FromForm`.
  * HTTP 413 (Payload Too Large) errors are now returned when the data limit is
    exceeded. (resolves #972)

Hierarchical Limits
  * Data limits are now hierarchical, delimited with `/`. A limit of `a/b/c`
    falls back to `a/b` then `a` when not set.

Temporary Files
  * A new `TempFile` data and form guard allows streaming data directly to a
    file which can then be persisted.
  * A new `temp_dir` config parameter specifies where to store `TempFile`.
  * The limits `file` and `file/$ext`, where `$ext` is the file extension,
    determines the data limit for a `TempFile`.

Forms Revamp
  * All form related types now reside in a new `form` module.
  * Multipart forms are supported. (resolves #106)
  * Collections are supported in body forms and queries. (resolves #205)
  * Nested forms and structures are supported. (resolves #313)
  * Form fields can be ad-hoc validated with `#[field(value = expr)]`.

Core:
  * `&RawStr` no longer implements `FromParam`.
  * `&str` implements `FromParam`, `FromData`, `FromForm`.
  * `FromTransformedData` was removed.
  * `FromData` gained a lifetime for use with request-local data.
  * All dynamic paramters in a query string must typecheck as `FromForm`.
  * `FromFormValue` removed in favor of `FromFormField`.
  * Dyanmic paramters, form values are always percent-decoded.
  * The default error HTML is more compact.
  * `&Config` is a request guard.
  * The `DataStream` interface was entirely revamped.
  * `State` is only exported via `rocket::State`.
  * A `request::local_cache!()` macro was added for storing values in
    request-local cache without consideration for type uniqueness by using a
    locally generated anonymous type.
  * `Request::get_param()` is now `Request::param()`.
  * `Request::get_segments()` is now `Request::segments()`, takes a range.
  * `Request::get_query_value()` is now `Request::query_value()`, can parse any
    `FromForm` including sequences.
  * `std::io::Error` implements `Responder` as `Debug<std::io::Error>`.
  * `(Status, R)` where `R: Responder` implements `Responder` by setting
    overriding the `Status` of `R`.
  * The name of a route is printed first during route matching.
  * `FlashMessage` now only has one lifetime generic.

HTTP:
  * `RawStr` implements `serde::{Serialize, Deserialize}`.
  * `RawStr` implements _many_ more methods, in particular, those related to the
    `Pattern` API.
  * `RawStr::from_str()` is now `RawStr::new()`.
  * `RawStr::url_decode()` and `RawStr::url_decode_lossy()` only allocate as
    necessary, return `Cow`.
  * `(Status, R)` where `R: Responder` is a responder that overwrites the status
    of `R` to `Status`.
  * `Status` implements `Default` with `Status::Ok`.
  * `Status` implements `PartialEq`, `Eq`, `Hash`, `PartialOrd`, `Ord`.
  * Authority and origin part of `Absolute` can be modified with new
    `Absolute::{with,set}_authority()`, `Absolute::{with,set}_origin()` methods.
  * `Origin::segments()` was removed in favor of methods split into query and
    path parts and into raw and decoded parts.
  * The `Segments` iterator is signficantly smarter. Returns `&str`.
  * `Segments::into_path_buf()` is now `Segments::to_path_buf()`, doesn't
    consume.
  * A new `QuerySegments` is the analogous query segment iterator.
  * Once set, the `expires` field on private cookies is not overwritten.
    (resolves #1506)
  * `Origin::path()` and `Origin::query()` return `&RawStr`, not `&str`.

Codegen:
  * Preserve more spans in `uri!` macro.
  * `FromFormValue` derive removed; `FromFormField` added.
  * The `form` `FromForm` and `FromFormField` field attribute is now named
    `field`. `#[form(field = ..)]` is now `#[form(name = ..)]`.

Examples:
  * `form_validation` and `form_kitchen_sink` removed in favor of `forms`
  * `rocket_contrib::Json` implements `FromForm`.
  * The `json!` macro is exported as `rocket_contrib::json::json`.
  * `rocket_contrib::MsgPack` implements `FromForm`.
  * Added clarifying docs to `StaticFiles`.
  * The `hello_world` example uses unicode in paths.

Internal:
  * Codegen uses new `exports` module with the following conventions:
    - Locals starts with `__` and are lowercased.
    - Rocket modules start with `_` are are lowercased.
    - Stdlib types start with `_` are are titlecased.
    - Rocket types are titlecased.
  * A `header` module was added to `http`, contains header types.
  * `SAFETY` is used as doc-string keyword for `unsafe` related comments.
  * The `Uri` parser no longer recognizes Rocket route URIs.
SergioBenitez added a commit that referenced this issue Mar 2, 2021
Routing:
  * All UTF-8 characters are accepted anywhere in route paths. (#998)
  * `path` is now `uri` in `route` attribute: `#[route(GET, path = "..")]`
    becomes `#[route(GET, uri = "..")]`.

Forms Revamp
  * All form related types now reside in a new `form` module.
  * Multipart forms are supported. (resolves #106)
  * Collections are supported in body forms and queries. (resolves #205)
  * Nested forms and structures are supported. (resolves #313)
  * Form fields can be ad-hoc validated with `#[field(value = expr)]`.
  * `FromFormValue` is now `FromFormField`, blanket implements `FromForm`.
  * Form field values are always percent-decoded apriori.

Temporary Files
  * A new `TempFile` data and form guard allows streaming data directly to a
    file which can then be persisted.
  * A new `temp_dir` config parameter specifies where to store `TempFile`.
  * The limits `file` and `file/$ext`, where `$ext` is the file extension,
    determines the data limit for a `TempFile`.

Capped
  * A new `Capped` type is used to indicate when data has been truncated due to
    incoming data limits. It allows checking whether data is complete or
    truncated. `DataStream` methods return `Capped` types.
  * Several `Capped<T>` types implement `FromData`, `FromForm`.
  * HTTP 413 (Payload Too Large) errors are now returned when data limits are
    exceeded. (resolves #972)

Hierarchical Limits
  * Data limits are now hierarchical, delimited with `/`. A limit of `a/b/c`
    falls back to `a/b` then `a`.

Core
  * `&RawStr` no longer implements `FromParam`.
  * `&str` implements `FromParam`, `FromData`, `FromForm`.
  * `FromTransformedData` was removed.
  * `FromData` gained a lifetime for use with request-local data.
  * The default error HTML is more compact.
  * `&Config` is a request guard.
  * The `DataStream` interface was entirely revamped.
  * `State` is only exported via `rocket::State`.
  * A `request::local_cache!()` macro was added for storing values in
    request-local cache without consideration for type uniqueness by using a
    locally generated anonymous type.
  * `Request::get_param()` is now `Request::param()`.
  * `Request::get_segments()` is now `Request::segments()`, takes a range.
  * `Request::get_query_value()` is now `Request::query_value()`, can parse any
    `FromForm` including sequences.
  * `std::io::Error` implements `Responder` as `Debug<std::io::Error>`.
  * `(Status, R)` where `R: Responder` implements `Responder` by overriding the
    `Status` of `R`.
  * The name of a route is printed first during route matching.
  * `FlashMessage` now only has one lifetime generic.

HTTP
  * `RawStr` implements `serde::{Serialize, Deserialize}`.
  * `RawStr` implements _many_ more methods, in particular, those related to the
    `Pattern` API.
  * `RawStr::from_str()` is now `RawStr::new()`.
  * `RawStr::url_decode()` and `RawStr::url_decode_lossy()` only allocate as
    necessary, return `Cow`.
  * `Status` implements `Default` with `Status::Ok`.
  * `Status` implements `PartialEq`, `Eq`, `Hash`, `PartialOrd`, `Ord`.
  * Authority and origin part of `Absolute` can be modified with new
    `Absolute::{with,set}_authority()`, `Absolute::{with,set}_origin()` methods.
  * `Origin::segments()` was removed in favor of methods split into query and
    path parts and into raw and decoded versions.
  * The `Segments` iterator is smarter, returns decoded `&str` items.
  * `Segments::into_path_buf()` is now `Segments::to_path_buf()`.
  * A new `QuerySegments` is the analogous query segment iterator.
  * Once set, `expires` on private cookies is not overwritten. (resolves #1506)
  * `Origin::path()` and `Origin::query()` return `&RawStr`, not `&str`.

Codegen
  * Preserve more spans in `uri!` macro.
  * Preserve spans `FromForm` field types.
  * All dynamic parameters in a query string must typecheck as `FromForm`.
  * `FromFormValue` derive removed; `FromFormField` added.
  * The `form` `FromForm` and `FromFormField` field attribute is now named
    `field`. `#[form(field = ..)]` is now `#[field(name = ..)]`.

Contrib
  * `Json` implements `FromForm`.
  * `MsgPack` implements `FromForm`.
  * The `json!` macro is exported as `rocket_contrib::json::json!`.
  * Added clarifying docs to `StaticFiles`.

Examples
  * `form_validation` and `form_kitchen_sink` removed in favor of `forms`.
  * The `hello_world` example uses unicode in paths.
  * The `json` example only allocates as necessary.

Internal
  * Codegen uses new `exports` module with the following conventions:
    - Locals starts with `__` and are lowercased.
    - Rocket modules start with `_` and are lowercased.
    - `std` types start with `_` and are titlecased.
    - Rocket types are titlecased.
  * A `header` module was added to `http`, contains header types.
  * `SAFETY` is used as doc-string keyword for `unsafe` related comments.
  * The `Uri` parser no longer recognizes Rocket route URIs.
SergioBenitez added a commit that referenced this issue Mar 4, 2021
Routing:
  * All UTF-8 characters are accepted anywhere in route paths. (#998)
  * `path` is now `uri` in `route` attribute: `#[route(GET, path = "..")]`
    becomes `#[route(GET, uri = "..")]`.

Forms Revamp
  * All form related types now reside in a new `form` module.
  * Multipart forms are supported. (resolves #106)
  * Collections are supported in body forms and queries. (resolves #205)
  * Nested forms and structures are supported. (resolves #313)
  * Form fields can be ad-hoc validated with `#[field(value = expr)]`.
  * `FromFormValue` is now `FromFormField`, blanket implements `FromForm`.
  * Form field values are always percent-decoded apriori.

Temporary Files
  * A new `TempFile` data and form guard allows streaming data directly to a
    file which can then be persisted.
  * A new `temp_dir` config parameter specifies where to store `TempFile`.
  * The limits `file` and `file/$ext`, where `$ext` is the file extension,
    determines the data limit for a `TempFile`.

Capped
  * A new `Capped` type is used to indicate when data has been truncated due to
    incoming data limits. It allows checking whether data is complete or
    truncated. `DataStream` methods return `Capped` types.
  * Several `Capped<T>` types implement `FromData`, `FromForm`.
  * HTTP 413 (Payload Too Large) errors are now returned when data limits are
    exceeded. (resolves #972)

Hierarchical Limits
  * Data limits are now hierarchical, delimited with `/`. A limit of `a/b/c`
    falls back to `a/b` then `a`.

Core
  * `&RawStr` no longer implements `FromParam`.
  * `&str` implements `FromParam`, `FromData`, `FromForm`.
  * `FromTransformedData` was removed.
  * `FromData` gained a lifetime for use with request-local data.
  * The default error HTML is more compact.
  * `&Config` is a request guard.
  * The `DataStream` interface was entirely revamped.
  * `State` is only exported via `rocket::State`.
  * A `request::local_cache!()` macro was added for storing values in
    request-local cache without consideration for type uniqueness by using a
    locally generated anonymous type.
  * `Request::get_param()` is now `Request::param()`.
  * `Request::get_segments()` is now `Request::segments()`, takes a range.
  * `Request::get_query_value()` is now `Request::query_value()`, can parse any
    `FromForm` including sequences.
  * `std::io::Error` implements `Responder` as `Debug<std::io::Error>`.
  * `(Status, R)` where `R: Responder` implements `Responder` by overriding the
    `Status` of `R`.
  * The name of a route is printed first during route matching.
  * `FlashMessage` now only has one lifetime generic.

HTTP
  * `RawStr` implements `serde::{Serialize, Deserialize}`.
  * `RawStr` implements _many_ more methods, in particular, those related to the
    `Pattern` API.
  * `RawStr::from_str()` is now `RawStr::new()`.
  * `RawStr::url_decode()` and `RawStr::url_decode_lossy()` only allocate as
    necessary, return `Cow`.
  * `Status` implements `Default` with `Status::Ok`.
  * `Status` implements `PartialEq`, `Eq`, `Hash`, `PartialOrd`, `Ord`.
  * Authority and origin part of `Absolute` can be modified with new
    `Absolute::{with,set}_authority()`, `Absolute::{with,set}_origin()` methods.
  * `Origin::segments()` was removed in favor of methods split into query and
    path parts and into raw and decoded versions.
  * The `Segments` iterator is smarter, returns decoded `&str` items.
  * `Segments::into_path_buf()` is now `Segments::to_path_buf()`.
  * A new `QuerySegments` is the analogous query segment iterator.
  * Once set, `expires` on private cookies is not overwritten. (resolves #1506)
  * `Origin::path()` and `Origin::query()` return `&RawStr`, not `&str`.

Codegen
  * Preserve more spans in `uri!` macro.
  * Preserve spans `FromForm` field types.
  * All dynamic parameters in a query string must typecheck as `FromForm`.
  * `FromFormValue` derive removed; `FromFormField` added.
  * The `form` `FromForm` and `FromFormField` field attribute is now named
    `field`. `#[form(field = ..)]` is now `#[field(name = ..)]`.

Contrib
  * `Json` implements `FromForm`.
  * `MsgPack` implements `FromForm`.
  * The `json!` macro is exported as `rocket_contrib::json::json!`.
  * Added clarifying docs to `StaticFiles`.

Examples
  * `form_validation` and `form_kitchen_sink` removed in favor of `forms`.
  * The `hello_world` example uses unicode in paths.
  * The `json` example only allocates as necessary.

Internal
  * Codegen uses new `exports` module with the following conventions:
    - Locals starts with `__` and are lowercased.
    - Rocket modules start with `_` and are lowercased.
    - `std` types start with `_` and are titlecased.
    - Rocket types are titlecased.
  * A `header` module was added to `http`, contains header types.
  * `SAFETY` is used as doc-string keyword for `unsafe` related comments.
  * The `Uri` parser no longer recognizes Rocket route URIs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted An accepted request or suggestion request Request for new functionality
Projects
None yet
Development

No branches or pull requests

4 participants