Migrating from tower-sessions 0.9 to 0.10 (session ID)

[Razican]

Hello, I'm currently using tower-sessions 0.9 with UUIDs in a PostgreSQL database. I use a custom diesel backend, and my session store table looks like this:

-- Create the session table
CREATE TABLE sys_session (
    id uuid PRIMARY KEY NOT NULL,
    data jsonb NOT NULL,
    expire_date TIMESTAMP WITH TIME ZONE NOT NULL
);

Unfortunately, the new IDs in 0.10 seem to be i128 and not UUIDs, so I guess I would need to migrate from UUIDs. Seeing how the sqlx postgres backend has done this, it seems to use a text column as primary key:

https://github.com/maxcountryman/tower-sessions-stores/blob/51cc75817cb98e875d8cf3975363f746857ad0b2/sqlx-store/src/postgres_store.rs#L110-L115

Is this what I have to do? Why would I want to change a very storage and performance v7 UUID for a non-checked, very slow text field? It seems that the performance would go down like crazy.

I also saw that the new IDs reduce security from 128 bits to 66 bits in this process, barely above the 64-bit minimum recommended by OWASP.

Is there a better way to do this?

[maxcountryman]

It seems that the performance would go down like crazy.

Have you benchmarked this? The UUID column type itself comes with some performance tradeoffs. That said, you don't need to use a text column, you could instead use a char type or even bytea type.

It's also important to point out that Django uses a fixed length CharField for their session table pk. So this is likely not a performance bottleneck in many real world scenarios.

I also saw that the new IDs reduce security from 128 bits to 66 bits in this process, barely above the 64-bit minimum recommended by OWASP.

Note that a UUIDv4 actually offers us 122-bits of entropy; 6 bits are fixed. We currently represent session IDs as base64-encoded i128s which are generated via rand; 128-bits of entropy.

It should be possible to make the ID length configurable. If that's important to you, I'm happy to review PRs that would enable that. Alternatively we could introduce signing via a feature flag (this is likely a larger change and it introduces significant overhead to the request).

If you have other ideas, please feel free to suggest potential ways of improving this.

[maxcountryman]

One more thing to add here, the fact we represent the ID as an i128 is helpful to folks that want to retain the use of UUIDs internally with their stores as we can then convert the ID to UUID.

For example, something like this (I haven't checked this, but to give you an idea):

fn i128_to_uuid(i: i128) -> Uuid {
    let bytes = i.to_be_bytes();
    let mut uuid_bytes = [0u8; 16];
    uuid_bytes[..16].copy_from_slice(&bytes[..16]);

    // Set the UUID version to 4
    uuid_bytes[6] = (uuid_bytes[6] & 0x0F) | 0x40;

    // Set the UUID variant to RFC4122
    uuid_bytes[8] = (uuid_bytes[8] & 0x3F) | 0x80;

    Uuid::from_bytes(uuid_bytes)
}

Or it looks like Uuid itself provides a from_u128 method. (As an aside, it seems to me tower-sessions would be better off using an u128.)

Edit: You would not set version or variant, as I did in the example above: this would lead to a lossy conversion of course! Instead, the entire UUID must be built from the i128. (Using Uuid::from_u128 seems like a preferable route.) My understanding is Postgres doesn't check UUID formatting, so I think this would work in practice.

[maxcountryman]

Given the fact that the i128 ID can be converted to a UUID I think this change is somewhat backwards compatible.

@Razican Does that work for you?

[Razican]

This works for me :) Thank you for your help!
Using u128 would probably simplify it a bit, as well

[bekriebel]

I'm running into this as well as I already have a (Firestore) database with existing sessions that I don't want to all suddenly get expired by upgrading.

For the backwards compatibility, are you suggesting that the logic for that should be in the store rather than the use of this chart? It seems it would have to be since the trait is sending Id.

Since Firestore doesn't support i128 or UUID values, they need to be stored as strings regardless. I have the logic done for the FirestoreStore to use the string form of Id, but I suppose I could flip it out to use the UUID conversion anyway. I'm just not sure if it makes sense for the store authors to be the ones making this call instead of leaving that up to the user through a setting at the tower-sessions level.

[maxcountryman]

For the backwards compatibility, are you suggesting that the logic for that should be in the store rather than the use of this chart? It seems it would have to be since the trait is sending Id.

I think existing sessions could either be migrated to their i128 (should we have made this u128?) values or the session store could indefinitely translate from i128 into UUID. (I suspect the former is preferable over the latter.)

Perhaps we could make this migration easier by providing methods on Id to assist with this?

[bekriebel]

Yeah, or even impl From<Uuid> for Id { and impl From<Id> for Uuid { might work. That's what I'm working on writing for FirestoreStore right now, actually.

However, unless the value is being sent to the store implementations as a string, I'm not sure we can get around there being logic in the stores with either method.

[maxcountryman]

I'm a little perplexed about what to do with the ID type as it stands: if we ever want to support cookie stores (and I think there's good reasons to not, but nevertheless) it's not really possible with i128. That said, some stores will benefit from it being a concrete type that can e.g. index well.

Anyway, I'm happy to make changes that would help reduce the migration burden here.

[bekriebel]

I could see that being helpful for some stores. In the case of Firestore, it seems like my best option is to coerce it into a string regardless, based on the size of the field and the fact that they index string fields by default anyway.

As for migration... Ideally, this would be agnostic of the session type or allow the developer to choose the type of the session. Since the app I'm using this for stores the session value outside of the browser as well, migrating to a new type is quite a bit more complicated. I'd rather either lock it in to UUID, or allow UUID for older tokens but begin generating the newer style tokens. I'm not sure what the best approach for either option is.