From 0beb842195c42489b9ebc0a7c5da8e5e70cb9bcf Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 7 Feb 2024 15:16:36 +0100 Subject: [PATCH] Make the user agree to T&C during registration --- crates/cli/src/commands/server.rs | 1 + crates/handlers/src/lib.rs | 1 + crates/handlers/src/site_config.rs | 3 ++ crates/handlers/src/test_utils.rs | 9 +++-- crates/handlers/src/upstream_oauth2/link.rs | 34 ++++++++++++++++++- crates/handlers/src/views/register.rs | 17 +++++++++- crates/storage/src/user/terms.rs | 3 +- crates/templates/src/context.rs | 10 ++++-- templates/components/field.html | 28 +++++++++++---- templates/pages/register.html | 13 +++++++ .../pages/upstream_oauth2/do_register.html | 14 ++++++++ translations/en.json | 22 +++++++----- 12 files changed, 133 insertions(+), 22 deletions(-) diff --git a/crates/cli/src/commands/server.rs b/crates/cli/src/commands/server.rs index 11a91a0b..14765ab6 100644 --- a/crates/cli/src/commands/server.rs +++ b/crates/cli/src/commands/server.rs @@ -160,6 +160,7 @@ impl Options { ); let site_config = SiteConfig { + tos_uri: config.branding.tos_uri.clone(), access_token_ttl: config.experimental.access_token_ttl, compat_token_ttl: config.experimental.compat_token_ttl, }; diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index e0a86e27..8c8dbe9f 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -319,6 +319,7 @@ where HttpClientFactory: FromRef, PasswordManager: FromRef, MetadataCache: FromRef, + SiteConfig: FromRef, BoxClock: FromRequestParts, BoxRng: FromRequestParts, Policy: FromRequestParts, diff --git a/crates/handlers/src/site_config.rs b/crates/handlers/src/site_config.rs index d24f7ea1..d52a6ebb 100644 --- a/crates/handlers/src/site_config.rs +++ b/crates/handlers/src/site_config.rs @@ -13,12 +13,14 @@ // limitations under the License. use chrono::Duration; +use url::Url; /// Random site configuration we don't now where to put yet. #[derive(Debug, Clone)] pub struct SiteConfig { pub access_token_ttl: Duration, pub compat_token_ttl: Duration, + pub tos_uri: Option, } impl Default for SiteConfig { @@ -26,6 +28,7 @@ impl Default for SiteConfig { Self { access_token_ttl: Duration::minutes(5), compat_token_ttl: Duration::minutes(5), + tos_uri: None, } } } diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 2356495c..f441ccb2 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -119,7 +119,9 @@ impl TestState { let url_builder = UrlBuilder::new("https://example.com/".parse()?, None, None); - let site_branding = SiteBranding::new("example.com").with_service_name("Example"); + let site_branding = SiteBranding::new("example.com") + .with_service_name("Example") + .with_tos_uri("https://example.com/tos"); let templates = Templates::load( workspace_root.join("templates"), @@ -154,7 +156,10 @@ impl TestState { let http_client_factory = HttpClientFactory::new().await?; - let site_config = SiteConfig::default(); + let site_config = SiteConfig { + tos_uri: Some("https://example.com/tos".parse().unwrap()), + ..SiteConfig::default() + }; let clock = Arc::new(MockClock::default()); let rng = Arc::new(Mutex::new(ChaChaRng::seed_from_u64(42))); diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 0906b49d..7bfb1e45 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -45,7 +45,9 @@ use tracing::warn; use ulid::Ulid; use super::{template::environment, UpstreamSessionsCookie}; -use crate::{impl_from_error_for_route, views::shared::OptionalPostAuthAction, PreferredLanguage}; +use crate::{ + impl_from_error_for_route, views::shared::OptionalPostAuthAction, PreferredLanguage, SiteConfig, +}; const DEFAULT_LOCALPART_TEMPLATE: &str = "{{ user.preferred_username }}"; const DEFAULT_DISPLAYNAME_TEMPLATE: &str = "{{ user.name }}"; @@ -170,6 +172,8 @@ pub(crate) enum FormData { import_email: Option, #[serde(default)] import_display_name: Option, + #[serde(default)] + accept_terms: Option, }, Link, } @@ -473,6 +477,7 @@ pub(crate) async fn post( PreferredLanguage(locale): PreferredLanguage, State(templates): State, State(url_builder): State, + State(site_config): State, Path(link_id): Path, Form(form): Form>, ) -> Result { @@ -533,6 +538,7 @@ pub(crate) async fn post( username, import_email, import_display_name, + accept_terms, }, ) => { // The user got the form to register a new account, and is not logged in. @@ -543,6 +549,7 @@ pub(crate) async fn post( // Those fields are Some("on") if the checkbox is checked let import_email = import_email.is_some(); let import_display_name = import_display_name.is_some(); + let accept_terms = accept_terms.is_some(); let id_token = upstream_session .id_token() @@ -695,6 +702,24 @@ pub(crate) async fn post( .into_response()); } + // If we need have a TOS in the config, make sure the user has accepted it + if site_config.tos_uri.is_some() && !accept_terms { + let form_state = form_state.with_error_on_field( + mas_templates::UpstreamRegisterFormField::AcceptTerms, + FieldError::Required, + ); + + let ctx = ctx + .with_form_state(form_state) + .with_csrf(csrf_token.form_value()) + .with_language(locale); + return Ok(( + cookie_jar, + Html(templates.render_upstream_oauth2_do_register(&ctx)?), + ) + .into_response()); + } + // Policy check let res = policy .evaluate_upstream_oauth_register(&username, email.as_deref()) @@ -731,6 +756,12 @@ pub(crate) async fn post( // Now we can create the user let user = repo.user().add(&mut rng, &clock, username).await?; + if let Some(terms_url) = &site_config.tos_uri { + repo.user_terms() + .accept_terms(&mut rng, &clock, &user, terms_url.clone()) + .await?; + } + // And schedule the job to provision it let mut job = ProvisionUserJob::new(&user); @@ -936,6 +967,7 @@ mod tests { "csrf": csrf_token, "action": "register", "import_email": "on", + "accept_terms": "on", }), ); let request = cookies.with_cookies(request); diff --git a/crates/handlers/src/views/register.rs b/crates/handlers/src/views/register.rs index 6e3cfec2..abac1bd5 100644 --- a/crates/handlers/src/views/register.rs +++ b/crates/handlers/src/views/register.rs @@ -43,7 +43,7 @@ use serde::{Deserialize, Serialize}; use zeroize::Zeroizing; use super::shared::OptionalPostAuthAction; -use crate::{passwords::PasswordManager, BoundActivityTracker, PreferredLanguage}; +use crate::{passwords::PasswordManager, BoundActivityTracker, PreferredLanguage, SiteConfig}; #[derive(Debug, Deserialize, Serialize)] pub(crate) struct RegisterForm { @@ -51,6 +51,8 @@ pub(crate) struct RegisterForm { email: String, password: String, password_confirm: String, + #[serde(default)] + accept_terms: String, } impl ToFormState for RegisterForm { @@ -108,6 +110,7 @@ pub(crate) async fn post( State(password_manager): State, State(templates): State, State(url_builder): State, + State(site_config): State, mut policy: Policy, mut repo: BoxRepository, activity_tracker: BoundActivityTracker, @@ -155,6 +158,11 @@ pub(crate) async fn post( state.add_error_on_field(RegisterFormField::PasswordConfirm, FieldError::Unspecified); } + // If the site has terms of service, the user must accept them + if site_config.tos_uri.is_some() && form.accept_terms != "on" { + state.add_error_on_field(RegisterFormField::AcceptTerms, FieldError::Required); + } + let res = policy .evaluate_register(&form.username, &form.password, &form.email) .await?; @@ -203,6 +211,13 @@ pub(crate) async fn post( } let user = repo.user().add(&mut rng, &clock, form.username).await?; + + if let Some(tos_uri) = &site_config.tos_uri { + repo.user_terms() + .accept_terms(&mut rng, &clock, &user, tos_uri.clone()) + .await?; + } + let password = Zeroizing::new(form.password.into_bytes()); let (version, hashed_password) = password_manager.hash(&mut rng, password).await?; let user_password = repo diff --git a/crates/storage/src/user/terms.rs b/crates/storage/src/user/terms.rs index 292197a2..07b98ec1 100644 --- a/crates/storage/src/user/terms.rs +++ b/crates/storage/src/user/terms.rs @@ -19,7 +19,8 @@ use url::Url; use crate::{repository_impl, Clock}; -/// A [`UserTermsRepository`] helps interacting with the terms of service agreed by a [`User`] +/// A [`UserTermsRepository`] helps interacting with the terms of service agreed +/// by a [`User`] #[async_trait] pub trait UserTermsRepository: Send + Sync { /// The error type returned by the repository diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 06529d8a..8e09982a 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -493,12 +493,15 @@ pub enum RegisterFormField { /// The password confirmation field PasswordConfirm, + + /// The terms of service agreement field + AcceptTerms, } impl FormField for RegisterFormField { fn keep(&self) -> bool { match self { - Self::Username | Self::Email => true, + Self::Username | Self::Email | Self::AcceptTerms => true, Self::Password | Self::PasswordConfirm => false, } } @@ -974,12 +977,15 @@ impl TemplateContext for UpstreamSuggestLink { pub enum UpstreamRegisterFormField { /// The username field Username, + + /// Accept the terms of service + AcceptTerms, } impl FormField for UpstreamRegisterFormField { fn keep(&self) -> bool { match self { - Self::Username => true, + Self::Username | Self::AcceptTerms => true, } } } diff --git a/templates/components/field.html b/templates/components/field.html index 61e8cc44..f930bc12 100644 --- a/templates/components/field.html +++ b/templates/components/field.html @@ -27,7 +27,7 @@ limitations under the License. {%- if value %} value="{{ value }}" {% endif %} {%- endmacro %} -{% macro field(label, name, form_state=false, class="") %} +{% macro field(label, name, form_state=false, class="", inline=false) %} {% set field_id = new_id() %} {% if not form_state %} {% set form_state = {"fields": {}} %} @@ -41,12 +41,24 @@ limitations under the License. "value": state.value, } %} -
- +
+ {% if not inline %} + + + {{ caller(field) }} + {% else %} +
+ {{ caller(field) }} +
+ +
+ + {% endif %} - {{ caller(field) }} {% if field.errors is not empty %} {% for error in field.errors %} @@ -65,6 +77,10 @@ limitations under the License. {% endif %} {% endfor %} {% endif %} + + {% if inline %} +
+ {% endif %}
{% endmacro %} diff --git a/templates/pages/register.html b/templates/pages/register.html index 7882a4ac..a7491578 100644 --- a/templates/pages/register.html +++ b/templates/pages/register.html @@ -56,6 +56,19 @@ limitations under the License. {% endcall %} + {% if branding.tos_uri %} + {% call(f) field.field(label=_("mas.register.terms_of_service", tos_uri=branding.tos_uri), name="accept_terms", form_state=form, inline=true, class="my-4") %} +
+
+ +
+ {{ icon.check() }} +
+
+
+ {% endcall %} + {% endif %} + {{ button.button(text=_("action.continue")) }} diff --git a/templates/pages/upstream_oauth2/do_register.html b/templates/pages/upstream_oauth2/do_register.html index c15f3014..127a13de 100644 --- a/templates/pages/upstream_oauth2/do_register.html +++ b/templates/pages/upstream_oauth2/do_register.html @@ -140,6 +140,20 @@ limitations under the License.
{% endif %} + {% if branding.tos_uri %} + {% call(f) field.field(label=_("mas.register.terms_of_service", tos_uri=branding.tos_uri), name="accept_terms", form_state=form_state, inline=true, class="my-4") %} +
+
+ +
+ {{ icon.check() }} +
+
+
+ {% endcall %} + {% endif %} + + {{ button.button(text=_("action.create_account")) }} diff --git a/translations/en.json b/translations/en.json index d6067446..22c37939 100644 --- a/translations/en.json +++ b/translations/en.json @@ -2,15 +2,15 @@ "action": { "cancel": "Cancel", "@cancel": { - "context": "pages/consent.html:72:11-29, pages/device_consent.html:94:13-31, pages/login.html:100:13-31, pages/policy_violation.html:52:13-31, pages/register.html:64:13-31" + "context": "pages/consent.html:72:11-29, pages/device_consent.html:94:13-31, pages/login.html:100:13-31, pages/policy_violation.html:52:13-31, pages/register.html:77:13-31" }, "continue": "Continue", "@continue": { - "context": "pages/account/emails/add.html:45:26-46, pages/account/emails/verify.html:60:26-46, pages/consent.html:60:28-48, pages/device_consent.html:91:13-33, pages/device_link.html:50:26-46, pages/login.html:62:30-50, pages/reauth.html:40:28-48, pages/register.html:59:28-48, pages/sso.html:45:28-48" + "context": "pages/account/emails/add.html:45:26-46, pages/account/emails/verify.html:60:26-46, pages/consent.html:60:28-48, pages/device_consent.html:91:13-33, pages/device_link.html:50:26-46, pages/login.html:62:30-50, pages/reauth.html:40:28-48, pages/register.html:72:28-48, pages/sso.html:45:28-48" }, "create_account": "Create Account", "@create_account": { - "context": "pages/login.html:72:35-61, pages/upstream_oauth2/do_register.html:143:26-52" + "context": "pages/login.html:72:35-61, pages/upstream_oauth2/do_register.html:157:26-52" }, "sign_in": "Sign in", "@sign_in": { @@ -167,11 +167,11 @@ "errors": { "denied_policy": "Denied by policy: %(policy)s", "@denied_policy": { - "context": "components/errors.html:23:7-58, components/field.html:60:17-68" + "context": "components/errors.html:23:7-58, components/field.html:72:17-68" }, "field_required": "This field is required", "@field_required": { - "context": "components/field.html:56:17-47" + "context": "components/field.html:68:17-47" }, "invalid_credentials": "Invalid credentials", "@invalid_credentials": { @@ -183,7 +183,7 @@ }, "username_taken": "This username is already taken", "@username_taken": { - "context": "components/field.html:58:17-47" + "context": "components/field.html:70:17-47" } }, "login": { @@ -251,7 +251,7 @@ }, "or_separator": "Or", "@or_separator": { - "context": "components/field.html:75:10-31", + "context": "components/field.html:91:10-31", "description": "Separator between the login methods" }, "policy_violation": { @@ -273,7 +273,7 @@ "register": { "call_to_login": "Already have an account?", "@call_to_login": { - "context": "pages/register.html:74:11-42", + "context": "pages/register.html:87:11-42", "description": "Displayed on the registration page to suggest to log in instead" }, "create_account": { @@ -288,7 +288,11 @@ }, "sign_in_instead": "Sign in instead", "@sign_in_instead": { - "context": "pages/register.html:78:31-64" + "context": "pages/register.html:91:31-64" + }, + "terms_of_service": "I agree to the Terms and Conditions", + "@terms_of_service": { + "context": "pages/register.html:60:37-97, pages/upstream_oauth2/do_register.html:144:35-95" } }, "scope": {