Is there a reason Record isn't a trait?

[Enduriel]

Been using both this and axum-login and in that crate I really like how AuthUser is handled, it makes me particularly aware of the fact that Record is not handled the same way in tower-sessions and that I need to work around the struct.

This is especially painful when it uses time when I use chrono for my worker-rs project due to the fact that worker-rs uses chrono, which leaves me in the amazing position of having to convert between the two time formats.

I'm not sure there's an easy way to solve that particular issue, since you guys have to choose something internally, so at the very least I'd have to convert to that (though with a bigger refactor even this could be worked around I think), but it would be nice to not have to convert the id and data fields at least. Are there any plans to improve this part of the API?

[maxcountryman]

There is no way around time: it's used by cookie which is the crate everything else is built over so that choice is made for us.

Generally, you shouldn't need to use Record outside of implementing SessionStore.

I could see a case for making IDs associated types of SessionStore but there's some nuance there as if an ID ever contains data it must be signed or encrypted and so it's not clear there's a good pattern that would enforce such a constraint.

That said, I'm not really clear on how Record being a trait would help e.g. the use case I just described. Maybe you can elaborate on what you mean by this? What's noteworthy is something has to describe the essential properties of the session data which includes at least an ID, the data itself, and an expiration.

[Enduriel]

Sure, for my API I use Uuid for the id, use chrono instead of time (because of worker-rs) and also have a DataMap wrapper around HashMap to implement from_bytes and to_bytes for the HashMap to allow me to easily (de)serialize it from/into a sqlite database.

This means I have to implement at least

impl From<&tower_sessions::session::Record> for Session {
	fn from(record: &tower_sessions::session::Record) -> Self {
		Self {
			session_uuid: Uuid::from_u128(record.id.0 as u128),
			expiry_date: chrono::DateTime::from_timestamp(record.expiry_date.unix_timestamp(), 0)
				.unwrap(),
			data: DataMap::from(record.data.clone()),
		}
	}
}

impl From<&Session> for tower_sessions::session::Record {
	fn from(session: Session) -> Self {
		Self {
			id: tower_sessions::session::Id(session.session_uuid.as_u128() as i128),
			expiry_date: time::OffsetDateTime::from_unix_timestamp(session.expiry_date.timestamp())
				.unwrap(),
			data: session.data.0.clone(),
		}
	}
}

which is obviously not ideal.

If it was possible to be able to define these ourselves, with members to implement (same as for SessionStore/AuthnBackend/AuthUser/etc) and maximizing the use of generics, I wouldn't have to write these wrappers and remember to use .into() a bunch of times.

Both crates mostly do this for the other stuff I've mentioned, I've noticed this as a particular spot where they don't play nice with my existing environment.

[Enduriel]

Actually, it's smarter to instead implement 4 different impl blocks

impl From<&Session> for &Record
impl From<Session> for Record
impl From<&Record> for &Session
impl From<Record> for Session

so it'd be twice as much code as before, but at least no unnecessary clones, my point still stands about the verbosity though.

EDIT: Actually not sure this is realistically possible.

[maxcountryman]

I think associated types on SessionStore would address the same things.

But again we can make far weaker guarantees about the secure nature of the crate if we e.g. don't provide a concrete ID type.

[Enduriel]

You could provide an example struct that would suffice for most people, and yes I agree with your point about doing this in SessionStore, I think that would be best

[maxcountryman]

It's a fairly significant undertaking to try to rework this and while I'm not opposed to it at a high level it's not clear it would really pay off.

Both the record and ID types are used directly in the session itself and so to make even just the ID a generic or associated type is nontrivial.

That coupled with the fact that any ID type that isn't specified by the crate is potentially weaker in terms of its secure nature makes me disinclined to pursue this.

If someone wanted to propose a design and explore this more concretely I would certainly be happy to take a look but I can't promise that design would be published.

From my perspective the most important outcome would be that any redesign like this enable storing data in the session ID directly but that should also require that any such cookie be signed or encrypted.