fix: close remaining Workspace auth gaps
This commit is contained in:
@@ -176,15 +176,27 @@ fn actor_for_user<S: ControlPlaneStore + ?Sized>(
|
|||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
pub struct SessionCookiePolicy<'a> {
|
||||||
|
pub cookie_name: &'a str,
|
||||||
|
pub path: &'a str,
|
||||||
|
pub domain: Option<&'a str>,
|
||||||
|
pub secure: bool,
|
||||||
|
}
|
||||||
|
|
||||||
pub fn session_set_cookie(
|
pub fn session_set_cookie(
|
||||||
cookie_name: &str,
|
policy: SessionCookiePolicy<'_>,
|
||||||
token: &str,
|
token: &str,
|
||||||
max_age_seconds: i64,
|
max_age_seconds: i64,
|
||||||
secure: bool,
|
|
||||||
) -> String {
|
) -> String {
|
||||||
let secure = if secure { "; Secure" } else { "" };
|
let domain = policy
|
||||||
|
.domain
|
||||||
|
.map(|domain| format!("; Domain={domain}"))
|
||||||
|
.unwrap_or_default();
|
||||||
|
let secure = if policy.secure { "; Secure" } else { "" };
|
||||||
format!(
|
format!(
|
||||||
"{cookie_name}={token}; Max-Age={max_age_seconds}; Path=/; HttpOnly; SameSite=Lax{secure}"
|
"{}={token}; Max-Age={max_age_seconds}; Path={}; HttpOnly; SameSite=Lax{domain}{secure}",
|
||||||
|
policy.cookie_name, policy.path
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -63,9 +63,9 @@ use workspace_api::{
|
|||||||
};
|
};
|
||||||
|
|
||||||
use crate::auth::{
|
use crate::auth::{
|
||||||
AuthPublicConfig, AuthenticatedUser, RequestActor, auth_error, is_expired, mint_secret, new_id,
|
ActorAuthMethod, AuthPublicConfig, AuthenticatedUser, RequestActor, SessionCookiePolicy,
|
||||||
new_user_code, normalize_handle, parse_cookie, resolve_request_actor, rfc3339_after,
|
auth_error, is_expired, mint_secret, new_id, new_user_code, normalize_handle, parse_cookie,
|
||||||
session_set_cookie, token_hash,
|
resolve_request_actor, rfc3339_after, session_set_cookie, token_hash,
|
||||||
};
|
};
|
||||||
use crate::authority::{
|
use crate::authority::{
|
||||||
MemoryAuthority, ObjectiveAuthority, ObjectiveCreateInput, ObjectiveEditInput,
|
MemoryAuthority, ObjectiveAuthority, ObjectiveCreateInput, ObjectiveEditInput,
|
||||||
@@ -837,7 +837,11 @@ async fn list_server_workspaces(
|
|||||||
) -> Response {
|
) -> Response {
|
||||||
let owner = match resolve_server_actor(&api, &headers).await {
|
let owner = match resolve_server_actor(&api, &headers).await {
|
||||||
Ok(Some(actor)) => Some(actor.account_id),
|
Ok(Some(actor)) => Some(actor.account_id),
|
||||||
Ok(None) => None,
|
Ok(None) => match api.catalog.list(None, 1) {
|
||||||
|
Ok(workspaces) if workspaces.is_empty() => return Json(workspaces).into_response(),
|
||||||
|
Ok(_) => return StatusCode::UNAUTHORIZED.into_response(),
|
||||||
|
Err(error) => return server_error_response(error),
|
||||||
|
},
|
||||||
Err(error) => return server_error_response(error),
|
Err(error) => return server_error_response(error),
|
||||||
};
|
};
|
||||||
match api
|
match api
|
||||||
@@ -936,15 +940,10 @@ async fn authorize_scoped_workspace_request(
|
|||||||
|
|
||||||
let actor = resolve_server_actor(api, request.headers())
|
let actor = resolve_server_actor(api, request.headers())
|
||||||
.await
|
.await
|
||||||
.map_err(server_error_response)?;
|
.map_err(server_error_response)?
|
||||||
if actor.is_none() {
|
.ok_or_else(|| StatusCode::UNAUTHORIZED.into_response())?;
|
||||||
return Err(StatusCode::UNAUTHORIZED.into_response());
|
|
||||||
}
|
|
||||||
|
|
||||||
let cookie_authenticated = request
|
let cookie_authenticated = matches!(actor.auth_method, ActorAuthMethod::BrowserSession);
|
||||||
.headers()
|
|
||||||
.get(axum::http::header::AUTHORIZATION)
|
|
||||||
.is_none();
|
|
||||||
let mutating = !matches!(
|
let mutating = !matches!(
|
||||||
*request.method(),
|
*request.method(),
|
||||||
Method::GET | Method::HEAD | Method::OPTIONS
|
Method::GET | Method::HEAD | Method::OPTIONS
|
||||||
@@ -1020,16 +1019,10 @@ async fn authorize_workspace_api_request(
|
|||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(actor) => actor,
|
Ok(Some(actor)) => actor,
|
||||||
Err(_) => return StatusCode::UNAUTHORIZED.into_response(),
|
Ok(None) | Err(_) => return StatusCode::UNAUTHORIZED.into_response(),
|
||||||
};
|
};
|
||||||
if actor.is_none() {
|
let cookie_authenticated = matches!(actor.auth_method, ActorAuthMethod::BrowserSession);
|
||||||
return StatusCode::UNAUTHORIZED.into_response();
|
|
||||||
}
|
|
||||||
let cookie_authenticated = request
|
|
||||||
.headers()
|
|
||||||
.get(axum::http::header::AUTHORIZATION)
|
|
||||||
.is_none();
|
|
||||||
let mutating = !matches!(
|
let mutating = !matches!(
|
||||||
*request.method(),
|
*request.method(),
|
||||||
Method::GET | Method::HEAD | Method::OPTIONS
|
Method::GET | Method::HEAD | Method::OPTIONS
|
||||||
@@ -9268,10 +9261,9 @@ fn issue_browser_session_response(api: &ServerAuthApi, user: UserRecord) -> ApiR
|
|||||||
headers.insert(
|
headers.insert(
|
||||||
SET_COOKIE,
|
SET_COOKIE,
|
||||||
session_set_cookie(
|
session_set_cookie(
|
||||||
&auth.cookie_name,
|
session_cookie_policy(&auth),
|
||||||
&session_token,
|
&session_token,
|
||||||
14 * 24 * 60 * 60,
|
14 * 24 * 60 * 60,
|
||||||
session_cookie_is_secure(&auth),
|
|
||||||
)
|
)
|
||||||
.parse()
|
.parse()
|
||||||
.map_err(|error| {
|
.map_err(|error| {
|
||||||
@@ -9448,7 +9440,7 @@ async fn post_auth_logout(
|
|||||||
let mut response_headers = HeaderMap::new();
|
let mut response_headers = HeaderMap::new();
|
||||||
response_headers.insert(
|
response_headers.insert(
|
||||||
SET_COOKIE,
|
SET_COOKIE,
|
||||||
session_set_cookie(&auth.cookie_name, "", 0, session_cookie_is_secure(&auth))
|
session_set_cookie(session_cookie_policy(&auth), "", 0)
|
||||||
.parse()
|
.parse()
|
||||||
.map_err(|error| {
|
.map_err(|error| {
|
||||||
auth_error(
|
auth_error(
|
||||||
@@ -9550,11 +9542,17 @@ fn passkey_credential_id(passkey: &Passkey) -> ApiResult<String> {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
fn session_cookie_is_secure(auth: &AuthPublicConfig) -> bool {
|
fn session_cookie_policy(auth: &AuthPublicConfig) -> SessionCookiePolicy<'_> {
|
||||||
[&auth.origin, &auth.public_base_url]
|
let secure = [&auth.origin, &auth.public_base_url]
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter_map(|url| reqwest::Url::parse(url).ok())
|
.filter_map(|url| reqwest::Url::parse(url).ok())
|
||||||
.any(|url| url.scheme() == "https")
|
.any(|url| url.scheme() == "https");
|
||||||
|
SessionCookiePolicy {
|
||||||
|
cookie_name: &auth.cookie_name,
|
||||||
|
path: "/",
|
||||||
|
domain: None,
|
||||||
|
secure,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn auth_public_config(config: &ServerConfig) -> AuthPublicConfig {
|
fn auth_public_config(config: &ServerConfig) -> AuthPublicConfig {
|
||||||
@@ -15490,6 +15488,23 @@ mod tests {
|
|||||||
let app = build_workspace_server_router(template, store)
|
let app = build_workspace_server_router(template, store)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
let empty_catalog = app
|
||||||
|
.clone()
|
||||||
|
.oneshot(
|
||||||
|
Request::builder()
|
||||||
|
.uri("/api/workspaces")
|
||||||
|
.body(Body::empty())
|
||||||
|
.unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(empty_catalog.status(), StatusCode::OK);
|
||||||
|
assert_eq!(
|
||||||
|
to_bytes(empty_catalog.into_body(), usize::MAX)
|
||||||
|
.await
|
||||||
|
.unwrap(),
|
||||||
|
"[]"
|
||||||
|
);
|
||||||
|
|
||||||
for (uri, expected) in [
|
for (uri, expected) in [
|
||||||
("/", "<main>Workspace chooser</main>"),
|
("/", "<main>Workspace chooser</main>"),
|
||||||
@@ -15665,6 +15680,30 @@ mod tests {
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(authenticated_legacy.status(), StatusCode::OK);
|
assert_eq!(authenticated_legacy.status(), StatusCode::OK);
|
||||||
|
|
||||||
|
let anonymous_catalog = app
|
||||||
|
.clone()
|
||||||
|
.oneshot(
|
||||||
|
Request::builder()
|
||||||
|
.uri("/api/workspaces")
|
||||||
|
.body(Body::empty())
|
||||||
|
.unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(anonymous_catalog.status(), StatusCode::UNAUTHORIZED);
|
||||||
|
let authenticated_catalog = app
|
||||||
|
.clone()
|
||||||
|
.oneshot(
|
||||||
|
Request::builder()
|
||||||
|
.uri("/api/workspaces")
|
||||||
|
.header(axum::http::header::AUTHORIZATION, "Bearer api-token-auth")
|
||||||
|
.body(Body::empty())
|
||||||
|
.unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(authenticated_catalog.status(), StatusCode::OK);
|
||||||
|
|
||||||
let ws_uri = format!("/api/w/{}/protocol/ws", workspace.workspace.workspace_id);
|
let ws_uri = format!("/api/w/{}/protocol/ws", workspace.workspace.workspace_id);
|
||||||
let anonymous_ws = app
|
let anonymous_ws = app
|
||||||
.clone()
|
.clone()
|
||||||
@@ -15720,6 +15759,25 @@ mod tests {
|
|||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(csrf_rejected.status(), StatusCode::FORBIDDEN);
|
assert_eq!(csrf_rejected.status(), StatusCode::FORBIDDEN);
|
||||||
|
|
||||||
|
let mixed_auth_csrf_rejected = app
|
||||||
|
.clone()
|
||||||
|
.oneshot(
|
||||||
|
Request::builder()
|
||||||
|
.method(Method::PUT)
|
||||||
|
.uri(&settings_uri)
|
||||||
|
.header(
|
||||||
|
axum::http::header::COOKIE,
|
||||||
|
"yoi_workspace_session=browser-session-auth",
|
||||||
|
)
|
||||||
|
.header(axum::http::header::AUTHORIZATION, "Bearer invalid-token")
|
||||||
|
.header(CONTENT_TYPE, "application/json")
|
||||||
|
.body(Body::from(r#"{"display_name":"Renamed"}"#))
|
||||||
|
.unwrap(),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(mixed_auth_csrf_rejected.status(), StatusCode::FORBIDDEN);
|
||||||
|
|
||||||
let csrf_accepted = app
|
let csrf_accepted = app
|
||||||
.oneshot(
|
.oneshot(
|
||||||
Request::builder()
|
Request::builder()
|
||||||
@@ -15771,6 +15829,10 @@ mod tests {
|
|||||||
assert_eq!(logout_cookie.contains("; Secure"), secure, "{scheme}");
|
assert_eq!(logout_cookie.contains("; Secure"), secure, "{scheme}");
|
||||||
assert!(logout_cookie.contains("Max-Age=0"));
|
assert!(logout_cookie.contains("Max-Age=0"));
|
||||||
assert!(logout_cookie.contains("; HttpOnly; SameSite=Lax"));
|
assert!(logout_cookie.contains("; HttpOnly; SameSite=Lax"));
|
||||||
|
for cookie in [login_cookie, logout_cookie] {
|
||||||
|
assert!(cookie.contains("; Path=/"));
|
||||||
|
assert!(!cookie.contains("; Domain="));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -19224,6 +19286,45 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn runtime_request_proof_rejects_trust_bound_to_another_workspace() {
|
||||||
|
let workspace = tempfile::tempdir().unwrap();
|
||||||
|
let mut api = test_api(workspace.path()).await;
|
||||||
|
let identity =
|
||||||
|
worker_runtime::auth::RuntimeIdentityMaterial::generate("runtime-test").unwrap();
|
||||||
|
configure_runtime_request_auth(&mut api, &identity, "runtime-test");
|
||||||
|
let other_workspace = "019d0000-0000-7000-8000-0000000000bb";
|
||||||
|
let path = format!("/api/runtime/v1/workspaces/{other_workspace}/resources/fetch");
|
||||||
|
let proof = worker_runtime::auth::RuntimeRequestSourceSigner::from_identity(&identity)
|
||||||
|
.issue(
|
||||||
|
"server-test",
|
||||||
|
other_workspace,
|
||||||
|
None,
|
||||||
|
worker_runtime::auth::BACKEND_RESOURCE_FETCH_PERMISSION,
|
||||||
|
"POST",
|
||||||
|
&path,
|
||||||
|
b"{}",
|
||||||
|
i64::try_from(worker_runtime::auth::unix_now_seconds()).unwrap_or(i64::MAX),
|
||||||
|
30,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
let result = crate::worker_source::verify_runtime_request_source_proof_with_store(
|
||||||
|
api.store.as_ref(),
|
||||||
|
&api.config,
|
||||||
|
&proof,
|
||||||
|
other_workspace,
|
||||||
|
worker_runtime::auth::BACKEND_RESOURCE_FETCH_PERMISSION,
|
||||||
|
"POST",
|
||||||
|
&path,
|
||||||
|
&worker_runtime::auth::request_body_digest(b"{}"),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
assert!(matches!(
|
||||||
|
result,
|
||||||
|
Err(crate::worker_source::WorkerMutationSourceProofError::WrongWorkspace)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn internal_resource_fetch_rest_returns_typed_missing_resource() {
|
async fn internal_resource_fetch_rest_returns_typed_missing_resource() {
|
||||||
let workspace = tempfile::tempdir().unwrap();
|
let workspace = tempfile::tempdir().unwrap();
|
||||||
|
|||||||
@@ -62,6 +62,11 @@ pub async fn verify_runtime_request_source_proof_with_store(
|
|||||||
.map_err(|error| WorkerMutationSourceProofError::Authority(error.to_string()))?
|
.map_err(|error| WorkerMutationSourceProofError::Authority(error.to_string()))?
|
||||||
.filter(|record| record.revoked_at.is_none())
|
.filter(|record| record.revoked_at.is_none())
|
||||||
.ok_or(WorkerMutationSourceProofError::RevokedRuntimeTrust)?;
|
.ok_or(WorkerMutationSourceProofError::RevokedRuntimeTrust)?;
|
||||||
|
let trusted_for_workspace = trusted.workspace_id.as_deref() == Some(workspace_id)
|
||||||
|
|| (unverified.iss == crate::hosts::EMBEDDED_RUNTIME_ID && trusted.workspace_id.is_none());
|
||||||
|
if !trusted_for_workspace {
|
||||||
|
return Err(WorkerMutationSourceProofError::WrongWorkspace);
|
||||||
|
}
|
||||||
let expected = RuntimeRequestSourceExpectation {
|
let expected = RuntimeRequestSourceExpectation {
|
||||||
identity_id: &unverified.iss,
|
identity_id: &unverified.iss,
|
||||||
audience: audience.as_ref(),
|
audience: audience.as_ref(),
|
||||||
@@ -203,6 +208,9 @@ async fn verify_worker_remove_source_with(
|
|||||||
.map_err(|error| WorkerMutationSourceProofError::Authority(error.to_string()))?
|
.map_err(|error| WorkerMutationSourceProofError::Authority(error.to_string()))?
|
||||||
.filter(|record| record.revoked_at.is_none())
|
.filter(|record| record.revoked_at.is_none())
|
||||||
.ok_or(WorkerMutationSourceProofError::RevokedRuntimeTrust)?;
|
.ok_or(WorkerMutationSourceProofError::RevokedRuntimeTrust)?;
|
||||||
|
if trusted.workspace_id.as_deref() != Some(config.workspace_id.as_str()) {
|
||||||
|
return Err(WorkerMutationSourceProofError::WrongWorkspace);
|
||||||
|
}
|
||||||
let expected = WorkerMutationSourceExpectation {
|
let expected = WorkerMutationSourceExpectation {
|
||||||
runtime_id: &unverified.iss,
|
runtime_id: &unverified.iss,
|
||||||
audience: audience.as_ref(),
|
audience: audience.as_ref(),
|
||||||
|
|||||||
Reference in New Issue
Block a user