Bläddra i källkod

feat: say which credential is which, by id and by what uses it

Two credentials can carry the same name, and then nothing on the screen
says which one a workflow is actually using - so neither can safely be
deleted. Each card now shows the start of its id, which copies on click,
and how many workflows name it.

The count is worked out by searching each workflow for credential ids
rather than by reading a list of the config keys a credential can hide
under. Nodes name them differently - credentialId here, imapCredential
there - and a list of those keys goes stale the moment somebody writes a
node, silently under-counting rather than failing. Counted once per
workflow however many of its nodes use the credential, since the question
is how many workflows would break without it.

Nothing-uses-this is drawn in amber rather than grey: it is either safe to
delete or a sign that something is pointing at a different credential with
the same name, and both are worth a second look.

Found while wiring it: the credentials mapper listed every field by name,
so usedByWorkflows arrived from the server and never reached the screen -
the same way the workflow mapper dropped the record version and broke
publishing. Both mappers now pass everything through and rename only what
needs renaming, so the next field added does not have to be remembered in
two places.

Verified in the browser: the counts match the API - 3 for the two shared
credentials, 1 for most, and 0 for the second "SB - OCR", which is the
ambiguity that prompted this.
fszontagh 1 månad sedan
förälder
incheckning
58b8b1270f

+ 43 - 1
src/webserver/api/credential_controller.cpp

@@ -1,3 +1,5 @@
+#include <cctype>
+#include <unordered_set>
 #include "credential_controller.hpp"
 #include "logging/logger.hpp"
 
@@ -7,11 +9,47 @@ using namespace credentials;
 
 CredentialController::CredentialController(CredentialStore& credential_store,
                                            auth::AccessControl& access,
+                                           storage::StorageClient& storage,
                                            auth::AuthMiddleware& middleware)
     : credential_store_(credential_store)
     , access_(access)
+    , storage_(storage)
     , middleware_(middleware) {}
 
+std::unordered_map<std::string, int> CredentialController::credentialUsage() {
+    std::unordered_map<std::string, int> usage;
+
+    storage::QueryOptions options;
+    options.page_size = 1000;
+    auto workflows = storage_.query("workflows", options);
+    if (workflows.failed()) return usage;
+
+    for (const auto& workflow : workflows.value().documents) {
+        // A credential id can appear in any node's config, under whatever key
+        // that node calls it - credentialId here, imapCredential there. Rather
+        // than keep a list of every key any node might use, which is a list
+        // that goes stale the moment somebody writes a node, the workflow is
+        // searched for the ids themselves.
+        const std::string text = workflow.dump();
+        std::unordered_set<std::string> counted;
+        size_t at = 0;
+        while ((at = text.find("cred_", at)) != std::string::npos) {
+            size_t end = at;
+            while (end < text.size() &&
+                   (std::isalnum(static_cast<unsigned char>(text[end])) || text[end] == '_' ||
+                    text[end] == '-')) {
+                ++end;
+            }
+            const std::string id = text.substr(at, end - at);
+            // Once per workflow, however many nodes in it use the credential -
+            // the question is how many workflows would break without it.
+            if (counted.insert(id).second) usage[id]++;
+            at = end;
+        }
+    }
+    return usage;
+}
+
 void CredentialController::registerRoutes(httplib::Server& server) {
     // List credentials - requires admin role
     server.Get("/api/v1/credentials", [this](const httplib::Request& req, httplib::Response& res) {
@@ -81,10 +119,14 @@ void CredentialController::listCredentials(const httplib::Request& req, httplib:
     // Only the projects this caller can reach. A credential is the most
     // sensitive thing here - it is somebody's password to another system.
     const auto reachable = access_.projectsFor(ctx);
+    const auto usage = credentialUsage();
     nlohmann::json credentials_json = nlohmann::json::array();
     for (const auto& cred : result.value()) {
         if (!reachable.contains(cred.project_id)) continue;
-        credentials_json.push_back(cred.toJson());
+        auto entry = cred.toJson();
+        const auto found = usage.find(cred.id);
+        entry["usedByWorkflows"] = found == usage.end() ? 0 : found->second;
+        credentials_json.push_back(entry);
     }
 
     sendJson(res, {{"credentials", credentials_json}});

+ 10 - 1
src/webserver/api/credential_controller.hpp

@@ -1,10 +1,12 @@
 #pragma once
 
 #include <httplib.h>
+#include <unordered_map>
 #include <nlohmann/json.hpp>
 #include "../auth/auth_middleware.hpp"
 #include "../auth/access.hpp"
 #include "credentials/credential_store.hpp"
+#include "storage/storage_client.hpp"
 
 namespace smartbotic::webserver::api {
 
@@ -12,7 +14,8 @@ class CredentialController {
 public:
     CredentialController(credentials::CredentialStore& credential_store,
                          auth::AccessControl& access,
-                        auth::AuthMiddleware& middleware);
+                         storage::StorageClient& storage,
+                         auth::AuthMiddleware& middleware);
 
     // Register routes
     void registerRoutes(httplib::Server& server);
@@ -43,11 +46,17 @@ private:
                      const auth::AuthContext& ctx);
 
     // Helper methods
+    // How many workflows name each credential, keyed by credential id. Two
+    // credentials can carry the same name, and then the only way to tell which
+    // is the live one is what uses it.
+    std::unordered_map<std::string, int> credentialUsage();
+
     void sendJson(httplib::Response& res, const nlohmann::json& data, int status = 200);
     void sendError(httplib::Response& res, const std::string& message, int status);
 
     credentials::CredentialStore& credential_store_;
     auth::AccessControl& access_;
+    storage::StorageClient& storage_;
     auth::AuthMiddleware& middleware_;
 };
 

+ 1 - 1
src/webserver/webserver_service.cpp

@@ -318,7 +318,7 @@ void WebServerService::setupRoutes() {
     database_ctrl_->registerRoutes(server);
 
     credential_ctrl_ = std::make_unique<api::CredentialController>(
-        *credential_store_, *access_, *auth_middleware_);
+        *credential_store_, *access_, *storage_, *auth_middleware_);
     credential_ctrl_->registerRoutes(server);
 
     LOG_INFO("API routes registered");

+ 12 - 5
webui/src/api/credentials.ts

@@ -14,6 +14,9 @@ export interface CredentialInfo {
   declaredType?: string
   updatedAt: number
   allowedWorkflows: string[]
+  /** How many workflows name this credential - the way to tell two of the same name apart. */
+  usedByWorkflows?: number
+  projectId?: string
   publicData?: Record<string, unknown>  // Non-secret fields for editing
 }
 
@@ -89,17 +92,21 @@ export interface UpdateCredentialRequest {
 
 // Transform backend credential data to frontend format
 function transformCredential(data: any): CredentialInfo {
+  // Everything the server sent, then the handful of fields that need a
+  // different name or a default.
+  //
+  // Listing the fields one by one is how "usedByWorkflows" arrived from the
+  // server and never reached the screen - the same way the workflow mapper
+  // dropped the record version. A mapper that names every field silently
+  // discards every field added after it was written, and nothing fails.
   return {
+    ...data,
     id: data.id || data._id,
-    name: data.name,
-    description: data.description,
-    type: data.type,
     createdBy: data.createdBy || data.ownerId,
     createdAt: data.createdAt || data._createdAt,
-    declaredType: data.declaredType || undefined,
     updatedAt: data.updatedAt || data._updatedAt,
+    declaredType: data.declaredType || undefined,
     allowedWorkflows: data.allowedWorkflows || [],
-    publicData: data.publicData,
   }
 }
 

+ 28 - 2
webui/src/pages/CredentialsPage.tsx

@@ -213,9 +213,35 @@ export default function CredentialsPage() {
                     <div className="w-10 h-10 bg-primary-50 dark:bg-primary-900/30 rounded-lg flex items-center justify-center">
                       <TypeIcon className="w-5 h-5 text-primary-600 dark:text-primary-400" />
                     </div>
-                    <div>
-                      <h3 className="font-semibold text-gray-900 dark:text-gray-100">{cred.name}</h3>
+                    <div className="min-w-0">
+                      <h3 className="font-semibold text-gray-900 dark:text-gray-100 truncate">{cred.name}</h3>
                       <span className="text-xs text-gray-500 dark:text-gray-400">{typeInfo.label}</span>
+                      {/* Two credentials can carry the same name, and then the
+                          name alone cannot say which one a workflow is using.
+                          The id and the count can. */}
+                      <div className="flex items-center gap-2 mt-0.5">
+                        <code
+                          className="text-[10px] text-gray-400 dark:text-gray-500 cursor-pointer hover:text-gray-600 dark:hover:text-gray-300"
+                          title={`${cred.id} - click to copy`}
+                          onClick={() => navigator.clipboard?.writeText(cred.id)}
+                        >
+                          {cred.id.replace(/^cred_/, '').slice(0, 8)}
+                        </code>
+                        <span
+                          className={`text-[10px] px-1.5 py-0.5 rounded ${
+                            (cred.usedByWorkflows ?? 0) > 0
+                              ? 'bg-gray-100 dark:bg-slate-700 text-gray-600 dark:text-gray-300'
+                              : 'bg-amber-50 dark:bg-amber-900/30 text-amber-700 dark:text-amber-400'
+                          }`}
+                          title={
+                            (cred.usedByWorkflows ?? 0) > 0
+                              ? 'Workflows that name this credential'
+                              : 'Nothing uses this one - safe to delete, or something is pointing at a different credential with the same name'
+                          }
+                        >
+                          {cred.usedByWorkflows ?? 0} in use
+                        </span>
+                      </div>
                     </div>
                   </div>
                   <div className="flex items-center gap-1">