Added refresh token support to backend

[jessevz]

closes #1526 adds refresh and accesstokens to the backend

[Copilot]

🟡 Changes recommended

Unresolved critical security and concurrency issues affect refresh-token rotation, revocation, CORS, and password invalidation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds rotating, database-backed refresh-token authentication for API v2, including cookies, persistence, revocation, CORS, OpenAPI updates, and tests.

Changes:

• Adds token schemas, models, issuance, rotation, replay detection, cleanup, and revocation.
• Integrates login, refresh, logout, user lifecycle, cookies, CORS, and JWT middleware.
• Updates documentation, fixtures, tests, and development database configuration.

File summaries

File	Summary and final review notes
src/migrations/postgres/20260914090000_refresh-token.sql	Adds the PostgreSQL refresh-token schema.
src/migrations/mysql/20260914090000_refresh-token.sql	Adds the MySQL refresh-token schema.
src/inc/utils/UserUtils.php	Adds user lifecycle revocation. Critical (3 votes): legacy password-change paths do not revoke existing refresh tokens.
src/inc/utils/RefreshTokenUtils.php	Implements token lifecycle management. Critical (1 vote): concurrent refreshes can both issue successors. Critical (2 votes): rotation can race family revocation. Moderate (1 vote): expired tokens for inactive users are not globally purged.
src/inc/StartupConfig.php	Adds refresh-token and cookie configuration. Moderate (1 vote): SameSite=None does not enable cross-domain use while CORS rejects those origins.
src/inc/apiv2/util/CorsHackMiddleware.php	Adds credentialed CORS handling. Critical (2 votes): reads the wrong origin key, preventing credentialed requests. Critical (1 vote): origin parsing can allow an untrusted origin when ports are omitted. Moderate (1 vote): the typed request parameter rejects existing test doubles.
src/inc/apiv2/openapi/StaticFragments.php	Documents refresh and logout operations. Moderate (2 votes): logout incorrectly requires a refresh-cookie security scheme. Nit (3 votes): logout documents the rotation cookie instead of the clearing header.
src/inc/apiv2/openapi/SpecBuilder.php	Registers the refresh security scheme and route.
src/inc/apiv2/error/HttpUnauthorized.php	Adds the HTTP 401 exception.
src/inc/apiv2/auth/token.routes.php	Adds login, refresh, and logout routes.
src/inc/apiv2/auth/RefreshTokenCookie.php	Handles refresh-token cookies.
src/dba/models/RefreshTokenFactory.php	Adds refresh-token database access.
src/dba/models/RefreshToken.php	Defines the refresh-token model.
src/dba/Factory.php	Registers the refresh-token factory.
src/api/v2/index.php	Exempts refresh routes from JWT middleware.
openapi.json	Publishes the updated API specification.
ci/phpunit/inc/utils/RefreshTokenUtilsTest.php	Adds unit coverage for token behavior.
ci/phpunit/fixtures/openapi/hashtype.spec.json	Updates the OpenAPI fixture.
ci/phpunit/fixtures/openapi/crackerbinarytype.spec.json	Updates the OpenAPI fixture.
ci/phpunit/fixtures/openapi/config.spec.json	Updates the OpenAPI fixture.
ci/phpunit/fixtures/openapi/abortchunk.spec.json	Updates the OpenAPI fixture.
ci/apiv2/test_refresh_token.py	Adds refresh-token integration tests. Nit (2 votes): captures the old token after logout clears it, so revocation is not actually asserted.
.devcontainer/docker-compose.postgres.yml	Configures PostgreSQL development settings.
.devcontainer/docker-compose.mysql.yml	Configures MySQL development settings.

Review details

Suppressed comments (3)

src/inc/StartupConfig.php:84

• SameSite=None is described as enabling a frontend on a different domain, but CorsHackMiddleware::CheckCORS() still rejects every origin whose host differs from HASHTOPOLIS_BACKEND_URL before this cookie is sent. Either add a configurable allowed frontend origin to the CORS check (with credentials) or stop advertising this deployment mode; changing SameSite alone cannot make it work.

      /* Frontend and API normally share a site even when they sit on different ports, and ports do not
         make a request cross-site, so Strict holds for the usual deployment. Only a frontend on a
         genuinely different domain needs None, which in turn only works on a Secure cookie. */
      self::REFRESH_TOKEN_COOKIE_SAMESITE => "Strict",

src/inc/apiv2/util/CorsHackMiddleware.php:98

• CheckCORS() is intentionally untyped and the existing PHPUnit callers pass DummyRequest, which only implements the two methods it needs; this new Request parameter rejects those calls with a TypeError whenever a valid origin reaches allowOrigin. Keep the helper parameter untyped to match CheckCORS, or make the test double implement ServerRequestInterface.

  private static function allowOrigin(Request $request, Response $response): Response {

src/inc/utils/RefreshTokenUtils.php:66

• issue() always calls purgeExpired with a user ID, and the only other repository caller is the user-scoped test; no production path invokes the nullable null form. Expired tokens for users who never log in again are consequently never removed, so abandoned sessions can accumulate rows indefinitely. Add a scheduled/global cleanup invocation rather than relying solely on the next login.

  public static function issue(int $userId, ?string $familyId = null): string {
    self::purgeExpired($userId);

• Files reviewed: 24/24 changed files
• Comments generated: 8
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Critical unresolved token-type, CORS, replay, and concurrency issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (8)

Previously missed (1) — in code that hasn't changed since the last review.

src/inc/utils/RefreshTokenUtils.php:25

• Issue #1526 explicitly calls for a refresh JWT carrying type: refresh and a refresh-only scope, but this implementation deliberately stores an opaque string with no claims at all. As written, the PR does not implement that refresh-token contract; either issue and validate the specified JWT or update the feature requirements instead of closing the issue.

src/inc/apiv2/auth/token.routes.php:213

• The linked issue requires manual revocation by an administrator, but the only new endpoint here is self-service logout for the cookie it receives. revokeFamily() is not exposed through an authenticated, authorized admin API, so an administrator cannot revoke a selected token/session as required.

  /* Logout: ends the session the cookie belongs to and drops the cookie. */
  $group->delete('', function (Request $request, Response $response, array $args): Response {
    $presented = RefreshTokenCookie::read($request);
    if ($presented !== null) {
      RefreshTokenUtils::revoke($presented);

src/inc/apiv2/util/CorsHackMiddleware.php:152

• process() invokes the route handler before CheckCORS() runs, so this credentialed-origin check rejects only the response after the state change. A disallowed Origin can still consume/rotate the refresh token on POST or revoke the family on DELETE, and with SameSite=None this is a cross-site request. Validate the origin before calling the handler; keep response decoration for allowed/error responses separate.

  private static function allowOrigin(string $origin, Response $response): Response {
    return $response->withHeader('Access-Control-Allow-Origin', $origin)
      ->withHeader('Access-Control-Allow-Credentials', 'true')
      ->withAddedHeader('Vary', 'Origin');

src/inc/utils/AccountUtils.php:160

• The deprecated-but-supported src/install/updates/reset.php also writes password hashes directly and does not call RefreshTokenUtils::revokeAllForUser, so an administrator using that reset path leaves existing refresh sessions valid after the password change. Route that script through the same revocation side effect or add the call there.

    $user = Factory::getUserFactory()->mset($user, [User::PASSWORD_HASH => $newHash, User::PASSWORD_SALT => $newSalt, USer::IS_COMPUTED_PASSWORD => 0]);
    RefreshTokenUtils::revokeAllForUser($user->getId());

src/inc/utils/RefreshTokenUtils.php:127

• A successor is inserted after the claim and can race with revokeFamily() or revokeAllForUser(). Since issue() always inserts it with isRevoked = 0, logout, password reset, or replay detection can commit its revocation before this insert and the newly minted token survives. Serialize family revocation with rotation (for example, a transaction/lock or durable family-revocation state checked atomically).

    return [
      "user" => $user,
      "token" => self::issue($token->getUserId(), $token->getFamilyId()),
    ];

src/inc/utils/RefreshTokenUtils.php:108

• Expiry is checked before consumed/replay state. A previously used token presented after its own expiry is returned as merely expired and never revokes its still-live successor family, even though expired rows are intentionally retained for replay detection. Check consumed/revoked state before returning expiration, or explicitly revoke the family for an expired-but-consumed token.

    $now = time();
    if ($token->getEndValid() < $now) {
      throw new HttpUnauthorized("Refresh token has expired");
    }
    

src/inc/utils/RefreshTokenUtils.php:252

• The documented global cleanup path is never used: every call from issue() passes a user ID, so expired tokens belonging to users who do not log in again remain in the table indefinitely. Add a scheduled/global purge or otherwise invoke purgeExpired() without a user filter; otherwise this table can grow without bound despite the method's stated purpose.

  public static function purgeExpired(?int $userId = null): void {
    $filters = [new QueryFilter(RefreshToken::END_VALID, time(), "<")];
    if ($userId !== null) {
      $filters[] = new QueryFilter(RefreshToken::USER_ID, $userId, "=");
    }

src/inc/utils/RefreshTokenUtils.php:211

• Although this adds internal family revocation, there is no authenticated admin-facing route or API that can invoke it; the only callers are logout and internal user/security flows, and RefreshToken is not registered in the API registry. This leaves issue #1526's manual administrator revocation requirement unavailable. Expose a permission-checked endpoint, for example to revoke all refresh-token families for a target user.

  public static function revokeFamily(string $familyId): void {
    Factory::getRefreshTokenFactory()->massUpdate([
      Factory::FILTER => [new QueryFilter(RefreshToken::FAMILY_ID, $familyId, "=")],
      Factory::UPDATE => [new UpdateSet(RefreshToken::IS_REVOKED, 1)]
    ]);

• Files reviewed: 31/31 changed files
• Comments generated: 3
• Review effort level: Lite