Skip to content
This repository was archived by the owner on Jul 8, 2026. It is now read-only.

HTTP MCP Server OAuth authentication (Visual Studio Code MCP Support) - #146

Closed
mdthorpe-sc wants to merge 9 commits into
mainfrom
mdthorpe/sc-305049/token-refresh-debug
Closed

mdthorpe-sc wants to merge 9 commits into
mainfrom
mdthorpe/sc-305049/token-refresh-debug

Conversation

@mdthorpe-sc

Copy link
Copy Markdown
Collaborator
  • OAuth proxy foundation (HTTP server)

    • Added full OAuth provider implementation in src/auth/provider.ts.
    • Integrated MCP SDK OAuth routes + bearer middleware in src/server-http.ts.
    • Added callback relay flow (/oauth/callback) and upstream token exchange/refresh handling.
    • Added token caching + refresh behavior for issued OAuth tokens.
  • Security hardening

    • Client secret exposure fix: registration/client lookup now return public client metadata (secret not exposed to MCP clients), while upstream secret remains server-side.
    • Session ownership binding fix: reintroduced session-bound token checks in src/server-http.ts (including helper methods and enforcement on session reuse paths).
  • Auth testing coverage

    • Added large mocked auth test suite: src/auth/oauth.test.ts.
    • Added staging integration auth tests: src/auth/oauth-integration.test.ts.
  • Operational config behavior

    • Redirect URI allowlisting now depends on OAUTH_ALLOWED_REDIRECT_URIS (comma-delimited env var) in provider startup path.
    • MCP_SERVER_URL is used to derive OAuth issuer/base/resource metadata identity (important for env isolation and VS Code cache behavior).
  • CI/CD and deploy pipeline changes

    • Updated .github/workflows/pipeline.yml to split/build dependencies (build-docker + build-scripts) and update deploy job wiring.
    • Updated build workflow (build_ecr_image.yml) and CodeDeploy startup behavior (including env file mount in start script).
  • Supporting app/library updates

    • src/client/shortcut.ts updates (token/client handling support for refreshed sessions).
    • Various test and tool file touch-ups.
    • README.md and docs/oauth-proxy-implementation.md updates.
    • package.json/package-lock.json churn from dependency/script updates.

Comment thread src/client/shortcut.test.ts Outdated
const fullMockDoc = { ...mockDoc, content_markdown: "This is test content" } as Doc;
const mockClient = {
getDoc: mock(async () => ({ data: fullMockDoc })),
getDoc: mock(async () => ({ data: mockDoc })),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this change intended or a bad merge? This doesn't match what is currently in main.

Comment thread src/client/shortcut.ts
return doc;
}

async updateDoc(docPublicId: string, params: UpdateDoc): Promise<Doc> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this removed on purpose?

Comment thread src/client/shortcut.ts Outdated
}

async getDocById(docId: string): Promise<Doc | null> {
async getDocById(docId: string): Promise<DocSlim | null> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this intentional?

Comment thread src/tools/base.ts

const additionalFields: Partial<SimplifiedTeam> = {};

if (kind === "simple") {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this intentional or a bad merge?

Comment thread src/tools/base.ts
}> {
if (!entity) return { users: {}, workflows: {} };

const { member_ids, workflow_ids, default_workflow_id } = entity;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Intentional or a bad merge?

Comment thread src/tools/base.ts Outdated
entity: Iteration | null | undefined,
kind: SimplifiedKind,
): Promise<{
private async getRelatedEntitiesForIteration(entity: Iteration | null | undefined): Promise<{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same question here.. this seems like a bad merge since it reverts recent changes to the code.

expect(mockWriteTool).toHaveBeenCalledTimes(2);
expect(mockWriteTool).toHaveBeenCalledTimes(1);
expect(mockWriteTool.mock.calls?.[0]?.[0]).toBe("documents-create");
expect(mockWriteTool.mock.calls?.[1]?.[0]).toBe("documents-update");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More merge issues?

Comment thread src/tools/documents.ts Outdated
server.addToolWithWriteAccess(
"documents-create",
"Create a new document in Shortcut with a title and content. Returns the document's id, title, and app_url. Note: Use Markdown format for the content.",
"Create a new document in Shortcut with a title and content. Returns the document's id, title, and app_url. Note: Use HTML markup for the content (e.g., <p>, <h1>, <ul>, <strong>) rather than Markdown.",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More merge issues.

Comment thread src/tools/teams.test.ts Outdated
}));
await mockTool.mock.calls?.[1]?.[3]({ includeArchived: true });
expect(tools.getTeams).toHaveBeenCalledWith(true);
await mockTool.mock.calls?.[1]?.[2]();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More merge issues.

Comment thread src/tools/teams.ts Outdated
},
async ({ includeArchived }: { includeArchived: boolean }) =>
await tools.getTeams(includeArchived),
"List all Shortcut teams",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More merge issues.

Comment thread src/server.ts
import { UserTools } from "./tools/user";
import { WorkflowTools } from "./tools/workflows";

let apiToken = process.env.SHORTCUT_API_TKN || process.env.SHORTCUT_API_TOKEN;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MOre merge issues.

Comment thread package.json Outdated
"dependencies": {
"@modelcontextprotocol/sdk": "^1.25.1",
"@shortcut/client": "^3.2.0",
"@shortcut/client": "^3.1.0",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

MOre merge issues.

Comment thread README.md
### Stories

- **stories-get-by-id** - Get a single Shortcut story by ID
- **stories-get-history** - Get the change history for a story

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This chagne removes newly added tools

Comment thread README.md
- **epics-get-by-id** - Get a Shortcut epic by ID
- **epics-search** - Find Shortcut epics with filtering and search options
- **epics-create** - Create a new Shortcut epic
- **epics-update** - Update an existing Shortcut epic

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

Comment thread README.md
- **iterations-get-by-id** - Get a Shortcut iteration by ID
- **iterations-search** - Find Shortcut iterations with filtering and search options
- **iterations-create** - Create a new Shortcut iteration with start/end dates
- **iterations-update** - Update an existing Shortcut iteration

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

Comment thread README.md
### Documents

- **documents-create** - Create a new document in Shortcut with Markdown content
- **documents-update** - Update content of an existing document by its ID

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

Comment thread README.md
| `teams-get-by-id` | Get a Shortcut team by ID |
| `teams-list` | List all Shortcut teams |

### Projects

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not "tableified".

Comment thread README.md

### Custom Fields

- **custom-fields-list** - List all custom fields in the workspace with their possible values

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not "tableified".

Comment thread README.md

## Issues and Troubleshooting

> [!IMPORTANT]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

DId we remove the "important" callout on purpose?

Comment thread README.md

### Common Issues and Solutions

#### MCP fails on startup in Gemini CLI

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't see these added to the new doc.

@mdthorpe-sc
mdthorpe-sc force-pushed the mdthorpe/sc-305049/token-refresh-debug branch from aee1721 to ef61043 Compare February 18, 2026 21:02
@mdthorpe-sc
mdthorpe-sc force-pushed the mdthorpe/sc-305049/token-refresh-debug branch from ef61043 to 64e07df Compare February 18, 2026 22:25
@mdthorpe-sc
mdthorpe-sc deleted the mdthorpe/sc-305049/token-refresh-debug branch February 19, 2026 20:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants