Skip to content

Commit ab5e82f

Browse files
ngoiyaericngoiyaeric
authored
Merge pull request #769 from QueueLab/fix/skyfi-mcp-authenticated-users-12245688311846760660
fix(skyfi): fix skyfi plugin mcp for authenticated users
2 parents 766c1ff + d54cdb7 commit ab5e82f

8 files changed

Lines changed: 85 additions & 35 deletions

File tree

‎app/api/skyfi/callback/route.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { skyfiOAuthTokens } from '@/lib/db/schema';
77
import { eq } from 'drizzle-orm';
88

99
export async function GET(request: NextRequest) {
10+
const fallbackSettingsUrl = new URL('/settings', request.url).toString();
1011
try {
1112
const userId = await getCurrentUserIdOnServer();
1213
if (!userId) {
@@ -16,6 +17,12 @@ export async function GET(request: NextRequest) {
1617
const { searchParams } = new URL(request.url);
1718
const code = searchParams.get('code');
1819
const state = searchParams.get('state');
20+
const oauthError = searchParams.get('error');
21+
22+
if (oauthError) {
23+
const description = searchParams.get('error_description') || oauthError;
24+
return NextResponse.redirect(`${fallbackSettingsUrl}?error=${encodeURIComponent(`skyfi_${description}`)}`);
25+
}
1926

2027
if (!code || !state) {
2128
return NextResponse.json({ error: 'Missing code or state parameters.' }, { status: 400 });
@@ -90,12 +97,11 @@ export async function GET(request: NextRequest) {
9097
return NextResponse.json({ error: `Token exchange failed or timed out: ${fetchError.message}` }, { status: 500 });
9198
}
9299

93-
// Redirect user back to the settings page
94-
const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000';
95-
return NextResponse.redirect(`${baseUrl}/settings`);
100+
// Preserve the same public origin used for the OAuth redirect. This works
101+
// with NEXT_PUBLIC_APP_URL as well as reverse proxies using forwarded hosts.
102+
return NextResponse.redirect(new URL('/settings', redirectUri).toString());
96103
} catch (error: any) {
97104
console.error('[SkyFiCallback] Unexpected error:', error.message);
98-
const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000';
99-
return NextResponse.redirect(`${baseUrl}/settings?error=skyfi_callback_failed`);
105+
return NextResponse.redirect(`${fallbackSettingsUrl}?error=skyfi_callback_failed`);
100106
}
101107
}

‎drizzle/migrations/0010_add_skyfi_redirect_uri.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE "skyfi_oauth_tokens" ADD COLUMN IF NOT EXISTS "redirect_uri" text;

‎drizzle/migrations/meta/_journal.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@
7171
"when": 1784838292039,
7272
"tag": "0009_add_skyfi_oauth_tokens",
7373
"breakpoints": true
74+
},
75+
{
76+
"idx": 10,
77+
"version": "7",
78+
"when": 1785549039000,
79+
"tag": "0010_add_skyfi_redirect_uri",
80+
"breakpoints": true
7481
}
7582
]
7683
}

‎lib/actions/skyfi.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,35 @@ import { headers } from 'next/headers';
1313
export async function getRedirectUri(): Promise<string> {
1414
try {
1515
const headersList = await headers();
16-
const host = headersList.get('host');
16+
const host = headersList.get('x-forwarded-host') || headersList.get('host');
1717
if (host) {
18-
const protocol = host.startsWith('localhost') || host.startsWith('127.0.0.1') ? 'http' : 'https';
19-
return `${protocol}://${host}/api/skyfi/callback`;
18+
const proto = headersList.get('x-forwarded-proto') || (host.startsWith('localhost') || host.startsWith('127.0.0.1') ? 'http' : 'https');
19+
return `${proto}://${host}/api/skyfi/callback`;
2020
}
2121
} catch (e) {
22-
console.warn('[Skyfi] Failed to get host from headers, falling back to ENV:', e);
22+
console.warn('[Skyfi] Failed to get host from headers, falling back to NEXT_PUBLIC_APP_URL:', e);
2323
}
24-
const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000';
25-
return `${baseUrl}/api/skyfi/callback`;
24+
if (process.env.NEXT_PUBLIC_APP_URL) {
25+
const baseUrl = process.env.NEXT_PUBLIC_APP_URL.replace(/\/$/, '');
26+
return `${baseUrl}/api/skyfi/callback`;
27+
}
28+
return 'http://localhost:3000/api/skyfi/callback';
2629
}
2730

2831
/**
2932
* Ensures the client is registered dynamically with the SkyFi MCP server.
3033
* Uses an AbortController with a 10s timeout to prevent hanging.
3134
*/
3235
async function ensureClientRegistered(provider: SkyfiOAuthProvider, forceRegister: boolean = false): Promise<string> {
36+
const currentUri = provider.redirectUrl?.toString();
3337
if (!forceRegister) {
3438
const currentInfo = await provider.clientInformation();
35-
if (currentInfo?.client_id) {
39+
if (currentInfo?.client_id && currentInfo?.redirect_uri === currentUri) {
3640
return currentInfo.client_id;
3741
}
3842
}
3943

40-
console.log('[SkyFiAction] Client not registered or force-register active. Registering dynamically...');
44+
console.log('[SkyFiAction] Client not registered or redirect URI changed. Registering dynamically...');
4145

4246
const controller = new AbortController();
4347
const timeoutId = setTimeout(() => controller.abort(), 10000);
@@ -91,7 +95,7 @@ export async function startSkyfiConnection(): Promise<{ url?: string; error?: st
9195
const redirectUri = await getRedirectUri();
9296
const provider = new SkyfiOAuthProvider(userId, redirectUri);
9397

94-
const clientId = await ensureClientRegistered(provider, true);
98+
const clientId = await ensureClientRegistered(provider, false);
9599

96100
const verifier = generateCodeVerifier();
97101
const challenge = generateCodeChallenge(verifier);
@@ -144,7 +148,7 @@ export async function getSkyfiConnectionStatus(): Promise<{ connected: boolean;
144148
method: 'POST',
145149
headers: {
146150
'Content-Type': 'application/json',
147-
'X-Skyfi-Api-Key': tokens.access_token,
151+
'Authorization': `Bearer ${tokens.access_token}`,
148152
},
149153
body: JSON.stringify({
150154
jsonrpc: '2.0',

‎lib/agents/tools/skyfi.tsx‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,14 @@ import { Client as MCPClientClass } from '@modelcontextprotocol/sdk/client/index
77
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
88
import { getCurrentUserIdOnServer } from '@/lib/auth/get-current-user';
99
import { SkyfiOAuthProvider } from '@/lib/skyfi/provider';
10+
import { getRedirectUri } from '@/lib/actions/skyfi';
1011
import { skyfiQuerySchema } from '@/lib/schema/skyfi';
1112
import { DrawnFeature } from '@/lib/agents/resolution-search';
1213
import { z } from 'zod';
1314
import crypto from 'crypto';
14-
import { headers } from 'next/headers';
1515

1616
export type McpClient = MCPClientClass;
1717

18-
async function getRedirectUri(): Promise<string> {
19-
try {
20-
const headersList = await headers();
21-
const host = headersList.get('host');
22-
if (host) {
23-
const protocol = host.startsWith('localhost') || host.startsWith('127.0.0.1') ? 'http' : 'https';
24-
return `${protocol}://${host}/api/skyfi/callback`;
25-
}
26-
} catch (e) {
27-
console.warn('[SkyfiTool] Failed to get host from headers, falling back to ENV:', e);
28-
}
29-
const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000';
30-
return `${baseUrl}/api/skyfi/callback`;
31-
}
32-
3318
function geoJsonToWkt(geometry: any): string | null {
3419
if (!geometry) return null;
3520
if (geometry.type === 'Polygon') {
@@ -63,7 +48,6 @@ async function getConnectedSkyfiMcpClient(accessToken: string, signal?: AbortSig
6348
requestInit: {
6449
headers: {
6550
'Authorization': `Bearer ${accessToken}`,
66-
'X-Skyfi-Api-Key': accessToken,
6751
},
6852
signal,
6953
}

‎lib/db/schema.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export const skyfiOAuthTokens = pgTable('skyfi_oauth_tokens', {
138138
tokenExpiry: timestamp('token_expiry', { withTimezone: true }),
139139
clientId: text('client_id'),
140140
clientSecret: text('client_secret'),
141+
redirectUri: text('redirect_uri'),
141142
codeVerifier: text('code_verifier'),
142143
state: text('state'),
143144
registrationClientUri: text('registration_client_uri'),

‎lib/skyfi/provider.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export class SkyfiOAuthProvider implements OAuthClientProvider {
7676
client_secret: decrypt(row.clientSecret) || undefined,
7777
registration_client_uri: row.registrationClientUri || undefined,
7878
registration_access_token: decrypt(row.registrationAccessToken) || undefined,
79+
redirect_uri: row.redirectUri || undefined,
7980
};
8081
}
8182
return undefined;
@@ -87,6 +88,7 @@ export class SkyfiOAuthProvider implements OAuthClientProvider {
8788
userId: this.userId,
8889
clientId: clientInformation.client_id,
8990
clientSecret: encrypt(clientInformation.client_secret || null),
91+
redirectUri: this.redirectUrlStr,
9092
registrationClientUri: clientInformation.registration_client_uri || null,
9193
registrationAccessToken: encrypt(clientInformation.registration_access_token || null),
9294
})
@@ -95,6 +97,7 @@ export class SkyfiOAuthProvider implements OAuthClientProvider {
9597
set: {
9698
clientId: clientInformation.client_id,
9799
clientSecret: encrypt(clientInformation.client_secret || null),
100+
redirectUri: this.redirectUrlStr,
98101
registrationClientUri: clientInformation.registration_client_uri || null,
99102
registrationAccessToken: encrypt(clientInformation.registration_access_token || null),
100103
updatedAt: new Date(),
@@ -122,9 +125,10 @@ export class SkyfiOAuthProvider implements OAuthClientProvider {
122125
const nowSeconds = Math.floor(Date.now() / 1000);
123126
const expiresAt = row.tokenExpiry ? Math.floor(row.tokenExpiry.getTime() / 1000) : null;
124127

125-
// Check if token is expired or expiring in less than 60 seconds
126-
if (expiresAt && expiresAt < nowSeconds + 60 && decryptedRefreshToken && row.clientId) {
127-
console.log('[SkyfiProvider] Token expired or expiring soon. Refreshing token...');
128+
// Check if token is expired, expiring in less than 60 seconds, or expiry details are missing (conservative treatment)
129+
const isExpiredOrMissing = !expiresAt || expiresAt < nowSeconds + 60;
130+
if (isExpiredOrMissing && decryptedRefreshToken && row.clientId) {
131+
console.log('[SkyfiProvider] Token expired, expiring soon, or missing expiry details. Refreshing token...');
128132
try {
129133
const response = await fetch('https://mcp.skyfi.com/oauth/token', {
130134
method: 'POST',
@@ -150,9 +154,15 @@ export class SkyfiOAuthProvider implements OAuthClientProvider {
150154
return newTokens;
151155
} else {
152156
console.error('[SkyfiProvider] Failed to refresh token:', await response.text());
157+
if (expiresAt && expiresAt < nowSeconds) {
158+
return undefined;
159+
}
153160
}
154161
} catch (err: any) {
155162
console.error('[SkyfiProvider] Error refreshing token:', err.message);
163+
if (expiresAt && expiresAt < nowSeconds) {
164+
return undefined;
165+
}
156166
}
157167
}
158168

‎tests-unit/skyfi.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { mock } from 'bun:test';
2+
3+
mock.module('next/cache', () => ({
4+
revalidatePath: () => {},
5+
}));
6+
7+
mock.module('next/headers', () => ({
8+
headers: async () => ({
9+
get: (key: string) => {
10+
if (key === 'x-forwarded-host') return 'proxy-host.com';
11+
if (key === 'x-forwarded-proto') return 'https';
12+
return null;
13+
},
14+
}),
15+
}));
16+
17+
const { getRedirectUri } = await import('../lib/actions/skyfi');
18+
19+
console.log('Starting SkyFi plugin unit tests...');
20+
21+
// 1. The public request host must win even when NEXT_PUBLIC_APP_URL is stale.
22+
process.env.NEXT_PUBLIC_APP_URL = 'https://custom-domain.com';
23+
const redirectUriEnv = await getRedirectUri();
24+
if (redirectUriEnv !== 'https://proxy-host.com/api/skyfi/callback') {
25+
throw new Error(`getRedirectUri request-host precedence failed. Expected 'https://proxy-host.com/api/skyfi/callback', got '${redirectUriEnv}'`);
26+
}
27+
console.log('✓ getRedirectUri prioritizing the public request host verified');
28+
29+
// 2. Test getRedirectUri fallback with proxy headers without NEXT_PUBLIC_APP_URL
30+
delete process.env.NEXT_PUBLIC_APP_URL;
31+
const redirectUriProxy = await getRedirectUri();
32+
if (redirectUriProxy !== 'https://proxy-host.com/api/skyfi/callback') {
33+
throw new Error(`getRedirectUri proxy fallback failed. Expected 'https://proxy-host.com/api/skyfi/callback', got '${redirectUriProxy}'`);
34+
}
35+
console.log('✓ getRedirectUri proxy headers fallback verified');
36+
37+
console.log('All SkyFi plugin unit tests passed!');

0 commit comments

Comments
 (0)