Merge pull request #969 from Chris0Jeky/fix/sec-31-oauth-scope-validation

SEC-31: Add OAuth scope validation for GitHub login

Author: Chris0Jeky

backend/src/Taskdeck.Api/Extensions/ApplicationServiceRegistration.cs

@@ -28,6 +28,7 @@ public static IServiceCollection AddApplicationServices(this IServiceCollection
         services.AddScoped<AuthenticationService>();
         services.AddScoped<AuthorizationService>();
         services.AddScoped<MfaService>();
+        services.AddSingleton<OAuthScopeValidator>();
         services.AddScoped<IAuthorizationService>(sp => sp.GetRequiredService<AuthorizationService>());
         services.AddScoped<UserService>();
         services.AddScoped<BoardAccessService>();

backend/src/Taskdeck.Api/Extensions/AuthenticationRegistration.cs

@@ -3,10 +3,13 @@
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Authentication.Cookies;
 using Microsoft.AspNetCore.Authentication.JwtBearer;
+using Microsoft.AspNetCore.Authentication.OAuth;
 using Microsoft.AspNetCore.Authentication.OAuth.Claims;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Http;
+using Microsoft.Extensions.Logging;
 using Microsoft.IdentityModel.Tokens;
 using Polly;
 using Polly.Extensions.Http;
@@ -135,15 +138,69 @@ await context.Response.WriteAsJsonAsync(new ApiErrorResponse(
                 options.Scope.Add("read:user");
                 options.Scope.Add("user:email");
 
+                // Ensure any additional RequiredScopes from configuration are also
+                // requested from GitHub. Without this, configuring a required scope
+                // that isn't requested causes all logins to be rejected.
+                foreach (var scope in gitHubOAuthSettings.RequiredScopes ?? Enumerable.Empty<string>())
+                {
+                    if (!options.Scope.Contains(scope))
+                        options.Scope.Add(scope);
+                }
+                foreach (var scope in gitHubOAuthSettings.ExpectedScopes ?? Enumerable.Empty<string>())
+                {
+                    if (!options.Scope.Contains(scope))
+                        options.Scope.Add(scope);
+                }
+
                 options.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id");
                 options.ClaimActions.MapJsonKey(ClaimTypes.Name, "login");
                 options.ClaimActions.MapJsonKey(ClaimTypes.Email, "email");
                 options.ClaimActions.MapJsonKey("urn:github:name", "name");
                 options.ClaimActions.MapJsonKey("urn:github:login", "login");
                 options.ClaimActions.MapJsonKey("urn:github:avatar", "avatar_url");
 
-                // OAuthHandler fetches UserInformationEndpoint automatically
-                // and applies ClaimActions — no custom OnCreatingTicket needed.
+                // Validate OAuth scopes after token exchange.
+                // GitHub returns the granted scopes in the token response body as "scope"
+                // (comma-separated). Reject authentication if required scopes are missing.
+                var scopeSettings = gitHubOAuthSettings;
+                options.Events = new OAuthEvents
+                {
+                    OnCreatingTicket = context =>
+                    {
+                        var scopeValidator = context.HttpContext.RequestServices
+                            .GetRequiredService<OAuthScopeValidator>();
+
+                        // GitHub returns granted scopes in the token response "scope" field.
+                        // The X-OAuth-Scopes header appears on API responses, but the token
+                        // response body is the authoritative source at auth time.
+                        var grantedScopesRaw = context.TokenResponse?.Response?.RootElement
+                            .TryGetProperty("scope", out var scopeElement) == true
+                            ? scopeElement.GetString()
+                            : null;
+
+                        var validationResult = scopeValidator.Validate(
+                            grantedScopesRaw,
+                            scopeSettings.RequiredScopes,
+                            scopeSettings.ExpectedScopes);
+
+                        if (!validationResult.IsValid)
+                        {
+                            var logger = context.HttpContext.RequestServices
+                                .GetRequiredService<ILoggerFactory>()
+                                .CreateLogger("Taskdeck.Api.OAuth.ScopeValidation");
+
+                            logger.LogError(
+                                "GitHub OAuth authentication rejected: missing required scopes. " +
+                                "Missing: [{MissingScopes}]. User must re-authorize with required permissions.",
+                                string.Join(", ", validationResult.MissingRequiredScopes));
+
+                            context.Fail(validationResult.ErrorMessage
+                                ?? "GitHub OAuth scope validation failed");
+                        }
+
+                        return Task.CompletedTask;
+                    }
+                };
 
                 // Circuit breaker for the OAuth backchannel (token exchange + user info).
                 if (circuitBreakerTracker is not null && circuitBreakerSettings is not null)

backend/src/Taskdeck.Application/Services/GitHubOAuthSettings.cs

@@ -9,6 +9,20 @@ public class GitHubOAuthSettings
     public string ClientId { get; set; } = string.Empty;
     public string ClientSecret { get; set; } = string.Empty;
 
+    /// <summary>
+    /// Scopes that must be present in the GitHub OAuth response.
+    /// Authentication is rejected if any required scope is missing.
+    /// Default: "user:email" (needed to retrieve the user's email address).
+    /// </summary>
+    public List<string> RequiredScopes { get; set; } = new() { "user:email" };
+
+    /// <summary>
+    /// Scopes that are expected but not mandatory.
+    /// A warning is logged if any expected scope is missing, but authentication proceeds.
+    /// Default: "read:user", "user:email".
+    /// </summary>
+    public List<string> ExpectedScopes { get; set; } = new() { "read:user", "user:email" };
+
     /// <summary>
     /// Returns true when GitHub OAuth is fully configured and should be active.
     /// </summary>

backend/src/Taskdeck.Application/Services/OAuthScopeValidator.cs

@@ -0,0 +1,143 @@
+using Microsoft.Extensions.Logging;
+
+namespace Taskdeck.Application.Services;
+
+/// <summary>
+/// Validates OAuth scopes granted by external providers against required and expected scopes.
+/// GitHub returns granted scopes as a comma-separated list in the token response body.
+/// </summary>
+public class OAuthScopeValidator
+{
+    private readonly ILogger<OAuthScopeValidator> _logger;
+
+    public OAuthScopeValidator(ILogger<OAuthScopeValidator> logger)
+    {
+        _logger = logger;
+    }
+
+    /// <summary>
+    /// Validates the granted scopes against the configured required and expected scopes.
+    /// </summary>
+    /// <param name="grantedScopesHeader">
+    /// The raw scope string from the provider (e.g. from GitHub's token response "scope" field).
+    /// GitHub uses comma-separated scopes; this method handles both comma and space separators.
+    /// </param>
+    /// <param name="requiredScopes">Scopes that must be present — authentication fails if any are missing.</param>
+    /// <param name="expectedScopes">Scopes that should be present — a warning is logged if missing, but auth proceeds.</param>
+    /// <returns>A result indicating whether validation passed, with details about any issues.</returns>
+    public OAuthScopeValidationResult Validate(
+        string? grantedScopesHeader,
+        IReadOnlyList<string>? requiredScopes,
+        IReadOnlyList<string>? expectedScopes)
+    {
+        var grantedScopes = ParseScopes(grantedScopesHeader);
+        // GitHub scopes are case-sensitive (e.g. "read:user" != "Read:User").
+        // Use Ordinal comparison to match exactly what the provider returns.
+        var grantedSet = new HashSet<string>(grantedScopes, StringComparer.Ordinal);
+
+        var effectiveRequired = requiredScopes ?? Array.Empty<string>();
+        var effectiveExpected = expectedScopes ?? Array.Empty<string>();
+
+        // Check required scopes
+        var missingRequired = effectiveRequired
+            .Where(scope => !grantedSet.Contains(scope))
+            .ToList();
+
+        if (missingRequired.Count > 0)
+        {
+            // Caller in AuthenticationRegistration logs a terminal LogError;
+            // this LogWarning provides structured detail for diagnostics.
+            _logger.LogWarning(
+                "OAuth scope validation failed: required scopes missing. Required: [{RequiredScopes}], Granted: [{GrantedScopes}], Missing: [{MissingScopes}]",
+                string.Join(", ", effectiveRequired),
+                string.Join(", ", grantedScopes),
+                string.Join(", ", missingRequired));
+
+            return OAuthScopeValidationResult.Failed(missingRequired, grantedScopes);
+        }
+
+        // Check expected (non-required) scopes
+        var missingExpected = effectiveExpected
+            .Where(scope => !grantedSet.Contains(scope))
+            .ToList();
+
+        if (missingExpected.Count > 0)
+        {
+            _logger.LogWarning(
+                "OAuth scope validation warning: expected scopes missing. Expected: [{ExpectedScopes}], Granted: [{GrantedScopes}], Missing: [{MissingScopes}]. Authentication will proceed.",
+                string.Join(", ", effectiveExpected),
+                string.Join(", ", grantedScopes),
+                string.Join(", ", missingExpected));
+        }
+
+        return OAuthScopeValidationResult.Succeeded(grantedScopes, missingExpected);
+    }
+
+    /// <summary>
+    /// Parses a scope string into a list of individual scopes.
+    /// Handles GitHub's comma-separated format and standard space-separated format.
+    /// Strips tabs and other whitespace to guard against malformed responses.
+    /// </summary>
+    internal static List<string> ParseScopes(string? scopeHeader)
+    {
+        if (string.IsNullOrWhiteSpace(scopeHeader))
+            return new List<string>();
+
+        // GitHub uses comma-separated scopes (e.g. "read:user, user:email")
+        // OAuth2 standard uses space-separated scopes
+        // Handle both by splitting on commas, spaces, and tabs, then trimming
+        var scopes = scopeHeader
+            .Split(new[] { ',', ' ', '\t', '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
+            .Select(s => s.Trim())
+            .Where(s => !string.IsNullOrWhiteSpace(s))
+            .ToList();
+
+        return scopes;
+    }
+}
+
+/// <summary>
+/// Result of OAuth scope validation.
+/// </summary>
+public class OAuthScopeValidationResult
+{
+    public bool IsValid { get; private init; }
+    public IReadOnlyList<string> GrantedScopes { get; private init; } = Array.Empty<string>();
+    public IReadOnlyList<string> MissingRequiredScopes { get; private init; } = Array.Empty<string>();
+    public IReadOnlyList<string> MissingExpectedScopes { get; private init; } = Array.Empty<string>();
+
+    /// <summary>
+    /// A user-facing error message when validation fails. Null when valid.
+    /// </summary>
+    public string? ErrorMessage { get; private init; }
+
+    public static OAuthScopeValidationResult Failed(
+        IReadOnlyList<string> missingRequired,
+        IReadOnlyList<string> grantedScopes)
+    {
+        var scopeList = string.Join(", ", missingRequired);
+        return new OAuthScopeValidationResult
+        {
+            IsValid = false,
+            GrantedScopes = grantedScopes,
+            MissingRequiredScopes = missingRequired,
+            MissingExpectedScopes = Array.Empty<string>(),
+            ErrorMessage = $"GitHub did not grant the required OAuth scopes: {scopeList}. " +
+                           "Please re-authorize the application with the required permissions."
+        };
+    }
+
+    public static OAuthScopeValidationResult Succeeded(
+        IReadOnlyList<string> grantedScopes,
+        IReadOnlyList<string> missingExpected)
+    {
+        return new OAuthScopeValidationResult
+        {
+            IsValid = true,
+            GrantedScopes = grantedScopes,
+            MissingRequiredScopes = Array.Empty<string>(),
+            MissingExpectedScopes = missingExpected,
+            ErrorMessage = null
+        };
+    }
+}