| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
| |
I've been doing this by hand anyways, and this makes it a _ton_ less tedious to maintain. I think it looks nice.
This does, however, require nightly - for formatting only.
|
| | |
|
| |
|
|
| |
As with `/api/setup`, the response was an ad-hoc choice, which we are not using and which constrains future development just by existing.
|
| |
|
|
| |
This API response was always ad-hoc, and the client doesn't use it. To free up some maneuvering room for server refactorings, stop sending it. We can add a response in the future if there's a need.
|
| |
|
|
| |
Having this buried under `crate::user` makes it hard to split up the roles `user` fulfils right now. Moving it out to its own module makes it a bit tidier to reuse it in a separate, authentication-only way.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rust 1.89 added a new warning:
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/setup/repo.rs:4:14
|
4 | fn setup(&mut self) -> Setup;
| ^^^^^^^^^ ----- the same lifetime is hidden here
| |
| the lifetime is elided here
|
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
help: use `'_` for type paths
|
4 | fn setup(&mut self) -> Setup<'_>;
| ++++
I don't entirely agree with the style advice here, but lifetime elision style is an evolving area in Rust and I'd rather track the Rust team's recommendations than invent my own, so I've added all of them.
|
| |
|
|
|
|
| |
Each domain module that exposes handlers does so through a `handlers` child module, ideally as a top-level symbol that can be plugged directly into Axum's `MethodRouter`. Modules could make exceptions to this - kill the doctrinaire inside yourself, after all - but none of the API modules that actually exist need such exceptions, and consistency is useful.
The related details of request types, URL types, response types, errors, &c &c are then organized into modules under `handlers`, along with their respective tests.
|
| |
|
|
|
|
|
|
| |
HTTP routes are now defined in a single, unified module, pulling them out of the topical modules they were formerly part of.
This is intended to improve the navigability of the codebase. Previously, finding the handler corresponding to a specific endpoint required prior familiarity, though in practice you could usually guess from topic area. Now, all routes are defined in `crate::routes`.
Other than changing visibility, I've avoided making changes to the handlers at the ends of those routes.
|
| |
|
|
|
|
| |
For endpoints that are unavailable, that default behaviour no longer needs to be specified: `Required(app)` will do that for you. For endpoints that are redirects until setup is completed, `Require(app).with_fallback(…response…)` will do that.
To make this a bit harder to break by accident, the default unavailable response is now its own type.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The two middlewares were identical but for the specific `IntoResponse` impl used to generate the response when setup has not been completed. However, unifying them while still using `from_fn_with_state` lead to this horrorshow:
.route_layer(middleware::from_fn_with_state(
app.clone(),
|state, req, next| {
setup::middeware::setup_required(UNAVAILABLE, state, req, next)
}
))
It's a lot to read, and it surfaces the entire signature of a state-driven middleware `fn` into the call site solely to close over one argument (`UNAVAILABLE`).
Rather than doing that, I've converted this middleware into a full blown Tower middleware, following <https://docs.rs/axum/latest/axum/middleware/index.html#towerservice-and-pinboxdyn-future>. I considered taking this further and implementing a custom future to remove the allocation for `Box::pin`, but honestly, that allocation isn't hurting anyone and this code already got long enough in the translation.
The new API looks like:
.route_layer(setup::Required::or_unavailable(app.clone()))
Or like:
.route_layer(setup::Required::with_fallback(app.clone(), RESPONSE))
One thing I would have liked to have avoided is the additional `app.clone()` argument, but there isn't a way to extract the _state_ from a request inside of an Axum middleware. It has to be passed in externally - that's what `from_fn_with_state` is doing under the hood, as well. Using `State` as an extractor doesn't work; the `State` extractor is special in a _bunch_ of ways, and this is one of them. Other extractors would work. Realistically, I'd probably want to explore interfaces like
.route_layer(setup::Required(app).or_unavailable())
or
.route_layer(app.setup().required().or_unavailable())
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
| |
There are a couple of migration suggestions from `cargo fix --edition` that I have deliberately skipped, which are intended to make sure that the changes to `if let` scoping don't bite us. They don't, I'm pretty sure, and if I turn out to be wrong, I'd rather fix the scoping issues (as they arise) than use `match` (`cargo fix --edition`'s suggestion).
This change also includes a bulk reformat and a clippy cleanup.
NOTA BENE: As this requires a new Rust toolchain, you'll need to update Rust (`rustup update`, normally) or the server won't build. This also applies to the Debian builder Docker image; it'll need to be rebuilt (from scratch, pulling its base image again) as well.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Nasty design corner. Logins need to be created in three places:
1. In tests, using app.logins().create(…);
2. On initial setup, using app.setup().initial(…); and
3. When accepting invites, using app.invites().accept(…).
These three places do the same thing with respect to logins, but also do a varying mix of other things. Testing is the simplest and _only_ creates a login. Initial setup and invite acceptance both issue a token for the newly-created login. Accepting an invite also invalidates the invite. Previously, those three functions have been copy-pasted variations on a theme. Now that we have validation, the copy-paste approach is no longer tenable; it will become increasingly hard to ensure that the three functions (plus any future functions) remain in synch.
To accommodate the variations while consolidating login creation, I've added a typestate-based state machine, which is driven by method calls:
* A creation attempt begins with `let create = Create::begin()`. This always succeeds; it packages up arguments used in later steps, but does nothing else.
* A creation attempt can be validated using `let validated = create.validate()?`. This may fail. Input validation and password hashing are carried out at this stage, making it potentially expensive.
* A validated attempt can be stored in the DB, using `let stored = validated.store(&mut tx).await?`. This may fail. The login will be written to the DB; the caller is responsible for transaction demarcation, to allow other things to take place in the same transaction.
* A fully-stored attempt can be used to publish events, using `let login = stored.publish(self.events)`. This always succeeds, and unwraps the state machine to its final product (a `login::History`).
|
| |
|
|
|
|
|
|
| |
There's no good reason to use an empty string as your login name, or to use one so long as to annoy others. Names beginning or ending with whitespace, or containing runs of whitespace, are also a technical problem, so they're also prohibited.
This change does not implement [UTS #39], as I haven't yet fully understood how to do so.
[UTS #39]: https://www.unicode.org/reports/tr39/
|
| | |
|
| |
|
|
|
|
|
|
|
| |
* A `cookie::Identity` (`IdentityCookie`) is a specialized CookieJar for working with identities.
* An `Identity` is a token/login pair.
I hope for this to be a bit more legible.
In service of this, `Login` is no longer extractable. You have to get an identity.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Canonicalization does two things:
* It prevents duplicate names that differ only by case or only by normalization/encoding sequence; and
* It makes certain name-based comparisons "case-insensitive" (generalizing via Unicode's case-folding rules).
This change is complicated, as it means that every name now needs to be stored in two forms. Unfortunately, this is _very likely_ a breaking schema change. The migrations in this commit perform a best-effort attempt to canonicalize existing channel or login names, but it's likely any existing channels or logins with non-ASCII characters will not be canonicalize correctly. Since clients look at all channel names and all login names on boot, and since the code in this commit verifies canonicalization when reading from the database, this will effectively make the server un-usuable until any incorrectly-canonicalized values are either manually canonicalized, or removed
It might be possible to do better with [the `icu` sqlite3 extension][icu], but (a) I'm not convinced of that and (b) this commit is already huge; adding database extension support would make it far larger.
[icu]: https://sqlite.org/src/dir/ext/icu
For some references on why it's worth storing usernames this way, see <https://www.b-list.org/weblog/2018/nov/26/case/> and the refernced talk, as well as <https://www.b-list.org/weblog/2018/feb/11/usernames/>. Bennett's treatment of this issue is, to my eye, much more readable than the referenced Unicode technical reports, and I'm inclined to trust his opinion given that he maintains a widely-used, internet-facing user registration library for Django.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This normalizes the following values:
* login names
* passwords
* channel names
* message bodies, because why not
The goal here is to have a canonical representation of these values, so that, for example, the service does not inadvertently host two channels whose names are semantically identical but differ in the specifics of how diacritics are encoded, or two users whose names are identical.
Normalization is done on input from the wire, using Serde hooks, and when reading from the database. The `crate::nfc::String` type implements these normalizations (as well as normalizing whenever converted from a `std::string::String` generally).
This change does not cover:
* Trying to cope with passwords that were created as non-normalized strings, which are now non-verifiable as all the paths to verify passwords normalize the input.
* Trying to ensure that non-normalized data in the database compares reasonably to normalized data. Fortunately, we don't _do_ very many string comparisons (I think only login names), so this isn't a huge deal at this stage. Login names will probably have to Get Fixed later on, when we figure out how to handle case folding for login name verification.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In general:
* If the client can only assume the response is immediately valid (mostly, login creation, where the client cannot monitor the event stream), then 200 Okay, with data describing the server's view of the request.
* If the client can monitor for completion by watching the event stream, then 202 Accepted, with data describing the server's view of the request.
This comes on the heels of a comment I made on Discord:
> hrm
>
> creating a login: 204 No Content, no body
> sending a message: 202 Accepted, no body
> creating a channel: 200 Okay, has a body
>
> past me, what were you on
There wasn't any principled reason for this inconsistency; it happened as the endpoints were written at different times and with different states of mind.
|
| | |
|
| |
|
|
| |
I've also aligned channel creation with this (it's 409 Conflict). To make server setup more distinct, it now returns 503 Service Unavailable if setup has not been completed.
|
| |
|
