From 3f8e841a1f5265362dbc55c9dba49cc7e4908377 Mon Sep 17 00:00:00 2001 From: Daniel Lerch <36048059+daniel-lerch@users.noreply.github.com> Date: Wed, 5 Feb 2025 01:02:33 +0100 Subject: [PATCH] Fix ASP.NET OAuth implementation --- server/Korga/Controllers/ProfileController.cs | 22 ++++++-------- .../IServiceCollectionExtensions.cs | 29 ++++++++++++++++--- webapp/src/services/client.ts | 3 +- webapp/src/services/profile.ts | 3 +- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/server/Korga/Controllers/ProfileController.cs b/server/Korga/Controllers/ProfileController.cs index 5cb3d9f..5f62435 100644 --- a/server/Korga/Controllers/ProfileController.cs +++ b/server/Korga/Controllers/ProfileController.cs @@ -1,6 +1,6 @@ using Korga.Models.Json; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; -using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using System.Security.Claims; @@ -14,10 +14,10 @@ public class ProfileController : ControllerBase [ProducesResponseType(typeof(ProfileResponse), StatusCodes.Status200OK)] public IActionResult Profile() { - string? id = User.FindFirstValue("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier"); - string? givenName = User.FindFirstValue("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname"); - string? familyName = User.FindFirstValue("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname"); - string? emailAddress = User.FindFirstValue("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress"); + string? id = User.FindFirstValue(ClaimTypes.NameIdentifier); + string? givenName = User.FindFirstValue(ClaimTypes.GivenName); + string? familyName = User.FindFirstValue(ClaimTypes.Surname); + string? emailAddress = User.FindFirstValue(ClaimTypes.Email); if (id == null || givenName == null || familyName == null || emailAddress == null) return new JsonResult(null); @@ -30,20 +30,16 @@ public IActionResult Profile() }); } - [Authorize] [HttpGet("~/api/challenge")] - public NoContentResult ChallengeLogin() + public IActionResult ChallengeLogin() { - return NoContent(); + AuthenticationProperties properties = new() { RedirectUri = "/" }; + return Challenge(properties, "OAuth"); } [HttpGet("~/api/logout")] public IActionResult Logout() { - return new SignOutResult(new[] - { - CookieAuthenticationDefaults.AuthenticationScheme, - "OAuth" - }); + return SignOut(CookieAuthenticationDefaults.AuthenticationScheme); } } diff --git a/server/Korga/Extensions/IServiceCollectionExtensions.cs b/server/Korga/Extensions/IServiceCollectionExtensions.cs index 8b19805..6791d38 100644 --- a/server/Korga/Extensions/IServiceCollectionExtensions.cs +++ b/server/Korga/Extensions/IServiceCollectionExtensions.cs @@ -2,6 +2,7 @@ using Korga.Configuration; using Korga.EmailDelivery; using Korga.EmailRelay; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -13,11 +14,14 @@ using System; using System.ComponentModel.DataAnnotations; using System.Net.Http; +using System.Security.Claims; using System.Text.Json; +using System.Threading.Tasks; using OAuthOptions = Microsoft.AspNetCore.Authentication.OAuth.OAuthOptions; using KorgaOAuthOptions = Korga.Configuration.OAuthOptions; using KorgaOpenIdConnectOptions = Korga.Configuration.OpenIdConnectOptions; +using Microsoft.Extensions.Hosting; namespace Korga.Extensions; @@ -87,15 +91,27 @@ public static IServiceCollection AddOpenIdConnectAuthentication(this IServiceCol .AddAuthentication(options => { options.DefaultScheme = CookieAuthenticationDefaults.AuthenticationScheme; - options.DefaultChallengeScheme = "OAuth"; + options.DefaultChallengeScheme = null; // Disable automatic challenge of OAuth }) .AddCookie(CookieAuthenticationDefaults.AuthenticationScheme, options => { options.ExpireTimeSpan = TimeSpan.FromDays(1); options.Cookie.SameSite = SameSiteMode.Strict; - options.Cookie.SecurePolicy = CookieSecurePolicy.Always; + options.Cookie.SecurePolicy = environment.IsDevelopment() ? CookieSecurePolicy.SameAsRequest : CookieSecurePolicy.Always; options.Cookie.HttpOnly = true; - options.LoginPath = PathString.Empty; + + // Disable automatic challenge of Cookie Authentication + // Setting LoginPath and AccessDeniedPath to null is not sufficient + options.Events.OnRedirectToLogin = context => + { + context.Response.StatusCode = 401; + return Task.CompletedTask; + }; + options.Events.OnRedirectToAccessDenied = context => + { + context.Response.StatusCode = 403; + return Task.CompletedTask; + }; }) .AddOAuth("OAuth", options => { }); @@ -112,7 +128,7 @@ public static IServiceCollection AddOpenIdConnectAuthentication(this IServiceCol } options.CorrelationCookie.SameSite = SameSiteMode.Lax; - options.CorrelationCookie.SecurePolicy = CookieSecurePolicy.Always; + options.CorrelationCookie.SecurePolicy = environment.IsDevelopment() ? CookieSecurePolicy.SameAsRequest : CookieSecurePolicy.Always; options.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; options.AuthorizationEndpoint = oauthOptions.Value.AuthorizationEndpoint; options.TokenEndpoint = oauthOptions.Value.TokenEndpoint; @@ -126,6 +142,11 @@ public static IServiceCollection AddOpenIdConnectAuthentication(this IServiceCol options.Scope.Add("openid"); options.Scope.Add("profile"); + options.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "sub"); + options.ClaimActions.MapJsonKey(ClaimTypes.GivenName, "firstName"); + options.ClaimActions.MapJsonKey(ClaimTypes.Surname, "lastName"); + options.ClaimActions.MapJsonKey(ClaimTypes.Email, "email"); + options.Events.OnCreatingTicket = async context => { var request = new HttpRequestMessage(HttpMethod.Get, context.Options.UserInformationEndpoint); diff --git a/webapp/src/services/client.ts b/webapp/src/services/client.ts index 4fba388..cad7c8d 100644 --- a/webapp/src/services/client.ts +++ b/webapp/src/services/client.ts @@ -1,4 +1,4 @@ -const baseUrl = import.meta.env.PROD +const baseUrl: string = import.meta.env.PROD ? window.basePath.slice(0, -1) : import.meta.env.VITE_API_BASE_PATH.slice(0, -1) @@ -18,6 +18,7 @@ const deleteInfo: RequestInit = { } export default { + baseUrl, async get(path: string) { const response = await fetch(baseUrl + path, getInfo) if (response.ok === false) { diff --git a/webapp/src/services/profile.ts b/webapp/src/services/profile.ts index 0f9d71b..8a1b467 100644 --- a/webapp/src/services/profile.ts +++ b/webapp/src/services/profile.ts @@ -18,7 +18,8 @@ export default { }, async challengeLogin() { - await client.getResponse("/api/challenge") + window.location.href = client.baseUrl + "/api/challenge" + //await client.getResponse("/api/challenge") }, async logout() {