Explorar o código

fix(keys): identify keys by stable non-secret id for edit/revoke

API keys were previously identified in PATCH/DELETE by their full secret
value, but GET /api/v1/keys masks the secret and only returns key_prefix.
This made it impossible for the web UI to edit or revoke any key it
didn't just create.

Add a stable `id` field (random hex, generated at create time, stored as
`key_id` in the DB document body to avoid the DB's reserved `id` field)
to ApiKey. toPublicJson() now includes `id`; the create response includes
it alongside the one-time secret. PATCH/DELETE /api/v1/keys/{id} now
route by this id instead of the secret. Authentication (lookup) still
matches on the full key value.

bootstrap() migrates any pre-existing keys that lack a `key_id` field by
assigning a fresh id and re-persisting them with id as the doc record id.

Tests updated to use id for update/remove/delete operations; the
CannotDelete/CannotDeAdmin last-admin tests look up the admin id from the
keys list. Frontend types, KeysAdmin.tsx actions, openapi.json path
(/api/v1/keys/{id}), and llms.txt all updated to match.
Fszontagh hai 1 mes
pai
achega
915a11d133

+ 4 - 4
api/llms.txt

@@ -20,10 +20,10 @@ Requires a valid key. Admin-only operations are noted.
 
 All key-management endpoints require an admin key.
 
-- `GET  /api/v1/keys` — List keys. Secret value is masked; only `key_prefix`, `label`, `projects`, `admin`, and `created_at` are returned.
-- `POST /api/v1/keys` `{label, projects:[], admin?}` — Generate a new key. Returns the secret key value **once**; store it immediately.
-- `PATCH /api/v1/keys/{key}` `{label?, projects?, admin?}` — Update grants for an existing key.
-- `DELETE /api/v1/keys/{key}` — Revoke a key. Active sessions using it are immediately invalidated.
+- `GET  /api/v1/keys` — List keys. Secret value is masked; returns `id`, `key_prefix`, `label`, `projects`, `admin`, and `created_at`. Use `id` to reference the key in PATCH/DELETE.
+- `POST /api/v1/keys` `{label, projects:[], admin?}` — Generate a new key. Returns the secret key value **once** (store it immediately) plus the stable `id`.
+- `PATCH /api/v1/keys/{id}` `{label?, projects?, admin?}` — Update grants for an existing key, identified by its `id` (not the secret).
+- `DELETE /api/v1/keys/{id}` — Revoke a key by `id`. Active sessions using it are immediately invalidated.
 
 ## Collections
 

+ 8 - 6
api/openapi.json

@@ -36,6 +36,7 @@
       "ApiKeyPublic": {
         "type": "object",
         "properties": {
+          "id": { "type": "string", "description": "Stable non-secret identifier used to reference the key in PATCH/DELETE" },
           "key_prefix": { "type": "string" },
           "label": { "type": "string" },
           "projects": { "type": "array", "items": { "type": "string" } },
@@ -161,7 +162,7 @@
     },
     "/api/v1/keys": {
       "get": {
-        "summary": "List API keys (admin only; secret masked, only key_prefix shown)",
+        "summary": "List API keys (admin only; secret masked; returns id and key_prefix)",
         "operationId": "listKeys",
         "responses": {
           "200": {
@@ -209,13 +210,14 @@
         },
         "responses": {
           "201": {
-            "description": "Key created; contains the secret key value (shown once)",
+            "description": "Key created; contains the secret key value (shown once) and the stable id",
             "content": {
               "application/json": {
                 "schema": {
                   "type": "object",
                   "properties": {
-                    "key": { "type": "string" },
+                    "id": { "type": "string", "description": "Stable non-secret identifier for this key" },
+                    "key": { "type": "string", "description": "Secret key value — shown only once" },
                     "label": { "type": "string" },
                     "projects": { "type": "array", "items": { "type": "string" } },
                     "admin": { "type": "boolean" }
@@ -230,14 +232,14 @@
         }
       }
     },
-    "/api/v1/keys/{key}": {
+    "/api/v1/keys/{id}": {
       "parameters": [
         {
-          "name": "key",
+          "name": "id",
           "in": "path",
           "required": true,
           "schema": { "type": "string" },
-          "description": "API key value"
+          "description": "Stable non-secret key id (from the id field in list/create responses)"
         }
       ],
       "patch": {

+ 7 - 2
src/apikey.cpp

@@ -8,6 +8,9 @@ namespace svapi {
 
 ApiKey ApiKey::fromJson(const nlohmann::json& j) {
     ApiKey k;
+    // "key_id" is the stable non-secret id stored in the document body.
+    // We avoid "id" as the DB reserves/strips that field.
+    k.id        = j.value("key_id", "");
     k.key       = j.value("key", "");
     k.label     = j.value("label", "");
     if (j.contains("projects") && j["projects"].is_array())
@@ -18,12 +21,14 @@ ApiKey ApiKey::fromJson(const nlohmann::json& j) {
 }
 
 nlohmann::json ApiKey::toJson() const {
-    return {{"key", key}, {"label", label}, {"projects", projects},
+    // "key_id" stores the stable non-secret id in the body.
+    // We avoid using "id" as the DB treats that as a reserved/internal field.
+    return {{"key_id", id}, {"key", key}, {"label", label}, {"projects", projects},
             {"admin", admin}, {"created_at", createdAt}};
 }
 
 nlohmann::json ApiKey::toPublicJson() const {
-    return {{"label", label}, {"projects", projects}, {"admin", admin},
+    return {{"id", id}, {"label", label}, {"projects", projects}, {"admin", admin},
             {"created_at", createdAt},
             {"key_prefix", key.substr(0, std::min<size_t>(8, key.size()))}};
 }

+ 2 - 1
src/apikey.hpp

@@ -7,7 +7,8 @@ namespace svapi {
 
 /// An API key with per-project grants and optional admin privilege.
 struct ApiKey {
-    std::string key;
+    std::string id;       // stable non-secret identifier (used as DB doc id)
+    std::string key;      // secret value (used for authentication only)
     std::string label;
     std::vector<std::string> projects;  // may contain "*" (all projects)
     bool     admin     = false;

+ 10 - 6
src/handlers/keys.cpp

@@ -28,7 +28,7 @@ void registerKeyRoutes(ApiServer& s) {
 
     svr.Patch(R"(/api/v1/keys/([^/]+))", [d](const httplib::Request& req, httplib::Response& res) {
         requireAdmin(requireKey(*d, req));
-        std::string key = req.matches[1];
+        std::string id = req.matches[1];
         auto body = bodyJson(req);
         std::optional<std::string> label;
         std::optional<std::vector<std::string>> projects;
@@ -37,28 +37,32 @@ void registerKeyRoutes(ApiServer& s) {
         if (body.contains("projects")) projects = body["projects"].get<std::vector<std::string>>();
         if (body.contains("admin")) admin = body["admin"].get<bool>();
         if (admin.has_value() && !*admin) {
-            auto target = d->keys.lookup(key);
+            // Find target by id in the key list
+            std::optional<ApiKey> target;
+            for (const auto& k : d->keys.list()) if (k.id == id) { target = k; break; }
             if (target && target->admin) {
                 int adminCount = 0;
                 for (const auto& k : d->keys.list()) if (k.admin) ++adminCount;
                 if (adminCount <= 1) throw ApiError(ErrCode::Unprocessable, "last_admin", "cannot remove the last admin key");
             }
         }
-        if (!d->keys.update(key, label, projects, admin))
+        if (!d->keys.update(id, label, projects, admin))
             throw ApiError(ErrCode::NotFound, "not_found", "no such key");
         sendJson(res, 200, {{"updated", true}});
     });
 
     svr.Delete(R"(/api/v1/keys/([^/]+))", [d](const httplib::Request& req, httplib::Response& res) {
         requireAdmin(requireKey(*d, req));
-        std::string key = req.matches[1];
-        auto target = d->keys.lookup(key);
+        std::string id = req.matches[1];
+        // Find target by id in the key list
+        std::optional<ApiKey> target;
+        for (const auto& k : d->keys.list()) if (k.id == id) { target = k; break; }
         if (target && target->admin) {
             int adminCount = 0;
             for (const auto& k : d->keys.list()) if (k.admin) ++adminCount;
             if (adminCount <= 1) throw ApiError(ErrCode::Unprocessable, "last_admin", "cannot remove the last admin key");
         }
-        if (!d->keys.remove(key)) throw ApiError(ErrCode::NotFound, "not_found", "no such key");
+        if (!d->keys.remove(id)) throw ApiError(ErrCode::NotFound, "not_found", "no such key");
         sendJson(res, 200, {{"deleted", true}});
     });
 }

+ 28 - 7
src/key_store.cpp

@@ -25,15 +25,35 @@ void KeyStore::bootstrap(const std::string& envAdminKey) {
     reload();
     if (snap_->empty()) {
         ApiKey k;
+        k.id        = generateApiKey();
         k.key       = envAdminKey.empty() ? generateApiKey() : envAdminKey;
         k.label     = "bootstrap-admin";
         k.projects  = {"*"};
         k.admin     = true;
         k.createdAt = static_cast<uint64_t>(std::time(nullptr));
-        db_.client().upsert(coll_, k.toJson(), k.key);
+        db_.client().upsert(coll_, k.toJson(), k.id);
         reload();
         if (envAdminKey.empty())
             spdlog::warn("Generated initial admin API key: {} — store it now; shown only once.", k.key);
+    } else {
+        // Migrate any existing keys that pre-date the stable-id feature:
+        // if key_id is empty, the document was stored with doc_id = key (old scheme).
+        // Re-store it with a fresh id as doc_id and key_id in body.
+        bool needsReload = false;
+        smartbotic::database::Client::QueryOptions opts;
+        opts.limit = 100000;
+        auto docs = db_.client().find(coll_, opts);
+        for (const auto& d : docs) {
+            ApiKey k = ApiKey::fromJson(d);
+            if (k.id.empty()) {
+                k.id = generateApiKey();
+                // Remove old doc (stored with doc_id = k.key) and insert new one (doc_id = k.id)
+                db_.client().remove(coll_, k.key);
+                db_.client().upsert(coll_, k.toJson(), k.id);
+                needsReload = true;
+            }
+        }
+        if (needsReload) reload();
     }
 }
 
@@ -53,33 +73,34 @@ ApiKey KeyStore::create(const std::string& label,
                         std::vector<std::string> projects,
                         bool admin) {
     ApiKey k;
+    k.id        = generateApiKey();
     k.key       = generateApiKey();
     k.label     = label;
     k.projects  = std::move(projects);
     k.admin     = admin;
     k.createdAt = static_cast<uint64_t>(std::time(nullptr));
-    db_.client().upsert(coll_, k.toJson(), k.key);
+    db_.client().upsert(coll_, k.toJson(), k.id);
     reload();
     return k;
 }
 
-bool KeyStore::update(const std::string& key,
+bool KeyStore::update(const std::string& id,
                       const std::optional<std::string>& label,
                       const std::optional<std::vector<std::string>>& projects,
                       const std::optional<bool>& admin) {
-    auto doc = db_.client().get(coll_, key);
+    auto doc = db_.client().get(coll_, id);
     if (!doc) return false;
     ApiKey k = ApiKey::fromJson(*doc);
     if (label)    k.label    = *label;
     if (projects) k.projects = *projects;
     if (admin)    k.admin    = *admin;
-    db_.client().upsert(coll_, k.toJson(), key);
+    db_.client().upsert(coll_, k.toJson(), id);
     reload();
     return true;
 }
 
-bool KeyStore::remove(const std::string& key) {
-    bool ok = db_.client().remove(coll_, key);
+bool KeyStore::remove(const std::string& id) {
+    bool ok = db_.client().remove(coll_, id);
     reload();
     return ok;
 }

+ 5 - 4
src/key_store.hpp

@@ -30,14 +30,15 @@ public:
     /// Persist a new key and refresh the snapshot.
     ApiKey create(const std::string& label, std::vector<std::string> projects, bool admin);
 
-    /// Patch an existing key.  Pass std::nullopt to leave a field unchanged.
+    /// Patch an existing key identified by its stable id.
+    /// Pass std::nullopt to leave a field unchanged.
     /// Returns false when the key does not exist.
-    bool update(const std::string& key, const std::optional<std::string>& label,
+    bool update(const std::string& id, const std::optional<std::string>& label,
                 const std::optional<std::vector<std::string>>& projects,
                 const std::optional<bool>& admin);
 
-    /// Delete a key.  Returns false when the key does not exist.
-    bool remove(const std::string& key);
+    /// Delete a key identified by its stable id.  Returns false when the key does not exist.
+    bool remove(const std::string& id);
 
     /// Subscribe to DB change events so the snapshot stays current automatically.
     void startWatch();

+ 39 - 10
tests/test_api_integration.cpp

@@ -49,31 +49,40 @@ TEST_F(ApiFixture, KeysCrudAndScopedAuthz) {
     auto mk = c.Post("/api/v1/keys",
         nlohmann::json{{"label","n8n"},{"projects",{project_}},{"admin",false}}.dump(), "application/json");
     ASSERT_TRUE(mk); ASSERT_EQ(mk->status, 201);
-    std::string scopedKey = nlohmann::json::parse(mk->body)["key"];
+    auto mkJson = nlohmann::json::parse(mk->body);
+    std::string scopedKey = mkJson["key"];
+    std::string scopedId  = mkJson["id"];
     ASSERT_FALSE(scopedKey.empty());
+    ASSERT_FALSE(scopedId.empty());
 
-    // list masks the secret
+    // list masks the secret but includes id
     auto list = c.Get("/api/v1/keys");
     ASSERT_EQ(list->status, 200);
     auto keysJson = nlohmann::json::parse(list->body);
-    for (auto& k : keysJson["keys"]) EXPECT_FALSE(k.contains("key"));
+    for (auto& k : keysJson["keys"]) {
+        EXPECT_FALSE(k.contains("key"));
+        EXPECT_TRUE(k.contains("id"));
+    }
 
     // scoped key can access its project, but not management or another project
     EXPECT_EQ(http(scopedKey).Get(("/api/v1/projects/" + project_).c_str())->status, 200);
     EXPECT_EQ(http(scopedKey).Get("/api/v1/keys")->status, 403);              // not admin
     EXPECT_EQ(http(scopedKey).Get("/api/v1/projects/svapitest_other")->status, 403);  // not granted
 
-    EXPECT_EQ(c.Delete(("/api/v1/keys/" + scopedKey).c_str())->status, 200);
+    // delete by id (not by secret)
+    EXPECT_EQ(c.Delete(("/api/v1/keys/" + scopedId).c_str())->status, 200);
     EXPECT_EQ(http(scopedKey).Get(("/api/v1/projects/" + project_).c_str())->status, 401);  // revoked
 }
 
 TEST_F(ApiFixture, NonAdminCannotCreateProject) {
     auto mk = admin().Post("/api/v1/keys",
         nlohmann::json{{"label","x"},{"projects",{project_}},{"admin",false}}.dump(), "application/json");
-    std::string scoped = nlohmann::json::parse(mk->body)["key"];
+    auto mkJson = nlohmann::json::parse(mk->body);
+    std::string scoped   = mkJson["key"];
+    std::string scopedId = mkJson["id"];
     auto r = http(scoped).Post("/api/v1/projects", nlohmann::json{{"name","svapitest_nope"}}.dump(), "application/json");
     ASSERT_TRUE(r); EXPECT_EQ(r->status, 403);
-    admin().Delete(("/api/v1/keys/" + scoped).c_str());
+    admin().Delete(("/api/v1/keys/" + scopedId).c_str());
 }
 
 TEST_F(ApiFixture, CollectionCrudUnderProject) {
@@ -237,7 +246,16 @@ TEST_F(ApiFixture, ProjectRecycleAfterDelete) {
 
 // Fix 5 regression: cannot delete the last admin key
 TEST_F(ApiFixture, CannotDeleteLastAdminKey) {
-    auto r = admin().Delete(("/api/v1/keys/" + adminKey_).c_str());
+    // Get the admin key's id from the keys list (find by key_prefix matching adminKey_'s first 8 chars)
+    auto listRes = admin().Get("/api/v1/keys");
+    ASSERT_TRUE(listRes); ASSERT_EQ(listRes->status, 200);
+    auto keysArr = nlohmann::json::parse(listRes->body)["keys"];
+    std::string prefix8 = adminKey_.substr(0, 8);
+    std::string adminId;
+    for (const auto& k : keysArr)
+        if (k["key_prefix"].get<std::string>() == prefix8) { adminId = k["id"].get<std::string>(); break; }
+    ASSERT_FALSE(adminId.empty());
+    auto r = admin().Delete(("/api/v1/keys/" + adminId).c_str());
     ASSERT_TRUE(r);
     EXPECT_EQ(r->status, 422);
     // Key still works
@@ -248,7 +266,16 @@ TEST_F(ApiFixture, CannotDeleteLastAdminKey) {
 
 // Fix 5 regression: cannot de-admin the last admin key
 TEST_F(ApiFixture, CannotDeAdminLastAdminKey) {
-    auto r = admin().Patch(("/api/v1/keys/" + adminKey_).c_str(),
+    // Get the admin key's id from the keys list (find by key_prefix matching adminKey_'s first 8 chars)
+    auto listRes = admin().Get("/api/v1/keys");
+    ASSERT_TRUE(listRes); ASSERT_EQ(listRes->status, 200);
+    auto keysArr = nlohmann::json::parse(listRes->body)["keys"];
+    std::string prefix8 = adminKey_.substr(0, 8);
+    std::string adminId;
+    for (const auto& k : keysArr)
+        if (k["key_prefix"].get<std::string>() == prefix8) { adminId = k["id"].get<std::string>(); break; }
+    ASSERT_FALSE(adminId.empty());
+    auto r = admin().Patch(("/api/v1/keys/" + adminId).c_str(),
                            nlohmann::json{{"admin", false}}.dump(), "application/json");
     ASSERT_TRUE(r);
     EXPECT_EQ(r->status, 422);
@@ -295,11 +322,13 @@ TEST_F(ApiFixture, SettingsPutChangesField) {
     auto mk = admin().Post("/api/v1/keys",
         nlohmann::json{{"label","scoped"},{"projects",{project_}},{"admin",false}}.dump(), "application/json");
     ASSERT_TRUE(mk); ASSERT_EQ(mk->status, 201);
-    std::string scopedKey = nlohmann::json::parse(mk->body)["key"];
+    auto mkJson2 = nlohmann::json::parse(mk->body);
+    std::string scopedKey = mkJson2["key"];
+    std::string scopedId2 = mkJson2["id"];
     auto denied = http(scopedKey).Get("/api/v1/settings");
     ASSERT_TRUE(denied);
     EXPECT_EQ(denied->status, 403);
-    admin().Delete(("/api/v1/keys/" + scopedKey).c_str());
+    admin().Delete(("/api/v1/keys/" + scopedId2).c_str());
 
     // Restore original model so other tests are unaffected
     admin().Put("/api/v1/settings",

+ 5 - 2
tests/test_stores.cpp

@@ -45,15 +45,18 @@ TEST(KeyStore, CreateUpdateRemove) {
     KeyStore ks(*db, coll); ks.bootstrap("");
 
     ApiKey made = ks.create("n8n", {"p1"}, /*admin*/ false);
+    EXPECT_FALSE(made.id.empty());
     EXPECT_FALSE(made.key.empty());
+    EXPECT_NE(made.id, made.key);  // id and secret are distinct random values
     ASSERT_TRUE(ks.lookup(made.key).has_value());
     EXPECT_TRUE(ks.lookup(made.key)->canAccess("p1"));
     EXPECT_FALSE(ks.lookup(made.key)->canAccess("p2"));
 
-    EXPECT_TRUE(ks.update(made.key, std::nullopt, std::vector<std::string>{"p1", "p2"}, std::nullopt));
+    // update/remove are keyed by id, not the secret
+    EXPECT_TRUE(ks.update(made.id, std::nullopt, std::vector<std::string>{"p1", "p2"}, std::nullopt));
     EXPECT_TRUE(ks.lookup(made.key)->canAccess("p2"));
 
-    EXPECT_TRUE(ks.remove(made.key));
+    EXPECT_TRUE(ks.remove(made.id));
     EXPECT_FALSE(ks.lookup(made.key).has_value());
     db->client().dropCollection(coll);
 }

+ 3 - 3
webui/src/pages/KeysAdmin.tsx

@@ -278,7 +278,7 @@ function EditGrantsModal({ keyRecord, allProjects, onClose }: EditGrantsModalPro
   const handleSave = async () => {
     setSubmitting(true)
     try {
-      await api.updateKey(keyRecord.key_prefix, { projects, admin: isAdmin })
+      await api.updateKey(keyRecord.id, { projects, admin: isAdmin })
       await queryClient.invalidateQueries({ queryKey: ['keys'] })
       push(`Key "${keyRecord.label}" updated.`, 'success')
       onClose()
@@ -362,7 +362,7 @@ function KeysTable({ keys, allProjects }: KeysTableProps) {
 
   const handleRevoke = async (keyRecord: ApiKeyPublic) => {
     try {
-      await api.deleteKey(keyRecord.key_prefix)
+      await api.deleteKey(keyRecord.id)
       await queryClient.invalidateQueries({ queryKey: ['keys'] })
       push(`Key "${keyRecord.label}" revoked.`, 'success')
     } catch (err) {
@@ -408,7 +408,7 @@ function KeysTable({ keys, allProjects }: KeysTableProps) {
           <tbody>
             {keys.map((k, idx) => (
               <tr
-                key={k.key_prefix}
+                key={k.id}
                 className={[
                   'border-b border-slate-700/20 transition hover:bg-slate-800/40',
                   idx % 2 === 0 ? 'bg-slate-900/30' : 'bg-slate-800/20',

+ 1 - 1
webui/src/types/index.ts

@@ -5,7 +5,7 @@ export interface CollectionMeta {
   document_count?: number; size_bytes?: number;
 }
 export interface ApiKeyPublic {
-  label: string; projects: string[]; admin: boolean; created_at: number; key_prefix: string;
+  id: string; label: string; projects: string[]; admin: boolean; created_at: number; key_prefix: string;
 }
 export interface ApiKeyCreated extends ApiKeyPublic { key: string }
 export interface SearchResult { id: string; score: number; data: Record<string, unknown> }