fix: ensure that only one user can be linked to a subject@provider id…

…entity

README.md

@@ -116,6 +116,15 @@ Some super helpful references to keep handy:
   - [+] feat: Avatar and login/logout/profile stuff in header
 
 - [+] chore: upgrade architect
+- [ ] chore: set up automatic deployment to staging in "staging" branch and production in "main" branch
+  - aws secrets to GH secrets
+  - oauth secrets to gh secrets
+    - build `arc env` script to set up configuration environment
+  - staging branch deploys to staging
+  - main branch deploys to production
+  - production needs a sane CNAME
+  - update readme to note deployment steps
+
 - [ ] feat: extract lambda/middleware into new package (@web-app-stack/lambda-auth)
 - [ ] chore: basic unit tests (the server is thoroughly tested with unit tests, the client no-so-much)
 - [ ] chore: git hooks for linting

server/src/shared/lambda/oauth/handlers/redirect.spec.ts

@@ -272,6 +272,7 @@ describe("redirect", () => {
 
     // SECOND redirect/auth:
     //   add the session for the last created user and do the auth again:
+    //   it SHOULD login the same user successfully but not create a second user in our DB
     assert(foundSession !== null)
     req = await mockAuthorizationCodeResponseRequest(foundSession)
     await oauthRedirectHandler(req)
@@ -404,17 +405,18 @@ describe("redirect", () => {
       userRepositoryFactory(),
       identityRepositoryFactory()
     )
-    const req = await mockAuthorizationCodeResponseRequest()
 
     mockProviderConfigInEnvironment()
 
     // invoke handler for production (root)
+    let req = await mockAuthorizationCodeResponseRequest()
     let res = await oauthRedirectHandler(req)
     expect(res).toHaveProperty("statusCode", 302)
     expect(res).toHaveProperty("headers.location", "/")
 
     // invoke handler for staging (it used to be deployed to /staging, but we deploy everything to / now. Consider removing this test)
     process.env.NODE_ENV = "staging"
+    req = await mockAuthorizationCodeResponseRequest()
     res = await oauthRedirectHandler(req)
     expect(res).toHaveProperty("statusCode", 302)
     expect(res).toHaveProperty("headers.location", "/")
@@ -506,25 +508,18 @@ function stubIdentityRepositoryFactory(): sinon.SinonStubbedInstance<IdentityRep
 /* eslint-disable @typescript-eslint/no-explicit-any */
 function mockFetchJsonWithEmail(email: string): jest.Mock<Promise<any>> {
   /* eslint-enable @typescript-eslint/no-explicit-any */
-  return mockFetchJson({
-    access_token: "foo-access",
-    expires_in: 3600,
-    refresh_token: "foo-refresh",
-    id_token: createIDToken(email),
-  })
+  return mockFetchJson(email)
 }
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
-function mockFetchJson(
-  fetchResult: TokenResponse = {
-    access_token: "foo-access",
-    expires_in: 3600,
-    refresh_token: "foo-refresh",
-    id_token: createIDToken(),
-  }
-): jest.Mock<Promise<any>> {
+function mockFetchJson(email: string = ""): jest.Mock<Promise<any>> {
   const fetchJson = jest.fn(async () => {
-    return fetchResult
+    return {
+      access_token: "foo-access",
+      expires_in: 3600,
+      refresh_token: "foo-refresh",
+      id_token: email ? createIDToken(email) : createIDToken(`foo-${randomInt()}@bar.com`),
+    }
   })
   jest.doMock("../../../fetch", () => {
     fetchJson

server/src/shared/lambda/oauth/repository/IdentityRepository.spec.ts

@@ -34,6 +34,19 @@ describe("upsert", () => {
     expect(list).toHaveLength(1)
   })
 
+  it("should reject if subject@provider already has an identity", async () => {
+    // we check this one because we don't want two Users to ever be able to be linked to the same subject+provider
+    const user1 = randomIdentity()
+    // now create a DIFFERENT user but with the same subject+provider:
+    const user2 = {
+      ...randomIdentity(),
+      provider: user1.provider,
+      subject: user1.subject
+    }
+    await repo.upsert(user1)
+    await expect(repo.upsert(user2)).rejects.toThrowError(/linked to another user/)
+  })
+
   it.todo("should reject if missing args")
 })
 

server/src/shared/lambda/oauth/repository/IdentityRepository.ts

@@ -96,6 +96,12 @@ class IdentityRepositoryImpl
   ): Promise<StoredIdentity> {
     this.throwIfRequiredPropertyMissing(identity, REQUIRED_ADD_PROPS)
 
+    // ensure nobody else is linked to this subject@provider:
+    const exists = await this.getByProviderSubject(identity.provider, identity.subject)
+    if (exists && exists.userID !== identity.userID) {
+      throw new Error(`The subject '${identity.subject}' at provider '${identity.provider}' is already linked to another user.`)
+    }
+
     const readyIdentity = {
       ...identity,
       id: this.idForIdentity(identity.userID, identity.provider),
@@ -164,7 +170,7 @@ class IdentityRepositoryImpl
     }
     assert(
       result.Items.length === 0 || result.Items.length === 1,
-      `unexpected number of items returned (${result.Items.length}) for provider ${provider} and subject ${subject}.`
+      `unexpected number of identities returned (${result.Items.length}) for provider ${provider} and subject ${subject}.`
     )
     const item = result.Items[0] as StoredIdentity
     assert(!("provider_subject" in item), "unexpected provider_subject")