Feat/setup mcp auth by akulistus · Pull Request #681 · codex-team/hawk.api.nodejs

akulistus · 2026-09-16T00:43:38Z

Adds implementation of MCP authorization flow based on OAuth 2.1 and official MCP Docs

Adds endpoints and logic for the OAuth 2.1 authorization flow
Stores AuthRequset data in shared redis store for multi-instance deployments and uses in-memory store for single-instance deployment

Copilot

🟡 Changes recommended

Token isolation, redirect validation, CORS, and distributed authorization-code storage must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds OAuth authorization and bearer-token protection to the MCP integration.

Changes:

Adds OAuth discovery, registration, consent, token, PKCE, and refresh flows.
Protects MCP routes and adds an authenticated user-ID tool.
Adds the MCP Express dependency.

File summaries

File	Description
`src/integrations/mcp/auth.ts`	Implements MCP OAuth flow.
`src/integrations/mcp/index.ts`	Applies authentication middleware.
`src/integrations/mcp/mcp.ts`	Adds an authenticated test tool.
`package.json`	Adds MCP Express dependency.
`yarn.lock`	Locks the new dependency.

Review details

Files reviewed: 4/5 changed files
Comments generated: 9
Review effort level: Balanced

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

Reversean · 2026-09-21T21:53:52Z

+  /**
+   * MCP
+   */
+  app.use("/integration/mcp", authMiddleware, router);


Under WHATWG Fetch §3.2.1, browser CORS preflights (OPTIONS) do not carry credentials (Authorization). Because OPTIONS currently cascades into authMiddleware (requireBearerAuth), it responds with 401 Unauthorized per RFC 6750 §3.1, causing browsers to abort cross-origin MCP requests.

Consider either:

intercepting OPTIONS before authMiddleware to respond with 204 to keep changes local to this route

terminating OPTIONS preflights with 204 and adding Authorization to Access-Control-Allow-Headers

For discovery endpoints (/.well-known/oauth-*), RFC 8414 §3 also recommends Access-Control-Allow-Origin: * for external clients.

There is a global cors policy that handles OPTION requests

…dis for multi-instance deploy or in-memory for single instance deploy, add JSDoc

neSpecc

Основное, что стоит поправить до мержа:

redirect_uri не валидируется — кража кода и XSS через consent-страницу.
expires_in отдаётся абсолютным timestamp.
Memory-store принимает просроченные коды.
Async-хендлеры без обработки ошибок — запрос виснет или падает процесс.

Отдельно стоит обсудить audience токенов: сейчас MCP-клиент получает полноценную сессию Hawk. Подробности в комментариях.

neSpecc · 2026-10-28T14:43:52Z

+    /**
+     * Add parameters to redirect_uri
+     */
+    const callback = new URL(redirect_uri);


Блокер: redirect_uri ничем не ограничен. /register ничего не сохраняет, поэтому client_id ни к чему не привязан, и consent примет любой redirect_uri.

Кража кода: атакующий даёт жертве ссылку ${GARAGE_URL}/concent?client_id=x&redirect_uri=https://evil/cb&code_challenge=<свой>. После «Разрешить» код уходит на evil, атакующий меняет его своим code_verifier на access + refresh жертвы.

XSS: redirect_uri=javascript:...// проходит new URL, а Concent.vue делает window.location.href = result.redirect_uri → скрипт выполняется на origin garage.

Предлагаю хранить зарегистрированных клиентов (Redis/Mongo или подписанный client_id с зашитыми redirect_uris), требовать точное совпадение redirect_uri и разрешать только https: и loopback http://localhost / http://127.0.0.1.

neSpecc · 2026-10-28T14:43:52Z

+      access_token: tokensPair.accessToken,
+      refresh_token: tokensPair.refreshToken,
+      token_type: 'Bearer',
+      expires_in: data.exp,


expires_in по RFC 6749 — время жизни токена в секундах, а data.exp — абсолютный timestamp (~1.8e9). Клиенты будут считать токен вечным и не станут обновлять его заранее; через 15 минут часть клиентов на 401 запускает полный flow заново вместо refresh.

Можно data.exp - Math.floor(Date.now() / 1000) или просто константа из TTL — тогда и jwt.verify только что подписанного токена (стр. 72) не понадобится.

neSpecc · 2026-10-28T14:43:52Z

+      return null;
+    }
+
+    if (Date.now() > data.expiresAt) {


После удаления просроченной записи не хватает return null (в SAML-сторе он есть). Сейчас просроченный код всё равно возвращается и обменивается на токены, пока его не удалит cleanup (до минуты).

neSpecc · 2026-10-28T14:43:52Z

+   * The redirect URI includes an authorization code and any local state
+   * provided by the client earlier
+   */
+  app.post('/concent/integration/mcp', async (req, res) => {


Express 4 не ловит ошибки из async-хендлеров. Здесь и в /token могут бросить: new URL(redirect_uri) при невалидном или отсутствующем redirect_uri, createHash().update(code_verifier) без code_verifier, generateTokensPair для удалённого пользователя, getClient() redis-стора при разрыве соединения. Запрос висит до таймаута, а без HAWK_CATCHER_TOKEN процесс падает на unhandled rejection — любой залогиненный пользователь может уронить API запросом с redirect_uri: "x".

Нужен try/catch + next(err), как в роутах GitHub/SAML, и валидация входа: строки client_id, redirect_uri, code_challenge, code_challenge_method === 'S256'. Код лучше сохранять в store уже после проверки redirect_uri.

neSpecc · 2026-10-28T14:43:53Z

+  verifyAccessToken: async (token: string) => {
+    const data = getUserJWTDataFromToken(token, 'access');
+
+    if (!data) {


Токены, которые получает MCP-клиент, — это обычные сессии Hawk: тот же секрет, без aud и скоупов. Поэтому verifier принимает любой access-токен Hawk, а токен стороннего MCP-клиента даёт полный доступ к GraphQL. Скоупы mcp:tools mcp:resources ни на что не влияют, параметр resource (RFC 8707) игнорируется, хотя спека MCP требует проверять, что токен выпущен именно для этого сервера. Refresh живёт 30 дней без ротации, отозвать его нельзя (OAuth 2.1 требует для public clients ротацию или sender-constrained токены).

Как минимум: aud: ${API_URL}/integration/mcp в токене, проверка aud здесь и запрет таких токенов в createContext; refresh сделать одноразовым (jti в Redis).

neSpecc · 2026-10-28T14:43:53Z

+   * @param userId - user ID
+   * @returns TokensPair - newly generated access and refresh tokens
+   */
+  async function generateTokensPair(userId: string) {


Почти дословная копия проверки enforced SSO из refreshTokens (src/resolvers/user.ts). Предлагаю вынести в модель или фабрику и звать в обоих местах, иначе при изменении SSO-политики MCP-путь разойдётся с основным.

neSpecc · 2026-10-28T14:43:53Z

+ *
+ * @returns MCPAuthRequest data store instance
+ */
+export function createMCPStore(): MCPAuthRequestStoreInterface {


Это уже третья копия фабрики и сторов (SAML, GitHub install state, теперь MCP), и при копировании потерялся return null в memory-сторе. Может, вынести общий одноразовый TTL-store с префиксом и TTL в параметрах?

Мелочи: ключ mcp:auth${code} без разделителя :; new RedisMCPAuthRequestStore() не получает уже взятый redisHelper; если Redis не подключён на старте, стор навсегда остаётся memory — в multi-instance это даст плавающий invalid_grant.

neSpecc · 2026-10-28T14:43:53Z

  );

+  server.registerTool(
+    'print_userId',


Тестовые hello_world / print_userId и 'A test server' уедут в прод, их увидят все пользователи. Если print_userId нужен для отладки, лучше брать проверенный AuthInfo (requireBearerAuth кладёт его в req.auth), а не jwt.decode из сырого заголовка с ! — иначе этот паттерн расползётся по настоящим тулам.

neSpecc · 2026-10-28T14:43:53Z

+        issuer: `${process.env.API_URL}/integration/mcp`,
+
+        authorization_endpoint:
+          `${process.env.GARAGE_URL}/concent`,


Опечатка: concent → consent. Этот URL публикуется в metadata и кешируется клиентами, после релиза переименовывать будет больно. Стоит поправить сразу здесь, в /concent/integration/mcp и в garage (Concent.vue, роут). В JSDoc ещё встречается «AuthRequset».

neSpecc · 2026-10-28T14:43:53Z

+   * ON 'refresh_token' GRANT TYPE:
+   *  - Validates refresh token. If valid, the authorization server responds back with an access token and a refresh token
+   */
+  app.post('/token/integration/mcp', async (req: Request<{}, {}, TokenRequest>, res) => {


На весь флоу нет ни одного теста, хотя для SAML есть test/sso/saml/store.test.ts и controller.test.ts. Тесты «просроченный код отклоняется» и «expires_in ≤ 900» поймали бы две проблемы выше. Хорошо бы покрыть хотя бы обмен кода (PKCE, несовпадение client_id / redirect_uri, повторное использование кода) и refresh.

neSpecc · 2026-10-28T17:49:35Z

why file is called getUserIdFromToken but an exported function is called getUserJWTDataFromToken

neSpecc · 2026-10-29T15:52:33Z

Отзыв доступа для MCP-клиентов

Сейчас MCP-клиент получает обычную сессию Hawk: полный доступ к GraphQL и refresh на 30 дней, который продлевается бесконечно. Отозвать такой доступ нельзя ни пользователю, ни нам: смена пароля JWT не инвалидирует, остаётся только сменить JWT_SECRET для всех. DCR открыт для любого клиента, так что утёкший токен из стороннего приложения — это доступ к аккаунту без срока. К тому же OAuth 2.1 требует для public clients ротацию refresh-токенов.

Заложить отзыв нужно в этом PR: если выпустить stateless-версию, уже выданные токены потом не отозвать без смены секрета. Что для этого нужно:

Хранить grant (Mongo): userId, clientId, clientName, redirectHost, scopes, refreshJti, tokenHint, createdAt, lastUsedAt, revokedAt. Создавать в /token при обмене кода (upsert по userId + clientId). Имя и redirect_uris брать из сохранённой регистрации — значит, /register тоже должен сохранять клиентов, это же нужно для проверки redirect_uri. tokenHint — превью mcp токена типа hwk_…91c2
Отдельные MCP-токены вместо user.generateTokensPair(): в access — gid, client_id и aud: ${API_URL}/integration/mcp, в refresh — gid и jti. В createContext не принимать токены с MCP-aud.
Ротация refresh: проверить JWT → загрузить grant по gid → если grant отозван или jti не совпадает с refreshJti, вернуть invalid_grant. При несовпадении jti отзывать весь grant: это признак повторного использования украденного токена. Иначе выдать новую пару и атомарно обновить refreshJti (findOneAndUpdate({ _id, refreshJti: old }, ...)).
tokenVerifier проверяет по gid, что grant не отозван (с кэшем в Redis на минуту, сбрасывать при отзыве), берёт clientId из токена и не чаще раза в час обновляет lastUsedAt.

UI можно сделать следующим PR: query со списком grant'ов пользователя и mutation revokeMcpGrant(id) для блока «Подключённые приложения» в настройках garage. Рядом с названием стоит показывать домен redirect_uri: client_name клиент присылает сам и может назваться как угодно.

akulistus added this pull request to stack #682 September 16, 2026 00:43

neSpecc reviewed Sep 16, 2026

View reviewed changes

neSpecc requested a balanced review from Copilot September 16, 2026 06:12

Copilot started reviewing on behalf of neSpecc September 16, 2026 06:12

View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Reversean requested changes Sep 21, 2026

View reviewed changes

akulistus force-pushed the feat/setup-mcp-auth branch from 4a261e7 to c83154d Compare October 14, 2026 15:40

akulistus force-pushed the feat/setup-mcp-auth branch 2 times, most recently from 6fe3738 to 7401a65 Compare October 22, 2026 20:16

akulistus and others added 4 commits October 24, 2026 00:14

feat(mcp-auth): add initial OAuth 2.1 authorization support

f446cfc

chore(mcp-auth): refresh token

d9d1a0e

feat(MCPAuthRequestStore): implement caching MCPAuthRequests using re…

3f8ef37

…dis for multi-instance deploy or in-memory for single instance deploy, add JSDoc

Bump version up to 1.6.2

08d5031

akulistus force-pushed the feat/setup-mcp-auth branch from 7401a65 to 08d5031 Compare October 23, 2026 21:16

akulistus added 4 commits October 25, 2026 20:02

fix(token-refresh): fix 400 status code error on token refresh

7e70c76

add JSDoc and fix lint

db7d5c0

chore(generateTokenPair): check enforced sso before issuing tokens

a869ee0

fix(print_userId tool): prevent possible unhandled error

170cf37

neSpecc reviewed Oct 28, 2026

View reviewed changes

Conversation

akulistus commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

neSpecc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neSpecc commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

akulistus commented Sep 16, 2026 •

edited

Loading

neSpecc commented Oct 29, 2026 •

edited

Loading