diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2024-10-29 23:29:22 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2024-10-29 23:29:22 -0400 |
| commit | 66d3fcf2e22f057bacce8d97d43a13c1c5a9ad09 (patch) | |
| tree | 60995943e14a6568cf2b37622ce97df121865a6d | |
| parent | e328d33fc7d6a0f2e3d260d8bddee3ef633318eb (diff) | |
Add `change password` UI + API.
The protocol here re-checks the caller's password, as a "I left myself logged in" anti-pranking check.
22 files changed, 517 insertions, 15 deletions
diff --git a/.sqlx/query-0b1543ec93e02c48c5cbaafd391b5812dc2d1d4a52ea3072b5dd52d71637b33d.json b/.sqlx/query-0b1543ec93e02c48c5cbaafd391b5812dc2d1d4a52ea3072b5dd52d71637b33d.json new file mode 100644 index 0000000..937b07e --- /dev/null +++ b/.sqlx/query-0b1543ec93e02c48c5cbaafd391b5812dc2d1d4a52ea3072b5dd52d71637b33d.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "\n delete\n from token\n where login = $1\n returning id as \"id: Id\"\n ", + "describe": { + "columns": [ + { + "name": "id: Id", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "0b1543ec93e02c48c5cbaafd391b5812dc2d1d4a52ea3072b5dd52d71637b33d" +} diff --git a/.sqlx/query-0f3bfb1ad8fad5213f733b32d8eb2d9c2bb4de2fbbf0b2280973966ef02f72b1.json b/.sqlx/query-0f3bfb1ad8fad5213f733b32d8eb2d9c2bb4de2fbbf0b2280973966ef02f72b1.json new file mode 100644 index 0000000..ffd81dc --- /dev/null +++ b/.sqlx/query-0f3bfb1ad8fad5213f733b32d8eb2d9c2bb4de2fbbf0b2280973966ef02f72b1.json @@ -0,0 +1,50 @@ +{ + "db_name": "SQLite", + "query": "\n select\n id as \"id: login::Id\",\n display_name as \"display_name: String\",\n canonical_name as \"canonical_name: String\",\n created_sequence as \"created_sequence: Sequence\",\n created_at as \"created_at: DateTime\",\n password_hash as \"password_hash: StoredHash\"\n from login\n where id = $1\n ", + "describe": { + "columns": [ + { + "name": "id: login::Id", + "ordinal": 0, + "type_info": "Text" + }, + { + "name": "display_name: String", + "ordinal": 1, + "type_info": "Text" + }, + { + "name": "canonical_name: String", + "ordinal": 2, + "type_info": "Text" + }, + { + "name": "created_sequence: Sequence", + "ordinal": 3, + "type_info": "Integer" + }, + { + "name": "created_at: DateTime", + "ordinal": 4, + "type_info": "Text" + }, + { + "name": "password_hash: StoredHash", + "ordinal": 5, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false, + false, + false, + false, + false + ] + }, + "hash": "0f3bfb1ad8fad5213f733b32d8eb2d9c2bb4de2fbbf0b2280973966ef02f72b1" +} diff --git a/.sqlx/query-c2b0ff7e2f27b6970a16fbc233ed32638e853e3b8b8f8de26b53f90c98b6ce11.json b/.sqlx/query-c2b0ff7e2f27b6970a16fbc233ed32638e853e3b8b8f8de26b53f90c98b6ce11.json new file mode 100644 index 0000000..4c99c42 --- /dev/null +++ b/.sqlx/query-c2b0ff7e2f27b6970a16fbc233ed32638e853e3b8b8f8de26b53f90c98b6ce11.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "\n update login\n set password_hash = $1\n where id = $2\n returning id as \"id: Id\"\n ", + "describe": { + "columns": [ + { + "name": "id: Id", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + false + ] + }, + "hash": "c2b0ff7e2f27b6970a16fbc233ed32638e853e3b8b8f8de26b53f90c98b6ce11" +} diff --git a/docs/api/authentication.md b/docs/api/authentication.md index 135e91b..93a8e52 100644 --- a/docs/api/authentication.md +++ b/docs/api/authentication.md @@ -113,3 +113,50 @@ The request must be an empty JSON object. This endpoint will respond with a status of `204 No Content` when successful. The response will include a `Set-Cookie` header that clears the `identity` cookie. Regardless of whether the client clears the cookie, the service also invalidates the token. + + +## `POST /api/password` + +Changes the current login's password, and invalidate all outstanding identity tokens. + +### Request + +```json +{ + "password": "my-old-password", + "to": "my-new-password" +} +``` + +The request must have the following fields: + +| Field | Type | Description | +|:-----------|:-------|:--| +| `password` | string | The login's _current_ password, in plain text. | +| `to` | string | The login's _new_ password, in plain text. | + +### Success + +This endpoint will respond with a status of `200 Okay` when successful. The body of the response will be a JSON object describing the authenticated login: + +```json +{ + "id": "Labcd1234", + "name": "Andrea" +} +``` + +The response will include the following fields: + +| Field | Type | Description | +|:------------|:-------|:--| +| `id` | string | The authenticated login's ID. | +| `name` | string | The authenticated login's name. | + +The response will include a `Set-Cookie` header for the `identity` cookie, providing the client with a newly-minted identity token associated with the login identified in the request. This token's value must be kept confidential. All previously-created identity cookies will cease to be valid. + +The cookie will expire if it is not used regularly. + +### Authentication failure + +This endpoint will respond with a status of `400 Bad Request` if the `password` does not match the login's current password. diff --git a/src/event/routes/test/token.rs b/src/event/routes/test/token.rs index 2039d9b..16ac7c3 100644 --- a/src/event/routes/test/token.rs +++ b/src/event/routes/test/token.rs @@ -93,3 +93,52 @@ async fn terminates_on_logout() { .expect_none("end of stream") .await; } + +#[tokio::test] +async fn terminates_on_password_change() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let channel = fixtures::channel::create(&app, &fixtures::now()).await; + let sender = fixtures::login::create(&app, &fixtures::now()).await; + + // Subscribe via the endpoint + + let creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let cookie = fixtures::cookie::logged_in(&app, &creds, &fixtures::now()).await; + let subscriber = fixtures::identity::from_cookie(&app, &cookie, &fixtures::now()).await; + + let get::Response(events) = get::handler( + State(app.clone()), + subscriber.clone(), + None, + Query::default(), + ) + .await + .expect("subscribe never fails"); + + // Verify the resulting stream's behaviour + + let (_, password) = creds; + let to = fixtures::login::propose_password(); + app.tokens() + .change_password(&subscriber.login, &password, &to, &fixtures::now()) + .await + .expect("expiring tokens succeeds"); + + // These should not be delivered. + + let messages = [ + fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await, + fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await, + fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await, + ]; + + events + .filter_map(fixtures::event::message) + .filter_map(fixtures::event::message::sent) + .filter(|event| future::ready(messages.iter().any(|message| &event.message == message))) + .next() + .expect_none("end of stream") + .await; +} diff --git a/src/login/repo.rs b/src/login/repo.rs index 611edd6..a972304 100644 --- a/src/login/repo.rs +++ b/src/login/repo.rs @@ -58,6 +58,29 @@ impl<'c> Logins<'c> { Ok(login) } + pub async fn set_password( + &mut self, + login: &History, + to: &StoredHash, + ) -> Result<(), sqlx::Error> { + let login = login.id(); + + sqlx::query_scalar!( + r#" + update login + set password_hash = $1 + where id = $2 + returning id as "id: Id" + "#, + to, + login, + ) + .fetch_one(&mut *self.0) + .await?; + + Ok(()) + } + pub async fn all(&mut self, resume_at: ResumePoint) -> Result<Vec<History>, LoadError> { let logins = sqlx::query!( r#" diff --git a/src/login/routes/mod.rs b/src/login/routes/mod.rs index 8cb8852..bbd0c3f 100644 --- a/src/login/routes/mod.rs +++ b/src/login/routes/mod.rs @@ -4,9 +4,11 @@ use crate::app::App; mod login; mod logout; +mod password; pub fn router() -> Router<App> { Router::new() + .route("/api/password", post(password::post::handler)) .route("/api/auth/login", post(login::post::handler)) .route("/api/auth/logout", post(logout::post::handler)) } diff --git a/src/login/routes/password/mod.rs b/src/login/routes/password/mod.rs new file mode 100644 index 0000000..36b384e --- /dev/null +++ b/src/login/routes/password/mod.rs @@ -0,0 +1,4 @@ +pub mod post; + +#[cfg(test)] +mod test; diff --git a/src/login/routes/password/post.rs b/src/login/routes/password/post.rs new file mode 100644 index 0000000..4723754 --- /dev/null +++ b/src/login/routes/password/post.rs @@ -0,0 +1,54 @@ +use axum::{ + extract::{Json, State}, + http::StatusCode, + response::{IntoResponse, Response}, +}; + +use crate::{ + app::App, + clock::RequestedAt, + error::Internal, + login::{Login, Password}, + token::{ + app, + extract::{Identity, IdentityCookie}, + }, +}; + +pub async fn handler( + State(app): State<App>, + RequestedAt(now): RequestedAt, + identity: Identity, + cookie: IdentityCookie, + Json(request): Json<Request>, +) -> Result<(IdentityCookie, Json<Login>), Error> { + let (login, secret) = app + .tokens() + .change_password(&identity.login, &request.password, &request.to, &now) + .await + .map_err(Error)?; + let cookie = cookie.set(secret); + Ok((cookie, Json(login))) +} + +#[derive(serde::Deserialize)] +pub struct Request { + pub password: Password, + pub to: Password, +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct Error(#[from] pub app::LoginError); + +impl IntoResponse for Error { + fn into_response(self) -> Response { + let Self(error) = self; + match error { + app::LoginError::Rejected => { + (StatusCode::BAD_REQUEST, "invalid name or password").into_response() + } + other => Internal::from(other).into_response(), + } + } +} diff --git a/src/login/routes/password/test.rs b/src/login/routes/password/test.rs new file mode 100644 index 0000000..c1974bf --- /dev/null +++ b/src/login/routes/password/test.rs @@ -0,0 +1,68 @@ +use axum::extract::{Json, State}; + +use super::post; +use crate::{ + test::fixtures, + token::app::{LoginError, ValidateError}, +}; + +#[tokio::test] +async fn password_change() { + // Set up the environment + let app = fixtures::scratch_app().await; + let creds = fixtures::login::create_with_password(&app, &fixtures::now()).await; + let cookie = fixtures::cookie::logged_in(&app, &creds, &fixtures::now()).await; + let identity = fixtures::identity::from_cookie(&app, &cookie, &fixtures::now()).await; + + // Call the endpoint + let (name, password) = creds; + let to = fixtures::login::propose_password(); + let request = post::Request { + password: password.clone(), + to: to.clone(), + }; + let (new_cookie, Json(response)) = post::handler( + State(app.clone()), + fixtures::now(), + identity.clone(), + cookie.clone(), + Json(request), + ) + .await + .expect("changing passwords succeeds"); + + // Verify that we have a new session + assert_ne!(cookie.secret(), new_cookie.secret()); + + // Verify that we're still ourselves + assert_eq!(identity.login, response); + + // Verify that our original token is no longer valid + let validate_err = app + .tokens() + .validate( + &cookie + .secret() + .expect("original identity cookie has a secret"), + &fixtures::now(), + ) + .await + .expect_err("validating the original identity secret should fail"); + assert!(matches!(validate_err, ValidateError::InvalidToken)); + + // Verify that our original password is no longer valid + let login_err = app + .tokens() + .login(&name, &password, &fixtures::now()) + .await + .expect_err("logging in with the original password should fail"); + assert!(matches!(login_err, LoginError::Rejected)); + + // Verify that our new password is valid + let (login, _) = app + .tokens() + .login(&name, &to, &fixtures::now()) + .await + .expect("logging in with the new password should succeed"); + assert_eq!(identity.login, login); +} diff --git a/src/test/fixtures/identity.rs b/src/test/fixtures/identity.rs index e438f2b..ffc44c6 100644 --- a/src/test/fixtures/identity.rs +++ b/src/test/fixtures/identity.rs @@ -15,11 +15,15 @@ pub async fn create(app: &App, created_at: &RequestedAt) -> Identity { logged_in(app, &credentials, created_at).await } -pub async fn from_cookie(app: &App, token: &IdentityCookie, issued_at: &RequestedAt) -> Identity { - let secret = token.secret().expect("identity token has a secret"); +pub async fn from_cookie( + app: &App, + cookie: &IdentityCookie, + validated_at: &RequestedAt, +) -> Identity { + let secret = cookie.secret().expect("identity token has a secret"); let (token, login) = app .tokens() - .validate(&secret, issued_at) + .validate(&secret, validated_at) .await .expect("always validates newly-issued secret"); diff --git a/src/token/app.rs b/src/token/app.rs index c19d6a0..5c0aeb0 100644 --- a/src/token/app.rs +++ b/src/token/app.rs @@ -13,7 +13,7 @@ use super::{ use crate::{ clock::DateTime, db::NotFound as _, - login::{Login, Password}, + login::{repo::Provider as _, Login, Password}, name::{self, Name}, }; @@ -61,6 +61,47 @@ impl<'a> Tokens<'a> { Ok((snapshot, token)) } + pub async fn change_password( + &self, + login: &Login, + password: &Password, + to: &Password, + changed_at: &DateTime, + ) -> Result<(Login, Secret), LoginError> { + let mut tx = self.db.begin().await?; + let (login, stored_hash) = tx + .auth() + .for_login(login) + .await + .optional()? + .ok_or(LoginError::Rejected)?; + // Split the transaction here to avoid holding the tx open (potentially blocking + // other writes) while we do the fairly expensive task of verifying the + // password. It's okay if the token issuance transaction happens some notional + // amount of time after retrieving the login, as inserting the token will fail + // if the account is deleted during that time. + tx.commit().await?; + + if !stored_hash.verify(password)? { + return Err(LoginError::Rejected); + } + + let snapshot = login.as_snapshot().ok_or(LoginError::Rejected)?; + let to_hash = to.hash()?; + + let mut tx = self.db.begin().await?; + let tokens = tx.tokens().revoke_all(&login).await?; + tx.logins().set_password(&login, &to_hash).await?; + let secret = tx.tokens().issue(&login, changed_at).await?; + tx.commit().await?; + + for event in tokens.into_iter().map(TokenEvent::Revoked) { + self.token_events.broadcast(event); + } + + Ok((snapshot, secret)) + } + pub async fn validate( &self, secret: &Secret, diff --git a/src/token/repo/auth.rs b/src/token/repo/auth.rs index bdc4c33..b51db8c 100644 --- a/src/token/repo/auth.rs +++ b/src/token/repo/auth.rs @@ -50,6 +50,35 @@ impl<'t> Auth<'t> { Ok((login, row.password_hash)) } + + pub async fn for_login(&mut self, login: &Login) -> Result<(History, StoredHash), LoadError> { + let row = sqlx::query!( + r#" + select + id as "id: login::Id", + display_name as "display_name: String", + canonical_name as "canonical_name: String", + created_sequence as "created_sequence: Sequence", + created_at as "created_at: DateTime", + password_hash as "password_hash: StoredHash" + from login + where id = $1 + "#, + login.id, + ) + .fetch_one(&mut *self.0) + .await?; + + let login = History { + login: Login { + id: row.id, + name: Name::new(row.display_name, row.canonical_name)?, + }, + created: Instant::new(row.created_at, row.created_sequence), + }; + + Ok((login, row.password_hash)) + } } #[derive(Debug, thiserror::Error)] diff --git a/src/token/repo/token.rs b/src/token/repo/token.rs index 35ea385..33b89d5 100644 --- a/src/token/repo/token.rs +++ b/src/token/repo/token.rs @@ -84,6 +84,24 @@ impl<'c> Tokens<'c> { Ok(()) } + // Revoke tokens for a login + pub async fn revoke_all(&mut self, login: &login::History) -> Result<Vec<Id>, sqlx::Error> { + let login = login.id(); + let tokens = sqlx::query_scalar!( + r#" + delete + from token + where login = $1 + returning id as "id: Id" + "#, + login, + ) + .fetch_all(&mut *self.0) + .await?; + + Ok(tokens) + } + // Expire and delete all tokens that haven't been used more recently than // `expire_at`. pub async fn expire(&mut self, expire_at: &DateTime) -> Result<Vec<Id>, sqlx::Error> { diff --git a/src/token/secret.rs b/src/token/secret.rs index 28c93bb..8f70646 100644 --- a/src/token/secret.rs +++ b/src/token/secret.rs @@ -1,6 +1,6 @@ use std::fmt; -#[derive(sqlx::Type)] +#[derive(PartialEq, Eq, sqlx::Type)] #[sqlx(transparent)] pub struct Secret(String); diff --git a/src/ui/routes/me.rs b/src/ui/routes/me.rs new file mode 100644 index 0000000..f1f118f --- /dev/null +++ b/src/ui/routes/me.rs @@ -0,0 +1,32 @@ +pub mod get { + use axum::response::{self, IntoResponse, Redirect}; + + use crate::{ + error::Internal, + token::extract::Identity, + ui::assets::{Asset, Assets}, + }; + + pub async fn handler(identity: Option<Identity>) -> Result<Asset, Error> { + let _ = identity.ok_or(Error::NotLoggedIn)?; + + Assets::index().map_err(Error::Internal) + } + + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error("not logged in")] + NotLoggedIn, + #[error("{0}")] + Internal(Internal), + } + + impl IntoResponse for Error { + fn into_response(self) -> response::Response { + match self { + Self::NotLoggedIn => Redirect::temporary("/login").into_response(), + Self::Internal(error) => error.into_response(), + } + } + } +} diff --git a/src/ui/routes/mod.rs b/src/ui/routes/mod.rs index 72d9a4a..48b3f90 100644 --- a/src/ui/routes/mod.rs +++ b/src/ui/routes/mod.rs @@ -6,6 +6,7 @@ mod ch; mod get; mod invite; mod login; +mod me; mod path; mod setup; @@ -16,6 +17,7 @@ pub fn router(app: &App) -> Router<App> { .route("/setup", get(setup::get::handler)), Router::new() .route("/", get(get::handler)) + .route("/me", get(me::get::handler)) .route("/login", get(login::get::handler)) .route("/ch/:channel", get(ch::channel::get::handler)) .route("/invite/:invite", get(invite::invite::get::handler)) diff --git a/ui/lib/apiServer.js b/ui/lib/apiServer.js index db554e2..19dcf60 100644 --- a/ui/lib/apiServer.js +++ b/ui/lib/apiServer.js @@ -30,6 +30,10 @@ export async function logOut() { return apiServer.post('/auth/logout', {}); } +export async function changePassword(password, to) { + return apiServer.post('/password', { password, to }); +} + export async function createChannel(name) { return apiServer.post('/channels', { name }); } diff --git a/ui/lib/components/LogOut.svelte b/ui/lib/components/CurrentUser.svelte index ba0861a..4b1b974 100644 --- a/ui/lib/components/LogOut.svelte +++ b/ui/lib/components/CurrentUser.svelte @@ -14,7 +14,7 @@ <form on:submit|preventDefault={handleLogout}> {#if $currentUser} - @{$currentUser.username} + <a href="/me">@{$currentUser.username}</a> {/if} <button class="border-slate-500 border-solid border-2 font-bold p-1 rounded" diff --git a/ui/routes/(app)/+layout.svelte b/ui/routes/(app)/+layout.svelte index 9abaaf4..08c6694 100644 --- a/ui/routes/(app)/+layout.svelte +++ b/ui/routes/(app)/+layout.svelte @@ -86,8 +86,4 @@ max-height: 100%; overflow: scroll; } - #interface .active-channel { - border: 1px solid grey; - border-radius: 1.25rem; - } </style> diff --git a/ui/routes/(app)/me/+page.svelte b/ui/routes/(app)/me/+page.svelte new file mode 100644 index 0000000..fb612b8 --- /dev/null +++ b/ui/routes/(app)/me/+page.svelte @@ -0,0 +1,41 @@ +<script> + import { changePassword } from '$lib/apiServer.js'; + + import Invite from '$lib/components/Invite.svelte'; + + let currentPassword, newPassword, confirmPassword, passwordForm; + let pending = false; + $: valid = (newPassword === confirmPassword) && (newPassword !== currentPassword); + $: disabled = pending || !valid; + + async function onPasswordChange() { + pending = true; + let response = await changePassword(currentPassword, newPassword); + switch (response.status) { + case 200: + passwordForm.reset(); + break; + } + pending = false; + } +</script> + +<form on:submit|preventDefault={onPasswordChange} bind:this={passwordForm} > + <label>current password + <input class="input" name="currentPassword" type="password" placeholder="password" bind:value={currentPassword}> + </label> + + <label>new password + <input class="input" name="newPassword" type="password" placeholder="password" bind:value={newPassword}> + </label> + + <label>confirm new password + <input class="input" name="confirmPassword" type="password" placeholder="password" bind:value={confirmPassword}> + </label> + + <button class="btn variant-filled" type="submit" disabled={disabled}> + change password + </button> +</form> + +<Invite /> diff --git a/ui/routes/+layout.svelte b/ui/routes/+layout.svelte index fdd3883..711b8bd 100644 --- a/ui/routes/+layout.svelte +++ b/ui/routes/+layout.svelte @@ -3,18 +3,16 @@ import "../app.css"; import { currentUser } from '$lib/store'; - import LogOut from '$lib/components/LogOut.svelte'; - import Invite from '$lib/components/Invite.svelte'; + import CurrentUser from '$lib/components/CurrentUser.svelte'; </script> <div id="app"> <AppBar> <svelte:fragment slot="lead">🌳</svelte:fragment> - <a href="/">understory</a> + <a href="/">understory</a> <svelte:fragment slot="trail"> {#if $currentUser} - <Invite /> - <LogOut /> + <CurrentUser /> {/if} </svelte:fragment> </AppBar> |
