feat: Add support for email address management #1565

Merged
ginger merged 26 commits from ginger/email-support into main 2026-03-31 02:21:00 +00:00
Owner

This pull request adds support for:

  • Requiring users to provide an email address when registering
  • Allowing users to sign in with their email, if they have one associated
  • Allowing users to reset their passwords via email, if they have one associated
  • Allowing users to manage their associated email address through their client

Pull request checklist:

  • This pull request targets the main branch, and the branch is named something other than
    main.
  • I have written an appropriate pull request title and my description is clear.
  • I understand I am responsible for the contents of this pull request.
  • I have followed the contributing guidelines:
<!-- In order to help reviewers know what your pull request does at a glance, you should ensure that 1. Your PR title is a short, single sentence describing what you changed 2. You have described in more detail what you have changed, why you have changed it, what the intended effect is, and why you think this will be beneficial to the project. If you have made any potentially strange/questionable design choices, but didn't feel they'd benefit from code comments, please don't mention them here - after opening your pull request, go to "files changed", and click on the "+" symbol in the line number gutter, and attach comments to the lines that you think would benefit from some clarification. --> This pull request adds support for: - Requiring users to provide an email address when registering - Allowing users to sign in with their email, if they have one associated - Allowing users to reset their passwords via email, if they have one associated - Allowing users to manage their associated email address through their client <!-- Example: This pull request allows us to warp through time and space ten times faster than before by double-inverting the warp drive with hyperheated jump fluid, both making the drive faster and more efficient. This resolves the common issue where we have to wait more than 10 milliseconds to engage, use, and disengage the warp drive when travelling between galaxies. --> <!-- Closes: #... --> <!-- Fixes: #... --> <!-- Uncomment the above line(s) if your pull request fixes an issue or closes another pull request by superseding it. Replace `#...` with the issue/pr number, such as `#123`. --> **Pull request checklist:** <!-- You need to complete these before your PR can be considered. If you aren't sure about some, feel free to ask for clarification in #dev:continuwuity.org. --> - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. - [x] I have written a [news fragment][n1] for this PR, if applicable<!--(can be done after hitting open!)-->. <!-- Notes on these requirements: - While not required, we encourage you to sign your commits with GPG or SSH to attest the authenticity of your changes. - While we allow LLM-assisted contributions, we do not appreciate contributions that are low quality, which is typical of machine-generated contributions that have not had a lot of love and care from a human. Please do not open a PR if all you have done is asked ChatGPT to tidy up the codebase with a +-100,000 diff. - In the case of code style violations, reviewers may leave review comments/change requests indicating what the ideal change would look like. For example, a reviewer may suggest you lower a log level, or use `match` instead of `if/else` etc. - In the case of code style violations, pre-commit check failures, minor things like typos/spelling errors, and in some cases commit format violations, reviewers may modify your branch directly, typically by making changes and adding a commit. Particularly in the latter case, a reviewer may rebase your commits to squash "spammy" ones (like "fix", "fix", "actually fix"), and reword commit messages that don't satisfy the format. - Pull requests MUST pass the `Checks` CI workflows to be capable of being merged. This can only be bypassed in exceptional circumstances. If your CI flakes, let us know in matrix:r/dev:continuwuity.org. - Pull requests have to be based on the latest `main` commit before being merged. If the main branch changes while you're making your changes, you should make sure you rebase on main before opening a PR. Your branch will be rebased on main before it is merged if it has fallen behind. - We typically only do fast-forward merges, so your entire commit log will be included. Once in main, it's difficult to get out cleanly, so put on your best dress, smile for the cameras! --> [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages [n1]: https://towncrier.readthedocs.io/en/stable/tutorial.html#creating-news-fragments
fix: Remove associated email on account deactivation
Some checks failed
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m19s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 2m51s
Update flake hashes / update-flake-hashes (pull_request) Successful in 52s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 25m48s
e8a06f51b7
chore: Fix typo
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m19s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m1s
Update flake hashes / update-flake-hashes (pull_request) Successful in 56s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 18m31s
7ff448b325
ginger added this to the (deleted) milestone 2026-03-23 15:02:55 +00:00
chore: Update news fragment
All checks were successful
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m27s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m53s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 11m16s
Update flake hashes / update-flake-hashes (pull_request) Successful in 48s
7b70cdba75
@ -0,0 +1,5 @@
{% extends "_base.txt" %}
Owner

Some inconsistency with the use of .j2 jinja extension between here and the web crate

Some inconsistency with the use of .j2 jinja extension between here and the web crate
Author
Owner

The j2 extension makes askama think the templates are HTML and escape them accordingly, which leads to ampersands in URL queries being replaced with &#38;.

The j2 extension makes askama think the templates are HTML and escape them accordingly, which leads to ampersands in URL queries being replaced with `&#38;`.
ginger marked this conversation as resolved
Jade requested review from Jade 2026-03-24 16:21:47 +00:00
nex requested changes 2026-03-24 19:15:13 +00:00
Dismissed
nex left a comment
Owner

just a few preliminary things, didn't get chance to review the UIAA service changes

just a few preliminary things, didn't get chance to review the UIAA service changes
@ -0,0 +51,4 @@
InsecureClientIp(client): InsecureClientIp,
body: Ruma<get_username_availability::v3::Request>,
) -> Result<get_username_availability::v3::Response> {
// workaround for https://github.com/matrix-org/matrix-appservice-irc/issues/1780 due to inactivity of fixing the issue
Owner

If we're refactoring we might as well just remove this workaround and the associated code below it. Just seems like a liability for no good reason.

If we're refactoring we might as well just remove this workaround and the associated code below it. Just seems like a liability for no good reason.
Owner

AAAA bad, someone will be using this. I know MetaBrainz have appservice-irc deployed

AAAA bad, someone will be using this. I know MetaBrainz have appservice-irc deployed
Owner
https://xkcd.com/1172/
Owner

someone will be using this.

i dont care 👍 upstreams should fix their bugs, we shouldn't be fixing them for them, especially when it's in a security hot zone like user registration

> someone will be using this. i dont care 👍 upstreams should fix their bugs, we shouldn't be fixing them for them, especially when it's in a security hot zone like user registration
Owner

I guess people should stop using abandoned software. Hard ask tho 😔

I guess people should stop using abandoned software. Hard ask tho 😔
Owner

getting people to not use abandoned software is literally what created this project 🧌

getting people to not use abandoned software is literally what created this project 🧌
ginger marked this conversation as resolved
@ -0,0 +185,4 @@
services
.threepid
.associate_localpart_email(user_id.localpart(), &email)
.await?;
Owner

This probably needs an explicit rollback if the services.users.create function fails below

This probably needs an explicit rollback if the `services.users.create` function fails below
ginger marked this conversation as resolved
@ -0,0 +499,4 @@
}
// Workaround for https://github.com/matrix-org/matrix-appservice-irc/issues/1780 due to inactivity of fixing the issue
let is_matrix_appservice_irc = appservice_info.is_some_and(|appservice| {
Owner

should also remove this (see earlier comment)

should also remove this (see earlier comment)
ginger marked this conversation as resolved
@ -0,0 +550,4 @@
Ok(user_id)
} else {
// The user is a guest or is lacking in creativity. Generate a username for
Owner

what does "creativity" mean??

what does "creativity" mean??
ginger marked this conversation as resolved
@ -0,0 +555,4 @@
loop {
let user_id = UserId::parse_with_server_name(
utils::random_string(RANDOM_USER_ID_LENGTH).to_lowercase(),
Owner

I'm gonna need a justification for potentially burdening someone with an immutable random string for their user ID. I hate that my spotify user ID is 32 random chars and can't be changed so im sure someone ending up here would hate this too

I'm gonna need a justification for potentially burdening someone with an immutable random string for their user ID. I hate that my spotify user ID is 32 random chars and can't be changed so im sure someone ending up here would hate this too
Author
Owner

From https://spec.matrix.org/v1.17/client-server-api/#post_matrixclientv3register regarding the username parameter:

The basis for the localpart of the desired Matrix ID. If omitted, the homeserver MUST generate a Matrix ID local part.

From https://spec.matrix.org/v1.17/client-server-api/#post_matrixclientv3register regarding the `username` parameter: > The basis for the localpart of the desired Matrix ID. If omitted, the homeserver MUST generate a Matrix ID local part.
Owner

I suspect the meaning of this was to generate the localpart based on the email provided, not random. iirc Synapse does something like this (which is why you see user IDs like @johnathan=46sunderland98=40gmail=46com:matrix.example), in theory we could just pull the email's identifier anyway.

I'm not sure there's another situation where localpart is ever empty. I guess if no identifiable 3pid is included and the user tries to register an empty username then they can be saddled with a random string for doing something dumb

I suspect the meaning of this was to generate the localpart based on the email provided, not *random*. iirc Synapse does something like this (which is why you see user IDs like `@johnathan=46sunderland98=40gmail=46com:matrix.example`), in theory we could just pull the email's identifier anyway. I'm not sure there's another situation where `localpart` is ever empty. I guess if no identifiable 3pid is included and the user tries to register an empty username then they can be saddled with a random string for doing something dumb
ginger marked this conversation as resolved
@ -415,13 +415,6 @@ impl<'a, 'de: 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
tracing::instrument(level = "trace", skip_all, fields(?self.buf))
)]
fn deserialize_any<V: Visitor<'de>>(self, visitor: V) -> Result<V::Value> {
debug_assert_eq!(
Owner

?

?
Author
Owner

This was causing panics when trying to register an account in a debug build because one type name had a lifetime parameter and the other didn't. I'm not sure what it was testing, but if it was panicking in code I didn't even touch that was working fine beforehand, I doubt that it was doing its job correctly.

This was causing panics when trying to register an account in a debug build because one type name had a lifetime parameter and the other didn't. I'm not sure what it was testing, but if it was panicking in code I didn't even touch that was working fine beforehand, I doubt that it was doing its job correctly.
nex marked this conversation as resolved
fix: Don't bail out on email association failures when registering a new user
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m25s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m5s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m50s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 20m37s
666adb705c
ginger requested review from nex 2026-03-26 16:51:57 +00:00
feat: Fall back to email when registering a user who didn't provide a username
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m19s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m58s
Update flake hashes / update-flake-hashes (pull_request) Successful in 2m7s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 20m17s
5bdaf478c4
refactor: Remove UiaaStatus enum
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m20s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m0s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m54s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 20m33s
9a0dd36b8d
Owner

Do we have any wal of removing expored tokens eventually? Probably an important TODO to write down at least

Do we have any wal of removing expored tokens eventually? Probably an important TODO to write down at least
Owner

oh kay then, in memory is one solution to this

oh kay then, in memory is *one* solution to this
@ -0,0 +64,4 @@
send_attempt: usize,
) -> Result<OwnedSessionId> {
let mailer = self.services.mailer.expect_mailer()?;
let mut sessions = self.sessions.lock().await;
Owner

This will hold the lock over the whole scope, including while sending the email

This will hold the lock over the whole scope, including *while sending the email*
ginger marked this conversation as resolved
@ -0,0 +141,4 @@
session_id: &SessionId,
supplied_token: &str,
) -> Result<(), Cow<'static, str>> {
let mut sessions = self.sessions.lock().await;
Owner

ditto, although less of an issue here

ditto, although less of an issue here
Author
Owner

The point of the function is to possibly mutate the session in question, and we can't do that without holding the lock.

The point of the function is to possibly mutate the session in question, and we can't do that without holding the lock.
Jade approved these changes 2026-03-27 15:42:47 +00:00
Dismissed
Jade left a comment
Owner

The only thing I really have concerns with is the in-memory session store, but that's fine for now. Just fix the lock held across the mailer I think?

The only thing I really have concerns with is the in-memory session store, but that's fine for now. Just fix the lock held across the mailer I think?
fix: Release session lock before sending threepid validation email
Some checks failed
Update flake hashes / update-flake-hashes (pull_request) Waiting to run
Check Changelog / Check for changelog (pull_request_target) Successful in 10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m30s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
9d651bed92
Author
Owner

Fixed.

Fixed.
ginger force-pushed ginger/email-support from 9d651bed92
Some checks failed
Update flake hashes / update-flake-hashes (pull_request) Waiting to run
Check Changelog / Check for changelog (pull_request_target) Successful in 10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m30s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
Checks / Prek / Pre-commit & Formatting (pull_request) Has been cancelled
to 3564c4ea4a
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m16s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m4s
Update flake hashes / update-flake-hashes (pull_request) Successful in 2m2s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 20m3s
2026-03-27 15:45:30 +00:00
Compare
ginger force-pushed ginger/email-support from 3564c4ea4a
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m16s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m4s
Update flake hashes / update-flake-hashes (pull_request) Successful in 2m2s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 20m3s
to 854901d79a
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m16s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m8s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m51s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 35m4s
2026-03-27 16:22:07 +00:00
Compare
Jade approved these changes 2026-03-27 16:48:01 +00:00
nex approved these changes 2026-03-29 21:42:54 +00:00
Dismissed
nex left a comment
Owner

woof

woof
nex dismissed nex's review 2026-03-29 21:47:03 +00:00
Reason:

ginger wants me to break stuff before approving

feat: Supply more informative error message if email is disabled
Some checks failed
Check Changelog / Check for changelog (pull_request_target) Successful in 10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m19s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m0s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m51s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
a7bdcc9ab9
chore: Update admin command docs
Some checks failed
Check Changelog / Check for changelog (pull_request_target) Successful in 21s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m22s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m56s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m53s
Checks / Prek / Clippy and Cargo Tests (pull_request) Has been cancelled
f6ce156acc
feat: Add a notice about email to the first-run banner
Some checks failed
Check Changelog / Check for changelog (pull_request_target) Successful in 9s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m13s
Checks / Prek / Pre-commit & Formatting (pull_request) Failing after 2m51s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m47s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 25m20s
a7f51d460e
fix: Don't allow UIAA stages to be completed if no flow includes them
All checks were successful
Check Changelog / Check for changelog (pull_request_target) Successful in 10s
Documentation / Build and Deploy Documentation (pull_request) Successful in 1m16s
Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m2s
Update flake hashes / update-flake-hashes (pull_request) Successful in 1m48s
Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 26m15s
0177810e2d
nex approved these changes 2026-03-30 17:18:32 +00:00
ginger scheduled this pull request to auto merge when all checks succeed 2026-03-30 17:19:55 +00:00
ginger merged commit e1c54f4dec into main 2026-03-31 02:21:00 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
continuwuation/continuwuity!1565
No description provided.