Skip to content

Commit 2e2247c

Browse files
TooAngelTooAngel
authored
fix: identify users by GitHub user ID instead of access token (#307)
This prevents creation of duplicate user records when OAuth tokens change during token refresh, scope changes, or multiple logins. Changes: - Add githubUserId field to User model and database schema - Add User.findByGithubUserId() method - Update OAuth callback to fetch GitHub user info - Update existing users' tokens instead of creating duplicates - Create migration script to populate githubUserId for existing users - Add unique index on githubUserId field Benefits: - One user record per GitHub user (no duplicates) - Automatic token updates without losing repository associations - Better security and data integrity - Simplified user management Closes #289
1 parent 169c733 commit 2e2247c

File tree

4 files changed

+264
-2
lines changed

4 files changed

+264
-2
lines changed

‎scripts/migrate-user-github-ids.js‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
#!/usr/bin/env node
2+
3+
import {
4+
database,
5+
client,
6+
connectionPromise,
7+
} from '../src/database/database.js';
8+
9+
/**
10+
* Fetch GitHub user info using an access token
11+
* @param {string} accessToken
12+
* @returns {Promise<{id: number, login: string} | null>}
13+
*/
14+
async function fetchGitHubUser(accessToken) {
15+
try {
16+
const response = await fetch('https://api.github.com/user', {
17+
headers: {
18+
Authorization: `Bearer ${accessToken}`,
19+
Accept: 'application/vnd.github.v3+json',
20+
},
21+
});
22+
23+
if (!response.ok) {
24+
console.error(
25+
`Failed to fetch GitHub user: ${response.status} ${response.statusText}`
26+
);
27+
return null;
28+
}
29+
30+
const user = await response.json();
31+
return { id: user.id, login: user.login };
32+
} catch (error) {
33+
console.error('Error fetching GitHub user:', error);
34+
return null;
35+
}
36+
}
37+
38+
async function migrateDatabase() {
39+
try {
40+
// Wait for database connection to be established
41+
await connectionPromise;
42+
43+
console.log('[MIGRATION] Checking user GitHub ID migration status...');
44+
45+
// Check if migration is already complete
46+
const usersWithoutGithubId = await database.users
47+
.find({ githubUserId: { $exists: false } })
48+
.toArray();
49+
50+
if (usersWithoutGithubId.length === 0) {
51+
console.log(
52+
'[MIGRATION] ✅ Already migrated - all users have githubUserId field'
53+
);
54+
return { alreadyMigrated: true, usersUpdated: 0 };
55+
}
56+
57+
console.log(
58+
`[MIGRATION] Found ${usersWithoutGithubId.length} users without githubUserId to migrate`
59+
);
60+
61+
const usersByGithubId = new Map();
62+
const failedUsers = [];
63+
64+
// Fetch GitHub user IDs for existing users
65+
for (const user of usersWithoutGithubId) {
66+
console.log(`[MIGRATION] Processing user ${user._id}...`);
67+
68+
const githubUser = await fetchGitHubUser(user.githubAccessToken);
69+
70+
if (!githubUser) {
71+
console.warn(
72+
`[MIGRATION] ⚠️ Could not fetch GitHub user info for user ${user._id} (token may be expired)`
73+
);
74+
failedUsers.push(user);
75+
continue;
76+
}
77+
78+
console.log(
79+
`[MIGRATION] ✓ Found GitHub user: ${githubUser.login} (ID: ${githubUser.id})`
80+
);
81+
82+
// Track users by GitHub ID to find duplicates
83+
if (!usersByGithubId.has(githubUser.id)) {
84+
usersByGithubId.set(githubUser.id, []);
85+
}
86+
usersByGithubId.get(githubUser.id).push({
87+
...user,
88+
githubUserId: githubUser.id,
89+
githubLogin: githubUser.login,
90+
});
91+
}
92+
93+
// Process users: update or merge duplicates
94+
let updatedCount = 0;
95+
let mergedCount = 0;
96+
97+
for (const [githubUserId, users] of usersByGithubId.entries()) {
98+
if (users.length === 1) {
99+
// Single user - just update
100+
const user = users[0];
101+
await database.users.updateOne(
102+
{ _id: user._id },
103+
{
104+
$set: {
105+
githubUserId: githubUserId,
106+
updatedAt: new Date(),
107+
},
108+
}
109+
);
110+
updatedCount++;
111+
console.log(
112+
`[MIGRATION] ✓ Updated user ${user._id} with githubUserId ${githubUserId}`
113+
);
114+
} else {
115+
// Multiple users with same GitHub ID - merge them
116+
console.log(
117+
`[MIGRATION] ⚠️ Found ${users.length} duplicate users for GitHub ID ${githubUserId}`
118+
);
119+
120+
// Keep the most recently updated user
121+
const sortedUsers = users.sort(
122+
(a, b) => b.updatedAt.getTime() - a.updatedAt.getTime()
123+
);
124+
const keepUser = sortedUsers[0];
125+
const deleteUsers = sortedUsers.slice(1);
126+
127+
// Update the keeper with githubUserId
128+
await database.users.updateOne(
129+
{ _id: keepUser._id },
130+
{
131+
$set: {
132+
githubUserId: githubUserId,
133+
updatedAt: new Date(),
134+
},
135+
}
136+
);
137+
138+
// Delete duplicate users
139+
for (const user of deleteUsers) {
140+
await database.users.deleteOne({ _id: user._id });
141+
console.log(`[MIGRATION] • Deleted duplicate user ${user._id}`);
142+
}
143+
144+
mergedCount += deleteUsers.length;
145+
console.log(
146+
`[MIGRATION] ✓ Kept user ${keepUser._id}, deleted ${deleteUsers.length} duplicates`
147+
);
148+
}
149+
}
150+
151+
console.log('\n[MIGRATION] Migration summary:');
152+
console.log(`[MIGRATION] • Updated: ${updatedCount} users`);
153+
console.log(`[MIGRATION] • Merged: ${mergedCount} duplicate users`);
154+
console.log(
155+
`[MIGRATION] • Failed: ${failedUsers.length} users (expired tokens)`
156+
);
157+
158+
if (failedUsers.length > 0) {
159+
console.log(
160+
'\n[MIGRATION] Users with expired tokens (will be updated on next login):'
161+
);
162+
for (const user of failedUsers) {
163+
console.log(`[MIGRATION] - ${user._id}`);
164+
}
165+
}
166+
167+
// Create unique index on githubUserId
168+
console.log('\n[MIGRATION] Creating unique index on githubUserId...');
169+
try {
170+
await database.users.createIndex(
171+
{ githubUserId: 1 },
172+
{ unique: true, sparse: true }
173+
);
174+
console.log('[MIGRATION] ✓ Index created successfully');
175+
} catch (error) {
176+
if (error.code === 85) {
177+
console.log('[MIGRATION] • Index already exists, skipping');
178+
} else {
179+
throw error;
180+
}
181+
}
182+
183+
console.log('\n[MIGRATION] ✓ User GitHub ID migration complete!');
184+
} catch (error) {
185+
console.error(
186+
'[MIGRATION] ❌ User GitHub ID migration failed:',
187+
error.message
188+
);
189+
throw error;
190+
}
191+
}
192+
193+
// Only close client if run directly (not imported)
194+
if (import.meta.url === `file://${process.argv[1]}`) {
195+
migrateDatabase()
196+
.then(() => {
197+
console.log('[MIGRATION] Migration completed successfully');
198+
process.exit(0);
199+
})
200+
.catch(error => {
201+
console.error('[MIGRATION] Unexpected error:', error);
202+
process.exit(1);
203+
})
204+
.finally(() => {
205+
client.close();
206+
});
207+
}
208+
209+
export { migrateDatabase };

‎src/database/database.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export const client = new MongoClient(url);
88
/**
99
* @typedef {object} User
1010
* @property {import("mongodb").ObjectId} [_id]
11+
* @property {number} githubUserId - GitHub user ID (stable identifier)
1112
* @property {string} githubAccessToken
1213
* @property {Date} createdAt
1314
* @property {Date} updatedAt

‎src/database/models.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const User = {
77
/**
88
* Create a new user
99
* @param {object} userData
10+
* @param {number} userData.githubUserId - GitHub user ID (stable identifier)
1011
* @param {string} userData.githubAccessToken
1112
* @returns {Promise<import('./database.js').User>}
1213
*/
@@ -39,6 +40,15 @@ export const User = {
3940
return await database.users.findOne({ githubAccessToken });
4041
},
4142

43+
/**
44+
* Find user by GitHub user ID
45+
* @param {number} githubUserId
46+
* @returns {Promise<import('./database.js').User|null>}
47+
*/
48+
async findByGithubUserId(githubUserId) {
49+
return await database.users.findOne({ githubUserId });
50+
},
51+
4252
/**
4353
* Find all users
4454
* @returns {Promise<import('./database.js').User[]>}

‎src/index.js‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
} from './helpers/installationHandler.js';
2929
import { removePatAuthentication } from '../scripts/remove-pat-auth.js';
3030
import { removeConfiguredField } from '../scripts/remove-configured-field.js';
31+
import { migrateDatabase as migrateUserGithubIds } from '../scripts/migrate-user-github-ids.js';
3132

3233
const mongoSessionStore = MongoStore.create({
3334
clientPromise: client.connect(),
@@ -59,6 +60,16 @@ async function startServer() {
5960
// Continue anyway - migration failure shouldn't prevent app startup
6061
}
6162

63+
try {
64+
await migrateUserGithubIds();
65+
} catch (error) {
66+
console.error(
67+
'[STARTUP] Failed to run user GitHub ID migration:',
68+
error.message
69+
);
70+
// Continue anyway - migration failure shouldn't prevent app startup
71+
}
72+
6273
const app = express();
6374

6475
// Setup Vite middleware for development
@@ -161,9 +172,40 @@ async function startServer() {
161172
return res.redirect('/');
162173
}
163174

164-
let user = await User.findByGithubToken(data.access_token);
165-
if (!user) {
175+
// Fetch GitHub user info to get the user ID
176+
const githubUserResponse = await fetch('https://api.github.com/user', {
177+
headers: {
178+
Authorization: `Bearer ${data.access_token}`,
179+
Accept: 'application/vnd.github.v3+json',
180+
},
181+
});
182+
183+
if (!githubUserResponse.ok) {
184+
console.error(
185+
'Failed to fetch GitHub user info:',
186+
githubUserResponse.status,
187+
githubUserResponse.statusText
188+
);
189+
return res.redirect('/');
190+
}
191+
192+
const githubUser = await githubUserResponse.json();
193+
if (!githubUser.id) {
194+
console.error('GitHub user response missing ID');
195+
return res.redirect('/');
196+
}
197+
198+
// Find user by GitHub user ID
199+
let user = await User.findByGithubUserId(githubUser.id);
200+
if (user) {
201+
// Update existing user's access token
202+
user = await User.update(user._id, {
203+
githubAccessToken: data.access_token,
204+
});
205+
} else {
206+
// Create new user with GitHub user ID and access token
166207
user = await User.create({
208+
githubUserId: githubUser.id,
167209
githubAccessToken: data.access_token,
168210
});
169211
}

0 commit comments

Comments
 (0)