Skip to content

Commit

Permalink
Merge pull request #41 from pubky/fix/detect-https-for-cookies
Browse files Browse the repository at this point in the history
fix(homeserver): detect requests in secure context for cookies
  • Loading branch information
Nuhvi authored Oct 3, 2024
2 parents 83ad51e + b543ffb commit 3d8d8a8
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 6 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pubky-homeserver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ tower-cookies = "0.10.0"
tower-http = { version = "0.5.2", features = ["cors", "trace"] }
tracing = "0.1.40"
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
url = "2.5.2"
47 changes: 41 additions & 6 deletions pubky-homeserver/src/routes/auth.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use axum::{
debug_handler,
extract::State,
http::{uri::Scheme, StatusCode, Uri},
extract::{Host, State},
http::StatusCode,
response::IntoResponse,
};
use axum_extra::{headers::UserAgent, TypedHeader};
Expand All @@ -25,12 +25,12 @@ pub async fn signup(
State(state): State<AppState>,
user_agent: Option<TypedHeader<UserAgent>>,
cookies: Cookies,
uri: Uri,
host: Host,
body: Bytes,
) -> Result<impl IntoResponse> {
// TODO: Verify invitation link.
// TODO: add errors in case of already axisting user.
signin(State(state), user_agent, cookies, uri, body).await
signin(State(state), user_agent, cookies, host, body).await
}

pub async fn session(
Expand Down Expand Up @@ -89,7 +89,7 @@ pub async fn signin(
State(state): State<AppState>,
user_agent: Option<TypedHeader<UserAgent>>,
cookies: Cookies,
uri: Uri,
Host(host): Host,
body: Bytes,
) -> Result<impl IntoResponse> {
let token = state.verifier.verify(&body)?;
Expand Down Expand Up @@ -124,7 +124,8 @@ pub async fn signin(
let mut cookie = Cookie::new(public_key.to_string(), session_secret);

cookie.set_path("/");
if *uri.scheme().unwrap_or(&Scheme::HTTP) == Scheme::HTTPS {

if is_secure(&host) {
cookie.set_secure(true);
cookie.set_same_site(SameSite::None);
}
Expand All @@ -136,3 +137,37 @@ pub async fn signin(

Ok(session)
}

/// Assuming that if the server is addressed by anything other than
/// localhost, or IP addresses, it is not addressed from a browser in an
/// secure (HTTPs) window, thus it no need to `secure` and `same_site=none` to cookies
fn is_secure(host: &str) -> bool {
url::Host::parse(host)
.map(|host| match host {
url::Host::Domain(domain) => domain != "localhost",
_ => false,
})
.unwrap_or_default()
}

#[cfg(test)]
mod tests {
use pkarr::Keypair;

use super::*;

#[test]
fn test_is_secure() {
assert_eq!(is_secure(""), false);
assert_eq!(is_secure("127.0.0.1"), false);
assert_eq!(is_secure("167.86.102.121"), false);
assert_eq!(
is_secure("[2001:0db8:0000:0000:0000:ff00:0042:8329]"),
false
);
assert_eq!(is_secure("localhost"), false);
assert_eq!(is_secure("localhost:23423"), false);
assert_eq!(is_secure(&Keypair::random().public_key().to_string()), true);
assert_eq!(is_secure("example.com"), true);
}
}

0 comments on commit 3d8d8a8

Please sign in to comment.