From 2f06ce1da531289ad77b6c742c56f99a21099bc0 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Wed, 18 Dec 2024 18:51:19 -0800 Subject: [PATCH] controllers/user/admin: add an unlock route --- src/controllers/user/admin.rs | 50 ++++++++++- src/router.rs | 2 +- ..._io__openapi__tests__openapi_snapshot.snap | 25 ++++++ src/tests/routes/users/admin.rs | 84 ++++++++++++++++++- ...es__users__admin__unlock__admin-found.snap | 20 +++++ ...users__admin__unlock__admin-not-found.snap | 5 ++ ..._users__admin__unlock__admin-reunlock.snap | 20 +++++ ...users__admin__unlock__anonymous-found.snap | 5 ++ ...s__admin__unlock__anonymous-not-found.snap | 5 ++ ...users__admin__unlock__non-admin-found.snap | 5 ++ ...s__admin__unlock__non-admin-not-found.snap | 5 ++ 11 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-found.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-not-found.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-reunlock.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-found.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-not-found.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-found.snap create mode 100644 src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-not-found.snap diff --git a/src/controllers/user/admin.rs b/src/controllers/user/admin.rs index 44e6fa55ab..17984e6a15 100644 --- a/src/controllers/user/admin.rs +++ b/src/controllers/user/admin.rs @@ -1,5 +1,5 @@ use axum::{extract::Path, Json}; -use chrono::NaiveDateTime; +use chrono::{NaiveDateTime, Utc}; use crates_io_database::schema::{emails, users}; use diesel::{pg::Pg, prelude::*}; use diesel_async::{scoped_futures::ScopedFutureExt, AsyncConnection, RunQueryDsl}; @@ -102,6 +102,54 @@ pub async fn lock( Ok(Json(user)) } +/// Unlock the given user. +/// +/// Only site admins can use this endpoint. +#[utoipa::path( + delete, + path = "/api/v1/users/{user}/lock", + params( + ("user" = String, Path, description = "Login name of the user"), + ), + tags = ["admin", "users"], + responses((status = 200, description = "Successful Response")), +)] +pub async fn unlock( + state: AppState, + Path(user_name): Path, + req: Parts, +) -> AppResult> { + let mut conn = state.db_read_prefer_primary().await?; + AuthCheck::only_cookie() + .require_admin() + .check(&req, &mut conn) + .await?; + + // Again, let's do this in a transaction, even though we _technically_ don't + // need to. + let user = conn + .transaction(|conn| { + // Although this is called via the `DELETE` method, this is + // implemented as a soft deletion by setting the lock until time to + // now, thereby allowing us to have some sense of history of whether + // an account has been locked in the past. + async move { + let id = diesel::update(users::table) + .filter(lower(users::gh_login).eq(lower(user_name))) + .set(users::account_lock_until.eq(Utc::now().naive_utc())) + .returning(users::id) + .get_result::(conn) + .await?; + + get_user(|query| query.filter(users::id.eq(id)), conn).await + } + .scope_boxed() + }) + .await?; + + Ok(Json(user)) +} + /// A helper to get an [`EncodableAdminUser`] based on whatever filter predicate /// is provided in the callback. /// diff --git a/src/router.rs b/src/router.rs index b06a3627b4..06cc8a57ea 100644 --- a/src/router.rs +++ b/src/router.rs @@ -62,7 +62,7 @@ pub fn build_axum_router(state: AppState) -> Router<()> { .routes(routes!(user::me::get_authenticated_user)) .routes(routes!(user::me::get_authenticated_user_updates)) .routes(routes!(user::admin::get)) - .routes(routes!(user::admin::lock)) + .routes(routes!(user::admin::lock, user::admin::unlock)) .routes(routes!(token::list_api_tokens, token::create_api_token)) .routes(routes!(token::find_api_token, token::revoke_api_token)) .routes(routes!(token::revoke_current_api_token)) diff --git a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap index d7ede9d134..58160a013f 100644 --- a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap +++ b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap @@ -1701,6 +1701,31 @@ expression: response.json() } }, "/api/v1/users/{user}/lock": { + "delete": { + "description": "Only site admins can use this endpoint.", + "operationId": "unlock", + "parameters": [ + { + "description": "Login name of the user", + "in": "path", + "name": "user", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Successful Response" + } + }, + "summary": "Unlock the given user.", + "tags": [ + "admin", + "users" + ] + }, "put": { "description": "Only site admins can use this endpoint.", "operationId": "lock", diff --git a/src/tests/routes/users/admin.rs b/src/tests/routes/users/admin.rs index 5a579af9d4..ca7ce33870 100644 --- a/src/tests/routes/users/admin.rs +++ b/src/tests/routes/users/admin.rs @@ -1,4 +1,5 @@ -use chrono::DateTime; +use chrono::{DateTime, Utc}; +use crates_io_database::schema::users; use http::StatusCode; use insta::{assert_json_snapshot, assert_snapshot}; use serde_json::json; @@ -155,6 +156,76 @@ mod lock { } } +mod unlock { + use super::*; + + #[tokio::test(flavor = "multi_thread")] + async fn unlock() { + let (app, anon, user) = TestApp::init().with_user().await; + let admin = app.db_new_admin_user("admin").await; + + use diesel::prelude::*; + use diesel_async::RunQueryDsl; + + // First up, let's lock the user. + let mut conn = app.db_conn().await; + diesel::update(user.as_model()) + .set(( + users::account_lock_reason.eq("naughty naughty"), + users::account_lock_until.eq(DateTime::parse_from_rfc3339("2050-01-01T01:02:03Z") + .unwrap() + .naive_utc()), + )) + .execute(&mut conn) + .await + .unwrap(); + + // Anonymous users should be forbidden. + let response = anon.delete::<()>("/api/v1/users/foo/lock").await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_snapshot!("anonymous-found", response.text()); + + let response = anon.delete::<()>("/api/v1/users/bar/lock").await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_snapshot!("anonymous-not-found", response.text()); + + // Regular users should also be forbidden, even if they're locking + // themself. + let response = user.delete::<()>("/api/v1/users/foo/lock").await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_snapshot!("non-admin-found", response.text()); + + let response = user.delete::<()>("/api/v1/users/bar/lock").await; + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_snapshot!("non-admin-not-found", response.text()); + + // Admin users are allowed, but still can't manifest users who don't + // exist. + let response = admin.delete::<()>("/api/v1/users/bar/lock").await; + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_snapshot!("admin-not-found", response.text()); + + // Admin users are allowed, and should be able to unlock the user. + let response = admin.delete::<()>("/api/v1/users/foo/lock").await; + assert_eq!(response.status(), StatusCode::OK); + assert_json_snapshot!("admin-found", response.json(), { + ".lock.until" => "[datetime]", + }); + + // Get the user again and validate that they are now unlocked. + let mut conn = app.db_conn().await; + let unlocked_user = User::find(&mut conn, user.as_model().id).await.unwrap(); + assert_user_is_unlocked(&unlocked_user); + + // Unlocking an unlocked user should succeed silently. + let response = admin.delete::<()>("/api/v1/users/foo/lock").await; + assert_eq!(response.status(), StatusCode::OK); + assert_json_snapshot!("admin-reunlock", response.json(), { + ".lock.until" => "[datetime]", + }); + } +} + #[track_caller] fn assert_user_is_locked(user: &User, reason: &str, until: &str) { assert_eq!(user.account_lock_reason.as_deref(), Some(reason)); @@ -169,3 +240,14 @@ fn assert_user_is_locked_indefinitely(user: &User, reason: &str) { assert_eq!(user.account_lock_reason.as_deref(), Some(reason)); assert_none!(user.account_lock_until); } + +#[track_caller] +fn assert_user_is_unlocked(user: &User) { + if user.account_lock_reason.is_some() { + if let Some(until) = user.account_lock_until { + assert_lt!(until, Utc::now().naive_utc()); + } else { + panic!("user account is locked indefinitely"); + } + } +} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-found.snap new file mode 100644 index 0000000000..00079b77bc --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-found.snap @@ -0,0 +1,20 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.json() +--- +{ + "avatar": null, + "email": "foo@example.com", + "email_verification_sent": true, + "email_verified": true, + "id": 1, + "is_admin": false, + "lock": { + "reason": "naughty naughty", + "until": "[datetime]" + }, + "login": "foo", + "name": null, + "publish_notifications": true, + "url": "https://github.com/foo" +} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-not-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-not-found.snap new file mode 100644 index 0000000000..83a124455e --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-not-found.snap @@ -0,0 +1,5 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.text() +--- +{"errors":[{"detail":"Not Found"}]} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-reunlock.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-reunlock.snap new file mode 100644 index 0000000000..00079b77bc --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__admin-reunlock.snap @@ -0,0 +1,20 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.json() +--- +{ + "avatar": null, + "email": "foo@example.com", + "email_verification_sent": true, + "email_verified": true, + "id": 1, + "is_admin": false, + "lock": { + "reason": "naughty naughty", + "until": "[datetime]" + }, + "login": "foo", + "name": null, + "publish_notifications": true, + "url": "https://github.com/foo" +} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-found.snap new file mode 100644 index 0000000000..4282cf055f --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-found.snap @@ -0,0 +1,5 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.text() +--- +{"errors":[{"detail":"this action requires authentication"}]} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-not-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-not-found.snap new file mode 100644 index 0000000000..4282cf055f --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__anonymous-not-found.snap @@ -0,0 +1,5 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.text() +--- +{"errors":[{"detail":"this action requires authentication"}]} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-found.snap new file mode 100644 index 0000000000..d016dc2b92 --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-found.snap @@ -0,0 +1,5 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.text() +--- +{"errors":[{"detail":"This account is locked until 2050-01-01 at 01:02:03 UTC. Reason: naughty naughty"}]} diff --git a/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-not-found.snap b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-not-found.snap new file mode 100644 index 0000000000..d016dc2b92 --- /dev/null +++ b/src/tests/routes/users/snapshots/crates_io__tests__routes__users__admin__unlock__non-admin-not-found.snap @@ -0,0 +1,5 @@ +--- +source: src/tests/routes/users/admin.rs +expression: response.text() +--- +{"errors":[{"detail":"This account is locked until 2050-01-01 at 01:02:03 UTC. Reason: naughty naughty"}]}