Skip to content

Commit

Permalink
Fix ASP.NET OAuth implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
daniel-lerch committed Feb 5, 2025
1 parent 2677bed commit 3f8e841
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 19 deletions.
22 changes: 9 additions & 13 deletions server/Korga/Controllers/ProfileController.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);

Expand All @@ -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);
}
}
29 changes: 25 additions & 4 deletions server/Korga/Extensions/IServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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 => { });

Expand All @@ -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;

Check warning on line 133 in server/Korga/Extensions/IServiceCollectionExtensions.cs

View workflow job for this annotation

GitHub Actions / Backend build and integration tests

Possible null reference assignment.

Check warning on line 133 in server/Korga/Extensions/IServiceCollectionExtensions.cs

View workflow job for this annotation

GitHub Actions / Backend build and integration tests

Possible null reference assignment.
options.TokenEndpoint = oauthOptions.Value.TokenEndpoint;

Check warning on line 134 in server/Korga/Extensions/IServiceCollectionExtensions.cs

View workflow job for this annotation

GitHub Actions / Backend build and integration tests

Possible null reference assignment.

Check warning on line 134 in server/Korga/Extensions/IServiceCollectionExtensions.cs

View workflow job for this annotation

GitHub Actions / Backend build and integration tests

Possible null reference assignment.
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion webapp/src/services/client.ts
Original file line number Diff line number Diff line change
@@ -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)

Expand All @@ -18,6 +18,7 @@ const deleteInfo: RequestInit = {
}

export default {
baseUrl,
async get<T>(path: string) {
const response = await fetch(baseUrl + path, getInfo)
if (response.ok === false) {
Expand Down
3 changes: 2 additions & 1 deletion webapp/src/services/profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 3f8e841

Please sign in to comment.