letro/letro-authentication-service
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Avoid using SameSite=None by re-submitting incoming form data

Browse Source
This commit is contained in:
Quentin Gliech
2024-11-21 14:30:08 +01:00
parent f3ed4dab43
commit ec883e15ef
10 changed files with 113 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
crates/axum-utils/src/cookies.rs
View File

@@ -101,15 +101,7 @@ impl CookieOption {
cookie.set_http_only(true);
cookie.set_secure(self.secure());
cookie.set_path(self.path().to_owned());
// The `form_post` callback requires that, as it means a 3rd party origin will
// POST to MAS. This is presumably fine, as our forms are protected with a CSRF
// token
cookie.set_same_site(if self.secure() {
SameSite::None
} else {
SameSite::Lax
});
cookie.set_same_site(SameSite::Lax);
cookie
}
}

4
crates/handlers/src/oauth2/authorization/callback.rs
View File

@@ -10,6 +10,7 @@ use std::collections::HashMap;
use axum::response::{Html, IntoResponse, Redirect, Response};
use mas_data_model::AuthorizationGrant;
use mas_i18n::DataLocale;
use mas_templates::{FormPostContext, Templates};
use oauth2_types::requests::ResponseMode;
use serde::Serialize;
@@ -103,6 +104,7 @@ impl CallbackDestination {
pub async fn go<T: Serialize + Send + Sync>(
self,
templates: &Templates,
locale: &DataLocale,
params: T,
) -> Result<Response, CallbackDestinationError> {
#[derive(Serialize)]
@@ -155,7 +157,7 @@ impl CallbackDestination {
state,
params,
};
let ctx = FormPostContext::new(redirect_uri, merged);
let ctx = FormPostContext::new_for_url(redirect_uri, merged).with_language(locale);
let rendered = templates.render_form_post(&ctx)?;
Ok(Html(rendered).into_response())
}

2
crates/handlers/src/oauth2/authorization/complete.rs
View File

@@ -141,7 +141,7 @@ pub(crate) async fn get(
.await
{
Ok(params) => {
let res = callback_destination.go(&templates, params).await?;
let res = callback_destination.go(&templates, &locale, params).await?;
Ok((cookie_jar, res).into_response())
}
Err(GrantCompletionError::RequiresReauth) => Ok((

23
crates/handlers/src/oauth2/authorization/mod.rs
View File

@@ -170,6 +170,7 @@ pub(crate) async fn get(
let res: Result<Response, RouteError> = ({
let templates = templates.clone();
let callback_destination = callback_destination.clone();
let locale = locale.clone();
async move {
let maybe_session = session_info.load_session(&mut repo).await?;
let prompt = params.auth.prompt.as_deref().unwrap_or_default();
@@ -180,6 +181,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestNotSupported),
)
.await?);
@@ -189,6 +191,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestUriNotSupported),
)
.await?);
@@ -200,6 +203,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnsupportedResponseType),
)
.await?);
@@ -211,6 +215,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)
.await?);
@@ -220,6 +225,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RegistrationNotSupported),
)
.await?);
@@ -230,6 +236,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::LoginRequired),
)
.await?);
@@ -241,6 +248,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)
.await?);
@@ -266,6 +274,7 @@ pub(crate) async fn get(
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::InvalidRequest),
)
.await?);
@@ -350,11 +359,12 @@ pub(crate) async fn get(
)
.await
{
Ok(params) => callback_destination.go(&templates, params).await?,
Ok(params) => callback_destination.go(&templates, &locale, params).await?,
Err(GrantCompletionError::RequiresConsent) => {
callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::ConsentRequired),
)
.await?
@@ -363,13 +373,14 @@ pub(crate) async fn get(
callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::InteractionRequired),
)
.await?
}
Err(GrantCompletionError::PolicyViolation(_grant, _res)) => {
callback_destination
.go(&templates, ClientError::from(ClientErrorCode::AccessDenied))
.go(&templates, &locale, ClientError::from(ClientErrorCode::AccessDenied))
.await?
}
Err(GrantCompletionError::Internal(e)) => {
@@ -400,7 +411,7 @@ pub(crate) async fn get(
)
.await
{
Ok(params) => callback_destination.go(&templates, params).await?,
Ok(params) => callback_destination.go(&templates, &locale, params).await?,
Err(GrantCompletionError::RequiresConsent) => {
url_builder.redirect(&mas_router::Consent(grant_id)).into_response()
}
@@ -440,7 +451,11 @@ pub(crate) async fn get(
Err(err) => {
tracing::error!(%err);
callback_destination
.go(&templates, ClientError::from(ClientErrorCode::ServerError))
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::ServerError),
)
.await?
}
};

38
crates/handlers/src/upstream_oauth2/callback.rs
View File

@@ -6,9 +6,10 @@
use axum::{
extract::{Path, Query, State},
response::IntoResponse,
response::{IntoResponse, Response},
Form,
};
use axum_extra::response::Html;
use hyper::StatusCode;
use mas_axum_utils::{cookies::CookieJar, sentry::SentryEventID};
use mas_data_model::{UpstreamOAuthProvider, UpstreamOAuthProviderResponseMode};
@@ -24,8 +25,9 @@ use mas_storage::{
},
BoxClock, BoxRepository, BoxRng, Clock,
};
use mas_templates::{FormPostContext, Templates};
use oauth2_types::errors::ClientErrorCode;
use serde::Deserialize;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use ulid::Ulid;
@@ -35,9 +37,9 @@ use super::{
template::{environment, AttributeMappingContext},
UpstreamSessionsCookie,
};
use crate::{impl_from_error_for_route, upstream_oauth2::cache::MetadataCache};
use crate::{impl_from_error_for_route, upstream_oauth2::cache::MetadataCache, PreferredLanguage};
#[derive(Deserialize)]
#[derive(Serialize, Deserialize)]
pub struct Params {
state: String,
@@ -45,7 +47,7 @@ pub struct Params {
code_or_error: CodeOrError,
}
#[derive(Deserialize)]
#[derive(Serialize, Deserialize)]
#[serde(untagged)]
enum CodeOrError {
Code {
@@ -115,6 +117,7 @@ pub(crate) enum RouteError {
Internal(Box<dyn std::error::Error>),
}
impl_from_error_for_route!(mas_templates::TemplateError);
impl_from_error_for_route!(mas_storage::RepositoryError);
impl_from_error_for_route!(mas_oidc_client::error::DiscoveryError);
impl_from_error_for_route!(mas_oidc_client::error::JwksError);
@@ -152,11 +155,13 @@ pub(crate) async fn handler(
State(encrypter): State<Encrypter>,
State(keystore): State<Keystore>,
State(client): State<reqwest::Client>,
State(templates): State<Templates>,
PreferredLanguage(locale): PreferredLanguage,
cookie_jar: CookieJar,
Path(provider_id): Path<Ulid>,
query_params: Option<Query<Params>>,
form_params: Option<Form<Params>>,
) -> Result<impl IntoResponse, RouteError> {
) -> Result<Response, RouteError> {
let provider = repo
.upstream_oauth_provider()
.lookup(provider_id)
@@ -164,11 +169,24 @@ pub(crate) async fn handler(
.filter(UpstreamOAuthProvider::enabled)
.ok_or(RouteError::ProviderNotFound)?;
let sessions_cookie = UpstreamSessionsCookie::load(&cookie_jar);
// Read the parameters from the query or the form, depending on what
// response_mode the provider uses
let params = match (provider.response_mode, query_params, form_params) {
(UpstreamOAuthProviderResponseMode::Query, Some(query_params), None) => query_params.0,
(UpstreamOAuthProviderResponseMode::FormPost, None, Some(form_params)) => form_params.0,
(UpstreamOAuthProviderResponseMode::Query, Some(Query(query_params)), None) => query_params,
(UpstreamOAuthProviderResponseMode::FormPost, None, Some(Form(form_params))) => {
// We got there from a cross-site form POST, so we need to render a form with
// the same values, which posts back to the same URL
if sessions_cookie.is_empty() {
let context =
FormPostContext::new_for_current_url(form_params).with_language(&locale);
let html = templates.render_form_post(&context)?;
return Ok(Html(html).into_response());
}
form_params
}
(UpstreamOAuthProviderResponseMode::Query, None, None) => {
return Err(RouteError::MissingQueryParams)
}
@@ -179,7 +197,6 @@ pub(crate) async fn handler(
(expected, _, _) => return Err(RouteError::InvalidParamsMode { expected }),
};
let sessions_cookie = UpstreamSessionsCookie::load(&cookie_jar);
let (session_id, _post_auth_action) = sessions_cookie
.find_session(provider_id, &params.state)
.map_err(|_| RouteError::MissingCookie)?;
@@ -327,5 +344,6 @@ pub(crate) async fn handler(
Ok((
cookie_jar,
url_builder.redirect(&mas_router::UpstreamOAuth2Link::new(link.id)),
))
)
.into_response())
}

5
crates/handlers/src/upstream_oauth2/cookie.rs
View File

@@ -61,6 +61,11 @@ impl UpstreamSessions {
}
}
/// Returns true if the cookie is empty
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}
/// Save the upstreams sessions to the cookie jar
pub fn save<C>(self, cookie_jar: CookieJar, clock: &C) -> CookieJar
where

31
crates/templates/src/context.rs
View File

@@ -1460,7 +1460,7 @@ impl TemplateContext for DeviceConsentContext {
/// Context used by the `form_post.html` template
#[derive(Serialize)]
pub struct FormPostContext<T> {
redirect_uri: Url,
redirect_uri: Option<Url>,
params: T,
}
@@ -1473,7 +1473,7 @@ impl<T: TemplateContext> TemplateContext for FormPostContext<T> {
sample_params
.into_iter()
.map(|params| FormPostContext {
redirect_uri: "https://example.com/callback".parse().unwrap(),
redirect_uri: "https://example.com/callback".parse().ok(),
params,
})
.collect()
@@ -1481,13 +1481,34 @@ impl<T: TemplateContext> TemplateContext for FormPostContext<T> {
}
impl<T> FormPostContext<T> {
/// Constructs a context for the `form_post` response mode form
pub fn new(redirect_uri: Url, params: T) -> Self {
/// Constructs a context for the `form_post` response mode form for a given
/// URL
pub fn new_for_url(redirect_uri: Url, params: T) -> Self {
Self {
redirect_uri,
redirect_uri: Some(redirect_uri),
params,
}
}
/// Constructs a context for the `form_post` response mode form for the
/// current URL
pub fn new_for_current_url(params: T) -> Self {
Self {
redirect_uri: None,
params,
}
}
/// Add the language to the context
///
/// This is usually implemented by the [`TemplateContext`] trait, but it is
/// annoying to make it work because of the generic parameter
pub fn with_language(self, lang: &DataLocale) -> WithLanguage<Self> {
WithLanguage {
lang: lang.to_string(),
inner: self,
}
}
}
/// Context used by the `error.html` template

2
crates/templates/src/lib.rs
View File

@@ -368,7 +368,7 @@ register_templates! {
pub fn render_reauth(WithLanguage<WithCsrf<WithSession<ReauthContext>>>) { "pages/reauth.html" }
/// Render the form used by the form_post response mode
pub fn render_form_post<T: Serialize>(FormPostContext<T>) { "form_post.html" }
pub fn render_form_post<T: Serialize>(WithLanguage<FormPostContext<T>>) { "form_post.html" }
/// Render the HTML error page
pub fn render_error(ErrorContext) { "pages/error.html" }

39
templates/form_post.html
View File

@@ -6,18 +6,27 @@ SPDX-License-Identifier: AGPL-3.0-only
Please see LICENSE in the repository root for full details.
-#}
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Redirecting to client</title>
<meta name="viewport" content="width=device-width, initial-scale=1">
</head>
<body onload="javascript:document.forms[0].submit()">
<form method="post" action="{{ redirect_uri }}">
{% for key, value in params|items %}
<input type="hidden" name="{{ key }}" value="{{ value }}" />
{% endfor %}
</form>
</body>
</html>
{% extends "base.html" %}
{% block content %}
<header class="page-heading">
<div class="header">
<h1 class="title">{{ _("common.loading") }}</h1>
</div>
</header>
<form method="post" class="flex flex-col"{% if redirect_uri %} action="{{ redirect_uri }}"{% endif %}>
{% for key, value in params|items %}
<input type="hidden" name="{{ key }}" value="{{ value }}" />
{% endfor %}
<noscript>
{# Display a button to submit the form in case JavaScript is disabled #}
{{ button.button(text=_("action.continue")) }}
</noscript>
</form>
{# Submit the form in JavaScript on the next tick, so that if the browser
wants to display the placeholder instead of a blank page, it can #}
<script>setTimeout(function() { document.forms[0].submit(); }, 0);</script>
{% endblock %}

6
translations/en.json
View File

@@ -10,7 +10,7 @@
},
"continue": "Continue",
"@continue": {
"context": "pages/account/emails/add.html:37:26-46, pages/account/emails/verify.html:52:26-46, pages/consent.html:55:28-48, pages/device_consent.html:121:13-33, pages/device_link.html:40:26-46, pages/login.html:58:30-50, pages/reauth.html:32:28-48, pages/recovery/start.html:38:26-46, pages/register.html:76:28-48, pages/sso.html:37:28-48"
"context": "form_post.html:25:28-48, pages/account/emails/add.html:37:26-46, pages/account/emails/verify.html:52:26-46, pages/consent.html:55:28-48, pages/device_consent.html:121:13-33, pages/device_link.html:40:26-46, pages/login.html:58:30-50, pages/reauth.html:32:28-48, pages/recovery/start.html:38:26-46, pages/register.html:76:28-48, pages/sso.html:37:28-48"
},
"create_account": "Create Account",
"@create_account": {
@@ -77,6 +77,10 @@
"@email_address": {
"context": "pages/account/emails/add.html:33:33-58, pages/recovery/start.html:34:33-58, pages/register.html:40:35-60, pages/upstream_oauth2/do_register.html:79:37-62"
},
"loading": "Loading…",
"@loading": {
"context": "form_post.html:14:27-46"
},
"mxid": "Matrix ID",
"@mxid": {
"context": "pages/upstream_oauth2/do_register.html:58:35-51"