diff --git a/src/core/User.ts b/src/core/User.ts index 05acd1a..ffe69aa 100644 --- a/src/core/User.ts +++ b/src/core/User.ts @@ -132,9 +132,16 @@ export default class User { * @returns the list of anonymized repositories */ async getRepositories() { - const query: Record = this.username - ? { $or: [{ owner: this.id }, { "coauthors.username": this.username }] } - : { owner: this.id }; + const memberships: Record[] = [{ owner: this.id }]; + const githubId = this.model.externalIDs?.github; + if (githubId) memberships.push({ "coauthors.githubId": githubId }); + if (this.username) { + memberships.push({ coauthors: { $elemMatch: { + username: this.username, + $or: [{ githubId: { $exists: false } }, { githubId: null }, { githubId: "" }], + } } }); + } + const query = { $or: memberships }; const repositories = ( await AnonymizedRepositoryModel.find(query).exec() ).map((d) => new Repository(d)); diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index d7d598e..c1b3138 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -817,9 +817,12 @@ router.delete( const user = await getUser(req); const target = req.params.username; const isOwner = repo.owner.id === user.model.id; - const isSelf = - !!user.username && - user.username.toLowerCase() === target.toLowerCase(); + const coauthor = (repo.model.coauthors || []).find( + (c) => c.username.toLowerCase() === target.toLowerCase() + ); + const isSelf = coauthor?.githubId + ? coauthor.githubId === user.model.externalIDs?.github + : !!user.username && user.username.toLowerCase() === target.toLowerCase(); if (!isOwner && !isSelf && !user.isAdmin) { throw new AnonymousError("not_authorized", { httpStatus: 401 }); } diff --git a/test/production-regressions.test.js b/test/production-regressions.test.js index 24dc22c..ed172bc 100644 --- a/test/production-regressions.test.js +++ b/test/production-regressions.test.js @@ -55,6 +55,18 @@ describe("production regressions", function () { expect(emitted).to.equal(false); }); + it("denies another GitHub identity access through a reused coauthor username", async function () { + const User = require("../src/core/User").default; + const UserModel = require("../src/core/model/users/users.model").default; + const user = new User(new UserModel({ username: "old-name", externalIDs: { github: "new-id" } })); + stub(RepoModel, "find", filter => { + expect(require("sift").default(filter)({ owner: "someone", coauthors: [{ username: "old-name", githubId: "old-id" }] })).to.equal(false); + expect(require("sift").default(filter)({ owner: "someone", coauthors: [{ username: "renamed", githubId: "new-id" }] })).to.equal(true); + return { exec: async () => [] }; + }); + expect(await user.getRepositories()).to.deep.equal([]); + }); + it("does not link a recycled OAuth username to an existing GitHub identity", async function () { require("../src/server/routes/connection"); const passport = require("passport");