Przeglądaj źródła

feat: let an administrator reset somebody's password

changePassword verifies the target's current password with bcrypt and
never looks at who is asking, so an administrator who does not know a
user's password could not reset it. The settings page shipped a button
for exactly that, and it had always answered "Invalid current password"
to anyone using it on somebody else.

Rather than loosen the existing path, which is self-service and should
keep demanding the old password, this is a separate endpoint that
requires the authority to administer users. A member is refused with 403
and cannot reach it even for their own account, because self-service
already exists and asks for the old password on purpose.

A reset ends the target's sessions. Otherwise the action taken to lock
somebody out would leave the intruder holding a working session.

**What a reset does not do, which is worth knowing before relying on
it.** Access tokens are stateless: auth_middleware verifies the
signature and never consults the session store, so a token already
issued keeps working until it expires - up to auth.access_token_lifetime_sec,
900 seconds by default. Ending the sessions kills the refresh path
immediately, so the intruder cannot renew, but they retain access for
the remainder of that window. Closing it properly means a revocation
check on every request, which is its own change with its own cost. The
same has always been true of deleting a user.

Verified against the running service with a throwaway member account:

  member's own session before reset               200
  member resetting the admin's password           403 Insufficient permissions
  admin resetting the member's password           200
  old password                                    rejected
  new password                                    accepted
  member's refresh token after the reset          Session not found
  admin's own session after resetting somebody    200

81 passed, 0 failed. The throwaway account was removed afterwards.
fszontagh 1 miesiąc temu
rodzic
commit
11b89cb640

+ 45 - 0
src/webserver/api/user_controller.cpp

@@ -51,6 +51,16 @@ void UserController::registerRoutes(httplib::Server& server) {
             changePassword(req, res, ctx);
         });
     });
+
+    // Admin-only: set somebody else's password without knowing their current
+    // one. Same "admin" gate as the other user-management routes above -
+    // requireRole ranks owner above admin above member, so this stays open to
+    // both without a separate owner check.
+    server.Post(R"(/api/v1/users/([^/]+)/reset-password)", [this](const httplib::Request& req, httplib::Response& res) {
+        middleware_.requireRole(req, res, "admin", [this](auto& req, auto& res, auto& ctx) {
+            resetPassword(req, res, ctx);
+        });
+    });
 }
 
 void UserController::listUsers(const httplib::Request& req, httplib::Response& res,
@@ -266,6 +276,41 @@ void UserController::changePassword(const httplib::Request& req, httplib::Respon
     }
 }
 
+void UserController::resetPassword(const httplib::Request& req, httplib::Response& res,
+                                    const auth::AuthContext& ctx) {
+    try {
+        std::string id = req.matches[1];
+
+        // Reset your own password through change-password instead, where a
+        // current password is still required. Allowing it here would let an
+        // admin end their own session by accident on the one route that is
+        // not supposed to ask them for proof they are who they say they are.
+        if (id == ctx.user_id) {
+            sendError(res, "Use change-password to reset your own password", 400);
+            return;
+        }
+
+        auto body = nlohmann::json::parse(req.body);
+        std::string new_password = body.value("newPassword", "");
+
+        if (new_password.empty()) {
+            sendError(res, "New password required", 400);
+            return;
+        }
+
+        auto result = auth_store_.resetPassword(id, new_password);
+        if (result.failed()) {
+            sendError(res, result.error().message(), 400);
+            return;
+        }
+
+        LOG_INFO("Password reset for user {} by admin {}", id, ctx.user_id);
+        sendJson(res, {{"success", true}});
+    } catch (const std::exception& e) {
+        sendError(res, "Invalid request body", 400);
+    }
+}
+
 void UserController::sendJson(httplib::Response& res, const nlohmann::json& data, int status) {
     res.status = status;
     res.set_content(data.dump(), "application/json");

+ 2 - 0
src/webserver/api/user_controller.hpp

@@ -29,6 +29,8 @@ private:
                     const auth::AuthContext& ctx);
     void changePassword(const httplib::Request& req, httplib::Response& res,
                         const auth::AuthContext& ctx);
+    void resetPassword(const httplib::Request& req, httplib::Response& res,
+                       const auth::AuthContext& ctx);
 
     void sendJson(httplib::Response& res, const nlohmann::json& data, int status = 200);
     void sendError(httplib::Response& res, const std::string& message, int status);

+ 21 - 0
src/webserver/auth/auth_store.cpp

@@ -339,6 +339,27 @@ Result<void> AuthStore::changePassword(const std::string& id,
     return Result<void>();
 }
 
+Result<void> AuthStore::resetPassword(const std::string& id, const std::string& new_password) {
+    auto user_result = getUser(id);
+    if (user_result.failed()) {
+        return user_result.error();
+    }
+
+    nlohmann::json updates;
+    updates["passwordHash"] = BcryptUtils::hashPassword(new_password);
+
+    auto result = storage_.update("users", id, updates, 0, true);
+    if (result.failed()) {
+        return result.error();
+    }
+
+    // Ends the target's sessions, same as changePassword - a reset that left a
+    // stolen session alive would defeat the point of resetting the password.
+    invalidateAllSessions(id);
+
+    return Result<void>();
+}
+
 Result<LoginResponse> AuthStore::login(const std::string& identifier,
                                         const std::string& password,
                                         const std::string& ip_address,

+ 9 - 0
src/webserver/auth/auth_store.hpp

@@ -86,6 +86,15 @@ public:
                                         const std::string& old_password,
                                         const std::string& new_password);
 
+    // Sets a user's password without checking their current one. Callers must
+    // establish on their own that whoever is asking has the authority to do
+    // this - unlike changePassword, there is no secret here that proves it.
+    // Ends every session the target holds, same as changePassword and for the
+    // same reason: a reset that leaves an old session alive does not lock
+    // anybody out.
+    common::Result<void> resetPassword(const std::string& id,
+                                       const std::string& new_password);
+
     // Authentication. `identifier` is matched against email first, then
     // username as a transitional fallback - see the .cpp for why.
     common::Result<LoginResponse> login(const std::string& identifier,

+ 12 - 0
webui/src/api/users.ts

@@ -44,6 +44,18 @@ export const usersApi = {
     const { data } = await api.post(`/users/${id}/change-password`, { oldPassword, newPassword })
     return data
   },
+
+  /**
+   * Sets somebody else's password without knowing their current one. Admin
+   * only on the backend - a member gets a 403. Ends every session the target
+   * holds, so a stolen session does not survive the reset that was meant to
+   * lock it out. Cannot be used on your own account; use changeOwnPassword
+   * for that, which still asks for your current password.
+   */
+  resetPassword: async (id: string, newPassword: string) => {
+    const { data } = await api.post(`/users/${id}/reset-password`, { newPassword })
+    return data
+  },
 }
 
 export interface ProjectMember {

+ 81 - 17
webui/src/pages/UsersPage.tsx

@@ -1,6 +1,6 @@
 import { useState } from 'react'
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
-import { UserPlus, Trash2, ShieldCheck, X, Info } from 'lucide-react'
+import { UserPlus, Trash2, ShieldCheck, X, KeyRound } from 'lucide-react'
 import { usersApi, User } from '../api/users'
 import { useAuthStore } from '../stores/authStore'
 
@@ -11,11 +11,11 @@ import { useAuthStore } from '../stores/authStore'
  * is decided by the project it lives in, not here; this is the difference
  * between "runs this installation" and "has been given a job in one project".
  *
- * Password resets for someone else are not offered here - see the note below
- * the table. The change-password endpoint verifies the account's *current*
- * password no matter who is calling it, so an admin who does not know that
- * password cannot use it to set a new one. Only a user's own password (with
- * their own current password) can be changed today, from Settings > Profile.
+ * Resetting someone else's password is an admin action: it sets a new
+ * password without knowing the old one, and ends every session the account
+ * currently holds. That is deliberately different from Settings > Profile,
+ * which only ever changes your own password and still asks for the current
+ * one - self-service password changes stay there.
  */
 
 const ROLES = [
@@ -40,6 +40,9 @@ export default function UsersPage() {
   const [showNew, setShowNew] = useState(false)
   const [newUser, setNewUser] = useState({ username: '', password: '', email: '', role: 'member' })
   const [error, setError] = useState<string | null>(null)
+  const [resetTarget, setResetTarget] = useState<User | null>(null)
+  const [resetPasswordValue, setResetPasswordValue] = useState('')
+  const [resetError, setResetError] = useState<string | null>(null)
 
   const { data: users = [], isLoading, isError } = useQuery({
     queryKey: ['users'],
@@ -72,6 +75,17 @@ export default function UsersPage() {
     onError: fail,
   })
 
+  const resetMutation = useMutation({
+    mutationFn: ({ id, newPassword }: { id: string; newPassword: string }) =>
+      usersApi.resetPassword(id, newPassword),
+    onSuccess: () => {
+      setResetTarget(null)
+      setResetPasswordValue('')
+      setResetError(null)
+    },
+    onError: (e: any) => setResetError(e?.response?.data?.error || e.message || 'That did not work'),
+  })
+
   const isOwner = (u: User) => u.role === 'owner'
 
   if (isError) {
@@ -164,6 +178,15 @@ export default function UsersPage() {
                   {user.active ? 'Deactivate' : 'Activate'}
                 </button>
 
+                <button
+                  onClick={() => { setResetTarget(user); setResetPasswordValue(''); setResetError(null) }}
+                  disabled={user.id === me?.id}
+                  className="p-1.5 text-gray-500 dark:text-gray-400 hover:text-primary-600 dark:hover:text-primary-400 hover:bg-primary-50 dark:hover:bg-primary-900/30 rounded disabled:opacity-40"
+                  title={user.id === me?.id ? 'Change your own password from Settings instead' : "Reset this account's password"}
+                >
+                  <KeyRound className="w-4 h-4" />
+                </button>
+
                 <button
                   onClick={() => {
                     if (confirm(
@@ -186,17 +209,6 @@ export default function UsersPage() {
         )}
       </section>
 
-      <div className="flex items-start gap-2 text-sm text-gray-500 dark:text-gray-400 px-1">
-        <Info className="w-4 h-4 shrink-0 mt-0.5" />
-        <p>
-          There is no password reset for somebody else's account here. The change-password endpoint always
-          verifies the account's current password, even when an admin is calling it, so it cannot be used to
-          set a new password for someone who has forgotten theirs. Only an account holder can change their
-          own password, from Settings, with their own current password. Resetting another user's password
-          needs a backend change - an endpoint that lets an admin set one without knowing the old one.
-        </p>
-      </div>
-
       {showNew && (
         <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50 p-4" onClick={() => setShowNew(false)}>
           <div className="bg-white dark:bg-slate-800 rounded-lg shadow-xl w-full max-w-sm p-5" onClick={(e) => e.stopPropagation()}>
@@ -260,6 +272,58 @@ export default function UsersPage() {
           </div>
         </div>
       )}
+
+      {resetTarget && (
+        <div
+          className="fixed inset-0 bg-black/50 flex items-center justify-center z-50 p-4"
+          onClick={() => { setResetTarget(null); setResetError(null) }}
+        >
+          <div className="bg-white dark:bg-slate-800 rounded-lg shadow-xl w-full max-w-sm p-5" onClick={(e) => e.stopPropagation()}>
+            <h3 className="font-semibold text-gray-900 dark:text-gray-100 mb-1">
+              Reset password for {resetTarget.username}
+            </h3>
+            <p className="text-xs text-gray-500 dark:text-gray-400 mb-4">
+              This is an administrative action. It sets a new password without knowing the old one, and
+              immediately signs {resetTarget.username} out everywhere - they will need the new password to
+              log in again.
+            </p>
+            {resetError && (
+              <div className="mb-3 px-3 py-2 rounded bg-red-50 dark:bg-red-900/20 text-red-700 dark:text-red-300 text-sm">
+                {resetError}
+              </div>
+            )}
+            <input
+              autoFocus
+              type="password"
+              placeholder="New password"
+              value={resetPasswordValue}
+              onChange={(e) => setResetPasswordValue(e.target.value)}
+              className="w-full px-3 py-2 border border-gray-300 dark:border-slate-600 rounded bg-white dark:bg-slate-700 text-gray-900 dark:text-gray-100 text-sm"
+            />
+            <div className="flex justify-end gap-2 mt-5">
+              <button
+                onClick={() => { setResetTarget(null); setResetError(null) }}
+                className="px-3 py-1.5 text-sm text-gray-600 dark:text-gray-400"
+              >
+                Cancel
+              </button>
+              <button
+                onClick={() => {
+                  if (confirm(
+                    `Reset ${resetTarget.username}'s password? They will be signed out of every session immediately.`
+                  )) {
+                    resetMutation.mutate({ id: resetTarget.id, newPassword: resetPasswordValue })
+                  }
+                }}
+                disabled={!resetPasswordValue || resetMutation.isPending}
+                className="px-3 py-1.5 text-sm bg-primary-600 text-white rounded hover:bg-primary-700 disabled:opacity-40"
+              >
+                {resetMutation.isPending ? 'Resetting...' : 'Reset password'}
+              </button>
+            </div>
+          </div>
+        </div>
+      )}
     </div>
   )
 }