Disclose that email is already in use after verification

This commit is contained in:
Quentin Gliech
2025-01-23 18:18:19 +01:00
parent ea6b80c5ac
commit 7f1b3866ba
13 changed files with 246 additions and 39 deletions

View File

@@ -242,7 +242,7 @@ enum StartEmailAuthenticationStatus {
RateLimited,
/// The email address isn't allowed by the policy
Denied,
/// The email address is already in use
/// The email address is already in use on this account
InUse,
}
@@ -308,6 +308,7 @@ enum CompleteEmailAuthenticationPayload {
Completed,
InvalidCode,
CodeExpired,
InUse,
RateLimited,
}
@@ -322,6 +323,8 @@ enum CompleteEmailAuthenticationStatus {
CodeExpired,
/// Too many attempts to complete an email authentication
RateLimited,
/// The email address is already in use
InUse,
}
#[Object(use_type_description)]
@@ -332,6 +335,7 @@ impl CompleteEmailAuthenticationPayload {
Self::Completed => CompleteEmailAuthenticationStatus::Completed,
Self::InvalidCode => CompleteEmailAuthenticationStatus::InvalidCode,
Self::CodeExpired => CompleteEmailAuthenticationStatus::CodeExpired,
Self::InUse => CompleteEmailAuthenticationStatus::InUse,
Self::RateLimited => CompleteEmailAuthenticationStatus::RateLimited,
}
}
@@ -588,10 +592,17 @@ impl UserEmailMutations {
let mut repo = state.repository().await?;
// Check if the email address is already in use
// Check if the email address is already in use by the same user
// We don't report here if the email address is already in use by another user,
// because we don't want to leak information about other users. We will do that
// only when the user enters the right verification code
let count = repo
.user_email()
.count(UserEmailFilter::new().for_email(&input.email))
.count(
UserEmailFilter::new()
.for_email(&input.email)
.for_user(&browser_session.user),
)
.await?;
if count > 0 {
return Ok(StartEmailAuthenticationPayload::InUse);
@@ -742,20 +753,24 @@ impl UserEmailMutations {
return Ok(CompleteEmailAuthenticationPayload::CodeExpired);
}
// Check that we can add the email address to the user
let count = repo
.user_email()
.count(UserEmailFilter::new().for_email(&authentication.email))
.await?;
if count > 0 {
return Ok(CompleteEmailAuthenticationPayload::CodeExpired);
}
let authentication = repo
.user_email()
.complete_authentication(&clock, authentication, &code)
.await?;
// Check the email is not already in use by anyone, including the current user
let count = repo
.user_email()
.count(UserEmailFilter::new().for_email(&authentication.email))
.await?;
if count > 0 {
// We still want to consume the code so that it can't be reused
repo.save().await?;
return Ok(CompleteEmailAuthenticationPayload::InUse);
}
repo.user_email()
.add(
&mut rng,

View File

@@ -25,7 +25,7 @@ use mas_policy::Policy;
use mas_router::UrlBuilder;
use mas_storage::{
queue::{QueueJobRepositoryExt as _, SendEmailAuthenticationCodeJob},
user::{UserEmailFilter, UserEmailRepository, UserRepository},
user::{UserEmailRepository, UserRepository},
BoxClock, BoxRepository, BoxRng, RepositoryAccess,
};
use mas_templates::{
@@ -191,17 +191,13 @@ pub(crate) async fn post(
homeserver_denied_username = true;
}
// Note that we don't check here if the email is already taken here, as
// we don't want to leak the information about other users. Instead, we will
// show an error message once the user confirmed their email address.
if form.email.is_empty() {
state.add_error_on_field(RegisterFormField::Email, FieldError::Required);
} else if Address::from_str(&form.email).is_err() {
state.add_error_on_field(RegisterFormField::Email, FieldError::Invalid);
} else if repo
.user_email()
.count(UserEmailFilter::new().for_email(&form.email))
.await?
> 0
{
state.add_error_on_field(RegisterFormField::Email, FieldError::Exists);
}
if form.password.is_empty() {

View File

@@ -6,7 +6,7 @@
use anyhow::Context as _;
use axum::{
extract::{Path, State},
response::IntoResponse,
response::{Html, IntoResponse, Response},
};
use axum_extra::TypedHeader;
use chrono::Duration;
@@ -19,10 +19,11 @@ use mas_storage::{
user::UserEmailFilter,
BoxClock, BoxRepository, BoxRng,
};
use mas_templates::{RegisterStepsEmailInUseContext, TemplateContext as _, Templates};
use ulid::Ulid;
use super::super::cookie::UserRegistrationSessions;
use crate::{views::shared::OptionalPostAuthAction, BoundActivityTracker};
use crate::{views::shared::OptionalPostAuthAction, BoundActivityTracker, PreferredLanguage};
#[tracing::instrument(
name = "handlers.views.register.steps.finish.get",
@@ -38,9 +39,11 @@ pub(crate) async fn get(
user_agent: Option<TypedHeader<headers::UserAgent>>,
State(url_builder): State<UrlBuilder>,
State(homeserver): State<BoxHomeserverConnection>,
State(templates): State<Templates>,
PreferredLanguage(lang): PreferredLanguage,
cookie_jar: CookieJar,
Path(id): Path<Ulid>,
) -> Result<impl IntoResponse, FancyError> {
) -> Result<Response, FancyError> {
let user_agent = user_agent.map(|ua| UserAgent::parse(ua.as_str().to_owned()));
let registration = repo
.user_registration()
@@ -60,7 +63,8 @@ pub(crate) async fn get(
return Ok((
cookie_jar,
OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
));
)
.into_response());
}
// Make sure the registration session hasn't expired
@@ -117,21 +121,33 @@ pub(crate) async fn get(
return Ok((
cookie_jar,
url_builder.redirect(&mas_router::RegisterVerifyEmail::new(id)),
));
)
.into_response());
}
// Check that the email address isn't already used
// It is important to do that here, as we we're not checking during the
// registration, because we don't want to disclose whether an email is
// already being used or not before we verified it
if repo
.user_email()
.count(UserEmailFilter::new().for_email(&email_authentication.email))
.await?
> 0
{
// XXX: this could have a better error message, but as this is unlikely to
// happen, we're fine with a vague message for now
return Err(FancyError::from(anyhow::anyhow!(
"Email address is already used"
)));
let action = registration
.post_auth_action
.map(serde_json::from_value)
.transpose()?;
let ctx = RegisterStepsEmailInUseContext::new(email_authentication.email, action)
.with_language(lang);
return Ok((
cookie_jar,
Html(templates.render_register_steps_email_in_use(&ctx)?),
)
.into_response());
}
// Check that the display name is set
@@ -139,7 +155,8 @@ pub(crate) async fn get(
return Ok((
cookie_jar,
url_builder.redirect(&mas_router::RegisterDisplayName::new(registration.id)),
));
)
.into_response());
}
// Everuthing is good, let's complete the registration
@@ -215,5 +232,6 @@ pub(crate) async fn get(
return Ok((
cookie_jar,
OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
));
)
.into_response());
}

View File

@@ -1000,6 +1000,32 @@ impl TemplateContext for RegisterStepsVerifyEmailContext {
}
}
/// Context used by the `pages/register/steps/email_in_use.html` template
#[derive(Serialize)]
pub struct RegisterStepsEmailInUseContext {
email: String,
action: Option<PostAuthAction>,
}
impl RegisterStepsEmailInUseContext {
/// Constructs a context for the email in use page
#[must_use]
pub fn new(email: String, action: Option<PostAuthAction>) -> Self {
Self { email, action }
}
}
impl TemplateContext for RegisterStepsEmailInUseContext {
fn sample(_now: chrono::DateTime<Utc>, _rng: &mut impl Rng) -> Vec<Self>
where
Self: Sized,
{
let email = "hello@example.com".to_owned();
let action = PostAuthAction::continue_grant(Ulid::nil());
vec![Self::new(email, Some(action))]
}
}
/// Fields for the display name form
#[derive(Serialize, Deserialize, Debug, Clone, Copy, Hash, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]

View File

@@ -42,10 +42,10 @@ pub use self::{
RecoveryFinishContext, RecoveryFinishFormField, RecoveryProgressContext,
RecoveryStartContext, RecoveryStartFormField, RegisterContext, RegisterFormField,
RegisterStepsDisplayNameContext, RegisterStepsDisplayNameFormField,
RegisterStepsVerifyEmailContext, RegisterStepsVerifyEmailFormField, SiteBranding,
SiteConfigExt, SiteFeatures, TemplateContext, UpstreamExistingLinkContext,
UpstreamRegister, UpstreamRegisterFormField, UpstreamSuggestLink, WithCaptcha, WithCsrf,
WithLanguage, WithOptionalSession, WithSession,
RegisterStepsEmailInUseContext, RegisterStepsVerifyEmailContext,
RegisterStepsVerifyEmailFormField, SiteBranding, SiteConfigExt, SiteFeatures,
TemplateContext, UpstreamExistingLinkContext, UpstreamRegister, UpstreamRegisterFormField,
UpstreamSuggestLink, WithCaptcha, WithCsrf, WithLanguage, WithOptionalSession, WithSession,
},
forms::{FieldError, FormError, FormField, FormState, ToFormState},
};
@@ -336,6 +336,9 @@ register_templates! {
/// Render the email verification page
pub fn render_register_steps_verify_email(WithLanguage<WithCsrf<RegisterStepsVerifyEmailContext>>) { "pages/register/steps/verify_email.html" }
/// Render the email in use page
pub fn render_register_steps_email_in_use(WithLanguage<RegisterStepsEmailInUseContext>) { "pages/register/steps/email_in_use.html" }
/// Render the display name page
pub fn render_register_steps_display_name(WithLanguage<WithCsrf<RegisterStepsDisplayNameContext>>) { "pages/register/steps/display_name.html" }
@@ -432,6 +435,7 @@ impl Templates {
check::render_register(self, now, rng)?;
check::render_password_register(self, now, rng)?;
check::render_register_steps_verify_email(self, now, rng)?;
check::render_register_steps_email_in_use(self, now, rng)?;
check::render_register_steps_display_name(self, now, rng)?;
check::render_consent(self, now, rng)?;
check::render_policy_violation(self, now, rng)?;

View File

@@ -78,6 +78,9 @@
"tablet": "Tablet",
"unknown": "Unknown device type"
},
"email_in_use": {
"heading": "The email address {{email}} is already in use."
},
"end_session_button": {
"confirmation_modal_title": "Are you sure you want to end this session?",
"text": "Sign out"
@@ -266,6 +269,10 @@
}
},
"verify_email": {
"code_expired_alert": {
"description": "The code has expired. Please request a new code.",
"title": "Code expired"
},
"code_field_error": "Code not recognised",
"code_field_label": "6-digit code",
"code_field_wrong_shape": "Code must be 6 digits",

View File

@@ -528,6 +528,10 @@ enum CompleteEmailAuthenticationStatus {
Too many attempts to complete an email authentication
"""
RATE_LIMITED
"""
The email address is already in use
"""
IN_USE
}
"""
@@ -1650,7 +1654,7 @@ enum StartEmailAuthenticationStatus {
"""
DENIED
"""
The email address is already in use
The email address is already in use on this account
"""
IN_USE
}

View File

@@ -349,6 +349,8 @@ export type CompleteEmailAuthenticationStatus =
| 'COMPLETED'
/** The authentication code is invalid */
| 'INVALID_CODE'
/** The email address is already in use */
| 'IN_USE'
/** Too many attempts to complete an email authentication */
| 'RATE_LIMITED';
@@ -1207,7 +1209,7 @@ export type StartEmailAuthenticationStatus =
| 'DENIED'
/** The email address is invalid */
| 'INVALID_EMAIL_ADDRESS'
/** The email address is already in use */
/** The email address is already in use on this account */
| 'IN_USE'
/** Too many attempts to start an email authentication */
| 'RATE_LIMITED'

View File

@@ -25,6 +25,7 @@ import { Route as PasswordRecoveryIndexImport } from './routes/password.recovery
import { Route as PasswordChangeIndexImport } from './routes/password.change.index'
import { Route as AccountSessionsIndexImport } from './routes/_account.sessions.index'
import { Route as EmailsIdVerifyImport } from './routes/emails.$id.verify'
import { Route as EmailsIdInUseImport } from './routes/emails.$id.in-use'
import { Route as AccountSessionsBrowsersImport } from './routes/_account.sessions.browsers'
import { Route as AccountSessionsIdImport } from './routes/_account.sessions.$id'
@@ -127,6 +128,12 @@ const EmailsIdVerifyRoute = EmailsIdVerifyImport.update({
import('./routes/emails.$id.verify.lazy').then((d) => d.Route),
)
const EmailsIdInUseRoute = EmailsIdInUseImport.update({
id: '/emails/$id/in-use',
path: '/emails/$id/in-use',
getParentRoute: () => rootRoute,
} as any)
const AccountSessionsBrowsersRoute = AccountSessionsBrowsersImport.update({
id: '/sessions/browsers',
path: '/sessions/browsers',
@@ -217,6 +224,13 @@ declare module '@tanstack/react-router' {
preLoaderRoute: typeof AccountSessionsBrowsersImport
parentRoute: typeof AccountImport
}
'/emails/$id/in-use': {
id: '/emails/$id/in-use'
path: '/emails/$id/in-use'
fullPath: '/emails/$id/in-use'
preLoaderRoute: typeof EmailsIdInUseImport
parentRoute: typeof rootRoute
}
'/emails/$id/verify': {
id: '/emails/$id/verify'
path: '/emails/$id/verify'
@@ -300,6 +314,7 @@ export interface FileRoutesByFullPath {
'/reset-cross-signing/': typeof ResetCrossSigningIndexRoute
'/sessions/$id': typeof AccountSessionsIdRoute
'/sessions/browsers': typeof AccountSessionsBrowsersRoute
'/emails/$id/in-use': typeof EmailsIdInUseRoute
'/emails/$id/verify': typeof EmailsIdVerifyRoute
'/password/change/success': typeof PasswordChangeSuccessLazyRoute
'/sessions': typeof AccountSessionsIndexRoute
@@ -316,6 +331,7 @@ export interface FileRoutesByTo {
'/reset-cross-signing': typeof ResetCrossSigningIndexRoute
'/sessions/$id': typeof AccountSessionsIdRoute
'/sessions/browsers': typeof AccountSessionsBrowsersRoute
'/emails/$id/in-use': typeof EmailsIdInUseRoute
'/emails/$id/verify': typeof EmailsIdVerifyRoute
'/password/change/success': typeof PasswordChangeSuccessLazyRoute
'/sessions': typeof AccountSessionsIndexRoute
@@ -335,6 +351,7 @@ export interface FileRoutesById {
'/reset-cross-signing/': typeof ResetCrossSigningIndexRoute
'/_account/sessions/$id': typeof AccountSessionsIdRoute
'/_account/sessions/browsers': typeof AccountSessionsBrowsersRoute
'/emails/$id/in-use': typeof EmailsIdInUseRoute
'/emails/$id/verify': typeof EmailsIdVerifyRoute
'/password/change/success': typeof PasswordChangeSuccessLazyRoute
'/_account/sessions/': typeof AccountSessionsIndexRoute
@@ -355,6 +372,7 @@ export interface FileRouteTypes {
| '/reset-cross-signing/'
| '/sessions/$id'
| '/sessions/browsers'
| '/emails/$id/in-use'
| '/emails/$id/verify'
| '/password/change/success'
| '/sessions'
@@ -370,6 +388,7 @@ export interface FileRouteTypes {
| '/reset-cross-signing'
| '/sessions/$id'
| '/sessions/browsers'
| '/emails/$id/in-use'
| '/emails/$id/verify'
| '/password/change/success'
| '/sessions'
@@ -387,6 +406,7 @@ export interface FileRouteTypes {
| '/reset-cross-signing/'
| '/_account/sessions/$id'
| '/_account/sessions/browsers'
| '/emails/$id/in-use'
| '/emails/$id/verify'
| '/password/change/success'
| '/_account/sessions/'
@@ -400,6 +420,7 @@ export interface RootRouteChildren {
ResetCrossSigningRoute: typeof ResetCrossSigningRouteWithChildren
ClientsIdRoute: typeof ClientsIdRoute
DevicesSplatRoute: typeof DevicesSplatRoute
EmailsIdInUseRoute: typeof EmailsIdInUseRoute
EmailsIdVerifyRoute: typeof EmailsIdVerifyRoute
PasswordChangeSuccessLazyRoute: typeof PasswordChangeSuccessLazyRoute
PasswordChangeIndexRoute: typeof PasswordChangeIndexRoute
@@ -411,6 +432,7 @@ const rootRouteChildren: RootRouteChildren = {
ResetCrossSigningRoute: ResetCrossSigningRouteWithChildren,
ClientsIdRoute: ClientsIdRoute,
DevicesSplatRoute: DevicesSplatRoute,
EmailsIdInUseRoute: EmailsIdInUseRoute,
EmailsIdVerifyRoute: EmailsIdVerifyRoute,
PasswordChangeSuccessLazyRoute: PasswordChangeSuccessLazyRoute,
PasswordChangeIndexRoute: PasswordChangeIndexRoute,
@@ -431,6 +453,7 @@ export const routeTree = rootRoute
"/reset-cross-signing",
"/clients/$id",
"/devices/$",
"/emails/$id/in-use",
"/emails/$id/verify",
"/password/change/success",
"/password/change/",
@@ -484,6 +507,9 @@ export const routeTree = rootRoute
"filePath": "_account.sessions.browsers.tsx",
"parent": "/_account"
},
"/emails/$id/in-use": {
"filePath": "emails.$id.in-use.tsx"
},
"/emails/$id/verify": {
"filePath": "emails.$id.verify.tsx"
},

View File

@@ -0,0 +1,56 @@
// Copyright 2025 New Vector Ltd.
//
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
import { useSuspenseQuery } from "@tanstack/react-query";
import { createFileRoute, notFound, redirect } from "@tanstack/react-router";
import IconArrowLeft from "@vector-im/compound-design-tokens/assets/web/icons/arrow-left";
import IconError from "@vector-im/compound-design-tokens/assets/web/icons/error";
import { useTranslation } from "react-i18next";
import { ButtonLink } from "../components/ButtonLink";
import Layout from "../components/Layout";
import PageHeading from "../components/PageHeading";
import { query } from "./emails.$id.verify";
export const Route = createFileRoute("/emails/$id/in-use")({
async loader({ context, params }): Promise<void> {
const data = await context.queryClient.ensureQueryData(query(params.id));
if (!data.userEmailAuthentication) {
throw notFound();
}
// If the user has not completed the verification process, it means they got
// to this page by mistake
if (!data.userEmailAuthentication.completedAt) {
throw redirect({ to: "/emails/$id/verify", params });
}
},
component: EmailInUse,
});
function EmailInUse(): React.ReactElement {
const { id } = Route.useParams();
const {
data: { userEmailAuthentication },
} = useSuspenseQuery(query(id));
if (!userEmailAuthentication) throw notFound();
const { t } = useTranslation();
return (
<Layout>
<PageHeading
Icon={IconError}
invalid
title={t("frontend.email_in_use.heading", {
email: userEmailAuthentication.email,
})}
/>
<ButtonLink as="a" Icon={IconArrowLeft} kind="tertiary" to="/">
{t("action.back")}
</ButtonLink>
</Layout>
);
}

View File

@@ -59,6 +59,8 @@ function EmailVerify(): React.ReactElement {
if (data.completeEmailAuthentication.status === "COMPLETED") {
await navigate({ to: "/" });
} else if (data.completeEmailAuthentication.status === "IN_USE") {
await navigate({ to: "/emails/$id/in-use", params: { id } });
}
},
});
@@ -99,6 +101,8 @@ function EmailVerify(): React.ReactElement {
"RESENT";
const invalidCode =
verifyEmail.data?.completeEmailAuthentication.status === "INVALID_CODE";
const codeExpired =
verifyEmail.data?.completeEmailAuthentication.status === "CODE_EXPIRED";
const rateLimited =
verifyEmail.data?.completeEmailAuthentication.status === "RATE_LIMITED";
@@ -135,6 +139,15 @@ function EmailVerify(): React.ReactElement {
</Alert>
)}
{codeExpired && (
<Alert
type="critical"
title={t("frontend.verify_email.code_expired_alert.title")}
>
{t("frontend.verify_email.code_expired_alert.description")}
</Alert>
)}
{rateLimited && (
<Alert
type="critical"

View File

@@ -0,0 +1,30 @@
{#
Copyright 2025 New Vector Ltd.
SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
-#}
{% extends "base.html" %}
{% block content %}
<main class="flex flex-col gap-6">
<header class="page-heading">
<div class="icon invalid">
{{ icon.error() }}
</div>
<div class="header">
<h1 class="title">
{{ _("mas.email_in_use.title", email=email) }}
</h1>
<p class="text">
{{ _("mas.email_in_use.description") }}
</p>
</div>
</header>
{% set params = action | default({}) | to_params(prefix="?") %}
{{ button.link_outline(text=_("action.start_over"), href="/register" ~ params) }}
</main>
{% endblock %}

View File

@@ -30,7 +30,7 @@
},
"start_over": "Start over",
"@start_over": {
"context": "pages/recovery/consumed.html:22:32-54, pages/recovery/expired.html:30:32-54"
"context": "pages/recovery/consumed.html:22:32-54, pages/recovery/expired.html:30:32-54, pages/register/steps/email_in_use.html:28:32-54"
}
},
"app": {
@@ -232,6 +232,16 @@
}
}
},
"email_in_use": {
"description": "If you have forgotten your account credentials, you can recover your account. You can also start over and use a different email address.",
"@description": {
"context": "pages/register/steps/email_in_use.html:22:13-46"
},
"title": "The email address <span>%(email)s</span> is already in use",
"@title": {
"context": "pages/register/steps/email_in_use.html:19:13-53"
}
},
"emails": {
"greeting": "Hello %(username)s,",
"@greeting": {