Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Commit

Permalink
Implement MSC2965 action parameter (#1673)
Browse files Browse the repository at this point in the history
* redirect session_end action to session detail

* fix react key warning in oauth session detail

* move Route type to /routing

* test getRouteActionRedirection

* comment

* frontend: Split the routing-related stuff in multiple files under routing/

* frontend: Cover all the redirections defined by MSC2965

* frontend: fix test

* Make the backend keep query parameters through login to the /account/ interface

* Fix frontend tests & clippy lints

---------

Co-authored-by: Quentin Gliech <quenting@element.io>
  • Loading branch information
Kerry and sandhose committed Sep 1, 2023
1 parent be5b527 commit 17f8dc4
Show file tree
Hide file tree
Showing 37 changed files with 662 additions and 328 deletions.
19 changes: 16 additions & 3 deletions crates/handlers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
clippy::let_with_type_underscore,
)]

use std::{convert::Infallible, time::Duration};
use std::{borrow::Cow, convert::Infallible, time::Duration};

use axum::{
body::{Bytes, HttpBody},
extract::{FromRef, FromRequestParts, OriginalUri, State},
extract::{FromRef, FromRequestParts, OriginalUri, RawQuery, State},
http::Method,
response::{Html, IntoResponse},
routing::{get, on, post, MethodFilter},
Expand Down Expand Up @@ -265,6 +265,7 @@ where
)
}

#[allow(clippy::too_many_lines)]
pub fn human_router<S, B>(templates: Templates) -> Router<S, B>
where
B: HttpBody + Send + 'static,
Expand All @@ -286,7 +287,19 @@ where
{
Router::new()
// XXX: hard-coded redirect from /account to /account/
.route("/account", get(|| async { mas_router::Account.go() }))
.route(
"/account",
get(|RawQuery(query): RawQuery| async {
let route = mas_router::Account::route();
let destination = if let Some(query) = query {
Cow::Owned(format!("{route}?{query}"))
} else {
Cow::Borrowed(route)
};

axum::response::Redirect::to(&destination)
}),
)
.route(mas_router::Account::route(), get(self::views::app::get))
.route(
mas_router::AccountWildcard::route(),
Expand Down
2 changes: 1 addition & 1 deletion crates/handlers/src/views/account/emails/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub(crate) async fn post(

next.go()
} else {
query.go_next_or_default(&mas_router::Account)
query.go_next_or_default(&mas_router::Account::default())
};

repo.save().await?;
Expand Down
4 changes: 2 additions & 2 deletions crates/handlers/src/views/account/emails/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub(crate) async fn get(

if user_email.confirmed_at.is_some() {
// This email was already verified, skip
let destination = query.go_next_or_default(&mas_router::Account);
let destination = query.go_next_or_default(&mas_router::Account::default());
return Ok((cookie_jar, destination).into_response());
}

Expand Down Expand Up @@ -145,6 +145,6 @@ pub(crate) async fn post(

repo.save().await?;

let destination = query.go_next_or_default(&mas_router::Account);
let destination = query.go_next_or_default(&mas_router::Account::default());
Ok((cookie_jar, destination).into_response())
}
2 changes: 1 addition & 1 deletion crates/handlers/src/views/account/password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub(crate) async fn get(
) -> Result<Response, FancyError> {
// If the password manager is disabled, we can go back to the account page.
if !password_manager.is_enabled() {
return Ok(mas_router::Account.go().into_response());
return Ok(mas_router::Account::default().go().into_response());
}

let (session_info, cookie_jar) = cookie_jar.session_info();
Expand Down
8 changes: 5 additions & 3 deletions crates/handlers/src/views/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

use axum::{
extract::State,
extract::{Query, State},
response::{Html, IntoResponse},
};
use mas_axum_utils::{cookies::CookieJar, FancyError, SessionInfoExt};
Expand All @@ -24,17 +24,19 @@ use mas_templates::{AppContext, Templates};
#[tracing::instrument(name = "handlers.views.app.get", skip_all, err)]
pub async fn get(
State(templates): State<Templates>,
action: Option<Query<mas_router::AccountAction>>,
mut repo: BoxRepository,
cookie_jar: CookieJar,
) -> Result<impl IntoResponse, FancyError> {
let (session_info, cookie_jar) = cookie_jar.session_info();
let session = session_info.load_session(&mut repo).await?;
let action = action.map(|Query(a)| a);

// TODO: keep the full path
// TODO: keep the full path, not just the action
if session.is_none() {
return Ok((
cookie_jar,
mas_router::Login::and_then(PostAuthAction::ManageAccount).go(),
mas_router::Login::and_then(PostAuthAction::manage_account(action)).go(),
)
.into_response());
}
Expand Down
2 changes: 1 addition & 1 deletion crates/handlers/src/views/reauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) async fn get(
) -> Result<Response, FancyError> {
if !password_manager.is_enabled() {
// XXX: do something better here
return Ok(mas_router::Account.go().into_response());
return Ok(mas_router::Account::default().go().into_response());
}

let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng);
Expand Down
2 changes: 1 addition & 1 deletion crates/handlers/src/views/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl OptionalPostAuthAction {
PostAuthContextInner::LinkUpstream { provider, link }
}

PostAuthAction::ManageAccount => PostAuthContextInner::ManageAccount,
PostAuthAction::ManageAccount { .. } => PostAuthContextInner::ManageAccount,
};

Ok(Some(PostAuthContext {
Expand Down
53 changes: 45 additions & 8 deletions crates/router/src/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,20 @@ pub use crate::traits::*;
#[derive(Deserialize, Serialize, Clone, Debug)]
#[serde(rename_all = "snake_case", tag = "next")]
pub enum PostAuthAction {
ContinueAuthorizationGrant { id: Ulid },
ContinueCompatSsoLogin { id: Ulid },
ContinueAuthorizationGrant {
id: Ulid,
},
ContinueCompatSsoLogin {
id: Ulid,
},
ChangePassword,
LinkUpstream { id: Ulid },
ManageAccount,
LinkUpstream {
id: Ulid,
},
ManageAccount {
#[serde(flatten)]
action: Option<AccountAction>,
},
}

impl PostAuthAction {
Expand All @@ -43,13 +52,21 @@ impl PostAuthAction {
PostAuthAction::LinkUpstream { id }
}

#[must_use]
pub const fn manage_account(action: Option<AccountAction>) -> Self {
PostAuthAction::ManageAccount { action }
}

pub fn go_next(&self) -> axum::response::Redirect {
match self {
Self::ContinueAuthorizationGrant { id } => ContinueAuthorizationGrant(*id).go(),
Self::ContinueCompatSsoLogin { id } => CompatLoginSsoComplete::new(*id, None).go(),
Self::ChangePassword => AccountPassword.go(),
Self::LinkUpstream { id } => UpstreamOAuth2Link::new(*id).go(),
Self::ManageAccount => Account.go(),
Self::ManageAccount { action } => Account {
action: action.clone(),
}
.go(),
}
}
}
Expand Down Expand Up @@ -406,12 +423,32 @@ impl AccountAddEmail {
}
}

/// Actions parameters as defined by MSC2965
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "snake_case", tag = "action")]
pub enum AccountAction {
Profile,
SessionsList,
SessionView { device_id: String },
SessionEnd { device_id: String },
}

/// `GET /account/`
#[derive(Default, Debug, Clone)]
pub struct Account;
pub struct Account {
action: Option<AccountAction>,
}

impl SimpleRoute for Account {
const PATH: &'static str = "/account/";
impl Route for Account {
type Query = AccountAction;

fn route() -> &'static str {
"/account/"
}

fn query(&self) -> Option<&Self::Query> {
self.action.as_ref()
}
}

/// `GET /account/*`
Expand Down
Loading

0 comments on commit 17f8dc4

Please sign in to comment.