From ca3460b49eec9d737657beed8ceb1fb77912da36 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Thu, 24 Aug 2023 18:18:36 +0200 Subject: [PATCH] Skip the "continue" screens on upstream IDP logins for new accounts --- crates/handlers/src/upstream_oauth2/link.rs | 58 +++++++++---------- crates/templates/src/lib.rs | 8 --- .../pages/upstream_oauth2/already_linked.html | 26 --------- templates/pages/upstream_oauth2/do_login.html | 49 ---------------- 4 files changed, 29 insertions(+), 112 deletions(-) delete mode 100644 templates/pages/upstream_oauth2/already_linked.html delete mode 100644 templates/pages/upstream_oauth2/do_login.html diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 6320adf8..7d0c0da4 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -33,8 +33,7 @@ use mas_storage::{ BoxClock, BoxRepository, BoxRng, RepositoryAccess, }; use mas_templates::{ - EmptyContext, TemplateContext, Templates, UpstreamExistingLinkContext, UpstreamRegister, - UpstreamSuggestLink, + TemplateContext, Templates, UpstreamExistingLinkContext, UpstreamRegister, UpstreamSuggestLink, }; use serde::Deserialize; use thiserror::Error; @@ -158,7 +157,6 @@ pub(crate) enum FormData { import_display_name: Option, }, Link, - Login, } #[tracing::instrument( @@ -176,10 +174,14 @@ pub(crate) async fn get( Path(link_id): Path, ) -> Result { let sessions_cookie = UpstreamSessionsCookie::load(&cookie_jar); - let (session_id, _post_auth_action) = sessions_cookie + let (session_id, post_auth_action) = sessions_cookie .lookup_link(link_id) .map_err(|_| RouteError::MissingCookie)?; + let post_auth_action = OptionalPostAuthAction { + post_auth_action: post_auth_action.cloned(), + }; + let link = repo .upstream_oauth_link() .lookup(link_id) @@ -206,7 +208,7 @@ pub(crate) async fn get( let (csrf_token, mut cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let maybe_user_session = user_session_info.load_session(&mut repo).await?; - let render = match (maybe_user_session, link.user_id) { + let response = match (maybe_user_session, link.user_id) { (Some(session), Some(user_id)) if session.user.id == user_id => { // Session already linked, and link matches the currently logged // user. Mark the session as consumed and renew the authentication. @@ -222,13 +224,7 @@ pub(crate) async fn get( repo.save().await?; - let ctx = EmptyContext - .with_session(session) - .with_csrf(csrf_token.form_value()); - - templates - .render_upstream_oauth2_already_linked(&ctx) - .await? + post_auth_action.go_next().into_response() } (Some(user_session), Some(user_id)) => { @@ -247,7 +243,7 @@ pub(crate) async fn get( .with_session(user_session) .with_csrf(csrf_token.form_value()); - templates.render_upstream_oauth2_link_mismatch(&ctx).await? + Html(templates.render_upstream_oauth2_link_mismatch(&ctx).await?).into_response() } (Some(user_session), None) => { @@ -256,7 +252,7 @@ pub(crate) async fn get( .with_session(user_session) .with_csrf(csrf_token.form_value()); - templates.render_upstream_oauth2_suggest_link(&ctx).await? + Html(templates.render_upstream_oauth2_suggest_link(&ctx).await?).into_response() } (None, Some(user_id)) => { @@ -268,9 +264,24 @@ pub(crate) async fn get( .filter(mas_data_model::User::is_valid) .ok_or(RouteError::UserNotFound)?; - let ctx = UpstreamExistingLinkContext::new(user).with_csrf(csrf_token.form_value()); + let session = repo.browser_session().add(&mut rng, &clock, &user).await?; - templates.render_upstream_oauth2_do_login(&ctx).await? + repo.upstream_oauth_session() + .consume(&clock, upstream_session) + .await?; + + repo.browser_session() + .authenticate_with_upstream(&mut rng, &clock, &session, &link) + .await?; + + cookie_jar = sessions_cookie + .consume_link(link_id)? + .save(cookie_jar, &clock); + cookie_jar = cookie_jar.set_session(&session); + + repo.save().await?; + + post_auth_action.go_next().into_response() } (None, None) => { @@ -322,11 +333,11 @@ pub(crate) async fn get( let ctx = ctx.with_csrf(csrf_token.form_value()); - templates.render_upstream_oauth2_do_register(&ctx).await? + Html(templates.render_upstream_oauth2_do_register(&ctx).await?).into_response() } }; - Ok((cookie_jar, Html(render))) + Ok((cookie_jar, response)) } #[tracing::instrument( @@ -388,17 +399,6 @@ pub(crate) async fn post( session } - (None, Some(user_id), FormData::Login) => { - let user = repo - .user() - .lookup(user_id) - .await? - .filter(mas_data_model::User::is_valid) - .ok_or(RouteError::UserNotFound)?; - - repo.browser_session().add(&mut rng, &clock, &user).await? - } - ( None, None, diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index 7c7c6830..f4ba609f 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -267,18 +267,12 @@ register_templates! { /// Render the email verification subject pub fn render_email_verification_subject(EmailVerificationContext) { "emails/verification.subject" } - /// Render the upstream already linked message - pub fn render_upstream_oauth2_already_linked(WithCsrf>) { "pages/upstream_oauth2/already_linked.html" } - /// Render the upstream link mismatch message pub fn render_upstream_oauth2_link_mismatch(WithCsrf>) { "pages/upstream_oauth2/link_mismatch.html" } /// Render the upstream suggest link message pub fn render_upstream_oauth2_suggest_link(WithCsrf>) { "pages/upstream_oauth2/suggest_link.html" } - /// Render the upstream login screen - pub fn render_upstream_oauth2_do_login(WithCsrf) { "pages/upstream_oauth2/do_login.html" } - /// Render the upstream register screen pub fn render_upstream_oauth2_do_register(WithCsrf) { "pages/upstream_oauth2/do_register.html" } } @@ -308,10 +302,8 @@ impl Templates { check::render_email_verification_txt(self, now, rng).await?; check::render_email_verification_html(self, now, rng).await?; check::render_email_verification_subject(self, now, rng).await?; - check::render_upstream_oauth2_already_linked(self, now, rng).await?; check::render_upstream_oauth2_link_mismatch(self, now, rng).await?; check::render_upstream_oauth2_suggest_link(self, now, rng).await?; - check::render_upstream_oauth2_do_login(self, now, rng).await?; check::render_upstream_oauth2_do_register(self, now, rng).await?; Ok(()) } diff --git a/templates/pages/upstream_oauth2/already_linked.html b/templates/pages/upstream_oauth2/already_linked.html deleted file mode 100644 index b8d5778f..00000000 --- a/templates/pages/upstream_oauth2/already_linked.html +++ /dev/null @@ -1,26 +0,0 @@ -{# -Copyright 2022 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -#} - -{% extends "base.html" %} - -{% block content %} - {{ navbar::top() }} -
-

- Your upstream account is already linked. -

-
-{% endblock content %} diff --git a/templates/pages/upstream_oauth2/do_login.html b/templates/pages/upstream_oauth2/do_login.html deleted file mode 100644 index 8d5582f9..00000000 --- a/templates/pages/upstream_oauth2/do_login.html +++ /dev/null @@ -1,49 +0,0 @@ -{# -Copyright 2022 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -#} - -{% extends "base.html" %} - -{% block content %} -
-
-

- Continue login -

- -
-
- Username: {{ linked_user.username }} -
-
- Identifier: {{ linked_user.sub }} -
-
- Primary email: - {% if linked_user.primary_email %} - {{ linked_user.primary_email.email }} - {% else %} - none - {% endif %} -
-
- - - - - {{ button::button(text="Continue") }} -
-
-{% endblock content %}