[server] update OIDC users on sign-in (email) – WEB-370 (#17633)

* [server] update OIDC users on sign-in (email)

* adding simple tests for update of email attribute

Author: AlexTugarev

components/server/src/iam/iam-session-app.spec.ts

@@ -20,7 +20,7 @@ import * as request from "supertest";
 import * as chai from "chai";
 import { OIDCCreateSessionPayload } from "./iam-oidc-create-session-payload";
 import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TeamDB } from "@gitpod/gitpod-db/lib";
-import { TeamMemberInfo } from "@gitpod/gitpod-protocol";
+import { TeamMemberInfo, User } from "@gitpod/gitpod-protocol";
 const expect = chai.expect;
 
 @suite(timeout(10000))
@@ -33,20 +33,25 @@ class TestIamSessionApp {
     protected knownSubjectID = "111";
     protected knownEmail = "[email protected]";
 
+    protected knownUser: Partial<User> = {
+        id: "id-known-user",
+        identities: [],
+    };
+
     protected userServiceMock: Partial<UserService> = {
         createUser: (params) => {
             return { id: "id-new-user" } as any;
         },
 
         findUserForLogin: async (params) => {
             if (params.candidate?.authId === this.knownSubjectID) {
-                return { id: "id-known-user" } as any;
+                return this.knownUser as any;
             }
             return undefined;
         },
         findOrgOwnedUser: async (params) => {
             if (params.email === this.knownEmail) {
-                return { id: "id-known-user" } as any;
+                return this.knownUser as any;
             }
             return undefined;
         },
@@ -88,6 +93,7 @@ class TestIamSessionApp {
 
     public before() {
         this.teamDbMock.memberships.clear();
+        this.knownUser.identities = [];
 
         const container = new Container();
         container.load(
@@ -231,6 +237,58 @@ class TestIamSessionApp {
         expect(this.teamDbMock.memberships.has(BUILTIN_INSTLLATION_ADMIN_USER_ID)).to.be.false;
         expect(this.teamDbMock.memberships.has("id-new-user")).to.be.true;
     }
+
+    @test public async testSessionRequest_updates_existing_user() {
+        const payload: OIDCCreateSessionPayload = { ...this.payload };
+        payload.claims.sub = this.knownSubjectID; // `userServiceMock.findUserForLogin` will match this value
+
+        this.knownUser.identities = [
+            {
+                authId: payload.claims.sub,
+                authProviderId: payload.claims.aud,
+                authName: "Test User",
+                primaryEmail: "[email protected]",
+            },
+        ];
+
+        let newEmail: string | undefined;
+        this.userServiceMock.updateUserIdentity = async (user, updatedIdentity) => {
+            newEmail = updatedIdentity.primaryEmail;
+        };
+
+        const result = await request(this.app.create())
+            .post("/session")
+            .set("Content-Type", "application/json")
+            .send(JSON.stringify(payload));
+
+        expect(result.statusCode, JSON.stringify(result.body)).to.equal(200);
+        expect(newEmail, "update was not called").not.to.be.undefined;
+        expect(newEmail).to.equal(payload.claims.email);
+    }
+
+    @test public async testSessionRequest_no_update_if_same_email() {
+        this.knownUser.identities = [
+            {
+                authId: this.payload.claims.sub,
+                authProviderId: this.payload.claims.aud,
+                authName: "Test User",
+                primaryEmail: this.payload.claims.email,
+            },
+        ];
+
+        let updateUserIdentityCalled = false;
+        this.userServiceMock.updateUserIdentity = async () => {
+            updateUserIdentityCalled = true;
+        };
+
+        const result = await request(this.app.create())
+            .post("/session")
+            .set("Content-Type", "application/json")
+            .send(JSON.stringify(this.payload));
+
+        expect(result.statusCode, JSON.stringify(result.body)).to.equal(200);
+        expect(updateUserIdentityCalled).to.be.false;
+    }
 }
 
 module.exports = new TestIamSessionApp();

components/server/src/iam/iam-session-app.ts

@@ -59,6 +59,10 @@ export class IamSessionApp {
         const payload = OIDCCreateSessionPayload.validate(req.body);
 
         const existingUser = await this.findExistingOIDCUser(payload);
+        if (existingUser) {
+            await this.updateOIDCUserOnSign(existingUser, payload);
+        }
+
         const user = existingUser || (await this.createNewOIDCUser(payload));
 
         await new Promise<void>((resolve, reject) => {
@@ -138,6 +142,20 @@ export class IamSessionApp {
         return existingUser;
     }
 
+    /**
+     * Updates `User.identities[current IdP].primaryEmail`
+     */
+    protected async updateOIDCUserOnSign(user: User, payload: OIDCCreateSessionPayload) {
+        const recent = this.mapOIDCProfileToIdentity(payload);
+        const existing = user.identities.find((identity) => identity.authId === recent.authId);
+
+        // Update email
+        if (existing && !!recent.primaryEmail && existing.primaryEmail !== recent.primaryEmail) {
+            existing.primaryEmail = recent.primaryEmail;
+            await this.userService.updateUserIdentity(user, existing);
+        }
+    }
+
     protected async createNewOIDCUser(payload: OIDCCreateSessionPayload): Promise<User> {
         const { claims, organizationId } = payload;
 

components/server/src/user/user-service.ts

@@ -302,20 +302,20 @@ export class UserService {
         user.name = user.name || authUser.name || authUser.primaryEmail;
         user.avatarUrl = user.avatarUrl || authUser.avatarUrl;
         await this.onAfterUserLoad(user);
-        await this.updateUserIdentity(user, candidate, token);
+        await this.updateUserIdentity(user, candidate);
+        await this.userDb.storeSingleToken(candidate, token);
     }
 
     async onAfterUserLoad(user: User): Promise<User> {
         return user;
     }
 
-    async updateUserIdentity(user: User, candidate: Identity, token: Token) {
+    async updateUserIdentity(user: User, candidate: Identity) {
         // ensure single identity per auth provider instance
         user.identities = user.identities.filter((i) => i.authProviderId !== candidate.authProviderId);
         user.identities.push(candidate);
 
         await this.userDb.storeUser(user);
-        await this.userDb.storeSingleToken(candidate, token);
     }
 
     async deauthorize(user: User, authProviderId: string) {