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

Rework how email verification works #3784

Merged
merged 28 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f84ccda
Fix the service worker not loading in dev mode
sandhose Jan 9, 2025
5d69b34
frontend: simplify email list
sandhose Jan 9, 2025
8db3642
Add more stories for the account index page
sandhose Jan 9, 2025
ee33e9c
Remove the primary email address concept
sandhose Jan 9, 2025
75526ff
storage: new email authentication codes
sandhose Jan 9, 2025
1f83b39
Remove the dedicated page to add an email address
sandhose Jan 9, 2025
0513f19
Rip out the email verification codes
sandhose Jan 9, 2025
5f5fc44
Job to send the new email authentication codes
sandhose Jan 10, 2025
23b019c
GraphQL API to use the new email authentication codes
sandhose Jan 10, 2025
e0f4882
Use the new GraphQL APIs in the frontend to add emails
sandhose Jan 10, 2025
dbb5316
Data model and storage layer for storing user registrations
sandhose Jan 13, 2025
3da27af
Move the registration-related views into a sub-module
sandhose Jan 14, 2025
a294b37
Fix the post auth action being lost during the registration flow
sandhose Jan 14, 2025
0bedaf3
Make the password registration create a user_registration
sandhose Jan 14, 2025
f8517a5
Implement email verification in the registration flow
sandhose Jan 14, 2025
621b648
Check that the email isn't used during the registration process
sandhose Jan 14, 2025
36aa1a0
Move the finishing of registration to a dedicated view
sandhose Jan 15, 2025
f50a386
Registration step to set a display name
sandhose Jan 15, 2025
5851584
Link the registration to the browser through a signed cookie
sandhose Jan 15, 2025
02db622
Expire registration sessions after an hour
sandhose Jan 15, 2025
1ec6192
Check with the homeserver the username is still available before regi…
sandhose Jan 15, 2025
ef74d47
Cleanup the unverified emails from the database
sandhose Jan 15, 2025
6092efe
Merge branch 'main' into quenting/optional-email
sandhose Jan 20, 2025
ef077d0
Rate-limit email authentications
sandhose Jan 23, 2025
7e6ab8f
Disclose that email is already in use after verification
sandhose Jan 23, 2025
244ec18
Merge remote-tracking branch 'origin/main' into quenting/optional-email
sandhose Jan 23, 2025
a83cdfb
Clarify that VerifyEmailJob is kept for flushing old jobs
sandhose Jan 23, 2025
8d50088
Apply code style suggestion
sandhose Jan 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/email/src/mailer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ impl Mailer {
fields(
email.to = %to,
email.language = %context.language(),
user.id = %context.user().id,
),
err,
)]
Expand Down
10 changes: 1 addition & 9 deletions crates/handlers/src/views/password_register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use mas_matrix::BoxHomeserverConnection;
use mas_policy::Policy;
use mas_router::UrlBuilder;
use mas_storage::{
queue::{ProvisionUserJob, QueueJobRepositoryExt as _, VerifyEmailJob},
queue::{ProvisionUserJob, QueueJobRepositoryExt as _},
user::{BrowserSessionRepository, UserEmailRepository, UserPasswordRepository, UserRepository},
BoxClock, BoxRepository, BoxRng, RepositoryAccess,
};
Expand Down Expand Up @@ -327,14 +327,6 @@ pub(crate) async fn post(
.authenticate_with_password(&mut rng, &clock, &session, &user_password)
.await?;

repo.queue_job()
.schedule_job(
&mut rng,
&clock,
VerifyEmailJob::new(&user_email).with_language(locale.to_string()),
)
.await?;

repo.queue_job()
.schedule_job(&mut rng, &clock, ProvisionUserJob::new(&user))
.await?;
Expand Down
48 changes: 30 additions & 18 deletions crates/storage/src/queue/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.

use mas_data_model::{Device, User, UserEmail, UserRecoverySession};
use mas_data_model::{Device, User, UserEmailAuthentication, UserRecoverySession};
use serde::{Deserialize, Serialize};
use ulid::Ulid;

Expand All @@ -17,37 +17,49 @@ pub struct VerifyEmailJob {
}

impl VerifyEmailJob {
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
/// Create a new job to verify an email address.
/// The ID of the email address to verify.
#[must_use]
pub fn new(user_email: &UserEmail) -> Self {
Self {
user_email_id: user_email.id,
language: None,
}
pub fn user_email_id(&self) -> Ulid {
self.user_email_id
}
}

/// Set the language to use for the email.
impl InsertableJob for VerifyEmailJob {
const QUEUE_NAME: &'static str = "verify-email";
}

/// A job to send an email authentication code to a user.
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct SendEmailAuthenticationCodeJob {
user_email_authentication_id: Ulid,
language: String,
}

impl SendEmailAuthenticationCodeJob {
/// Create a new job to send an email authentication code to a user.
#[must_use]
pub fn with_language(mut self, language: String) -> Self {
self.language = Some(language);
self
pub fn new(user_email_authentication: &UserEmailAuthentication, language: String) -> Self {
Self {
user_email_authentication_id: user_email_authentication.id,
language,
}
}

/// The language to use for the email.
#[must_use]
pub fn language(&self) -> Option<&str> {
self.language.as_deref()
pub fn language(&self) -> &str {
&self.language
}

/// The ID of the email address to verify.
/// The ID of the email authentication to send the code for.
#[must_use]
pub fn user_email_id(&self) -> Ulid {
self.user_email_id
pub fn user_email_authentication_id(&self) -> Ulid {
self.user_email_authentication_id
}
}

impl InsertableJob for VerifyEmailJob {
const QUEUE_NAME: &'static str = "verify-email";
impl InsertableJob for SendEmailAuthenticationCodeJob {
const QUEUE_NAME: &'static str = "send-email-authentication-code";
}

/// A job to provision the user on the homeserver.
Expand Down
91 changes: 90 additions & 1 deletion crates/tasks/src/email.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
// Please see LICENSE in the repository root for full details.

use async_trait::async_trait;
use mas_storage::queue::VerifyEmailJob;
use chrono::Duration;
use mas_email::{Address, EmailVerificationContext, Mailbox};
use mas_storage::queue::{SendEmailAuthenticationCodeJob, VerifyEmailJob};
use mas_templates::TemplateContext as _;
use rand::{distributions::Uniform, Rng};
use tracing::info;

use crate::{
new_queue::{JobContext, JobError, RunnableJob},
Expand All @@ -27,3 +32,87 @@ impl RunnableJob for VerifyEmailJob {
Err(JobError::fail(anyhow::anyhow!("Not implemented")))
}
}

#[async_trait]
impl RunnableJob for SendEmailAuthenticationCodeJob {
#[tracing::instrument(
name = "job.send_email_authentication_code",
fields(user_email_authentication.id = %self.user_email_authentication_id()),
skip_all,
err,
)]
async fn run(&self, state: &State, _context: JobContext) -> Result<(), JobError> {
let clock = state.clock();
let mailer = state.mailer();
let mut rng = state.rng();
let mut repo = state.repository().await.map_err(JobError::retry)?;

let user_email_authentication = repo
.user_email()
.lookup_authentication(self.user_email_authentication_id())
.await
.map_err(JobError::retry)?
.ok_or(JobError::fail(anyhow::anyhow!(
"User email authentication not found"
)))?;

if user_email_authentication.completed_at.is_some() {
return Err(JobError::fail(anyhow::anyhow!(
"User email authentication already completed"
)));
}

// Load the browser session, if any
let browser_session =
if let Some(browser_session) = user_email_authentication.user_session_id {
Some(
repo.browser_session()
.lookup(browser_session)
.await
.map_err(JobError::retry)?
.ok_or(JobError::fail(anyhow::anyhow!(
"Failed to load browser session"
)))?,
)
} else {
None
};

// Generate a new 6-digit authentication code
let range = Uniform::<u32>::from(0..1_000_000);
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
let code = rng.sample(range);
let code = format!("{code:06}");
let code = repo
.user_email()
.add_authentication_code(
&mut rng,
&clock,
Duration::minutes(5), // TODO: make this configurable
&user_email_authentication,
code,
)
.await
.map_err(JobError::retry)?;

let address: Address = user_email_authentication
.email
.parse()
.map_err(JobError::fail)?;
let username = browser_session.as_ref().map(|s| s.user.username.clone());
let mailbox = Mailbox::new(username, address);

info!("Sending email verification code to {}", mailbox);

let language = self.language().parse().map_err(JobError::fail)?;

let context = EmailVerificationContext::new(code, browser_session).with_language(language);
mailer
.send_verification_email(mailbox, &context)
.await
.map_err(JobError::fail)?;

repo.save().await.map_err(JobError::fail)?;

Ok(())
}
}
3 changes: 2 additions & 1 deletion crates/tasks/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2021-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand Down Expand Up @@ -125,6 +125,7 @@ pub async fn init(
.register_handler::<mas_storage::queue::ProvisionUserJob>()
.register_handler::<mas_storage::queue::ReactivateUserJob>()
.register_handler::<mas_storage::queue::SendAccountRecoveryEmailsJob>()
.register_handler::<mas_storage::queue::SendEmailAuthenticationCodeJob>()
.register_handler::<mas_storage::queue::SyncDevicesJob>()
.register_handler::<mas_storage::queue::VerifyEmailJob>()
.add_schedule(
Expand Down
45 changes: 31 additions & 14 deletions crates/templates/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2021-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -22,7 +22,8 @@ use mas_data_model::{
AuthorizationGrant, BrowserSession, Client, CompatSsoLogin, CompatSsoLoginState,
DeviceCodeGrant, UpstreamOAuthLink, UpstreamOAuthProvider, UpstreamOAuthProviderClaimsImports,
UpstreamOAuthProviderDiscoveryMode, UpstreamOAuthProviderPkceMode,
UpstreamOAuthProviderTokenAuthMethod, User, UserAgent, UserEmail, UserRecoverySession,
UpstreamOAuthProviderTokenAuthMethod, User, UserAgent, UserEmail, UserEmailAuthenticationCode,
UserRecoverySession,
};
use mas_i18n::DataLocale;
use mas_iana::jose::JsonWebSignatureAlg;
Expand Down Expand Up @@ -877,27 +878,33 @@ impl TemplateContext for EmailRecoveryContext {
/// Context used by the `emails/verification.{txt,html,subject}` templates
#[derive(Serialize)]
pub struct EmailVerificationContext {
user: User,
verification: serde_json::Value,
browser_session: Option<BrowserSession>,
authentication_code: UserEmailAuthenticationCode,
}

impl EmailVerificationContext {
/// Constructs a context for the verification email
#[must_use]
pub fn new(user: User, verification: serde_json::Value) -> Self {
Self { user, verification }
pub fn new(
authentication_code: UserEmailAuthenticationCode,
browser_session: Option<BrowserSession>,
) -> Self {
Self {
browser_session,
authentication_code,
}
}

/// Get the user to which this email is being sent
#[must_use]
pub fn user(&self) -> &User {
&self.user
pub fn user(&self) -> Option<&User> {
self.browser_session.as_ref().map(|s| &s.user)
}

/// Get the verification code being sent
#[must_use]
pub fn verification(&self) -> &serde_json::Value {
&self.verification
pub fn code(&self) -> &str {
&self.authentication_code.code
}
}

Expand All @@ -906,11 +913,21 @@ impl TemplateContext for EmailVerificationContext {
where
Self: Sized,
{
User::samples(now, rng)
BrowserSession::samples(now, rng)
.into_iter()
.map(|user| {
let verification = serde_json::json!({"code": "123456"});
Self { user, verification }
.map(|browser_session| {
let authentication_code = UserEmailAuthenticationCode {
id: Ulid::from_datetime_with_source(now.into(), rng),
user_email_authentication_id: Ulid::from_datetime_with_source(now.into(), rng),
code: "123456".to_owned(),
created_at: now - Duration::try_minutes(5).unwrap(),
expires_at: now + Duration::try_minutes(25).unwrap(),
};

Self {
browser_session: Some(browser_session),
authentication_code,
}
})
.collect()
}
Expand Down
6 changes: 3 additions & 3 deletions templates/emails/verification.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{#
Copyright 2024 New Vector Ltd.
Copyright 2024, 2025 New Vector Ltd.
Copyright 2021-2024 The Matrix.org Foundation C.I.C.

SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -8,6 +8,6 @@

{%- set _ = translator(lang) -%}

{{ _("mas.emails.greeting", username=user.username) }}<br />
{{ _("mas.emails.greeting", username=browser_session.user.username | default("user")) }}<br />
<br />
{{ _("mas.emails.verify.body_html", code=verification.code) }}<br />
{{ _("mas.emails.verify.body_html", code=authentication_code.code) }}<br />
4 changes: 2 additions & 2 deletions templates/emails/verification.subject
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{#
Copyright 2024 New Vector Ltd.
Copyright 2024, 2025 New Vector Ltd.
Copyright 2021-2024 The Matrix.org Foundation C.I.C.

SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -8,4 +8,4 @@ Please see LICENSE in the repository root for full details.

{%- set _ = translator(lang) -%}

{{ _("mas.emails.verify.subject", code=verification.code) }}
{{ _("mas.emails.verify.subject", code=authentication_code.code) }}
6 changes: 3 additions & 3 deletions templates/emails/verification.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{#
Copyright 2024 New Vector Ltd.
Copyright 2024, 2025 New Vector Ltd.
Copyright 2021-2024 The Matrix.org Foundation C.I.C.

SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -8,6 +8,6 @@ Please see LICENSE in the repository root for full details.

{%- set _ = translator(lang) -%}

{{ _("mas.emails.greeting", username=user.username) }}
{{ _("mas.emails.greeting", username=browser_session.user.username | default("user")) }}

{{ _("mas.emails.verify.body_text", code=verification.code) }}
{{ _("mas.emails.verify.body_text", code=authentication_code.code) }}
8 changes: 4 additions & 4 deletions translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@
"emails": {
"greeting": "Hello %(username)s,",
"@greeting": {
"context": "emails/verification.html:11:3-51, emails/verification.txt:11:3-51",
"context": "emails/verification.html:11:3-85, emails/verification.txt:11:3-85",
"description": "Greeting at the top of emails sent to the user"
},
"recovery": {
Expand Down Expand Up @@ -262,17 +262,17 @@
"verify": {
"body_html": "Your verification code to confirm this email address is: <strong>%(code)s</strong>",
"@body_html": {
"context": "emails/verification.html:13:3-59",
"context": "emails/verification.html:13:3-66",
"description": "The body of the email sent to verify an email address (HTML)"
},
"body_text": "Your verification code to confirm this email address is: %(code)s",
"@body_text": {
"context": "emails/verification.txt:13:3-59",
"context": "emails/verification.txt:13:3-66",
"description": "The body of the email sent to verify an email address (text)"
},
"subject": "Your email verification code is: %(code)s",
"@subject": {
"context": "emails/verification.subject:11:3-57",
"context": "emails/verification.subject:11:3-64",
"description": "The subject line of the email sent to verify an email address"
}
}
Expand Down