Skip to content

Conversation

@MarceloRGonc
Copy link
Contributor

Part of OPS-3127.

Copilot AI review requested due to automatic review settings November 27, 2025 11:25
@linear
Copy link

linear bot commented Nov 27, 2025

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mg/OPS-3127

Comment @coderabbitai help to get the list of available commands and usage tips.

@MarceloRGonc MarceloRGonc requested review from rita-gorokhod and removed request for Copilot November 27, 2025 11:25
@greptile-apps
Copy link

greptile-apps bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

Added MCP token type to authentication system and integrated token generation into MCP tool initialization. The PR introduces a new generateMCPToken function that creates specialized tokens for MCP operations by extracting user principals and applying the MCP type.

Critical issue found:

  • The generateMCPToken function uses incorrect field name token instead of type on line 45, which doesn't match the Principal interface structure and will cause authentication failures

Changes made:

  • Added MCP to PrincipalType enum in packages/shared/src/lib/authentication/model/principal-type.ts:2
  • Created generateMCPToken function in packages/server/api/src/app/authentication/context/access-token-manager.ts:34-50
  • Updated getOpenOpsTools to generate MCP tokens before passing to MCP client in packages/server/api/src/app/ai/mcp/openops-tools.ts:83-91

Confidence Score: 0/5

  • This PR has a critical bug that will break MCP authentication
  • The generateMCPToken function uses wrong field name (token instead of type), meaning the generated payload won't match the Principal interface. This will cause authentication validation to fail when decoded, as the type field is required for principal identification. All other generation functions (generateEngineToken, generateWorkerToken) correctly use the type field.
  • packages/server/api/src/app/authentication/context/access-token-manager.ts - line 45 must be fixed before merge

Important Files Changed

File Analysis

Filename Score Overview
packages/server/api/src/app/authentication/context/access-token-manager.ts 0/5 added generateMCPToken function but incorrectly uses token field instead of type field, will cause authentication failures
packages/server/api/src/app/ai/mcp/openops-tools.ts 5/5 renamed parameter from authToken to userAuthToken and generates MCP token before passing to client
packages/shared/src/lib/authentication/model/principal-type.ts 5/5 added new MCP principal type to enum

Sequence Diagram

sequenceDiagram
    participant Client as API Client
    participant OpenOps as getOpenOpsTools
    participant TokenMgr as accessTokenManager
    participant JWT as jwtUtils
    participant MCP as MCP Server

    Client->>OpenOps: getOpenOpsTools(app, userAuthToken)
    OpenOps->>TokenMgr: generateMCPToken(userAuthToken)
    TokenMgr->>JWT: extractPrincipal(userToken)
    JWT-->>TokenMgr: principal (with user info)
    TokenMgr->>TokenMgr: spread principal, add token field
    TokenMgr->>JWT: sign(payload, key, expiry)
    JWT-->>TokenMgr: mcpToken
    TokenMgr-->>OpenOps: mcpToken
    OpenOps->>MCP: createMCPClient with mcpToken
    MCP-->>OpenOps: client with tools
    OpenOps-->>Client: MCPTool
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

…n-manager.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings November 27, 2025 11:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for MCP (Model Context Protocol) token authentication by introducing a new principal type and token generation functionality. This change enables the system to issue specialized tokens for MCP clients that are derived from user tokens but carry distinct authorization semantics.

Key Changes:

  • Added MCP as a new principal type to the authentication system
  • Implemented token generation for MCP clients that inherits user permissions with MCP-specific type designation
  • Updated the OpenOps MCP tools integration to use MCP tokens instead of passing user tokens directly

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/shared/src/lib/authentication/model/principal-type.ts Added MCP enum value to PrincipalType to support MCP token authentication
packages/server/api/src/app/authentication/context/access-token-manager.ts Implemented generateMCPToken method to create MCP-specific tokens from user tokens
packages/server/api/src/app/ai/mcp/openops-tools.ts Updated to generate and use MCP tokens instead of passing user authentication tokens directly to the MCP server

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MarceloRGonc MarceloRGonc changed the base branch from main to mg/OPS-3127-2 November 27, 2025 12:40
@MarceloRGonc MarceloRGonc marked this pull request as draft November 27, 2025 12:42
@sonarqubecloud
Copy link

@MarceloRGonc MarceloRGonc changed the title Add MCP token type Add MCP Principal type Nov 27, 2025
@MarceloRGonc MarceloRGonc changed the title Add MCP Principal type Use an MCP token for the mcp tools Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants