From 5b0cb7fe83a9034682c8ea650107775298acc6de Mon Sep 17 00:00:00 2001
From: Bernhard Windisch <bwi@cocoar.dev>
Date: Tue, 2 Jun 2026 08:07:37 +0200
Subject: [PATCH] fix(identity): reject deactivated/deleted users on external
 (OIDC/SAML) login

The admin recycle-bin deactivates a user (IsActive=false) but deliberately keeps
their external identity links. Password, magic-link, and passkey login all gate on
IsActive -- external login did not, so a binned (or deactivated) user holding an
IdP link could re-authenticate through that provider and receive a fresh app
cookie, fully bypassing the bin. RevokeAllAccessAsync only kills EXISTING
sessions/tokens; it does not stop a fresh login.

Add the IsActive/IsDeleted gate at the single sign-in choke point
(ExternalLoginProcessor.Success), covering the returning-link, link-to-authenticated,
and email-match paths. JIT-created users are IsActive=true and pass unaffected.

Surfaced by an adversarial review of the account-lifecycle hardening (#42). The gap
is pre-existing but became security-relevant once the recycle-bin lifecycle made
IsActive the sole lockout for binned users.

Regression test: DeactivatedUser_WithExternalLink_CannotSignIn.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../ExternalLoginProcessorTests.cs            | 29 +++++++++++++++++++
 .../ExternalAuth/ExternalLoginProcessor.cs    | 14 +++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/dotnet/Modgud.Api.Tests/ExternalAuth/ExternalLoginProcessorTests.cs b/src/dotnet/Modgud.Api.Tests/ExternalAuth/ExternalLoginProcessorTests.cs
index d038df68..07990db9 100644
--- a/src/dotnet/Modgud.Api.Tests/ExternalAuth/ExternalLoginProcessorTests.cs
+++ b/src/dotnet/Modgud.Api.Tests/ExternalAuth/ExternalLoginProcessorTests.cs
@@ -250,6 +250,35 @@ public async Task MissingSubject_Fails()
         Assert.Equal("Idp.InvalidToken", result.ErrorCode);
     }
 
+    [Fact]
+    public async Task DeactivatedUser_WithExternalLink_CannotSignIn()
+    {
+        // Regression: the admin recycle-bin deactivates a user (IsActive=false) but
+        // deliberately keeps their external identity links. External login must
+        // refuse a deactivated/deleted user, exactly like password/magic-link/
+        // passkey do — otherwise a binned user could re-authenticate via their IdP
+        // and bypass the bin.
+        var config = await CreateEnabledEntraConfig();
+        var user = await Factory.CreateTestUserWithIdentityAsync("Ban", "Ned", "BN", "banned@acme.com");
+        await LinkUserAsync(user.Id, config.Id, subject: "sub-banned-1");
+
+        using (var scope = Factory.Services.CreateScope())
+        {
+            var userManager = scope.ServiceProvider.GetRequiredService<UserManager<ApplicationUser>>();
+            var appUser = await userManager.FindByIdAsync(user.Id.ToString());
+            appUser!.IsActive = false;
+            await userManager.UpdateAsync(appUser);
+        }
+
+        using var scope2 = Factory.Services.CreateScope();
+        var processor = scope2.ServiceProvider.GetRequiredService<ExternalLoginProcessor>();
+        var external = BuildExternalPrincipal(subject: "sub-banned-1", email: "banned@acme.com", name: "Ban Ned");
+        var result = await processor.ProcessAsync(external, config.Id, default);
+
+        Assert.False(result.Succeeded);
+        Assert.Equal("Idp.UserInactive", result.ErrorCode);
+    }
+
     private async Task<LoginProvider> CreateEnabledEntraConfig(
         bool autoCreate = false,
         bool trustForEmailLink = false,
diff --git a/src/dotnet/Modgud.Authentication/Api/ExternalAuth/ExternalLoginProcessor.cs b/src/dotnet/Modgud.Authentication/Api/ExternalAuth/ExternalLoginProcessor.cs
index c3145470..fd5d646a 100644
--- a/src/dotnet/Modgud.Authentication/Api/ExternalAuth/ExternalLoginProcessor.cs
+++ b/src/dotnet/Modgud.Authentication/Api/ExternalAuth/ExternalLoginProcessor.cs
@@ -366,6 +366,20 @@ private async Task<ExternalLoginResult> Success(
         IReadOnlyList<string> externalGroups,
         CancellationToken ct)
     {
+        // A deactivated or deleted user must never receive a fresh app cookie via
+        // federation. Password / magic-link / passkey login all gate this, and the
+        // admin recycle-bin relies on IsActive as its ONLY lockout — so a binned
+        // user holding an external IdP link could otherwise re-authenticate through
+        // that provider and bypass the bin. RevokeAllAccessAsync only kills EXISTING
+        // sessions/tokens; it does not stop a fresh login. JIT-created users are
+        // IsActive=true, so the JIT path passes this gate unaffected.
+        if (user.IsDeleted || !user.IsActive)
+        {
+            logger.LogWarning(
+                "Auth: External login rejected — user {UserId} is inactive or deleted", user.Id);
+            return ExternalLoginResult.Failed("Idp.UserInactive", "This account is not active.");
+        }
+
         // Build the sign-in ClaimsPrincipal. It carries session mechanics — link
         // id + issuer for logout routing, amr for TwoFactorFederated — PLUS, for a
         // provider trusted for authorization, the federation v1 "session group"