| Commit message (Collapse) | Author | Age |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`crate::app::App`'s internals.
In the course of working on web push, I determined that we probably need to make `App` generic over the web push client we're using, so that tests can use a dummy client while the real app uses a client created at startup and maintained over the life of the program's execution. The most direct implementation of that is to render App as `App<P>`, where the parameter is occupied by the specific web push client type in use. However, doing this requires refactoring at _every_ site that mentions `App`, including every handler, even though the vast majority of those sites will not be concerned with web push.
I reviewed a few options with @wlonk:
* Accept the type parameter and apply it everywhere, as the cost of supporting web push.
* Hard-code the use of a specific web push client.
* Insulate handlers &c from `App` via provider traits, mimicing what we do for repository provider traits today.
* Treat each app type as a freestanding state in its own right, so that only push-related components need to consider push clients (as far as is feasible).
This is a prototype towards that last point, using a simple app component (boot) as a testbed. `FromRef` allows handlers that take a `Boot` to be used in routes that provide an `App`, so this is a contained change. However, the structure of `FromRef` prevents `Boot` from carrying any lifetime narrower than `'static`, so it now holds clones of the state fields it acquires from App, instead of references. This is fine - that's just a database pool, and sqlx's pool type is designed to be shared via cloning. From <https://docs.rs/sqlx/latest/sqlx/struct.Pool.html>:
> Cloning Pool is cheap as it is simply a reference-counted handle to the inner pool state.
|
| |
|
|
| |
We've made stylistic changes that follow from Rust 1.86 through 1.89 anyways, and I'm not at all confident we aren't using APIs that only exist in those versions.
|
| | |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Conversations, users, messages, and all other "synchronized" entities now have an in-memory implementation of their lifecycle, rather than a database-backed one. These operations take a history, apply one lifecycle change to that history, and emit a new history. Storage is then implemented by applying the events in this new history to the database.
The storage methods in repo types, which process these events by emitting SQL statements, make necessary assumptions that the events being passed to them are coherent with the data already in storage. For example, the code to handle a conversation's delete event is allowed to assume that the database already contains a row for that conversation, inserted in response to a prior conversation creation event.
Data retrieval is not modified in this commit, and probably never will be without a more thorough storage rewrite. The whole intention of the data modelling approach I've been using is that a single row per entity represents its entire history, in turn so that the data in the database should be legible to people approaching it using normal SQL tools.
Developed as an aesthetic response to increasing unease with the lack of an ORM versus the boring-ness of our actual queries.
Merges event-based-storage into main.
|
| | |
| |
| |
| |
| |
| | |
Per <https://docs.rs/tokio/latest/tokio/sync/broadcast/struct.Sender.html>, a `Sender` is safe to share between threads. The clone behaviour we want is also provided by its `Clone` impl directly, and we don't need to wrap the sender in an `Arc` to share it.
It's amazing what you can find in the docs.
|
| | |
| |
| |
| |
| |
| | |
This conversion, from an iterator of type-specific events (say, `user::Event` or `message::Event`), into a `Vec<event::Event>`, is prevasive, and it needs to be done each time. Having Broadcaster expose a support method for this cuts down on the repetition, at the cost of a slightly alarming amount of type-system nonsense in `broadcast_from`.
Historical footnote: the internal message structure is a Vec and not an individual message so that bulk operations, like expiring channels and messages, won't disconnect everyone if they happen to dispatch more than sixteen messages (current queue depth limit) at once. We trade allocation and memory pressure for keeping the connections alive. _Most_ event publishing is an iterator of one item, so the Vec allocation is redundant.
|
| | | |
|
| | |
| |
| |
| | |
I found a test bug! The tests for deleting previously-deleted or previously-expired tests were using the wrong user to try to delete those messages. The tests happened to pass anyways because the message authorship check was done after the message lifecycle check. They would have no longer passed; the tests are fixed to use the sender, instead.
|
| | |
| |
| |
| | |
This replaces the approach of having the repo type know about conversation lifecycle in detail. Instead, the repo type accepts events and applies them to the DB blindly. The SQL written to implement each event does, however, embed assumptions about what order events will happen in.
|
| |/ |
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The taxonomy is now as follows:
* A _login_ is someone's identity for the purposes of authenticating to the service. Logins are not synchronized, and in fact are not published anywhere in the current API. They have a login ID, a name and a password.
* A _user_ is someone's identity for the purpose of participating in conversations. Users _are_ synchronized, as before. They have a user ID, a name, and a creation instant for the purposes of synchronization.
## API changes
* `GET /api/boot` method now returns a `login` key instead of a `user` key. The structure of the nested value is unchanged. This change is not backwards-compatible; the included client and the docs have been updated accordingly.
## Server implementation
* Most app methods that took a `&User` as an identity now take a `&Login` as an identity, instead. Where a `User` is needed, the new `tx.users().for_login(&login)` database access method resolves a `Login` to its corresponding `user::History`, which can then be turned into a `User` at whatever point in time is most appropriate.
This adds a few new error cases to methods that traverse the login-to-history-to-user chain. Those cases are presently unreachable, but I've fully fleshed them out so that they don't bite us later. Most of the resulting errors, however, are captured as internal server errors.
* There is a new `app.logins()` application entry point, dealing with login identities and password-based logins.
* `app.tokens()` is a bit more limited in scope to only things that work with an existing token.
That has the side effect of splitting up logging in (in `app.logins().with_password(…)`) and logging out (in `app.tokens().logout(…)`).
## Schema changes
The `user` table has been split:
* `login` holds the data needed for the user to log in - their login ID, their name, and their password.
* `user` now holds only the user ID and the event data for the user's `created` instant. Reconstructing a `User` struct requires joining in data from both `login` and `user`.
In theory, the relationship is one-way: every user has a login. In practice, it's reciprocal: every login has a user and every user has a login.
Relationships with downstream tables have been modified to suit:
* `message` still refers to `user` for authorship information.
* `invite` still refers to `user` for originator information.
* `token` refers to `login` for authentication information.
## Blimy, that's big
Yeah, I know. It's hard to avoid and I'm not sure the effort of making this in incremental steps is worth it.
Authentication logic has a way of getting into all sorts of corners, and Pilcrow is no different. In order for the new taxonomy to make sense, all of the places that previously used `User` as a representation of an authenticated identity have to be updated, and it's easier to do that all at once, so that we can retire all the code that _supports_ using a `User` that way.
Merges split-user into main.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The taxonomy is now as follows:
* A _login_ is someone's identity for the purposes of authenticating to the service. Logins are not synchronized, and in fact are not published anywhere in the current API. They have a login ID, a name and a password.
* A _user_ is someone's identity for the purpose of participating in conversations. Users _are_ synchronized, as before. They have a user ID, a name, and a creation instant for the purposes of synchronization.
In practice, a user exists for every login - in fact, users' names are stored in the login table and are joined in, rather than being stored redundantly in the user table. A login ID and its corresponding user ID are always equal, and the user and login ID types support conversion and comparison to facilitate their use in this context.
Tokens are now associated with logins, not users. The currently-acting identity is passed down into app types as a login, not a user, and then resolved to a user where appropriate within the app methods.
As a side effect, the `GET /api/boot` method now returns a `login` key instead of a `user` key. The structure of the nested value is unchanged.
|
| | |
| |
| |
| | |
This is the leading edge of a larger storage refactoring, where repo types stop doing things like generating secrets or deciding whether to carry out an operation. To make this work, there is now a `Token` type that holds the complete state of a token, in memory.
|
| | |
| |
| |
| | |
We'll be building separate entities around this in future commits, to better separate the authentication data (non-synchronized and indeed "not public") from the chat data (synchronized and public).
|
| | |
| |
| |
| | |
These checks tended to be wordy, and were prone to being done subtly differently in different locations for no good reason. Centralizing them cleans this up and makes the tests easier to follow, at the expense of making it somewhat harder to follow what the test is specifically checking.
|
| | |
| |
| |
| |
| |
| | |
identity token.
This is a small refactoring that's been possible for a while, and we only just noticed.
|
| |/ |
|
| |
|
|
|
|
| |
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.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This removes the response body from the following methods:
* `POST /api/setup`
* `POST /api/auth/login`
* `POST /api/invite/:id`
* `POST /api/password`
The bodies returned from these methods were something of a rough guess as to what might be useful. Actual client development has shown that we don't use _any_ of the data from any of these API responses, so let's not tie ourselves to future compatibility by continuing to send them. We can add a body to a bodyless method a _lot_ more easily than we can change the body of a method that already returns one, after all.
These changes are not backwards compatible for clients which care about the existing bodies. To my knowledge, there are no such clients; the included client definitely doesn't care.
## Internals
Not only does this change stop returning bodies at the API surface, but it also stops retrieving and returning values used internally to construct those responses, simplifying the code a bit in the process.
One side effect of this is that tests that need to log in a user now need to manually verify the returned token secret, to convert it back into a user, whereas the previous versions returned both a token secret and a user during password login. I don't love the increase in the size of the tests, but I think it's the right tradeoff (and this change is code net-negative anyways).
Merges no-content into main.
|
| | |
| |
| |
| |
| |
| | |
the current token is invalid.
It's inconsistent with the behaviour when the current token is unset. Shrug.
|
| | | |
|
| | | |
|
| | |
| |
| |
| | |
As with the previous commits, the body was never actually being used.
|
| | |
| |
| |
| | |
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.
|
| |/
|
|
| |
This is a bit tidier and easier to assert on than returning a bare HTTP status code, but is otherwise interchangeable with it.
|
| |
|
|
| |
These were separated as there wasn't an obvious way to serialize two fields with the same _type_ with different _prefixes_. Turns out this is a common problem, and someone's written a crate for it that remaps the names for you.
|
| |
|
|
| |
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.
|
| |
|
|
| |
There's already an implicit conversion (via serialization), it's just awkward to use. However, we now need those conversions more directly.
|
| |
|
|
|
|
| |
later on) when checking validity of Rust code.
I inadvertantly broke a test and my pre-commit hook, which runs `tools/check-lint`, didn't catch it.
|
| |
|
|
| |
This is a recurring and nameable operation; let's give it a name before we use it further.
|
| |\ |
|
| |/
|
|
| |
This simplifies data flow, at the potential expense of re-rendering HTML more often than strictly necessary. Requiring every path that produces a message-shaped object to pre-render markdown made things more interdependent than intended and slowed me down.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
| |
In 792de8e49fa8a3c04bfb747adadf71572d753055, `crate::cli::Error` was made private. I forgot to update the doctest that mentions it.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is based heavily on the work done for normalized strings, in `crate::normalize`. The key realization in that module is that the logic distinguishing one kind of thing (normalized strings in that case, IDs, in this case) can be packaged up as a type token, and that doing so may reduce the overall complexity. This implementation for ID also borrows heavily from the implementation for normalized strings.
It's less flexible: an ID implemented this way can't expose _less_ of `crate::id::ID`'s interface, whereas newtype wrappers can, for example. However, our code doesn't use that flexiblity on purpose anywhere and we're relatively unlikely to change that. In return, the individual ID types require substantially less code - they do not, for example, need to re-implement `Display` for themselves.
I very nearly made the trait `Prefix`:
```rust
pub trait Prefix {
const PREFIX: &str;
}
```
however, I think having an effectively-constant method is less surprising overall.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Pilcrow isn't meant to be used as a library, and the only public interface the `pilcrow` lib crate exposes is the CLI entry point. However, we will likely be publishing Pilcrow via crates.io (among other options) one day, and so it will _be usable_ as a library if someone's desperate enough to try. To that end, let's try to be good citizens.
This change fixes two issues:
* The docs contained links to internal items, which are not actually included in the library documentation. The links are simply removed; the uses of those items were already private anyways.
* The CLI `Error` type is no longer part of the public interface, using `impl Trait` (`impl std::error::Error`) shenanigans to hide the error type from callers. (To be clear, this would be _extremely_ rude in code intended for library use.) This frees us up to change the structure of the error type - or to replace it entirely - without making the world's most pedantic semver change in the process.
Merges lib-crate-weirdness into main.
|
| | |
| |
| |
| | |
This might be the pettiest rude change I've ever made to a Rust program. If I saw this - or did this - in code _intend_ to be used as a library, I'd be appalled.
|
| |/
|
|
| |
The Pilcrow crate library docs are something of a wart; Pilcrow isn't meant to be used as a library, and the only public interface it exposes is the CLI entry point. However, we will likely be publishing Pilcrow via crates.io (among other options), and so it will _be usable_ as a library if you're desperate enough to try. The docs should at least be coherent.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The new `--umask` option takes one of three values:
* `--umask masked`, the default, takes the inherited umask and forces o+rwx on.
* `--umask inherit` takes the inherited umask as-is.
* `--umask OCTAL` sets the umask to exactly `OCTAL` and is broadly equivalent to `umask OCTAL && pilcrow --umask inherit`.
This fell out of a conversation with @wlonk, who is working on notifications. Since notifications may require [VAPID] keys, the server will need a way to store those keys. That would generally be "in the pilcrow database," which lead me to the observation that Pilcrow creates that database as world-readable by default. "World-readable" and "encryption/signing keys" are not things that belong in the same sentence.
[VAPID]: https://datatracker.ietf.org/doc/html/rfc8292
The most "obvious" solution would be to set the permissions used for the sqlite database when it's created. That's harder than it sounds: sqlite has no built-in facility for doing this. The closest thing that exists today is the [`modeof`] query parameter, which copies the permissions (and ownership) from some other file. We also can't reliably set the permissions ourselves, as sqlite may - depending on build options and configuration - [create multiple files][wal].
[`modeof`]: https://www.sqlite.org/uri.html
[wal]: https://www.sqlite.org/wal.html
Using `umask` is a whole-process solution to this. As Pilcrow doesn't attempt to create other files, there's little issue with doing it this way, but this is a design risk for future work if it creates files that are _intended_ to be readable by more than just the Pilcrow daemon user.
Merges options-umask into main.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The new `--umask` option takes one of three values:
* `--umask masked`, the default, takes the inherited umask and forces o+rwx on.
* `--umask inherit` takes the inherited umask as-is.
* `--umask OCTAL` sets the umask to exactly `OCTAL` and is broadly equivalent to `umask OCTAL && pilcrow --umask inherit`.
This fell out of a conversation with @wlonk, who is working on notifications. Since notifications may require [VAPID] keys, the server will need a way to store those keys. That would generally be "in the pilcrow database," which lead me to the observation that Pilcrow creates that database as world-readable by default. "World-readable" and "encryption/signing keys" are not things that belong in the same sentence.
[VAPID]: https://datatracker.ietf.org/doc/html/rfc8292
The most "obvious" solution would be to set the permissions used for the sqlite database when it's created. That's harder than it sounds: sqlite has no built-in facility for doing this. The closest thing that exists today is the [`modeof`] query parameter, which copies the permissions (and ownership) from some other file. We also can't reliably set the permissions ourselves, as sqlite may - depending on build options and configuration - [create multiple files][wal].
[`modeof`]: https://www.sqlite.org/uri.html
[wal]: https://www.sqlite.org/wal.html
Using `umask` is a whole-process solution to this. As Pilcrow doesn't attempt to create other files, there's little issue with doing it this way, but this is a design risk for future work if it creates files that are _intended_ to be readable by more than just the Pilcrow daemon user.
|
| |/
|
|
| |
This was a leftover from the idea that different swatches might have different input notations - and they do, but we turned out not to need to style them differently. And, in any event, this class was applied (only) to inputs that _aren't HTML_, because of 01ed82ac4f89810161fbc3aa1cb8e4691fb8938b.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To reproduce:
* Make any change (even just `touch`) to a file in `ui`.
* Run `tools/run`.
There's a decent chance that the script will fail, with esoteric and variable errors:
```
failed to load config from .../pilcrow/vite.config.js_api(buil...
error when starting dev server:
Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@sveltejs/kit' imported from .../pilcrow/vite.config.js.timestamp-1752608239248-52b6b8965ad28.mjs
```
is one such exmple.
These errors are due to the `npm ci` carried out by `cargo` at build time (see `build.rs`), which deletes and re-creates `node_modules`. Vite and Svelte do not like having `node_modules` deleted out from under them. As `cargo` will rerun `build.rs` any time `ui` changes, this means that `tools/run` effectively requires a complete build _first_, before it can be run.
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
A swatch is a live, and ideally editable, example of an element of the service. They serve as:
* Documentation: what is this element, how do you use it, what does it do?
* Demonstration: what does this element look like?
* Manual test scaffolding: when I change this element like _so_, what happens?
Swatches are collectively available under `/.swatch/` on a running instance. They do not require setup or login for simplicity's sake and because they don't _do_ anything that requires either of those things.
Swatches are manually curated. First, we lack the technical infrastructure needed to do this based on static analysis, and second, manual curation lets us include affordances like "interesting values," that would be tricky to express as part of the type or schema for the component. The tradeoff, however, is that they will fall out of step with the components if not reviewed regularly.
Swatches are _possible_ because we've gone to efforts to avoid global data access or direct side effects (including API requests) in our components, delegating that upwards to `+page`s and `+layout`s. However, the isolation is imperfect. For example, the swatch for `Conversation`, which renders the conversation sidebar entries, causes actual attempts to boot the app as browsers pre-fetch the links on mouseover, and clicking them will take the user to the "real" application because they really are links.
Merges swatch into main.
|
| | |
| |
| |
| |
| |
| |
| |
| | |
You can inject Javascript into a swatch that uses `{@html <expr>}` fairly easily. `<script>foo()</script>` doesn't appear to work, but `<img src="x" onerror="foo()">` does, for example. That code then runs with the same access to cookies, and the same access to local data, as the Pilcrow client. This change removes that capability, by replacing the two swatches that exposed it with more limited examples.
I love the generality and flexibility of generic HTML entry here, and I think it might have been useful for swatching components that are generic DOM containers (which both `Message` and `MessageRun` are today), but swatches are a user interface and are exposed to _all_ users. A user who is unfamiliar with HTML and Javascript, but who is persuaded to open a swatch and enter some code into it (think about an attacker who tells their victim "hey check out this funny thing that happens," preying on curiousity, while providing a lightly-obfuscated payload) can then impersonate that user, exfiltrate anything saved locally, or potentially install persistent code using JS' various background-processing APIs. Gnarly stuff.
We're not up to mitigating that in place. Anyone who knows JS can likely learn to build the client from source, and can experiment with arbitrary input that way, taking responsibility for the results in the process, while anyone who doesn't is unlikely to be persuaded to set up an entire Node toolchain just for an exploit.
|
| | |\ \ \ \ \ \ \ \ \ \ |
|
| | | | | | | | | | | | | |
|
| | | | | | | | | | | |/ |
|
| | | | | | | | | | |/ |
|
| | | | | | | | | |/ |
|
