Jelajahi Sumber

feat: hand a credential to somebody else

Credentials could be shared but never handed over, so a credential made
by one person stayed theirs whatever happened to them or their role.

The first attempt did it by overwriting createdBy, because credentials had
no owner field at all. That would have destroyed a historical fact - who
made this - with nothing recoverable afterwards, and bought nothing:
neither field decides anything. So credentials now carry owner_id of their
own, transfers move it, and createdBy is left alone. A credential stored
before this reads its owner as createdBy when owner_id is absent, so
nothing needed migrating.

The owner is also now in the API response and shown on the credentials
page. It was absent from both, which is why a transfer would have been
invisible even when it worked.

**Ownership here is attribution, not authority, and the UI says so.** Who
may use a credential is project Read access or being in sharedWith; who
may manage it is project Write access. Neither consults owner_id or
createdBy. Transferring says who a credential belongs to; it moves no
capability, revokes nothing, and leaves existing shares in place.

Refusals: an unknown user id, and a new owner with no access to the
credential's project - the second explains what to do rather than only
saying no, since the fix is to add them to the project first.

Verified against the running service with a throwaway user, a throwaway
membership of a team project and a throwaway credential:

  before   ownerId admin        createdBy admin
  transfer 200
  after    ownerId the new user createdBy admin

Unknown user id and an owner without project access were both refused. The
throwaway user, membership and credential were removed afterwards.

81 passed, 0 failed, on a run with nothing restarting the services under
it - an earlier run showed 11 failures that were all connection errors
from a mid-run restart, not regressions.
fszontagh 1 bulan lalu
induk
melakukan
9935b02ee5

+ 18 - 0
lib/credentials/credential_store.cpp

@@ -402,6 +402,24 @@ Result<CredentialInfo> CredentialStore::setSharedWith(const std::string& id,
     return CredentialInfo::fromMetadata(doc.metadata);
 }
 
+Result<CredentialInfo> CredentialStore::setOwner(const std::string& id, const std::string& new_owner_id) {
+    auto stored = storage_.get(COLLECTION, id);
+    if (stored.failed()) {
+        return Error(ErrorCode::NotFound, "Credential not found: " + id);
+    }
+
+    // created_by is a historical fact - who made this - and is never touched
+    // by a transfer. Only owner_id moves.
+    auto doc = CredentialDocument::fromJson(stored.value());
+    doc.metadata.owner_id = new_owner_id;
+
+    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()) {

+ 10 - 0
lib/credentials/credential_store.hpp

@@ -36,6 +36,16 @@ public:
     // credential should not go anywhere near the secret itself.
     common::Result<CredentialInfo> setSharedWith(const std::string& id,
                                                  const std::vector<std::string>& user_ids);
+
+    // Change who this credential is attributed to now, by setting owner_id.
+    // created_by - who actually made it - is never touched: that is a
+    // historical fact a transfer should not be able to erase. Attribution
+    // only, either way - nothing reads owner_id or created_by to decide who
+    // may see, use, edit or delete a credential, so this changes a label, not
+    // a right. The encrypted blob is carried across untouched, the same way
+    // setSharedWith leaves it alone.
+    common::Result<CredentialInfo> setOwner(const std::string& id, const std::string& new_owner_id);
+
     common::Result<std::vector<CredentialInfo>> list();
 
     // Get decrypted credential for use (checks workflow access)

+ 17 - 0
lib/credentials/credential_types.cpp

@@ -282,6 +282,12 @@ nlohmann::json CredentialMetadata::toJson() const {
         {"createdBy", created_by},
         {"allowedWorkflows", allowed_workflows}
     };
+    // Only written once a transfer has actually happened. Writing it
+    // unconditionally - even as a copy of created_by - would quietly migrate
+    // every legacy credential the first time anything else about it changed.
+    if (!owner_id.empty()) {
+        j["ownerId"] = owner_id;
+    }
     if (!public_data.is_null() && !public_data.empty()) {
         j["publicData"] = public_data;
     }
@@ -305,6 +311,11 @@ CredentialMetadata CredentialMetadata::fromJson(const nlohmann::json& j) {
     meta.description = j.value("description", "");
     meta.type = credentialTypeFromString(j.value("type", "basic"));
     meta.created_by = j.value("createdBy", j.value("created_by", ""));
+    // Raw, not resolved: stays empty for a credential nobody has transferred
+    // yet. Callers that want the effective owner of a possibly-legacy
+    // credential fall back to created_by themselves, the way
+    // CredentialInfo::fromMetadata does for the API response.
+    meta.owner_id = j.value("ownerId", "");
     meta.declared_type = j.value("declaredType", "");
     meta.project_id = j.value("projectId", "");
     if (j.contains("sharedWith") && j["sharedWith"].is_array()) {
@@ -355,6 +366,9 @@ nlohmann::json CredentialInfo::toJson() const {
         {"description", description},
         {"type", credentialTypeToString(type)},
         {"createdBy", created_by},
+        // Always resolved by fromMetadata - equal to created_by until the
+        // credential has been transferred, the transferred-to user id after.
+        {"ownerId", owner_id},
         {"createdAt", created_at},
         {"updatedAt", updated_at},
         {"allowedWorkflows", allowed_workflows}
@@ -379,6 +393,9 @@ CredentialInfo CredentialInfo::fromMetadata(const CredentialMetadata& metadata)
     info.description = metadata.description;
     info.type = metadata.type;
     info.created_by = metadata.created_by;
+    // The fallback a legacy credential needs: no transfer has ever written
+    // ownerId for it, so its owner is still whoever created it.
+    info.owner_id = metadata.owner_id.empty() ? metadata.created_by : metadata.owner_id;
     info.created_at = metadata.created_at;
     info.updated_at = metadata.updated_at;
     info.allowed_workflows = metadata.allowed_workflows;

+ 17 - 0
lib/credentials/credential_types.hpp

@@ -148,7 +148,17 @@ struct CredentialMetadata {
     std::string name;
     std::string description;
     CredentialType type;
+    // Who made this - a historical fact, set once at creation and never
+    // changed again, not even by a transfer. Overwriting it on transfer would
+    // destroy the one thing nothing else can recover.
     std::string created_by;
+    // Who this credential is attributed to now. Raw and possibly empty: a
+    // credential stored before ownership transfer existed has no ownerId on
+    // disk, and none is written for it until somebody actually transfers it -
+    // that would be a migration wearing a read's clothes. Code that wants the
+    // effective owner of a possibly-legacy credential should fall back to
+    // created_by when this is empty, the way CredentialInfo::fromMetadata does.
+    std::string owner_id;
     int64_t created_at = 0;
     int64_t updated_at = 0;
     std::vector<std::string> allowed_workflows;  // Empty = all workflows
@@ -196,6 +206,13 @@ struct CredentialInfo {
     std::string description;
     CredentialType type;
     std::string created_by;
+    // The effective owner - always resolved, never empty as long as
+    // created_by is set. This is created_by's fallback value when no transfer
+    // has ever happened, and the transferred-to user id afterward. Neither
+    // this nor created_by grants any capability: what a caller may do with a
+    // credential comes from their role in project_id and from shared_with,
+    // never from who made it or who it is attributed to now.
+    std::string owner_id;
     int64_t created_at = 0;
     int64_t updated_at = 0;
     std::vector<std::string> allowed_workflows;

+ 82 - 0
src/webserver/api/credential_controller.cpp

@@ -222,6 +222,15 @@ void CredentialController::registerRoutes(httplib::Server& server) {
         });
     });
 
+    server.Post(R"(/api/v1/credentials/([^/]+)/transfer-owner)",
+                [this](const httplib::Request& req, httplib::Response& res) {
+        middleware_.requireAuth(req, res, [this](const httplib::Request& req,
+                                                 httplib::Response& res,
+                                                 const auth::AuthContext& ctx) {
+            transferCredentialOwner(req, res, ctx);
+        });
+    });
+
     LOG_INFO("Credential API routes registered");
 }
 
@@ -430,6 +439,79 @@ void CredentialController::refreshToken(const httplib::Request& req, httplib::Re
     sendJson(res, {{"success", true}});
 }
 
+bool CredentialController::checkTransferTarget(httplib::Response& res, const std::string& new_owner_id,
+                                               const std::string& project_id) {
+    if (new_owner_id.empty()) {
+        sendError(res, "newOwnerId is required", 400);
+        return false;
+    }
+
+    auto user = storage_.get("users", new_owner_id);
+    if (user.failed()) {
+        sendError(res, "Unknown user id", 400);
+        return false;
+    }
+
+    // The new owner has to be somebody who can actually reach the project this
+    // credential lives in, the same rule transferWorkflowOwner uses - a
+    // transfer that names somebody who cannot see the project is worse than an
+    // error that says why.
+    auth::AuthContext target_ctx;
+    target_ctx.user_id = new_owner_id;
+    target_ctx.role = user.value().value("role", "user");
+    if (access_.roleIn(target_ctx, project_id) == auth::ProjectRole::None) {
+        sendError(res, "That user has no access to the project this belongs to. "
+                       "Add them to the project before transferring ownership to them.", 400);
+        return false;
+    }
+    return true;
+}
+
+void CredentialController::transferCredentialOwner(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;
+    }
+    // Manage, not Write: transferring somebody's credential is an
+    // administrative act on the project, not an edit to the credential
+    // itself - the same distinction transferWorkflowOwner draws.
+    if (!access_.allowed(ctx, existing.value().project_id, auth::Action::Manage)) {
+        sendError(res, "Only a project admin can transfer ownership of this credential", 403);
+        return;
+    }
+
+    nlohmann::json body;
+    try {
+        body = nlohmann::json::parse(req.body);
+    } catch (const std::exception&) {
+        sendError(res, "Invalid request body", 400);
+        return;
+    }
+
+    const std::string new_owner_id = body.value("newOwnerId", "");
+    if (!checkTransferTarget(res, new_owner_id, existing.value().project_id)) return;
+
+    auto result = credential_store_.setOwner(id, new_owner_id);
+    if (result.failed()) {
+        sendError(res, result.error().message(), 500);
+        return;
+    }
+
+    LOG_INFO("Credential {} ownership transferred from {} to {} by {}", id,
+             existing.value().owner_id, new_owner_id, ctx.user_id);
+    sendJson(res, result.value().toJson());
+}
+
 void CredentialController::sendJson(httplib::Response& res, const nlohmann::json& data, int status) {
     res.status = status;
     res.set_content(data.dump(), "application/json");

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

@@ -57,6 +57,23 @@ private:
     void refreshToken(const httplib::Request& req, httplib::Response& res,
                      const auth::AuthContext& ctx);
 
+    // POST /api/v1/credentials/:id/transfer-owner - Change who this
+    // credential is attributed to, by setting ownerId. Attribution, not
+    // authority: neither ownerId nor createdBy is ever consulted by mayUse or
+    // mayManage, which look only at project membership and shared_with. This
+    // changes whose name is on the credential, nothing about who can use,
+    // edit or delete it - the previous owner keeps whatever access their
+    // project role or a share already gave them, and every existing share is
+    // untouched. createdBy - who actually made it - is left alone; only
+    // ownerId moves.
+    void transferCredentialOwner(const httplib::Request& req, httplib::Response& res,
+                                 const auth::AuthContext& ctx);
+
+    // Whether newOwnerId names an account that exists and can reach
+    // project_id. On failure, writes the error response itself.
+    bool checkTransferTarget(httplib::Response& res, const std::string& new_owner_id,
+                             const std::string& project_id);
+
     // 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

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

@@ -7,7 +7,15 @@ export interface CredentialInfo {
   name: string
   description?: string
   type: CredentialType
+  /** Who made this - a historical fact, unchanged by an ownership transfer. */
   createdBy: string
+  /**
+   * Who this credential is attributed to now. Equal to createdBy until a
+   * transfer, the transferred-to user id after. Attribution only: nothing on
+   * the server reads ownerId or createdBy to decide who may see, use, edit
+   * or delete a credential - that comes from project role and sharedWith.
+   */
+  ownerId: string
   createdAt: number
   // The named type this was created as, when a node registered one. The type
   // above is still what decides how it is stored and used.
@@ -111,6 +119,10 @@ function transformCredential(data: any): CredentialInfo {
     ...data,
     id: data.id || data._id,
     createdBy: data.createdBy || data.ownerId,
+    // The server always resolves this (falling back to createdBy for a
+    // credential nobody has transferred), but a defensive fallback here too
+    // in case an older response shape is ever replayed.
+    ownerId: data.ownerId || data.createdBy,
     // Sent by the server in milliseconds; it reads the document metadata itself.
     createdAt: data.createdAt,
     updatedAt: data.updatedAt,
@@ -169,4 +181,17 @@ export const credentialsApi = {
     const response = await api.post(`/credentials/${id}/refresh`)
     return response.data
   },
+
+  /**
+   * Change who this credential is attributed to, by setting ownerId. createdBy
+   * - who actually made it - is left untouched. Attribution only: nothing on
+   * the server reads ownerId or createdBy to decide who may see, use, edit or
+   * delete a credential, so this changes a label, not a right. Every existing
+   * share stays in place, and the previous owner keeps whatever access their
+   * project role or a share already gave them.
+   */
+  transferOwner: async (id: string, newOwnerId: string): Promise<CredentialInfo> => {
+    const response = await api.post(`/credentials/${id}/transfer-owner`, { newOwnerId })
+    return transformCredential(response.data)
+  },
 }

+ 67 - 0
webui/src/pages/CredentialsPage.tsx

@@ -1337,6 +1337,9 @@ function ShareCredentialDialog({
 }) {
   const [error, setError] = useState<string | null>(null)
   const [pick, setPick] = useState('')
+  const [transferPick, setTransferPick] = useState('')
+  const [confirmingTransfer, setConfirmingTransfer] = useState(false)
+  const [transferring, setTransferring] = useState(false)
 
   // Only somebody who can manage users can read the whole list, so this is
   // allowed to fail quietly - it only fills the picker.
@@ -1354,6 +1357,7 @@ function ShareCredentialDialog({
     return user ? (user.username as string) : '(deleted user)'
   }
   const candidates = users.filter((u: any) => !shared.includes(u._id || u.id))
+  const transferCandidates = users.filter((u: any) => (u._id || u.id) !== credential.ownerId)
 
   const run = async (fn: () => Promise<CredentialInfo>) => {
     setError(null)
@@ -1422,6 +1426,69 @@ function ShareCredentialDialog({
           </button>
         </div>
 
+        <div className="mt-5 pt-4 border-t border-gray-200 dark:border-slate-700">
+          <h4 className="text-sm font-semibold text-gray-900 dark:text-gray-100 mb-1">Ownership</h4>
+          <p className="text-xs text-gray-500 dark:text-gray-400 mb-1">
+            Owned by {nameOf(credential.ownerId)}.
+          </p>
+          <p className="text-xs text-gray-500 dark:text-gray-400 mb-3">
+            This only changes who the credential is attributed to. It does not change
+            who can see, use, edit or delete it - that still depends on project
+            membership and the shares above, both of which stay exactly as they are.
+            The current owner does not lose access by transferring it away.
+          </p>
+
+          {!confirmingTransfer ? (
+            <div className="flex items-center gap-2">
+              <select
+                value={transferPick}
+                onChange={(e) => setTransferPick(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="">Transfer to...</option>
+                {transferCandidates.map((u: any) => (
+                  <option key={u._id || u.id} value={u._id || u.id}>{u.username}</option>
+                ))}
+              </select>
+              <button
+                onClick={() => transferPick && setConfirmingTransfer(true)}
+                disabled={!transferPick}
+                className="px-3 py-2 text-sm bg-gray-100 dark:bg-slate-700 text-gray-700 dark:text-gray-200 rounded hover:bg-gray-200 dark:hover:bg-slate-600 disabled:opacity-40"
+              >
+                Transfer
+              </button>
+            </div>
+          ) : (
+            <div className="px-3 py-2 rounded bg-amber-50 dark:bg-amber-900/20 text-sm">
+              <p className="text-amber-800 dark:text-amber-300 mb-2">
+                Transfer ownership of "{credential.name}" to {nameOf(transferPick)}?
+              </p>
+              <div className="flex justify-end gap-2">
+                <button
+                  onClick={() => { setConfirmingTransfer(false); setTransferPick('') }}
+                  disabled={transferring}
+                  className="px-3 py-1.5 text-sm text-gray-600 dark:text-gray-400"
+                >
+                  Cancel
+                </button>
+                <button
+                  onClick={async () => {
+                    setTransferring(true)
+                    await run(() => credentialsApi.transferOwner(credential.id, transferPick))
+                    setTransferring(false)
+                    setConfirmingTransfer(false)
+                    setTransferPick('')
+                  }}
+                  disabled={transferring}
+                  className="px-3 py-1.5 text-sm bg-primary-600 text-white rounded hover:bg-primary-700 disabled:opacity-50"
+                >
+                  {transferring ? 'Transferring...' : 'Confirm transfer'}
+                </button>
+              </div>
+            </div>
+          )}
+        </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