Explorar o código

feat(access-token-parser): add shared token-source extractor with header support

Introduce extractAccessToken() and X_GROWI_ACCESS_TOKEN_HEADER_NAME as the single
source of truth for token-source precedence (Bearer > X-GROWI-ACCESS-TOKEN header >
query > body). A non-string header value falls through to the remaining sources (3.4).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Yuki Takei hai 1 semana
pai
achega
787feb7c4e

+ 7 - 5
.kiro/specs/access-token-parser/design.md

@@ -164,8 +164,11 @@ apps/app/src/server/middlewares/access-token-parser/
 **Responsibilities & Constraints**
 - Resolve the token in order: Bearer → `X-GROWI-ACCESS-TOKEN` header → `access_token`
   query → `access_token` body.
-- Return the resolved token only when it is a single string; otherwise return `null`
-  (covers array-valued or absent header — 3.4).
+- A non-string (e.g. array-valued, from a duplicated header) `X-GROWI-ACCESS-TOKEN` value
+  is **skipped** so resolution falls through to the remaining sources, rather than
+  short-circuiting (3.4). Implement by coercing a non-string header to `undefined` before
+  the precedence chain so `??` continues past it.
+- Return `null` when no string-typed source resolves; otherwise return the string token.
 - Does not validate the token; resolution only.
 
 **Dependencies**
@@ -180,10 +183,9 @@ export const X_GROWI_ACCESS_TOKEN_HEADER_NAME = 'x-growi-access-token';
 export const extractAccessToken = (req: AccessTokenParserReq): string | null;
 ```
 - Preconditions: `req` is an Express request (`AccessTokenParserReq`).
-- Postconditions: returns a non-empty-or-empty string token, or `null` when no
-  string-typed source is present.
+- Postconditions: returns a string token, or `null` when no string-typed source resolves.
 - Invariants: precedence order is `Bearer ?? header ?? query ?? body`; Bearer always wins
-  when present (3.1).
+  when present (3.1); a non-string header is skipped so resolution falls through (3.4).
 
 #### parserForAccessToken / parserForApiToken (modified)
 

+ 2 - 2
.kiro/specs/access-token-parser/tasks.md

@@ -4,8 +4,8 @@
 > `imprv/x-access-token-header` branch, which carries unrelated MongoDB-regex work).
 > Test-first per repo TDD policy.
 
-- [ ] 1. Foundation: shared token-source extraction utility
-- [ ] 1.1 Create the shared token-source extractor with unit tests (test-first)
+- [x] 1. Foundation: shared token-source extraction utility
+- [x] 1.1 Create the shared token-source extractor with unit tests (test-first)
   - Write failing unit tests first, covering: precedence Bearer > `X-GROWI-ACCESS-TOKEN` header > query > body; non-string / array-valued header is ignored; header key resolves case-insensitively
   - Define the canonical header-name constant and implement the pure extractor that returns the resolved token string or null
   - Observable: a new unit test file passes, exercising every precedence, guard, and casing case; the no-header case resolves exactly to the prior Bearer/query/body result

+ 135 - 0
apps/app/src/server/middlewares/access-token-parser/extract-access-token.spec.ts

@@ -0,0 +1,135 @@
+import type { IncomingHttpHeaders } from 'node:http';
+import type { AccessTokenParserReq } from '@growi/core/dist/interfaces/server';
+
+import {
+  extractAccessToken,
+  X_GROWI_ACCESS_TOKEN_HEADER_NAME,
+} from './extract-access-token';
+
+// Build a minimal request shaped like the real Express request: only explicitly-set
+// properties exist, so unset sources are genuinely `undefined` (unlike a deep auto-mock,
+// which would stub every accessed path and break the `??` precedence chain).
+const buildReq = (parts: {
+  headers?: IncomingHttpHeaders;
+  query?: { access_token?: string };
+  body?: { access_token?: string };
+}): AccessTokenParserReq =>
+  ({
+    headers: parts.headers ?? {},
+    query: parts.query ?? {},
+    body: parts.body ?? {},
+  }) as AccessTokenParserReq;
+
+describe('extractAccessToken', () => {
+  it('returns the Bearer token when present, even if other sources exist (3.1)', () => {
+    // arrange
+    const req = buildReq({
+      headers: {
+        authorization: 'Bearer bearer-token',
+        [X_GROWI_ACCESS_TOKEN_HEADER_NAME]: 'header-token',
+      },
+      query: { access_token: 'query-token' },
+      body: { access_token: 'body-token' },
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('bearer-token');
+  });
+
+  it('returns the header token when no Bearer is present (3.2)', () => {
+    // arrange
+    const req = buildReq({
+      headers: { [X_GROWI_ACCESS_TOKEN_HEADER_NAME]: 'header-token' },
+      query: { access_token: 'query-token' },
+      body: { access_token: 'body-token' },
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('header-token');
+  });
+
+  it('returns the query token when no Bearer and no header (3.3)', () => {
+    // arrange
+    const req = buildReq({
+      headers: {},
+      query: { access_token: 'query-token' },
+      body: { access_token: 'body-token' },
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('query-token');
+  });
+
+  it('returns the body token when only the body has it (3.3)', () => {
+    // arrange
+    const req = buildReq({
+      headers: {},
+      query: {},
+      body: { access_token: 'body-token' },
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('body-token');
+  });
+
+  it('ignores an array-valued (non-string) header and falls through to query (3.4)', () => {
+    // arrange
+    // Express represents repeated headers as string[]; a non-string header value must be
+    // skipped so resolution continues to the remaining sources rather than failing.
+    const req = buildReq({
+      headers: { [X_GROWI_ACCESS_TOKEN_HEADER_NAME]: ['a', 'b'] },
+      query: { access_token: 'query-token' },
+      body: {},
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('query-token');
+  });
+
+  it('returns null when the only source is an array-valued header (3.4)', () => {
+    // arrange
+    const req = buildReq({
+      headers: { [X_GROWI_ACCESS_TOKEN_HEADER_NAME]: ['a', 'b'] },
+      query: {},
+      body: {},
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBeNull();
+  });
+
+  it('returns null when no string-typed source is present (3.4)', () => {
+    // arrange
+    const req = buildReq({ headers: {}, query: {}, body: {} });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBeNull();
+  });
+
+  it('resolves the header case-insensitively via the lowercase constant (1.3)', () => {
+    // arrange
+    // Express lowercases incoming header keys, so the canonical constant is lowercase;
+    // indexing by it resolves a header regardless of the sender's casing.
+    const req = buildReq({
+      headers: { 'x-growi-access-token': 'header-token' },
+      query: {},
+      body: {},
+    });
+
+    // act / assert
+    expect(X_GROWI_ACCESS_TOKEN_HEADER_NAME).toBe('x-growi-access-token');
+    expect(extractAccessToken(req)).toBe('header-token');
+  });
+
+  it('matches the prior Bearer/query/body precedence when no header is present', () => {
+    // arrange
+    const req = buildReq({
+      headers: { authorization: 'Bearer bearer-token' },
+      query: { access_token: 'query-token' },
+      body: { access_token: 'body-token' },
+    });
+
+    // act / assert
+    expect(extractAccessToken(req)).toBe('bearer-token');
+  });
+});

+ 31 - 0
apps/app/src/server/middlewares/access-token-parser/extract-access-token.ts

@@ -0,0 +1,31 @@
+import type { AccessTokenParserReq } from '@growi/core/dist/interfaces/server';
+
+import { extractBearerToken } from './extract-bearer-token';
+
+// Canonical header name for passing an access token outside the Authorization header.
+// Express lowercases incoming header keys, so indexing by this lowercase constant
+// resolves the header case-insensitively. Mirrors X_GROWI_TRANSFER_KEY_HEADER_NAME.
+export const X_GROWI_ACCESS_TOKEN_HEADER_NAME = 'x-growi-access-token';
+
+/**
+ * Resolve the access token from a request using the single source-of-truth precedence:
+ * Bearer > X-GROWI-ACCESS-TOKEN header > access_token query > access_token body.
+ *
+ * A non-string X-GROWI-ACCESS-TOKEN value (e.g. an array from a duplicated header) is
+ * coerced to `undefined` before the precedence chain, so resolution falls through to the
+ * remaining sources instead of short-circuiting (3.4). `null` is returned only when no
+ * string-typed source resolves.
+ */
+export const extractAccessToken = (
+  req: AccessTokenParserReq,
+): string | null => {
+  const headerToken = req.headers[X_GROWI_ACCESS_TOKEN_HEADER_NAME];
+
+  const token =
+    extractBearerToken(req.headers.authorization) ??
+    (typeof headerToken === 'string' ? headerToken : undefined) ??
+    req.query.access_token ??
+    req.body.access_token;
+
+  return typeof token === 'string' ? token : null;
+};