Przeglądaj źródła

feat: share a credential with people outside its project

A credential lived in exactly one project, which is usually right and
occasionally in the way: whoever holds the API key is often not in the
project that needs it, and the only ways round it were to add them to the
project - far more than they need - or to keep a second copy of the
secret.

It can be shared with named people now. Sharing grants use: they can pick
the credential and run a workflow that uses it. It is not a lesser form
of ownership - a sharee cannot change it, delete it, or pass it on, and
the plaintext still leaves the store only for a runner. The dialog says
all of that rather than leaving somebody to find out by trying.

setSharedWith is its own method rather than a field on the update
request, because that path decrypts and re-encrypts the secret. Changing
who may use a credential should not go near the secret at all - and this
codebase has already had one bug where an update wrote an empty secret
over a real one.

Two smaller things fell out of it. The refusals now distinguish "you
cannot see this" from "you can see this but not change it": once a
credential is shared with somebody, answering 404 to their edit attempt
is a lie to a person who is looking straight at it. And somebody may
always give up their own share, which needs no rights over the credential
- only taking away somebody else's does.

Verified with a second account: invisible before sharing, including to
the share endpoint itself, which answers 404 rather than confirming it
exists; visible and fetchable after; refused with a reason on update,
delete and re-share; gone again after either party removes the share. The
secret survived the round trip - a workflow using that credential still
authenticated afterwards, which is the check that matters here. Exercised
through the browser as well as the API. 70 passed, 0 failed.
fszontagh 1 miesiąc temu
rodzic
commit
ca42034370

+ 21 - 0
lib/credentials/credential_store.cpp

@@ -381,6 +381,27 @@ Result<void> CredentialStore::update(const std::string& id, const UpdateCredenti
     return Result<void>();
 }
 
+Result<CredentialInfo> CredentialStore::setSharedWith(const std::string& id,
+                                                     const std::vector<std::string>& user_ids) {
+    auto stored = storage_.get(COLLECTION, id);
+    if (stored.failed()) {
+        return Error(ErrorCode::NotFound, "Credential not found: " + id);
+    }
+
+    // Read the whole document and put it back with only this field changed. The
+    // encrypted blob is carried across untouched: sharing must never be a path
+    // that decrypts, re-encrypts, or - as an earlier bug here did - writes an
+    // empty secret over a real one.
+    auto doc = CredentialDocument::fromJson(stored.value());
+    doc.metadata.shared_with = user_ids;
+
+    auto written = storage_.update(COLLECTION, id, doc.toJson());
+    if (written.failed()) {
+        return Error(written.error().code(), written.error().message());
+    }
+    return CredentialInfo::fromMetadata(doc.metadata);
+}
+
 Result<void> CredentialStore::remove(const std::string& id) {
     auto result = storage_.remove(COLLECTION, id);
     if (result.failed()) {

+ 7 - 0
lib/credentials/credential_store.hpp

@@ -29,6 +29,13 @@ public:
     common::Result<CredentialInfo> get(const std::string& id);
     common::Result<void> update(const std::string& id, const UpdateCredentialRequest& request);
     common::Result<void> remove(const std::string& id);
+
+    // Replace the list of people outside the project who may use this
+    // credential. Its own method rather than a field on UpdateCredentialRequest
+    // because that path re-encrypts the secret, and changing who may use a
+    // credential should not go anywhere near the secret itself.
+    common::Result<CredentialInfo> setSharedWith(const std::string& id,
+                                                 const std::vector<std::string>& user_ids);
     common::Result<std::vector<CredentialInfo>> list();
 
     // Get decrypted credential for use (checks workflow access)

+ 10 - 0
lib/credentials/credential_types.cpp

@@ -291,6 +291,9 @@ nlohmann::json CredentialMetadata::toJson() const {
     if (!project_id.empty()) {
         j["projectId"] = project_id;
     }
+    // Written even when empty: removing the last share has to be recorded, and a
+    // field only written when non-empty leaves the old list in the document.
+    j["sharedWith"] = shared_with;
     return j;
 }
 
@@ -304,6 +307,11 @@ CredentialMetadata CredentialMetadata::fromJson(const nlohmann::json& j) {
     meta.created_by = j.value("createdBy", j.value("created_by", ""));
     meta.declared_type = j.value("declaredType", "");
     meta.project_id = j.value("projectId", "");
+    if (j.contains("sharedWith") && j["sharedWith"].is_array()) {
+        for (const auto& user : j["sharedWith"]) {
+            if (user.is_string()) meta.shared_with.push_back(user.get<std::string>());
+        }
+    }
     // The database writes _created_at and _updated_at - snake_case, with the
     // leading underscore, in nanoseconds. The chain here tried _createdAt,
     // createdAt and created_at and matched none of them, so every credential
@@ -360,6 +368,7 @@ nlohmann::json CredentialInfo::toJson() const {
     if (!project_id.empty()) {
         j["projectId"] = project_id;
     }
+    j["sharedWith"] = shared_with;
     return j;
 }
 
@@ -376,6 +385,7 @@ CredentialInfo CredentialInfo::fromMetadata(const CredentialMetadata& metadata)
     info.public_data = metadata.public_data;
     info.declared_type = metadata.declared_type;
     info.project_id = metadata.project_id;
+    info.shared_with = metadata.shared_with;
     return info;
 }
 

+ 12 - 0
lib/credentials/credential_types.hpp

@@ -164,6 +164,17 @@ struct CredentialMetadata {
     // whole record, so a field the model does not know about is a field that
     // disappears the first time somebody edits the credential.
     std::string project_id;
+    // People outside the credential's project who may use it, by user id.
+    //
+    // A credential lives in one project, which is usually right and occasionally
+    // in the way: whoever holds the API key is often not in the project that
+    // needs it, and the alternatives were to add them to the project - far more
+    // than they need - or to keep a second copy of the secret.
+    //
+    // Being shared means being able to pick it and run a workflow that uses it.
+    // It does not mean editing or deleting it, and it never means seeing the
+    // secret: the plaintext leaves the store only for a runner.
+    std::vector<std::string> shared_with;
 
     nlohmann::json toJson() const;
     static CredentialMetadata fromJson(const nlohmann::json& j);
@@ -191,6 +202,7 @@ struct CredentialInfo {
     nlohmann::json public_data;  // Non-secret fields for editing
     std::string declared_type;   // See CredentialMetadata::declared_type
     std::string project_id;
+    std::vector<std::string> shared_with;  // See CredentialMetadata::shared_with
 
     nlohmann::json toJson() const;
     static CredentialInfo fromMetadata(const CredentialMetadata& metadata);

+ 129 - 6
src/webserver/api/credential_controller.cpp

@@ -1,3 +1,4 @@
+#include <algorithm>
 #include <cctype>
 #include <unordered_set>
 #include "credential_controller.hpp"
@@ -50,7 +51,123 @@ std::unordered_map<std::string, int> CredentialController::credentialUsage() {
     return usage;
 }
 
+bool CredentialController::mayUse(const auth::AuthContext& ctx,
+                                  const credentials::CredentialInfo& cred) const {
+    if (access_.allowed(ctx, cred.project_id, auth::Action::Read)) return true;
+    for (const auto& user_id : cred.shared_with) {
+        if (user_id == ctx.user_id) return true;
+    }
+    return false;
+}
+
+bool CredentialController::mayManage(const auth::AuthContext& ctx,
+                                     const credentials::CredentialInfo& cred) const {
+    // Being shared with never carries the right to change it, delete it or pass
+    // it on. Otherwise sharing would quietly hand over the credential itself.
+    return access_.allowed(ctx, cred.project_id, auth::Action::Write);
+}
+
+void CredentialController::shareCredential(const httplib::Request& req, httplib::Response& res,
+                                           const auth::AuthContext& ctx) {
+    const std::string id = req.matches[1].str();
+    auto existing = credential_store_.get(id);
+    if (existing.failed()) {
+        sendError(res, "Credential not found", 404);
+        return;
+    }
+    // 404 rather than 403 for somebody who cannot see it at all: whether a
+    // credential exists is only told to people who can reach it.
+    if (!mayUse(ctx, existing.value())) {
+        sendError(res, "Credential not found", 404);
+        return;
+    }
+    if (!mayManage(ctx, existing.value())) {
+        sendError(res, "Only someone who can change this credential can share it", 403);
+        return;
+    }
+
+    nlohmann::json body;
+    try {
+        body = nlohmann::json::parse(req.body);
+    } catch (...) {
+        sendError(res, "Invalid JSON body", 400);
+        return;
+    }
+    const std::string user_id = body.value("userId", "");
+    if (user_id.empty()) {
+        sendError(res, "Say which user to share it with, as userId", 400);
+        return;
+    }
+    if (user_id == ctx.user_id) {
+        sendError(res, "You already reach this credential - sharing it with yourself "
+                       "would do nothing", 400);
+        return;
+    }
+
+    auto shared = existing.value().shared_with;
+    if (std::find(shared.begin(), shared.end(), user_id) == shared.end()) {
+        shared.push_back(user_id);
+    }
+    auto updated = credential_store_.setSharedWith(id, shared);
+    if (updated.failed()) {
+        sendError(res, updated.error().message(), 500);
+        return;
+    }
+    LOG_INFO("Credential {} shared with user {} by {}", id, user_id, ctx.user_id);
+    sendJson(res, updated.value().toJson());
+}
+
+void CredentialController::unshareCredential(const httplib::Request& req, httplib::Response& res,
+                                             const auth::AuthContext& ctx) {
+    const std::string id = req.matches[1].str();
+    const std::string user_id = req.matches[2].str();
+
+    auto existing = credential_store_.get(id);
+    if (existing.failed()) {
+        sendError(res, "Credential not found", 404);
+        return;
+    }
+    if (!mayUse(ctx, existing.value())) {
+        sendError(res, "Credential not found", 404);
+        return;
+    }
+    // Somebody may always give up their own access, which needs no rights over
+    // the credential at all.
+    if (user_id != ctx.user_id && !mayManage(ctx, existing.value())) {
+        sendError(res, "Only someone who can change this credential can take a share away", 403);
+        return;
+    }
+
+    auto shared = existing.value().shared_with;
+    shared.erase(std::remove(shared.begin(), shared.end(), user_id), shared.end());
+    auto updated = credential_store_.setSharedWith(id, shared);
+    if (updated.failed()) {
+        sendError(res, updated.error().message(), 500);
+        return;
+    }
+    LOG_INFO("Credential {} unshared from user {} by {}", id, user_id, ctx.user_id);
+    sendJson(res, updated.value().toJson());
+}
+
 void CredentialController::registerRoutes(httplib::Server& server) {
+    server.Post(R"(/api/v1/credentials/([^/]+)/shares)",
+                [this](const httplib::Request& req, httplib::Response& res) {
+        middleware_.requireAuth(req, res, [this](const httplib::Request& req,
+                                                 httplib::Response& res,
+                                                 const auth::AuthContext& ctx) {
+            shareCredential(req, res, ctx);
+        });
+    });
+
+    server.Delete(R"(/api/v1/credentials/([^/]+)/shares/([^/]+))",
+                  [this](const httplib::Request& req, httplib::Response& res) {
+        middleware_.requireAuth(req, res, [this](const httplib::Request& req,
+                                                 httplib::Response& res,
+                                                 const auth::AuthContext& ctx) {
+            unshareCredential(req, res, ctx);
+        });
+    });
+
     // List credentials - requires admin role
     server.Get("/api/v1/credentials", [this](const httplib::Request& req, httplib::Response& res) {
         middleware_.requireAuth(req, res, [this](const httplib::Request& req,
@@ -122,7 +239,7 @@ void CredentialController::listCredentials(const httplib::Request& req, httplib:
     const auto usage = credentialUsage();
     nlohmann::json credentials_json = nlohmann::json::array();
     for (const auto& cred : result.value()) {
-        if (!reachable.contains(cred.project_id)) continue;
+        if (!reachable.contains(cred.project_id) && !mayUse(ctx, cred)) continue;
         auto entry = cred.toJson();
         const auto found = usage.find(cred.id);
         entry["usedByWorkflows"] = found == usage.end() ? 0 : found->second;
@@ -142,7 +259,7 @@ void CredentialController::getCredential(const httplib::Request& req, httplib::R
         sendError(res, result.error().message(), status);
         return;
     }
-    if (!access_.allowed(ctx, result.value().project_id, auth::Action::Read)) {
+    if (!mayUse(ctx, result.value())) {
         sendError(res, "Credential not found", 404);
         return;
     }
@@ -206,11 +323,14 @@ void CredentialController::updateCredential(const httplib::Request& req, httplib
             sendError(res, "Credential not found", 404);
             return;
         }
-        if (!access_.allowed(ctx, existing.value().project_id, auth::Action::Read)) {
+        // Somebody it was shared with can see it, so "not found" would be a lie
+        // to them - they get the real reason instead. To anybody who cannot see
+        // it at all, whether it exists is still not something to disclose.
+        if (!mayUse(ctx, existing.value())) {
             sendError(res, "Credential not found", 404);
             return;
         }
-        if (!access_.allowed(ctx, existing.value().project_id, auth::Action::Write)) {
+        if (!mayManage(ctx, existing.value())) {
             sendError(res, "You can see this credential but not change it", 403);
             return;
         }
@@ -254,11 +374,14 @@ void CredentialController::deleteCredential(const httplib::Request& req, httplib
             sendError(res, "Credential not found", 404);
             return;
         }
-        if (!access_.allowed(ctx, existing.value().project_id, auth::Action::Read)) {
+        // Somebody it was shared with can see it, so "not found" would be a lie
+        // to them - they get the real reason instead. To anybody who cannot see
+        // it at all, whether it exists is still not something to disclose.
+        if (!mayUse(ctx, existing.value())) {
             sendError(res, "Credential not found", 404);
             return;
         }
-        if (!access_.allowed(ctx, existing.value().project_id, auth::Action::Write)) {
+        if (!mayManage(ctx, existing.value())) {
             sendError(res, "You can see this credential but not delete it", 403);
             return;
         }

+ 12 - 0
src/webserver/api/credential_controller.hpp

@@ -22,6 +22,18 @@ public:
 
 private:
     // GET /api/v1/credentials - List all credentials (metadata only)
+    // Who may use a credential, in one place. Sharing is an addition to the
+    // project rule, never a replacement: a sharee may pick it and run it, and
+    // may not edit, delete or re-share it.
+    bool mayUse(const auth::AuthContext& ctx, const credentials::CredentialInfo& cred) const;
+    bool mayManage(const auth::AuthContext& ctx, const credentials::CredentialInfo& cred) const;
+
+    // Share and unshare.
+    void shareCredential(const httplib::Request& req, httplib::Response& res,
+                         const auth::AuthContext& ctx);
+    void unshareCredential(const httplib::Request& req, httplib::Response& res,
+                           const auth::AuthContext& ctx);
+
     void listCredentials(const httplib::Request& req, httplib::Response& res,
                         const auth::AuthContext& ctx);
 

+ 17 - 0
webui/src/api/credentials.ts

@@ -17,6 +17,12 @@ export interface CredentialInfo {
   /** How many workflows name this credential - the way to tell two of the same name apart. */
   usedByWorkflows?: number
   projectId?: string
+  /**
+   * User ids outside the credential's project who may use it. Being shared with
+   * means being able to pick it and run a workflow that uses it - not to change
+   * it, delete it, share it on, or see the secret.
+   */
+  sharedWith?: string[]
   publicData?: Record<string, unknown>  // Non-secret fields for editing
 }
 
@@ -110,6 +116,7 @@ function transformCredential(data: any): CredentialInfo {
     updatedAt: data.updatedAt,
     declaredType: data.declaredType || undefined,
     allowedWorkflows: data.allowedWorkflows || [],
+    sharedWith: data.sharedWith || [],
   }
 }
 
@@ -141,6 +148,16 @@ export const credentialsApi = {
     await api.delete(`/credentials/${id}`)
   },
 
+  share: async (id: string, userId: string): Promise<CredentialInfo> => {
+    const response = await api.post(`/credentials/${id}/shares`, { userId })
+    return transformCredential(response.data)
+  },
+
+  unshare: async (id: string, userId: string): Promise<CredentialInfo> => {
+    const response = await api.delete(`/credentials/${id}/shares/${userId}`)
+    return transformCredential(response.data)
+  },
+
   refreshOAuth2: async (id: string): Promise<{ success: boolean; expiresAt?: number }> => {
     const response = await api.post(`/credentials/${id}/refresh`)
     return response.data

+ 146 - 1
webui/src/pages/CredentialsPage.tsx

@@ -1,5 +1,5 @@
 import { useState } from 'react'
-import { projectsApi, Project } from '../api/users'
+import { projectsApi, Project, usersApi } from '../api/users'
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'
 import { nodesApi } from '../api/workflows'
 import { format } from 'date-fns'
@@ -20,6 +20,7 @@ import {
   Key,
   Plus,
   Trash2,
+  Users2,
   Edit3,
   Shield,
   User,
@@ -105,6 +106,7 @@ export default function CredentialsPage() {
   const [searchTerm, setSearchTerm] = useState('')
   const [showCreateModal, setShowCreateModal] = useState(false)
   const [editingCredential, setEditingCredential] = useState<CredentialInfo | null>(null)
+  const [sharingCredential, setSharingCredential] = useState<CredentialInfo | null>(null)
   const [showDeleteModal, setShowDeleteModal] = useState(false)
   const [credentialToDelete, setCredentialToDelete] = useState<CredentialInfo | null>(null)
 
@@ -258,6 +260,21 @@ export default function CredentialsPage() {
                         />
                       </button>
                     )}
+                    <button
+                      onClick={() => setSharingCredential(cred)}
+                      className={`p-1.5 rounded hover:bg-gray-100 dark:hover:bg-slate-700 ${
+                        (cred.sharedWith?.length ?? 0) > 0
+                          ? 'text-primary-600 dark:text-primary-400'
+                          : 'text-gray-400 hover:text-primary-600 dark:hover:text-primary-400'
+                      }`}
+                      title={
+                        (cred.sharedWith?.length ?? 0) > 0
+                          ? `Shared with ${cred.sharedWith!.length} ${cred.sharedWith!.length === 1 ? 'person' : 'people'}`
+                          : 'Share with people outside this project'
+                      }
+                    >
+                      <Users2 className="w-4 h-4" />
+                    </button>
                     <button
                       onClick={() => setEditingCredential(cred)}
                       className="p-1.5 text-gray-400 hover:text-primary-600 dark:hover:text-primary-400 hover:bg-gray-100 dark:hover:bg-slate-700 rounded"
@@ -313,6 +330,17 @@ export default function CredentialsPage() {
         </div>
       )}
 
+      {sharingCredential && (
+        <ShareCredentialDialog
+          credential={sharingCredential}
+          onClose={() => setSharingCredential(null)}
+          onChanged={(updated) => {
+            setSharingCredential(updated)
+            queryClient.invalidateQueries({ queryKey: ['credentials'] })
+          }}
+        />
+      )}
+
       {/* Create/Edit Modal */}
       {(showCreateModal || editingCredential) && (
         <CredentialModal
@@ -1284,3 +1312,120 @@ function CredentialModal({ credential, onClose, onSuccess }: CredentialModalProp
     </div>
   )
 }
+
+/**
+ * Who, outside this credential's project, may use it.
+ *
+ * A credential lives in one project, which is usually right and occasionally in
+ * the way: whoever holds the API key is often not in the project that needs it,
+ * and the alternatives were to add them to the project - far more than they
+ * need - or to keep a second copy of the secret.
+ *
+ * Sharing grants use, and only use. The dialog says so rather than leaving
+ * somebody to find out by trying.
+ */
+function ShareCredentialDialog({
+  credential,
+  onClose,
+  onChanged,
+}: {
+  credential: CredentialInfo
+  onClose: () => void
+  onChanged: (updated: CredentialInfo) => void
+}) {
+  const [error, setError] = useState<string | null>(null)
+  const [pick, setPick] = useState('')
+
+  // Only somebody who can manage users can read the whole list, so this is
+  // allowed to fail quietly - it only fills the picker.
+  const { data: users = [] } = useQuery({
+    queryKey: ['users'],
+    queryFn: () => usersApi.list(),
+    retry: false,
+  })
+
+  const shared = credential.sharedWith || []
+  const nameOf = (id: string) => {
+    const user = users.find((u: any) => (u._id || u.id) === id)
+    // Somebody whose account has gone. Said plainly rather than shown as a
+    // bare identifier nobody can act on.
+    return user ? (user.username as string) : '(deleted user)'
+  }
+  const candidates = users.filter((u: any) => !shared.includes(u._id || u.id))
+
+  const run = async (fn: () => Promise<CredentialInfo>) => {
+    setError(null)
+    try {
+      onChanged(await fn())
+      setPick('')
+    } catch (e: any) {
+      setError(e?.response?.data?.error || e.message || 'That did not work')
+    }
+  }
+
+  return (
+    <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50 p-4" onClick={onClose}>
+      <div className="bg-white dark:bg-slate-800 rounded-lg shadow-xl w-full max-w-md p-5" onClick={(e) => e.stopPropagation()}>
+        <h3 className="font-semibold text-gray-900 dark:text-gray-100 mb-1">
+          Share "{credential.name}"
+        </h3>
+        <p className="text-xs text-gray-500 dark:text-gray-400 mb-4">
+          They will be able to pick this credential and run workflows that use it.
+          They cannot change it, delete it, share it on, or see the secret.
+        </p>
+
+        {error && (
+          <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">
+            {error}
+          </div>
+        )}
+
+        {shared.length === 0 ? (
+          <p className="text-sm text-gray-500 dark:text-gray-400 mb-4">
+            Not shared with anybody. Only people who can reach its project can use it.
+          </p>
+        ) : (
+          <ul className="mb-4 space-y-1">
+            {shared.map((id) => (
+              <li key={id} className="flex items-center justify-between text-sm px-2 py-1 rounded bg-gray-50 dark:bg-slate-900">
+                <span className="text-gray-700 dark:text-gray-300">{nameOf(id)}</span>
+                <button
+                  onClick={() => run(() => credentialsApi.unshare(credential.id, id))}
+                  className="text-xs text-gray-500 hover:text-red-600 dark:hover:text-red-400"
+                >
+                  Remove
+                </button>
+              </li>
+            ))}
+          </ul>
+        )}
+
+        <div className="flex items-center gap-2">
+          <select
+            value={pick}
+            onChange={(e) => setPick(e.target.value)}
+            className="flex-1 px-3 py-2 text-sm border border-gray-300 dark:border-slate-600 rounded bg-white dark:bg-slate-900 text-gray-900 dark:text-gray-100"
+          >
+            <option value="">Add somebody...</option>
+            {candidates.map((u: any) => (
+              <option key={u._id || u.id} value={u._id || u.id}>{u.username}</option>
+            ))}
+          </select>
+          <button
+            onClick={() => pick && run(() => credentialsApi.share(credential.id, pick))}
+            disabled={!pick}
+            className="px-3 py-2 text-sm bg-primary-600 text-white rounded hover:bg-primary-700 disabled:opacity-40"
+          >
+            Share
+          </button>
+        </div>
+
+        <div className="flex justify-end mt-5">
+          <button onClick={onClose} className="px-3 py-1.5 text-sm text-gray-600 dark:text-gray-400">
+            Done
+          </button>
+        </div>
+      </div>
+    </div>
+  )
+}