fix(oidc): drop the old provider's avatar and identity when the IdP changes

Pointing the instance at a different OIDC provider left two things from the old
one on every account. The avatar kept a URL on a host this instance no longer
talks to, so it rendered as a broken image and stayed that way: the sync only
ran when the new provider sent a picture claim, and having no picture for a user
is a perfectly normal answer.

The account also stayed pinned to the old provider. Login still worked, because
the verified-email lookup finds the row, but the relink sat behind a check for a
missing sub, so oidc_sub and oidc_issuer never moved. The SSO note in settings
went on naming a provider that was gone.

An uploaded avatar is a bare filename rather than a URL and is left alone on
both paths, exactly as before.

Fixes #2110

server/src/nest/oidc/oidc.service.ts

@@ -525,6 +525,14 @@ export class OidcService implements OnModuleDestroy {
           return { error: 'email_not_verified' };
         }
         this.db.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run(sub, config.issuer, user.id);
+        user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer } as User;
+      } else if (user.oidc_issuer !== config.issuer || user.oidc_sub !== sub) {
+        // The admin pointed the instance at a different IdP. We got here through the
+        // verified-email lookup, so this is the same person arriving from the new
+        // provider; leaving the old sub and issuer on the row would keep the account
+        // pinned to a provider that no longer exists (#2110).
+        this.db.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run(sub, config.issuer, user.id);
+        user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer } as User;
       }
       // Update role based on OIDC claims on every login (if claim mapping is configured)
       if (readEnv().oidc.adminValue) {
@@ -547,10 +555,16 @@ export class OidcService implements OnModuleDestroy {
         }
       }
       // Keep the avatar in sync with the OIDC picture, but never clobber a custom
-      // upload: only fill it when empty or when the current value is itself an OIDC
+      // upload: only touch it when empty or when the current value is itself an OIDC
       // picture URL, so the picture refreshes on each login without overriding an
       // uploaded one. #1399
-      if (picture && picture !== user.avatar && (!user.avatar || /^https:\/\//i.test(user.avatar))) {
+      //
+      // "In sync" includes the provider having no picture for this user any more. That
+      // is what a provider switch looks like from here, and the old value points at a
+      // host this instance no longer talks to, so it renders as a broken image forever
+      // (#2110). An uploaded avatar is a bare filename and stays untouched either way.
+      const avatarIsOidc = !!user.avatar && /^https:\/\//i.test(user.avatar);
+      if (picture ? picture !== user.avatar && (!user.avatar || avatarIsOidc) : avatarIsOidc) {
         this.db.prepare('UPDATE users SET avatar = ? WHERE id = ?').run(picture, user.id);
         user = { ...user, avatar: picture } as User;
       }

server/tests/unit/nest/oidc.service.test.ts

@@ -678,6 +678,54 @@ describe('findOrCreateUser', () => {
     expect(row.avatar).toBe('https://idp.example.com/u/new.png');
   });
 
+  // #2110 — the admin repoints the instance at a different IdP. The user still logs in,
+  // because the verified-email lookup finds the row, but everything the old provider put
+  // on it used to survive: an avatar URL on a host this instance no longer talks to, and
+  // a sub/issuer pair pinning the account to a provider that is gone.
+  it('OIDC-SVC-060: a provider without a picture claim clears the previous provider avatar', () => {
+    const { user } = createUser(testDb, { email: 'switch1@example.com' });
+    testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?')
+      .run('sub-old-1', 'https://old-idp.example.com', 'https://old-idp.example.com/u/me.png', user.id);
+
+    svc.findOrCreateUser(
+      { sub: 'sub-new-1', email: 'switch1@example.com', name: 'Switcher', email_verified: true },
+      { ...MOCK_CONFIG, issuer: 'https://new-idp.example.com' },
+    );
+
+    const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any;
+    expect(row.avatar).toBeNull();
+  });
+
+  it('OIDC-SVC-061: an uploaded avatar survives the same switch', () => {
+    const { user } = createUser(testDb, { email: 'switch2@example.com' });
+    // A local upload is a bare filename, not a URL, and belongs to the user.
+    testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?')
+      .run('sub-old-2', 'https://old-idp.example.com', 'uploaded-abc.jpg', user.id);
+
+    svc.findOrCreateUser(
+      { sub: 'sub-new-2', email: 'switch2@example.com', name: 'Switcher', email_verified: true },
+      { ...MOCK_CONFIG, issuer: 'https://new-idp.example.com' },
+    );
+
+    const row = testDb.prepare('SELECT avatar FROM users WHERE id = ?').get(user.id) as any;
+    expect(row.avatar).toBe('uploaded-abc.jpg');
+  });
+
+  it('OIDC-SVC-062: the account is relinked to the new provider sub and issuer', () => {
+    const { user } = createUser(testDb, { email: 'switch3@example.com' });
+    testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?')
+      .run('sub-old-3', 'https://old-idp.example.com', user.id);
+
+    svc.findOrCreateUser(
+      { sub: 'sub-new-3', email: 'switch3@example.com', name: 'Switcher', email_verified: true },
+      { ...MOCK_CONFIG, issuer: 'https://new-idp.example.com' },
+    );
+
+    const row = testDb.prepare('SELECT oidc_sub, oidc_issuer FROM users WHERE id = ?').get(user.id) as any;
+    expect(row.oidc_sub).toBe('sub-new-3');
+    expect(row.oidc_issuer).toBe('https://new-idp.example.com');
+  });
+
   it('OIDC-SVC-053: returns no_email when the email claim is missing (no throw)', () => {
     const result = svc.findOrCreateUser({ sub: 'sub-no-email', name: 'No Email' }, MOCK_CONFIG);
     expect('error' in result).toBe(true);