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

Rebuild actix-identity on top of actix-session #246

Merged
merged 63 commits into from
Jul 9, 2022

Conversation

LukeMathWalker
Copy link
Contributor

@LukeMathWalker LukeMathWalker commented May 18, 2022

This is a rewrite of actix-identity.

Why?

actix-identity and actix-session have historically created confusion in developers that are approaching actix-web's ecosystem for the first time: which one should I use if I want to add session-based authentication to my application?
actix-session offered a more general-purpose API (lower-level, if you want), while actix-identity had some pretty convenient extractors. Starting with actix-identity was usually easier (a more opinionated API, geared for auth), but if you outgrew it (e.g. you now want to store more than identity into the session) you'd have to rewrite everything from scratch to use actix-session, including the auth part.

From a maintenance perspective, actix-identity and actix-session shared no code.
Any change/fix/improvement to the logic related to the management of the session cookie, additional storage backend, etc. required duplication of effort. In reality, they often diverged or features were available in one of the two but not the other (e.g. actix-identity never supported remote storage backends).

By rebuilding actix-identity on top of actix-session we can solve both problems at once: we get, out of the box, lower maintenance overhead, feature parity and a clear onboarding path for developers who want to work with sessions (for auth or otherwise).

How?

I ported over all existing features of actix-identity: extractor, login deadline, visit deadline.
This PR is built on top of the branch for #233, therefore #233 must be merged first into trunk.
This PR introduces a few more breaking changes to actix-session, on top of the ones already in #233:

  • We return a custom error type for Session::get and Session::insert. This will make it easier to evolve the API in the future and gives us a way to provide a more helpful error message (i.e. it includes details on the key you were trying to access/insert).

LukeMathWalker and others added 30 commits March 21, 2022 08:25
…henever it fires a request to the server.

We also make sure not to extend the session TTL if `refresh_ttl_when_active` is set to false. Previously, the TTL would be refreshed when the session state changed.
…g changes in the future.

Each variant get its own type whose fields are not exposed. We allow configuration via the builder pattern which empowers us to add more configuration knobs in the future in a non-breaking fashion.
…odule to highlight the actually relevant types on the homepage of the documentation for the crate.
@LukeMathWalker LukeMathWalker marked this pull request as ready for review May 18, 2022 08:09
@LukeMathWalker LukeMathWalker added A-identity Project: actix-identity A-session Project: actix-session B-semver-major breaking change requiring a major version bump labels May 18, 2022
@LukeMathWalker
Copy link
Contributor Author

This has a few breaking changes in actix-session as well @robjtede - it should go in before cutting a new release.

@LukeMathWalker
Copy link
Contributor Author

Resolved all conflicts ✅

@robjtede robjtede merged commit b1cea64 into actix:master Jul 9, 2022
@robjtede
Copy link
Member

robjtede commented Jul 9, 2022

@LukeMathWalker good to release -session and -identity ?

@LukeMathWalker
Copy link
Contributor Author

Good to go for me!

@robjtede
Copy link
Member

robjtede commented Jul 10, 2022

@LukeMathWalker released -session v0.7.0 (🎉) but realized that -identity didn't get anything added to the changelog. Do you mind writing something up and I'll get -identity out, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-identity Project: actix-identity A-session Project: actix-session B-semver-major breaking change requiring a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants