fix: address CodeQL and code-quality scan findings

Author: adbergen

src/SEBT.Portal.Api/Controllers/Auth/OidcController.cs

@@ -50,13 +50,16 @@ public async Task<IActionResult> GetConfig(
         [FromQuery] bool stepUp = false,
         CancellationToken cancellationToken = default)
     {
+        // Validate and normalize the route parameter against the server-side allowlist.
+        // After this point, stateCode is a known-good value — not raw user input.
         if (!stateAllowlist.Contains(code))
         {
             logger.LogWarning(
                 "OIDC GetConfig rejected: stateCode {StateCode} not in allowlist (reason=unknown_state)",
                 SanitizeForLog(code));
             return BadRequest(new ErrorResponse("Unknown or unsupported stateCode."));
         }
+        var stateCode = code.ToLowerInvariant();
 
         var authorizationEndpoint = stepUp
             ? config["Oidc:StepUp:AuthorizationEndpoint"]
@@ -69,7 +72,7 @@ public async Task<IActionResult> GetConfig(
         {
             logger.LogWarning(
                 "OIDC config missing for stateCode {StateCode} (reason=oidc_not_configured)",
-                SanitizeForLog(code));
+                stateCode);
             var hint = environment.IsDevelopment()
                 ? "Set Oidc:AuthorizationEndpoint, Oidc:ClientId, and Oidc:CallbackRedirectUri in appsettings."
                 : "";
@@ -83,7 +86,7 @@ public async Task<IActionResult> GetConfig(
 
         // Create the pre-auth session and set the cookie.
         var session = await sessionStore.CreateAsync(
-            code.ToLowerInvariant(), state, codeVerifier, redirectUri, stepUp, cancellationToken);
+            stateCode, state, codeVerifier, redirectUri, stepUp, cancellationToken);
         OidcSessionCookie.Set(Response, session.Id);
 
         var languageParam = config["Oidc:LanguageParam"] ?? "en";
@@ -118,11 +121,15 @@ public async Task<IActionResult> Callback(
         if (body == null || string.IsNullOrEmpty(body.Code) || string.IsNullOrEmpty(body.StateCode))
             return BadRequest(new ErrorResponse("Missing code or stateCode."));
 
-        if (!stateAllowlist.Contains(body.StateCode))
+        // Bind to locals after null checks so downstream code uses validated references.
+        var requestedCode = body.Code;
+        var requestedStateCode = body.StateCode;
+
+        if (!stateAllowlist.Contains(requestedStateCode))
         {
             logger.LogWarning(
                 "OIDC Callback rejected: stateCode {StateCode} not in allowlist (reason=unknown_state)",
-                SanitizeForLog(body.StateCode));
+                SanitizeForLog(requestedStateCode));
             return BadRequest(new ErrorResponse("Unknown or unsupported stateCode."));
         }
 
@@ -150,7 +157,7 @@ public async Task<IActionResult> Callback(
         }
 
         // --- Validate stateCode matches stored value (prevents tenant switching) ---
-        if (!string.Equals(body.StateCode, session.StateCode, StringComparison.OrdinalIgnoreCase))
+        if (!string.Equals(requestedStateCode, session.StateCode, StringComparison.OrdinalIgnoreCase))
         {
             logger.LogWarning(
                 "OIDC Callback rejected: stateCode mismatch (reason=mismatched_stateCode, SessionId={SessionId})", sessionId);
@@ -168,7 +175,7 @@ public async Task<IActionResult> Callback(
 
         // --- Exchange code using the stored code_verifier (never from the body) ---
         var result = await exchangeService.ExchangeCodeAsync(
-            body.Code,
+            requestedCode,
             session.CodeVerifier,
             session.RedirectUri,
             session.IsStepUp,
@@ -213,11 +220,15 @@ public async Task<IActionResult> CompleteLogin(
         if (body == null || string.IsNullOrEmpty(body.StateCode) || string.IsNullOrEmpty(body.CallbackToken))
             return BadRequest(new ErrorResponse("Missing stateCode or callbackToken."));
 
-        if (!stateAllowlist.Contains(body.StateCode))
+        // Validate and bind to local variables so downstream code uses validated values.
+        var requestedStateCode = body.StateCode;
+        var callbackToken = body.CallbackToken;
+
+        if (!stateAllowlist.Contains(requestedStateCode))
         {
             logger.LogWarning(
                 "OIDC CompleteLogin rejected: stateCode {StateCode} not in allowlist (reason=unknown_state)",
-                SanitizeForLog(body.StateCode));
+                SanitizeForLog(requestedStateCode));
             return BadRequest(new ErrorResponse("Unknown or unsupported stateCode."));
         }
 
@@ -230,7 +241,7 @@ public async Task<IActionResult> CompleteLogin(
         }
 
         // --- Verify the callback token matches this session and hasn't been consumed ---
-        var tokenHash = IPreAuthSessionStore.HashCallbackToken(body.CallbackToken);
+        var tokenHash = IPreAuthSessionStore.HashCallbackToken(callbackToken);
         var advanced = await sessionStore.TryAdvanceToLoginCompletedAsync(sessionId, tokenHash, cancellationToken);
         if (!advanced)
         {
@@ -244,7 +255,7 @@ public async Task<IActionResult> CompleteLogin(
         OidcSessionCookie.Clear(Response);
         await sessionStore.RemoveAsync(sessionId, cancellationToken);
 
-        var stateKey = body.StateCode.ToLowerInvariant();
+        var stateKey = requestedStateCode.ToLowerInvariant();
         var signingKey = config["Oidc:CompleteLoginSigningKey"];
         if (string.IsNullOrEmpty(signingKey))
         {
@@ -274,12 +285,12 @@ public async Task<IActionResult> CompleteLogin(
         ClaimsPrincipal principal;
         try
         {
-            principal = handler.ValidateToken(body.CallbackToken, validationParams, out _);
+            principal = handler.ValidateToken(callbackToken, validationParams, out _);
         }
         catch (Exception ex)
         {
             logger.LogWarning(ex, "Invalid or expired callback token for state {StateCode}",
-                SanitizeForLog(body.StateCode));
+                SanitizeForLog(requestedStateCode));
             return BadRequest(new ErrorResponse("Invalid or expired callback token."));
         }
 

src/SEBT.Portal.Api/Services/OidcExchangeService.cs

@@ -163,7 +163,7 @@ public async Task<OidcExchangeResult> ExchangeCodeAsync(
 
         // --- Exchange authorization code for tokens ---
         using var client = _httpFactory.CreateClient();
-        var tokenParams = new FormUrlEncodedContent(new Dictionary<string, string>
+        using var tokenParams = new FormUrlEncodedContent(new Dictionary<string, string>
         {
             ["grant_type"] = "authorization_code",
             ["code"] = code,
@@ -332,7 +332,15 @@ void TrySet(string jsonKey, string claimName)
 
             TrySet("sub", "sub");
             TrySet("email", "email");
-            TrySet("preferred_username", "email"); // fallback for IdPs that put email here
+            // Some IdPs put email in preferred_username — only use it as email if it looks like one.
+            if (!claims.ContainsKey("email")
+                && doc.RootElement.TryGetProperty("preferred_username", out var pu)
+                && pu.ValueKind == JsonValueKind.String
+                && !string.IsNullOrEmpty(pu.GetString())
+                && pu.GetString()!.Contains('@'))
+            {
+                claims.TryAdd("email", pu.GetString()!);
+            }
             TrySet("phone", "phone");
             TrySet("phone_number", "phone_number");
             TrySet("given_name", "givenName");

src/SEBT.Portal.Api/Services/PreAuthSessionStore.cs

@@ -123,7 +123,7 @@ public async Task<PreAuthSession> CreateAsync(
         KnownSessionIds.TryAdd(sessionId, 0);
         _logger.LogInformation(
             "Pre-auth session created: SessionId={SessionId}, StateCode={StateCode} (reason=session_created)",
-            sessionId, stateCode);
+            sessionId, SanitizeForLog(stateCode));
         return session;
     }
 
@@ -141,6 +141,14 @@ public async Task<PreAuthSession> CreateAsync(
             _ => ValueTask.FromResult<PreAuthSession?>(null),
             CacheOptions,
             cancellationToken: cancellationToken);
+
+        // Session expired in cache — clean up tracking dictionaries so they don't grow unbounded.
+        if (result == null)
+        {
+            KnownSessionIds.TryRemove(sessionId, out _);
+            SessionLocks.TryRemove(sessionId, out _);
+        }
+
         return result;
     }
 
@@ -231,6 +239,16 @@ public async Task RemoveAsync(string sessionId, CancellationToken cancellationTo
 
     private static string CacheKey(string sessionId) => $"{CacheKeyPrefix}{sessionId}";
 
+    private static string SanitizeForLog(string? value)
+    {
+        if (string.IsNullOrEmpty(value)) return string.Empty;
+        return value
+            .Replace("\r", string.Empty, StringComparison.Ordinal)
+            .Replace("\n", string.Empty, StringComparison.Ordinal)
+            .Replace("\u2028", string.Empty, StringComparison.Ordinal)
+            .Replace("\u2029", string.Empty, StringComparison.Ordinal);
+    }
+
     private static string GenerateSessionId()
     {
         Span<byte> bytes = stackalloc byte[32];

test/SEBT.Portal.Tests/Integration/AuthCookieAuthenticationTests.cs

@@ -70,7 +70,7 @@ public async Task GetStatus_WithExpiredSessionCookie_ReturnsUnauthorized()
     private async Task<HttpResponseMessage> GetStatusWithCookie(string? token)
     {
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Get, "/api/auth/status");
+        using var request = new HttpRequestMessage(HttpMethod.Get, "/api/auth/status");
         if (token != null)
             request.Headers.Add("Cookie", $"{AuthCookies.AuthCookieName}={token}");
         return await client.SendAsync(request);

test/SEBT.Portal.Tests/Integration/OidcPreAuthSecurityTests.cs

@@ -43,7 +43,7 @@ public async Task V01_T03_CompleteLogin_WithoutSessionCookie_Returns403()
     {
         var client = _factory.CreateClient();
         var callbackToken = MintCallbackToken("user@example.com");
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
         {
             Content = JsonContent.Create(new { stateCode = "co", callbackToken }),
             Headers = { { "Origin", "http://localhost:3000" } }
@@ -60,7 +60,7 @@ public async Task V01_T03_CompleteLogin_WithSpoofedOrigin_Returns403()
     {
         var client = _factory.CreateClient();
         var callbackToken = MintCallbackToken("user@example.com");
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
         {
             Content = JsonContent.Create(new { stateCode = "co", callbackToken }),
             Headers = { { "Origin", "https://attacker.example.com" } }
@@ -77,7 +77,7 @@ public async Task V01_T03_CompleteLogin_WithMissingOrigin_Returns403()
     {
         var client = _factory.CreateClient();
         var callbackToken = MintCallbackToken("user@example.com");
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
         {
             Content = JsonContent.Create(new { stateCode = "co", callbackToken })
             // No Origin header at all — curl from command line
@@ -119,7 +119,7 @@ public async Task V02_T02_CompleteLogin_SecondCallWithConsumedSession_Returns403
         await sessionStore.TryAdvanceToLoginCompletedAsync(session.Id, tokenHash);
 
         // Replay attempt: exact same token + cookie → must fail
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
         {
             Content = JsonContent.Create(new { stateCode = "co", callbackToken }),
             Headers =
@@ -146,7 +146,7 @@ public async Task V03_T04_Callback_WithTamperedState_Returns400()
         var session = await sessionStore.CreateAsync("co", "real-state-value", "verifier1", "http://localhost:3000/callback", false);
 
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {
@@ -187,7 +187,7 @@ public async Task V05_T07a_CompleteLogin_WithInvalidStateCode_Returns400(string
     {
         var client = _factory.CreateClient();
         var callbackToken = MintCallbackToken("user@example.com");
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/complete-login")
         {
             Content = JsonContent.Create(new { stateCode, callbackToken }),
             Headers = { { "Origin", "http://localhost:3000" } }
@@ -204,7 +204,7 @@ public async Task V05_T07a_CompleteLogin_WithInvalidStateCode_Returns400(string
     public async Task V05_T07a_Callback_WithInvalidStateCode_Returns400(string stateCode)
     {
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new { code = "code", state = "state", stateCode }),
             Headers = { { "Origin", "http://localhost:3000" } }
@@ -225,7 +225,7 @@ public async Task V05_T07a_Callback_WithInvalidStateCode_Returns400(string state
     public async Task V06_T08aA_Callback_WithoutSessionCookie_Returns403()
     {
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {
@@ -246,7 +246,7 @@ public async Task V06_T08aA_Callback_WithoutSessionCookie_Returns403()
     public async Task V06_T08aA_Callback_WithSpoofedOrigin_Returns403()
     {
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {
@@ -274,7 +274,7 @@ public async Task V06_T08aB_Callback_WithValidSessionButWrongState_Returns400()
         var session = await sessionStore.CreateAsync("co", "correct-state", "verifier", "http://localhost:3000/callback", false);
 
         var client = _factory.CreateClient();
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {
@@ -308,7 +308,7 @@ public async Task V06_T08aE_Callback_WithValidSessionButWrongStateCode_Returns40
         var client = _factory.CreateClient();
         // Session was created for "co" but we send "az" (stateCode mismatch)
         // Note: "az" isn't in the allowlist either, so this gets caught at both layers
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {
@@ -344,7 +344,7 @@ public async Task SessionReplay_CallbackOnAlreadyUsedSession_Returns400()
 
         var client = _factory.CreateClient();
         // Try to use the same session for another callback — should fail
-        var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
+        using var request = new HttpRequestMessage(HttpMethod.Post, "/api/auth/oidc/callback")
         {
             Content = JsonContent.Create(new
             {