Skip to content

Commit 71d9deb

Browse files
yash-pouranikyash-pouranik
authored
Merge pull request #85 from geturbackend/copilot/fix-social-auth-demo-issues
Fix social auth follow-up issues from PR #81
2 parents 4fd5a18 + 4411c1c commit 71d9deb

7 files changed

Lines changed: 278 additions & 18 deletions

File tree

‎apps/dashboard-api/src/controllers/project.controller.js‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,7 +1099,12 @@ module.exports.updateProject = async (req, res) => {
10991099
try {
11001100
const { name, siteUrl, resendApiKey, resendFromEmail } = req.body;
11011101
const updateFields = {};
1102-
if (name !== undefined) updateFields.name = name;
1102+
if (name !== undefined) {
1103+
if (typeof name !== "string" || !name.trim()) {
1104+
return res.status(400).json({ error: "name must be a non-empty string." });
1105+
}
1106+
updateFields.name = name.trim();
1107+
}
11031108
if (resendFromEmail !== undefined) {
11041109
if (typeof resendFromEmail !== "string") {
11051110
return res.status(400).json({ error: "resendFromEmail must be a string." });
@@ -1338,11 +1343,21 @@ module.exports.toggleAuth = async (req, res) => {
13381343
const { projectId } = req.params;
13391344
const { enable } = req.body; // true or false
13401345

1341-
// Ensure user owns project
1346+
// Ensure user owns project, and load authProviders secrets so sanitizeAuthProviders
1347+
// can correctly compute hasClientSecret in the response.
1348+
// NOTE: If new OAuth providers are added to SOCIAL_PROVIDER_KEYS, extend this select list
1349+
// to include their clientSecret fields as well.
13421350
const project = await Project.findOne({
13431351
_id: projectId,
13441352
owner: req.user._id,
1345-
});
1353+
}).select(
1354+
"+authProviders.github.clientSecret.encrypted " +
1355+
"+authProviders.github.clientSecret.iv " +
1356+
"+authProviders.github.clientSecret.tag " +
1357+
"+authProviders.google.clientSecret.encrypted " +
1358+
"+authProviders.google.clientSecret.iv " +
1359+
"+authProviders.google.clientSecret.tag"
1360+
);
13461361
if (!project) return res.status(404).json({ error: "Project not found" });
13471362

13481363
if (enable) {

‎apps/public-api/src/__tests__/verifyApiKey.test.js‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
'use strict';
2+
3+
const mockFindOne = jest.fn();
4+
const mockSelect = jest.fn().mockReturnThis();
5+
const mockPopulate = jest.fn().mockReturnThis();
6+
const mockLean = jest.fn();
7+
8+
jest.mock('@urbackend/common', () => ({
9+
Project: {
10+
findOne: jest.fn(() => ({
11+
select: mockSelect,
12+
populate: mockPopulate,
13+
lean: mockLean,
14+
})),
15+
},
16+
hashApiKey: jest.fn((key) => `hashed_${key}`),
17+
getProjectByApiKeyCache: jest.fn().mockResolvedValue(null),
18+
setProjectByApiKeyCache: jest.fn().mockResolvedValue(undefined),
19+
}));
20+
21+
const { hashApiKey, getProjectByApiKeyCache, Project } = require('@urbackend/common');
22+
const verifyApiKey = require('../middlewares/verifyApiKey');
23+
24+
// ---------------------------------------------------------------------------
25+
// Helpers
26+
// ---------------------------------------------------------------------------
27+
28+
function makeProject(overrides = {}) {
29+
return {
30+
_id: 'proj_1',
31+
owner: { isVerified: true },
32+
resources: { db: { isExternal: false }, storage: { isExternal: false } },
33+
allowedDomains: ['*'],
34+
...overrides,
35+
};
36+
}
37+
38+
function makeReq({ headers = {}, query = {} } = {}) {
39+
return {
40+
header: jest.fn((name) => headers[name.toLowerCase()] || undefined),
41+
headers,
42+
query: { ...query },
43+
};
44+
}
45+
46+
function makeRes() {
47+
const res = {
48+
status: jest.fn().mockReturnThis(),
49+
json: jest.fn().mockReturnThis(),
50+
};
51+
return res;
52+
}
53+
54+
// ---------------------------------------------------------------------------
55+
// Tests
56+
// ---------------------------------------------------------------------------
57+
58+
describe('verifyApiKey middleware', () => {
59+
const next = jest.fn();
60+
61+
beforeEach(() => {
62+
jest.clearAllMocks();
63+
getProjectByApiKeyCache.mockResolvedValue(null);
64+
mockLean.mockResolvedValue(makeProject());
65+
});
66+
67+
test('accepts publishable key via x-api-key header', async () => {
68+
const req = makeReq({ headers: { 'x-api-key': 'pk_live_headerkey' } });
69+
const res = makeRes();
70+
71+
await verifyApiKey(req, res, next);
72+
73+
expect(next).toHaveBeenCalled();
74+
expect(req.keyRole).toBe('publishable');
75+
});
76+
77+
test('accepts publishable key via ?key= query param', async () => {
78+
const req = makeReq({ query: { key: 'pk_live_querykey' } });
79+
const res = makeRes();
80+
81+
await verifyApiKey(req, res, next);
82+
83+
expect(next).toHaveBeenCalled();
84+
expect(req.keyRole).toBe('publishable');
85+
});
86+
87+
test('strips ?key= from req.query after reading it', async () => {
88+
const req = makeReq({ query: { key: 'pk_live_querykey', other: 'keep' } });
89+
const res = makeRes();
90+
91+
await verifyApiKey(req, res, next);
92+
93+
expect(req.query.key).toBeUndefined();
94+
expect(req.query.other).toBe('keep');
95+
});
96+
97+
test('rejects secret key supplied via ?key= query param', async () => {
98+
const req = makeReq({ query: { key: 'sk_live_secretkey' } });
99+
const res = makeRes();
100+
101+
await verifyApiKey(req, res, next);
102+
103+
expect(next).not.toHaveBeenCalled();
104+
expect(res.status).toHaveBeenCalledWith(401);
105+
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ error: 'API key not found' }));
106+
});
107+
108+
test('header takes precedence when both x-api-key header and ?key= query param are present', async () => {
109+
const req = makeReq({
110+
headers: { 'x-api-key': 'pk_live_headerkey' },
111+
query: { key: 'pk_live_querykey' },
112+
});
113+
const res = makeRes();
114+
115+
await verifyApiKey(req, res, next);
116+
117+
expect(next).toHaveBeenCalled();
118+
// hashApiKey is called with the header key, not the query key
119+
expect(hashApiKey).toHaveBeenCalledWith('pk_live_headerkey');
120+
expect(hashApiKey).not.toHaveBeenCalledWith('pk_live_querykey');
121+
});
122+
123+
test('?key= is stripped even when it is not a valid pk_live_ key', async () => {
124+
const req = makeReq({
125+
headers: { 'x-api-key': 'pk_live_headerkey' },
126+
query: { key: 'sk_live_secretkey' },
127+
});
128+
const res = makeRes();
129+
130+
await verifyApiKey(req, res, next);
131+
132+
expect(req.query.key).toBeUndefined();
133+
expect(next).toHaveBeenCalled();
134+
});
135+
136+
test('returns 401 when no key is provided at all', async () => {
137+
const req = makeReq();
138+
const res = makeRes();
139+
140+
await verifyApiKey(req, res, next);
141+
142+
expect(next).not.toHaveBeenCalled();
143+
expect(res.status).toHaveBeenCalledWith(401);
144+
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ error: 'API key not found' }));
145+
});
146+
147+
test('returns 401 when project is not found in DB', async () => {
148+
mockLean.mockResolvedValueOnce(null);
149+
const req = makeReq({ query: { key: 'pk_live_unknown' } });
150+
const res = makeRes();
151+
152+
await verifyApiKey(req, res, next);
153+
154+
expect(next).not.toHaveBeenCalled();
155+
expect(res.status).toHaveBeenCalledWith(401);
156+
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ error: 'API key is expired or invalid.' }));
157+
});
158+
159+
test('returns 401 when owner is not verified', async () => {
160+
getProjectByApiKeyCache.mockResolvedValueOnce(makeProject({ owner: { isVerified: false } }));
161+
const req = makeReq({ query: { key: 'pk_live_unverifiedowner' } });
162+
const res = makeRes();
163+
164+
await verifyApiKey(req, res, next);
165+
166+
expect(next).not.toHaveBeenCalled();
167+
expect(res.status).toHaveBeenCalledWith(401);
168+
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ error: 'Owner not verified' }));
169+
});
170+
171+
test('uses cache when available and does not query DB', async () => {
172+
getProjectByApiKeyCache.mockResolvedValueOnce(makeProject());
173+
const req = makeReq({ query: { key: 'pk_live_cached' } });
174+
const res = makeRes();
175+
176+
await verifyApiKey(req, res, next);
177+
178+
expect(next).toHaveBeenCalled();
179+
expect(Project.findOne).not.toHaveBeenCalled();
180+
});
181+
});

‎apps/public-api/src/controllers/userAuth.controller.js‎

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,11 @@ const getSocialRefreshExchangeKey = (rtCode) => `project:social-auth:refresh-exc
5858
*/
5959
const getFrontendCallbackBaseUrl = (project) => {
6060
const configured = String(project?.siteUrl || '').trim();
61-
const base = configured || process.env.FRONTEND_URL || 'http://localhost:5173';
62-
return `${base.replace(/\/$/, '')}/auth/callback`;
61+
const base = configured || process.env.FRONTEND_URL || '';
62+
if (!base) {
63+
console.warn('[social-auth] getFrontendCallbackBaseUrl: siteUrl is not set on the project and FRONTEND_URL env is not configured. Falling back to http://localhost:5173. Set siteUrl in Project Settings or configure FRONTEND_URL.');
64+
}
65+
return `${(base || 'http://localhost:5173').replace(/\/$/, '')}/auth/callback`;
6366
};
6467

6568
/**
@@ -69,6 +72,58 @@ const getFrontendCallbackBaseUrl = (project) => {
6972
*/
7073
const toBase64UrlBuffer = (input) => Buffer.from(input.replace(/-/g, '+').replace(/_/g, '/').padEnd(Math.ceil(input.length / 4) * 4, '='), 'base64');
7174

75+
/**
76+
* In-memory cache for Google's public JWK keys.
77+
* Keys are refreshed when the cache expires (based on Cache-Control max-age).
78+
* An in-flight promise is stored to prevent redundant concurrent fetches (single-flight).
79+
*/
80+
const googleJwkCache = { keys: null, expiresAt: 0, inflight: null };
81+
82+
/**
83+
* Fetches Google's public JWK keys, using an in-memory cache keyed by Cache-Control max-age.
84+
* Uses a single-flight pattern so that concurrent requests share one fetch instead of many.
85+
* @returns {Promise<Array>} Array of JWK key objects
86+
*/
87+
const getGooglePublicKeys = async () => {
88+
const now = Date.now();
89+
if (googleJwkCache.keys && now < googleJwkCache.expiresAt) {
90+
return googleJwkCache.keys;
91+
}
92+
93+
// Single-flight: reuse in-flight promise if a fetch is already in progress.
94+
if (googleJwkCache.inflight) {
95+
return googleJwkCache.inflight;
96+
}
97+
98+
googleJwkCache.inflight = (async () => {
99+
try {
100+
const certsResponse = await fetch('https://www.googleapis.com/oauth2/v3/certs');
101+
if (!certsResponse.ok) {
102+
throw new Error('Unable to fetch Google JWK keys');
103+
}
104+
105+
const certsPayload = await certsResponse.json();
106+
const keys = certsPayload.keys || [];
107+
108+
// Parse Cache-Control max-age from response headers to determine TTL.
109+
const cacheControl = (typeof certsResponse.headers?.get === 'function')
110+
? (certsResponse.headers.get('cache-control') || '')
111+
: '';
112+
const maxAgeMatch = cacheControl.match(/max-age=(\d+)/);
113+
const ttlMs = maxAgeMatch ? parseInt(maxAgeMatch[1], 10) * 1000 : 3600 * 1000;
114+
115+
googleJwkCache.keys = keys;
116+
googleJwkCache.expiresAt = Date.now() + ttlMs;
117+
118+
return keys;
119+
} finally {
120+
googleJwkCache.inflight = null;
121+
}
122+
})();
123+
124+
return googleJwkCache.inflight;
125+
};
126+
72127
/**
73128
* Asserts that a project has auth enabled and a valid users collection schema.
74129
* Throws with an appropriate statusCode on failure.
@@ -315,10 +370,9 @@ const verifyGoogleIdToken = async ({ idToken, clientId }) => {
315370
throw new Error('Unsupported Google id_token signature');
316371
}
317372

318-
const certsResponse = await fetch('https://www.googleapis.com/oauth2/v3/certs');
319-
const certsPayload = await certsResponse.json();
320-
const signingKey = certsPayload.keys?.find((key) => key.kid === header.kid);
321-
if (!certsResponse.ok || !signingKey) {
373+
const certsKeys = await getGooglePublicKeys();
374+
const signingKey = certsKeys.find((key) => key.kid === header.kid);
375+
if (!signingKey) {
322376
throw new Error('Unable to verify Google id_token signing key');
323377
}
324378

‎apps/public-api/src/middlewares/verifyApiKey.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,19 @@ const {
88
module.exports = async (req, res, next) => {
99
try {
1010
// x-api-key header is preferred. For browser-navigation endpoints (e.g. social OAuth start),
11+
// a publishable key may be supplied via the `key` query parameter instead.
1112
const headerKey = req.header('x-api-key');
13+
const queryKey = typeof req.query?.key === 'string' ? req.query.key : undefined;
14+
15+
// Only allow publishable keys (pk_live_) via query param; secret keys must use the header.
16+
const apiKey = headerKey || (queryKey?.startsWith('pk_live_') ? queryKey : undefined);
17+
18+
// Strip the key from req.query immediately after reading so it is not forwarded to
19+
// downstream middleware, controllers, or access logs.
20+
if (req.query && typeof req.query === 'object') {
21+
delete req.query.key;
22+
}
1223

13-
const apiKey = headerKey;
1424
if (!apiKey) {
1525
return res.status(401).json({ error: 'API key not found' });
1626
}

‎examples/social-demo/COLLECTIONS_GUIDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ For GitHub social auth in this demo, these extra `users` fields are recommended:
3838
```
3939
Field Name | Type | Required | Unique | Default
4040
------------------|---------|----------|--------|----------
41-
githubId | String | No | No | -
41+
githubId | String | No | Yes | -
4242
authProviders | Array | No | No | []
4343
```
4444

‎examples/social-demo/client/src/lib/api.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export const authApi = {
129129
return response;
130130
},
131131
getSocialStartUrl: (provider) => (
132-
`${PROXY_URL}/userAuth/social/${encodeURIComponent(provider)}/start`
132+
`${API_BASE_URL}${API_PREFIX}/userAuth/social/${encodeURIComponent(provider)}/start?key=${encodeURIComponent(PUBLIC_KEY)}`
133133
),
134134
exchangeSocialAuth: async ({ token, rtCode }) => {
135135
const response = await axios.post(

‎package-lock.json‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)