Let the API create a token that carries a vault

CLAUDE.md

@@ -632,26 +632,33 @@ against the code as it stood, not reasoned about: where a claim needed a fact, t
   asserts each is *still* unrouted and *still* never handed out — so the list cannot silently
   excuse a new one. Everything not on it must resolve **and** satisfy the dispatch contract.
 
-## Known gap — the API cannot create a token that carries a vault
-
-`POST /api/v1/auth-tokens` answers **500 "Error while retrieving master password from context"** for
-every action a token carries a vault for — the five `SECURED_ACTIONS` and the three
-`CAN_USE_SECURE_TOKEN_ACTIONS`, so `ACCOUNT_VIEW` and `ACCOUNT_CREATE` among them. With or without a
-`password`. The help documents `actionId` with no restriction, and the web creates these tokens
-fine, so this is an oversight rather than a policy.
-
-The cause is not in the controller. `AuthToken::injectSecureData()` needs the master password on the
-context to seal the vault; the API only ever loads it from the *calling* token's own vault
-(`Api::requireMasterPass()`), and `AUTHTOKEN_CREATE` is on neither list, so it has no vault to load
-it from. Making this work therefore means putting `AUTHTOKEN_CREATE` on
-`CAN_USE_SECURE_TOKEN_ACTIONS` — which is a decision, not a fix: it makes a token that can mint
-tokens also a token that carries the master password. **That is why it is recorded here rather than
-patched.** The alternative is to answer a clear refusal instead of a 500, which is smaller but
-encodes "permanently unavailable" for `ACCOUNT_VIEW` tokens, and that looks wrong.
-
-Do not "fix" it by having `injectSecureData()` fall back to an empty key: that seals the vault with
-the empty string, and `Api::getMasterPassFromVault()` requires a non-empty `tokenPass`, so the token
-can never be opened. That exact failure is what the web form's password rule now prevents.
+## A token that carries a vault, and what that costs
+
+Some actions need the master password, so a token issued for one carries a **vault**: the master
+password sealed with the token's own password *and* the token itself. `AuthToken::needsSecureToken()`
+is the single definition of which actions those are — `SECURED_ACTIONS` plus
+`CAN_USE_SECURE_TOKEN_ACTIONS` — and both the web form and the API ask it.
+
+`POST /api/v1/auth-tokens` used to answer **500 "Error while retrieving master password from
+context"** for every one of them, with or without a password, while the web created the same tokens
+without difficulty: sealing a vault needs the master password on the context, and the API only ever
+loads that from the *calling* token's own vault, which `AUTHTOKEN_CREATE` did not have.
+
+It works now, and the way it was made to work is a decision worth knowing rather than a detail:
+**`AUTHTOKEN_CREATE` and `AUTHTOKEN_EDIT` are themselves on `CAN_USE_SECURE_TOKEN_ACTIONS`, so a
+token that can mint tokens also carries the master password.** That is the authority the web already
+grants — an administrator who can reach the tokens page has unlocked the vault with their own
+password — but on the API it is a bearer credential sitting in somebody's script, so it is worth as
+much as the vault. Two things follow, and both are enforced in
+`AuthTokenBase::prepareSecureToken()`:
+
+- the password on such a token is **required**, not optional. Without it the vault is sealed with
+  the empty string and nothing can open it: `Api::getMasterPassFromVault()` reads `tokenPass` as a
+  required parameter, and required refuses the empty string, so the one password that would work
+  cannot be presented. Do not "fix" that by letting the empty key through;
+- creating one requires a calling token that already carries a vault, so an `AUTHTOKEN_CREATE`
+  token minted before this existed has none and gets a 401 until it is re-issued. The web can
+  always issue the first one.
 
 ## Escaping: on output, never on input
 

src/Application/Auth/Services/AuthToken.php

@@ -76,6 +76,18 @@ final class AuthToken extends Service implements AuthTokenService
         AclActionsInterface::ACCOUNT_VIEW,
         AclActionsInterface::CATEGORY_VIEW,
         AclActionsInterface::CLIENT_VIEW,
+        // Sealing the master password into a new token needs the master password, and the API can
+        // only get it out of the calling token's own vault. Without these two, every action a
+        // token carries a vault for — ACCOUNT_VIEW and ACCOUNT_CREATE among them — could be
+        // created from the web and not from the API, which answered 500 for all of them.
+        //
+        // It follows that a token which can mint tokens also carries the master password. That is
+        // the same authority the web grants: an administrator who can reach the tokens page has
+        // already unlocked the vault with their own password. It is not free, though — such a
+        // token is worth as much as the vault, which is why the password protecting it is required
+        // rather than optional (`AuthTokenBase::prepareSecureToken()`).
+        AclActionsInterface::AUTHTOKEN_CREATE,
+        AclActionsInterface::AUTHTOKEN_EDIT,
     ];
 
     /**

src/Infrastructure/Adapter/In/Api/Controllers/AuthToken/AuthTokenBase.php

@@ -8,9 +8,14 @@
 use SP\Application\Auth\Ports\AuthTokenService;
 use SP\Domain\Auth\Models\AuthToken as AuthTokenModel;
 use SP\Domain\Core\Acl\AclInterface;
+use SP\Domain\Core\Exceptions\ValidationException;
+use SP\Domain\Common\Services\ServiceException;
+use SP\Application\Auth\Services\AuthToken;
 use SP\Infrastructure\Adapter\In\Api\Controllers\ControllerBase;
 use SP\Infrastructure\Adapter\In\Api\Controllers\Help\AuthTokenHelp;
 
+use function SP\__u;
+
 abstract class AuthTokenBase extends ControllerBase
 {
     /**
@@ -45,4 +50,34 @@ protected static function withoutSecrets(AuthTokenModel $authToken): array
     {
         return $authToken->toArray(null, AuthTokenModel::SECRET_COLS, true);
     }
+
+    /**
+     * What a token that carries a vault needs before it can be written.
+     *
+     * A vault is the master password sealed with the token's own password and the token itself, so
+     * two things have to be true and neither was checked here. The password cannot be blank, or the
+     * vault is sealed with the empty string and nothing can open it again — `Api` reads `tokenPass`
+     * as a required parameter, and required refuses the empty string, so the one password that
+     * would work cannot be presented. And the master password has to be on the context to seal it
+     * there at all, which on the API means loading it from the calling token's own vault.
+     *
+     * Without the second, this endpoint answered 500 "Error while retrieving master password from
+     * context" for every action a token carries a vault for, with or without a password, while the
+     * web created the same tokens without difficulty.
+     *
+     * @throws ServiceException
+     * @throws ValidationException
+     */
+    final protected function prepareSecureToken(int $actionId, ?string $password): void
+    {
+        if (!AuthToken::needsSecureToken($actionId)) {
+            return;
+        }
+
+        if (empty($password)) {
+            throw ValidationException::error(__u('Password cannot be blank'));
+        }
+
+        $this->apiService->requireMasterPass();
+    }
 }

src/Infrastructure/Adapter/In/Api/Controllers/AuthToken/CreateController.php

@@ -17,10 +17,15 @@ public function createAction(): ApiResponse
     {
         $this->setupApi(AclActionsInterface::AUTHTOKEN_CREATE);
 
+        $actionId = $this->apiService->getParamInt('actionId', true);
+        $password = $this->apiService->getParamRaw('password');
+
+        $this->prepareSecureToken($actionId, $password);
+
         $tokenData = new AuthToken([
             'userId'   => $this->apiService->getParamInt('userId', true),
-            'actionId' => $this->apiService->getParamInt('actionId', true),
-            'hash'     => $this->apiService->getParamRaw('password'),
+            'actionId' => $actionId,
+            'hash'     => $password,
         ]);
 
         $id = $this->authTokenService->create($tokenData);

src/Infrastructure/Adapter/In/Api/Controllers/AuthToken/EditController.php

@@ -17,11 +17,16 @@ public function editAction(): ApiResponse
     {
         $this->setupApi(AclActionsInterface::AUTHTOKEN_EDIT);
 
+        $actionId = $this->apiService->getParamInt('actionId', true);
+        $password = $this->apiService->getParamRaw('password');
+
+        $this->prepareSecureToken($actionId, $password);
+
         $tokenData = new AuthToken([
             'id'       => $this->apiService->getParamInt('id', true),
             'userId'   => $this->apiService->getParamInt('userId', true),
-            'actionId' => $this->apiService->getParamInt('actionId', true),
-            'hash'     => $this->apiService->getParamRaw('password'),
+            'actionId' => $actionId,
+            'hash'     => $password,
         ]);
 
         $this->authTokenService->update($tokenData);

tests/Integration/Infrastructure/Adapter/In/Api/Controllers/AuthTokenRoundTripTest.php

@@ -26,6 +26,8 @@
 namespace SP\Tests\Integration\Infrastructure\Adapter\In\Api\Controllers;
 
 use PHPUnit\Framework\Attributes\Group;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Test;
 use SP\Domain\Core\Acl\AclActionsInterface;
 use SP\Tests\Integration\Infrastructure\Adapter\In\Api\ApiTestCase;
 use stdClass;
@@ -89,6 +91,116 @@ public function testATokenThatWasNeverIssuedIsRefused(): void
         $this->assertObjectHasProperty('error', $r->body);
     }
 
+    /**
+     * A token for an action that carries a vault can be created through the API, and works.
+     *
+     * It could not be. `AuthToken::injectSecureData()` seals the master password into the new
+     * token, which needs the master password on the context, and the API only ever loads that from
+     * the *calling* token's own vault — which `AUTHTOKEN_CREATE` did not have. So every action a
+     * token carries a vault for, `ACCOUNT_VIEW` and `ACCOUNT_CREATE` among them, answered 500
+     * "Error while retrieving master password from context", with or without a password, while the
+     * web created the same tokens without difficulty.
+     *
+     * The round trip is the point: minting it is only half of it, so the token it answers with is
+     * then used to read an account, custom fields and all, which is the path that needs the vault.
+     *
+     * @throws \Exception
+     */
+    #[Test]
+    public function aTokenForAnActionThatCarriesAVaultCanBeCreatedAndUsed(): void
+    {
+        $token = $this->createTokenFor(AclActionsInterface::ACCOUNT_VIEW);
+
+        $r = $this->callApiPath(
+            'GET',
+            '/api/v1/accounts/1',
+            ['customFields' => '1', 'tokenPass' => self::AUTH_TOKEN_PASS],
+            'Bearer ' . $token
+        );
+
+        $this->assertSame(200, $r->status, 'the token has to open the account it was issued for');
+        $this->assertNotEmpty($r->body->data);
+    }
+
+    /**
+     * Every one of them, so this is not a fix for the one action that happened to be tried.
+     *
+     * @throws \Exception
+     */
+    #[Test]
+    #[DataProvider('vaultCarryingActionProvider')]
+    public function everyActionThatCarriesAVaultCanBeMinted(int $actionId): void
+    {
+        $created = $this->callApi(
+            AclActionsInterface::AUTHTOKEN_CREATE,
+            [
+                'userId' => self::ADMIN_USER_ID,
+                'actionId' => $actionId,
+                'password' => self::AUTH_TOKEN_PASS,
+            ]
+        );
+
+        $this->assertSame(201, $created->status, json_encode($created->body));
+        $this->assertNotEmpty($created->body->data->token);
+    }
+
+    /**
+     * @return array<string, array{int}>
+     */
+    public static function vaultCarryingActionProvider(): array
+    {
+        return [
+            'ACCOUNT_VIEW_PASS' => [AclActionsInterface::ACCOUNT_VIEW_PASS],
+            'ACCOUNT_EDIT_PASS' => [AclActionsInterface::ACCOUNT_EDIT_PASS],
+            'ACCOUNT_CREATE' => [AclActionsInterface::ACCOUNT_CREATE],
+            'PUBLICLINK_CREATE' => [AclActionsInterface::PUBLICLINK_CREATE],
+            'PUBLICLINK_REFRESH' => [AclActionsInterface::PUBLICLINK_REFRESH],
+            'ACCOUNT_VIEW' => [AclActionsInterface::ACCOUNT_VIEW],
+            'CATEGORY_VIEW' => [AclActionsInterface::CATEGORY_VIEW],
+            'CLIENT_VIEW' => [AclActionsInterface::CLIENT_VIEW],
+        ];
+    }
+
+    /**
+     * Without a password the vault would be sealed with the empty string, and nothing could open
+     * it again: `Api` reads `tokenPass` as a required parameter, and required refuses the empty
+     * string, so the one password that would work cannot be presented. The web form has refused
+     * this since #833; the API refuses it now too, rather than issuing a token that can never be
+     * used.
+     *
+     * @throws \Exception
+     */
+    #[Test]
+    public function aTokenThatCarriesAVaultIsRefusedWithoutAPassword(): void
+    {
+        $created = $this->callApi(
+            AclActionsInterface::AUTHTOKEN_CREATE,
+            ['userId' => self::ADMIN_USER_ID, 'actionId' => AclActionsInterface::ACCOUNT_VIEW]
+        );
+
+        $this->assertInstanceOf(stdClass::class, $created->body->error ?? null);
+        $this->assertSame('Password cannot be blank', $created->body->error->message);
+    }
+
+    /**
+     * The control: an action that carries no vault still needs no password, so the rule above did
+     * not become "every token needs one".
+     *
+     * @throws \Exception
+     */
+    #[Test]
+    public function aTokenThatCarriesNoVaultStillNeedsNoPassword(): void
+    {
+        $created = $this->callApi(
+            AclActionsInterface::AUTHTOKEN_CREATE,
+            ['userId' => self::ADMIN_USER_ID, 'actionId' => AclActionsInterface::CATEGORY_SEARCH]
+        );
+
+        $this->assertSame(201, $created->status, json_encode($created->body));
+        $this->assertNotEmpty($created->body->data->token);
+    }
+
+
     /**
      * Create a token through the API and hand back the bearer string it answered with.
      *