From bd1bbd6d37c4ded9cbfa39a0986781683e591392 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Tue, 10 Mar 2026 17:28:36 -0400 Subject: [PATCH 01/17] Implement OIDC --- .../LdapAuthenticationServiceTests.cs | 34 +-- .../OidcAuthenticationServiceTests.cs | 224 ++++++++++++++++++ .../Web/Pages/Account/LoginModelTests.cs | 8 +- .../Pages/Admin/Settings/LdapModelTests.cs | 2 +- .../Services/GroupMappingSyncServiceTests.cs | 46 ++++ .../OidcAuthenticationServiceTests.cs | 60 +++++ .../Services/OidcPostConfigureOptionsTests.cs | 106 +++++++++ SAMA.Web/Pages/Account/Login.cshtml | 11 + SAMA.Web/Pages/Account/Login.cshtml.cs | 52 +++- SAMA.Web/Pages/Account/Logout.cshtml.cs | 10 + .../Pages/Admin/Settings/GroupMappings.cshtml | 3 +- SAMA.Web/Pages/Admin/Settings/Oidc.cshtml | 161 +++++++++++++ SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs | 122 ++++++++++ .../Admin/Settings/_SettingsLayout.cshtml | 7 + SAMA.Web/Program.cs | 9 + SAMA.Web/SAMA.Web.csproj | 1 + SAMA.Web/Services/GlobalSettingsService.cs | 55 +++++ SAMA.Web/Services/GroupMappingSyncService.cs | 133 +++++++++++ .../Services/LdapAuthenticationService.cs | 104 +------- .../Services/OidcAuthenticationService.cs | 88 +++++++ SAMA.Web/Services/OidcPostConfigureOptions.cs | 45 ++++ 21 files changed, 1141 insertions(+), 140 deletions(-) create mode 100644 SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs create mode 100644 SAMA.Tests.Unit/Web/Services/GroupMappingSyncServiceTests.cs create mode 100644 SAMA.Tests.Unit/Web/Services/OidcAuthenticationServiceTests.cs create mode 100644 SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs create mode 100644 SAMA.Web/Pages/Admin/Settings/Oidc.cshtml create mode 100644 SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs create mode 100644 SAMA.Web/Services/GroupMappingSyncService.cs create mode 100644 SAMA.Web/Services/OidcAuthenticationService.cs create mode 100644 SAMA.Web/Services/OidcPostConfigureOptions.cs diff --git a/SAMA.Tests.Integration/Web/Services/LdapAuthenticationServiceTests.cs b/SAMA.Tests.Integration/Web/Services/LdapAuthenticationServiceTests.cs index b2cddb5..a5d01b0 100644 --- a/SAMA.Tests.Integration/Web/Services/LdapAuthenticationServiceTests.cs +++ b/SAMA.Tests.Integration/Web/Services/LdapAuthenticationServiceTests.cs @@ -29,7 +29,8 @@ public override async Task InitializeTestAsync() await EnsureAdminRoleExistsAsync(); var globalSettings = Substitute.For(null!, null!, null!, null!); - _service = new LdapAuthenticationService(globalSettings, ServiceProvider, Substitute.For>()); + var groupMappingSync = new GroupMappingSyncService(Substitute.For>()); + _service = new LdapAuthenticationService(globalSettings, groupMappingSync, ServiceProvider, Substitute.For>()); } [TestMethod] @@ -522,37 +523,6 @@ public void ValidateWithCustomCaShouldThrowWhenCertIsNull() StringAssert.Contains(ex.Message, "certificate or chain is null"); } - [TestMethod] - public void ExtractCnFromDnShouldParseCnFromFullDn() - { - var result = LdapAuthenticationService.ExtractCnFromDn("CN=Developers,OU=Groups,DC=example,DC=com"); - - Assert.AreEqual("Developers", result); - } - - [TestMethod] - public void ExtractCnFromDnShouldReturnNullForNonCnDn() - { - var result = LdapAuthenticationService.ExtractCnFromDn("OU=Groups,DC=example,DC=com"); - - Assert.IsNull(result); - } - - [TestMethod] - public void ExtractCnFromDnShouldReturnNullForNullOrWhitespace() - { - Assert.IsNull(LdapAuthenticationService.ExtractCnFromDn("")); - Assert.IsNull(LdapAuthenticationService.ExtractCnFromDn(" ")); - } - - [TestMethod] - public void ExtractCnFromDnShouldHandleCnOnly() - { - var result = LdapAuthenticationService.ExtractCnFromDn("CN=Admins"); - - Assert.AreEqual("Admins", result); - } - [TestMethod] public void ResolveBindDnShouldApplyTemplateForPlainUsername() { diff --git a/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs b/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs new file mode 100644 index 0000000..d4a7e34 --- /dev/null +++ b/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs @@ -0,0 +1,224 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Identity; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NSubstitute; +using SAMA.Data.Entities; +using SAMA.Web.Constants; +using SAMA.Web.Services; + +namespace SAMA.Tests.Integration.Web.Services; + +[TestClass] +public class OidcAuthenticationServiceTests : IntegrationTestBase +{ + private UserManager _userManager = null!; + private RoleManager> _roleManager = null!; + private OidcAuthenticationService _service = null!; + private GlobalSettingsService _globalSettings = null!; + + [TestInitialize] + public override async Task InitializeTestAsync() + { + await base.InitializeTestAsync(); + + _userManager = ServiceProvider.GetRequiredService>(); + _roleManager = ServiceProvider.GetRequiredService>>(); + _globalSettings = ServiceProvider.GetRequiredService(); + + await EnsureAdminRoleExistsAsync(); + + var groupMappingSync = new GroupMappingSyncService(Substitute.For>()); + _service = new OidcAuthenticationService(_globalSettings, groupMappingSync, ServiceProvider, Substitute.For>()); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldCreateNewUser() + { + var principal = CreatePrincipal("newuser@example.com", "sub-123"); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + Assert.IsNotNull(user); + Assert.AreEqual("newuser@example.com", user.Email); + Assert.IsTrue(user.EmailConfirmed); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldAddOidcLoginToExistingUser() + { + var existingUser = await CreateUserAsync("existing@example.com"); + + var principal = CreatePrincipal("existing@example.com", "sub-456"); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + Assert.AreEqual(existingUser.Id, user.Id); + + var logins = await _userManager.GetLoginsAsync(user); + Assert.IsTrue(logins.Any(l => l.LoginProvider == AuthConstants.OidcSource)); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldNotDuplicateOidcLogin() + { + var principal = CreatePrincipal("repeat@example.com", "sub-789"); + + await _service.ProvisionOrUpdateUserAsync(principal); + await _service.ProvisionOrUpdateUserAsync(principal); + + var user = await _userManager.FindByEmailAsync("repeat@example.com"); + var logins = await _userManager.GetLoginsAsync(user!); + Assert.AreEqual(1, logins.Count(l => l.LoginProvider == AuthConstants.OidcSource)); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldThrowWhenNoEmailClaim() + { + var claims = new List { new("sub", "sub-no-email") }; + var identity = new ClaimsIdentity(claims, AuthConstants.OidcSource); + var principal = new ClaimsPrincipal(identity); + + await Assert.ThrowsAsync(async () => + await _service.ProvisionOrUpdateUserAsync(principal)); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldNotAffectLdapMappings() + { + _globalSettings.OidcGroupClaimType = "groups"; + var workspace = await CreateWorkspaceAsync("Mixed Workspace"); + await CreateGroupMappingAsync(workspace.Id, AuthConstants.LdapSource, "ldap-team", AuthConstants.EditorRole); + + var principal = CreatePrincipal("mixed@example.com", "sub-mixed", ["ldap-team"]); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + var assignments = await DbContext.UserWorkspaces + .Where(uw => uw.UserId == user.Id) + .ToListAsync(); + + Assert.AreEqual(0, assignments.Count); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldUseConfiguredGroupClaimType() + { + _globalSettings.OidcGroupClaimType = "roles"; + await CreateGroupMappingAsync(null, AuthConstants.OidcSource, "admin-role", AuthConstants.AdminRole); + + var claims = new List + { + new("sub", "sub-custom-claim"), + new("email", "customclaim@example.com"), + new("roles", "admin-role"), + }; + var identity = new ClaimsIdentity(claims, AuthConstants.OidcSource); + var principal = new ClaimsPrincipal(identity); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + DbContext.ChangeTracker.Clear(); + var refreshedUser = await _userManager.FindByEmailAsync("customclaim@example.com"); + Assert.IsTrue(await _userManager.IsInRoleAsync(refreshedUser!, AuthConstants.AdminRole)); + } + + [TestMethod] + public async Task ProvisionOrUpdateUserShouldSkipGroupsWhenClaimTypeEmpty() + { + _globalSettings.OidcGroupClaimType = ""; + await CreateGroupMappingAsync(null, AuthConstants.OidcSource, "admins-group", AuthConstants.AdminRole); + + var principal = CreatePrincipal("noclaimtype@example.com", "sub-no-ct", ["admins-group"]); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + DbContext.ChangeTracker.Clear(); + var refreshedUser = await _userManager.FindByEmailAsync("noclaimtype@example.com"); + Assert.IsFalse(await _userManager.IsInRoleAsync(refreshedUser!, AuthConstants.AdminRole)); + } + + private static ClaimsPrincipal CreatePrincipal(string email, string subject, List? groups = null) + { + var claims = new List + { + new("sub", subject), + new("email", email), + new("name", email), + }; + + if (groups != null) + { + claims.AddRange(groups.Select(g => new Claim("groups", g))); + } + + var identity = new ClaimsIdentity(claims, AuthConstants.OidcSource); + return new ClaimsPrincipal(identity); + } + + private async Task EnsureAdminRoleExistsAsync() + { + if (!await _roleManager.RoleExistsAsync(AuthConstants.AdminRole)) + { + await _roleManager.CreateAsync(new IdentityRole(AuthConstants.AdminRole)); + } + } + + private async Task CreateUserAsync(string email) + { + var user = new ApplicationUser + { + UserName = email, + Email = email, + EmailConfirmed = true, + CreatedAt = DateTimeOffset.UtcNow, + }; + + var result = await _userManager.CreateAsync(user, "Test-Password-123456789!"); + Assert.IsTrue(result.Succeeded, $"Failed to create user: {string.Join(", ", result.Errors.Select(e => e.Description))}"); + + DbContext.ChangeTracker.Clear(); + return user; + } + + private async Task CreateWorkspaceAsync(string name) + { + var workspace = new Workspace + { + Name = name, + IsPublic = false, + CreatedAt = DateTimeOffset.UtcNow, + UpdatedAt = DateTimeOffset.UtcNow, + }; + + DbContext.Workspaces.Add(workspace); + await DbContext.SaveChangesAsync(); + DbContext.ChangeTracker.Clear(); + + return workspace; + } + + private async Task CreateGroupMappingAsync(Guid? workspaceId, string identityProvider, string externalGroupId, string role) + { + DbContext.WorkspaceGroupMappings.Add(new WorkspaceGroupMapping + { + WorkspaceId = workspaceId, + IdentityProvider = identityProvider, + ExternalGroupId = externalGroupId, + Role = role, + CreatedAt = DateTimeOffset.UtcNow, + UpdatedAt = DateTimeOffset.UtcNow, + }); + await DbContext.SaveChangesAsync(); + DbContext.ChangeTracker.Clear(); + } + + private async Task EnsureRoleExistsAsync(string roleName) + { + if (!await _roleManager.RoleExistsAsync(roleName)) + { + await _roleManager.CreateAsync(new IdentityRole(roleName)); + } + } +} diff --git a/SAMA.Tests.Unit/Web/Pages/Account/LoginModelTests.cs b/SAMA.Tests.Unit/Web/Pages/Account/LoginModelTests.cs index ffd2828..0f54905 100644 --- a/SAMA.Tests.Unit/Web/Pages/Account/LoginModelTests.cs +++ b/SAMA.Tests.Unit/Web/Pages/Account/LoginModelTests.cs @@ -15,6 +15,8 @@ public class LoginModelTests { private SignInManager _mockSignInManager = null!; private LdapAuthenticationService _mockLdapService = null!; + private OidcAuthenticationService _mockOidcService = null!; + private GlobalSettingsService _mockGlobalSettings = null!; private ILogger _mockLogger = null!; private LoginModel _pageModel = null!; @@ -33,10 +35,12 @@ public void Setup() null, null); - _mockLdapService = Substitute.For(null!, null!, null!); + _mockLdapService = Substitute.For(null!, null!, null!, null!); + _mockOidcService = Substitute.For(null!, null!, null!, null!); + _mockGlobalSettings = Substitute.For(null!, null!, null!, null!); _mockLogger = Substitute.For>(); - _pageModel = new LoginModel(_mockSignInManager, _mockLdapService, _mockLogger); + _pageModel = new LoginModel(_mockSignInManager, _mockLdapService, _mockOidcService, _mockGlobalSettings, _mockLogger); PageModelTestHelpers.ConfigurePageModel(_pageModel); } diff --git a/SAMA.Tests.Unit/Web/Pages/Admin/Settings/LdapModelTests.cs b/SAMA.Tests.Unit/Web/Pages/Admin/Settings/LdapModelTests.cs index 95fe962..e934b7a 100644 --- a/SAMA.Tests.Unit/Web/Pages/Admin/Settings/LdapModelTests.cs +++ b/SAMA.Tests.Unit/Web/Pages/Admin/Settings/LdapModelTests.cs @@ -21,7 +21,7 @@ public class LdapModelTests public void Setup() { _mockGlobalSettings = Substitute.For(null!, null!, null!, null!); - _mockLdapService = Substitute.For(null!, null!, null!); + _mockLdapService = Substitute.For(null!, null!, null!, null!); _mockLogger = Substitute.For>(); _pageModel = new LdapModel(_mockGlobalSettings, _mockLdapService, _mockLogger); diff --git a/SAMA.Tests.Unit/Web/Services/GroupMappingSyncServiceTests.cs b/SAMA.Tests.Unit/Web/Services/GroupMappingSyncServiceTests.cs new file mode 100644 index 0000000..2a72a82 --- /dev/null +++ b/SAMA.Tests.Unit/Web/Services/GroupMappingSyncServiceTests.cs @@ -0,0 +1,46 @@ +using SAMA.Web.Services; + +namespace SAMA.Tests.Unit.Web.Services; + +[TestClass] +public class GroupMappingSyncServiceTests +{ + [TestMethod] + public void ExtractCnFromDnShouldParseCnFromFullDn() + { + var result = GroupMappingSyncService.ExtractCnFromDn("CN=Developers,OU=Groups,DC=example,DC=com"); + + Assert.AreEqual("Developers", result); + } + + [TestMethod] + public void ExtractCnFromDnShouldReturnNullForNonCnDn() + { + var result = GroupMappingSyncService.ExtractCnFromDn("OU=Groups,DC=example,DC=com"); + + Assert.IsNull(result); + } + + [TestMethod] + public void ExtractCnFromDnShouldReturnNullForEmptyOrWhitespace() + { + Assert.IsNull(GroupMappingSyncService.ExtractCnFromDn("")); + Assert.IsNull(GroupMappingSyncService.ExtractCnFromDn(" ")); + } + + [TestMethod] + public void ExtractCnFromDnShouldHandleCnOnly() + { + var result = GroupMappingSyncService.ExtractCnFromDn("CN=Admins"); + + Assert.AreEqual("Admins", result); + } + + [TestMethod] + public void ExtractCnFromDnShouldBeCaseInsensitive() + { + var result = GroupMappingSyncService.ExtractCnFromDn("cn=DevOps,OU=Groups,DC=example,DC=com"); + + Assert.AreEqual("DevOps", result); + } +} diff --git a/SAMA.Tests.Unit/Web/Services/OidcAuthenticationServiceTests.cs b/SAMA.Tests.Unit/Web/Services/OidcAuthenticationServiceTests.cs new file mode 100644 index 0000000..231805a --- /dev/null +++ b/SAMA.Tests.Unit/Web/Services/OidcAuthenticationServiceTests.cs @@ -0,0 +1,60 @@ +using NSubstitute; +using SAMA.Web.Services; + +namespace SAMA.Tests.Unit.Web.Services; + +[TestClass] +public class OidcAuthenticationServiceTests +{ + private GlobalSettingsService _mockGlobalSettings = null!; + + [TestInitialize] + public void Setup() + { + _mockGlobalSettings = Substitute.For(null!, null!, null!, null!); + } + + [TestMethod] + public void IsOidcEnabledShouldReturnTrueWhenAllConditionsMet() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com/tenant"); + _mockGlobalSettings.OidcClientId.Returns("my-client-id"); + var service = new OidcAuthenticationService(_mockGlobalSettings, null!, null!, null!); + + Assert.IsTrue(service.IsOidcEnabled); + } + + [TestMethod] + public void IsOidcEnabledShouldReturnFalseWhenDisabled() + { + _mockGlobalSettings.OidcEnabled.Returns(false); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com/tenant"); + _mockGlobalSettings.OidcClientId.Returns("my-client-id"); + var service = new OidcAuthenticationService(_mockGlobalSettings, null!, null!, null!); + + Assert.IsFalse(service.IsOidcEnabled); + } + + [TestMethod] + public void IsOidcEnabledShouldReturnFalseWhenAuthorityEmpty() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns(""); + _mockGlobalSettings.OidcClientId.Returns("my-client-id"); + var service = new OidcAuthenticationService(_mockGlobalSettings, null!, null!, null!); + + Assert.IsFalse(service.IsOidcEnabled); + } + + [TestMethod] + public void IsOidcEnabledShouldReturnFalseWhenClientIdEmpty() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com/tenant"); + _mockGlobalSettings.OidcClientId.Returns(" "); + var service = new OidcAuthenticationService(_mockGlobalSettings, null!, null!, null!); + + Assert.IsFalse(service.IsOidcEnabled); + } +} diff --git a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs new file mode 100644 index 0000000..369a656 --- /dev/null +++ b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs @@ -0,0 +1,106 @@ +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using NSubstitute; +using SAMA.Web.Constants; +using SAMA.Web.Services; + +namespace SAMA.Tests.Unit.Web.Services; + +[TestClass] +public class OidcPostConfigureOptionsTests +{ + private GlobalSettingsService _mockGlobalSettings = null!; + private OidcPostConfigureOptions _postConfigure = null!; + + [TestInitialize] + public void Setup() + { + _mockGlobalSettings = Substitute.For(null!, null!, null!, null!); + _postConfigure = new OidcPostConfigureOptions(_mockGlobalSettings); + } + + [TestMethod] + public void PostConfigureShouldIgnoreNonOidcSchemes() + { + var options = new OpenIdConnectOptions(); + var originalAuthority = options.Authority; + + _postConfigure.PostConfigure("Cookies", options); + + Assert.AreEqual(originalAuthority, options.Authority); + } + + [TestMethod] + public void PostConfigureShouldSetDummyConfigWhenDisabled() + { + _mockGlobalSettings.OidcEnabled.Returns(false); + var options = new OpenIdConnectOptions(); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + + Assert.AreEqual("https://localhost", options.Authority); + Assert.AreEqual("disabled", options.ClientId); + Assert.AreEqual(string.Empty, options.MetadataAddress); + Assert.IsNotNull(options.Configuration); + } + + [TestMethod] + public void PostConfigureShouldMapSettingsWhenEnabled() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com/tenant"); + _mockGlobalSettings.OidcClientId.Returns("my-client-id"); + _mockGlobalSettings.OidcClientSecret.Returns("my-secret"); + _mockGlobalSettings.OidcScopes.Returns("openid profile email"); + var options = new OpenIdConnectOptions(); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + + Assert.AreEqual("https://login.example.com/tenant", options.Authority); + Assert.AreEqual("my-client-id", options.ClientId); + Assert.AreEqual("my-secret", options.ClientSecret); + Assert.AreEqual(OpenIdConnectResponseType.Code, options.ResponseType); + Assert.AreEqual("/signin-oidc", options.CallbackPath.Value); + Assert.AreEqual("/signout-callback-oidc", options.SignedOutCallbackPath.Value); + Assert.IsTrue(options.SaveTokens); + Assert.IsFalse(options.MapInboundClaims); + Assert.AreEqual("name", options.TokenValidationParameters.NameClaimType); + } + + [TestMethod] + public void PostConfigureShouldSplitAndSetScopes() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com"); + _mockGlobalSettings.OidcClientId.Returns("client"); + _mockGlobalSettings.OidcClientSecret.Returns(""); + _mockGlobalSettings.OidcScopes.Returns("openid profile email groups"); + var options = new OpenIdConnectOptions(); + options.Scope.Add("pre-existing"); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + + Assert.AreEqual(4, options.Scope.Count); + CollectionAssert.AreEquivalent( + new[] { "openid", "profile", "email", "groups" }, + options.Scope.ToArray()); + } + + [TestMethod] + public void PostConfigureShouldHandleExtraWhitespaceInScopes() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com"); + _mockGlobalSettings.OidcClientId.Returns("client"); + _mockGlobalSettings.OidcClientSecret.Returns(""); + _mockGlobalSettings.OidcScopes.Returns("openid profile email"); + var options = new OpenIdConnectOptions(); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + + Assert.AreEqual(3, options.Scope.Count); + CollectionAssert.AreEquivalent( + new[] { "openid", "profile", "email" }, + options.Scope.ToArray()); + } +} diff --git a/SAMA.Web/Pages/Account/Login.cshtml b/SAMA.Web/Pages/Account/Login.cshtml index fc6c4c3..9fbee90 100644 --- a/SAMA.Web/Pages/Account/Login.cshtml +++ b/SAMA.Web/Pages/Account/Login.cshtml @@ -48,6 +48,17 @@ + + @if (Model.OidcEnabled) + { +
+
+ + + Sign in with @Model.OidcProviderName + +
+ } diff --git a/SAMA.Web/Pages/Account/Login.cshtml.cs b/SAMA.Web/Pages/Account/Login.cshtml.cs index a293f13..e241253 100644 --- a/SAMA.Web/Pages/Account/Login.cshtml.cs +++ b/SAMA.Web/Pages/Account/Login.cshtml.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; @@ -14,6 +15,8 @@ namespace SAMA.Web.Pages.Account; public class LoginModel( SignInManager signInManager, LdapAuthenticationService ldapService, + OidcAuthenticationService oidcService, + GlobalSettingsService globalSettings, ILogger logger) : PageModel { [BindProperty] @@ -23,6 +26,10 @@ public class LoginModel( public bool LdapEnabled => ldapService.IsLdapEnabled; + public bool OidcEnabled => oidcService.IsOidcEnabled; + + public string OidcProviderName => globalSettings.OidcProviderName; + public class InputModel { [Required(ErrorMessage = "Email or username is required")] @@ -81,12 +88,12 @@ public async Task OnPostAsync(string? returnUrl = null) } // Fall back to local password authentication - // Block local login for LDAP-sourced users + // Block local login for LDAP or OIDC-sourced users var localUser = await signInManager.UserManager.FindByEmailAsync(identifier); if (localUser != null) { var logins = await signInManager.UserManager.GetLoginsAsync(localUser); - if (logins.Any(l => l.LoginProvider == AuthConstants.LdapSource)) + if (logins.Any(l => l.LoginProvider == AuthConstants.LdapSource || l.LoginProvider == AuthConstants.OidcSource)) { ModelState.AddModelError(string.Empty, "Invalid login attempt."); return Page(); @@ -109,4 +116,45 @@ public async Task OnPostAsync(string? returnUrl = null) ModelState.AddModelError(string.Empty, "Invalid login attempt."); return Page(); } + + public IActionResult OnGetOidcLogin(string? returnUrl = null) + { + returnUrl ??= Url.Content("~/"); + + var properties = new AuthenticationProperties + { + RedirectUri = Url.Page("/Account/Login", "OidcCallback", new { returnUrl }), + }; + + return Challenge(properties, AuthConstants.OidcSource); + } + + public async Task OnGetOidcCallbackAsync(string? returnUrl = null) + { + returnUrl ??= Url.Content("~/"); + + var authResult = await HttpContext.AuthenticateAsync(AuthConstants.OidcSource); + if (!authResult.Succeeded || authResult.Principal == null) + { + logger.LogWarning("OIDC authentication failed: {Failure}", authResult.Failure?.Message); + ModelState.AddModelError(string.Empty, "External login failed. Please try again."); + ReturnUrl = returnUrl; + return Page(); + } + + try + { + var user = await oidcService.ProvisionOrUpdateUserAsync(authResult.Principal); + await signInManager.SignInAsync(user, isPersistent: false); + logger.LogInformation("User {Email} logged in via OIDC", user.Email); + return LocalRedirect(returnUrl); + } + catch (Exception ex) + { + logger.LogError(ex, "Error during OIDC login"); + ModelState.AddModelError(string.Empty, "An error occurred during login. Please try again."); + ReturnUrl = returnUrl; + return Page(); + } + } } diff --git a/SAMA.Web/Pages/Account/Logout.cshtml.cs b/SAMA.Web/Pages/Account/Logout.cshtml.cs index 454a874..3421521 100644 --- a/SAMA.Web/Pages/Account/Logout.cshtml.cs +++ b/SAMA.Web/Pages/Account/Logout.cshtml.cs @@ -1,14 +1,18 @@ +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; using SAMA.Data.Entities; +using SAMA.Web.Constants; +using SAMA.Web.Services; namespace SAMA.Web.Pages.Account; [AllowAnonymous] public class LogoutModel( SignInManager signInManager, + OidcAuthenticationService oidcService, ILogger logger) : PageModel { @@ -17,6 +21,12 @@ public async Task OnPostAsync(string? returnUrl = null) await signInManager.SignOutAsync(); logger.LogInformation("User logged out"); + // If OIDC is enabled, also sign out from the OIDC provider + if (oidcService.IsOidcEnabled) + { + await HttpContext.SignOutAsync(AuthConstants.OidcSource); + } + if (returnUrl != null) { return LocalRedirect(returnUrl); diff --git a/SAMA.Web/Pages/Admin/Settings/GroupMappings.cshtml b/SAMA.Web/Pages/Admin/Settings/GroupMappings.cshtml index 8c2b044..83ddb47 100644 --- a/SAMA.Web/Pages/Admin/Settings/GroupMappings.cshtml +++ b/SAMA.Web/Pages/Admin/Settings/GroupMappings.cshtml @@ -132,7 +132,7 @@
- The LDAP group DN or CN. Must match what appears in the user's group memberships. Use the LDAP Test Login to discover group names. + The group name, DN, or ID from the identity provider. For LDAP, use the group DN or CN. For OIDC, use the exact value from the group claim (e.g. group name or Object ID).
@@ -157,6 +157,7 @@
The identity provider that supplies group membership.
diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml new file mode 100644 index 0000000..8120040 --- /dev/null +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml @@ -0,0 +1,161 @@ +@page +@model SAMA.Web.Pages.Admin.Settings.OidcModel +@{ + Layout = "_SettingsLayout"; + ViewData["Title"] = "System Options - OIDC"; + ViewData["ActiveSettingsTab"] = "Oidc"; +} + +
+
+
+
+
+ + OpenID Connect (OIDC) +
+

+ Configure single sign-on with an OpenID Connect provider such as Azure AD / Entra ID, Okta, Auth0, or Keycloak. + When enabled, a "Sign in with" button appears on the login page. +

+ + @if (TempData["OidcSuccess"] != null) + { + + } + + @if (TempData["OidcError"] != null) + { + + } + +
+ + How OIDC interacts with local accounts: When a user logs in via OIDC, their account is matched by email. + If a local user with the same email exists, it is linked to OIDC and local password login is disabled for that account. + New users are automatically provisioned on first OIDC login. + Use Group Mappings to automatically assign workspace access and admin roles based on OIDC group claims. +
+ +
+
+ + +
+ +
Provider
+
+
+ + +
+ The issuer URL of your OIDC provider. Must expose a /.well-known/openid-configuration endpoint. +
Azure AD: https://login.microsoftonline.com/{tenant-id}/v2.0 +
Okta: https://{domain}.okta.com/oauth2/default +
Keycloak: https://{host}/realms/{realm} +
+
+
+ + +
Display name shown on the login button (e.g. "Azure AD", "Okta", "Company SSO")
+
+
+ +
Client Credentials
+
+
+ + +
The application/client ID from your OIDC provider
+
+
+ + +
+ @if (Model.HasExistingClientSecret) + { + Client secret is set. Leave unmodified to keep existing secret, or enter a new one to change it. + } + else + { + The client secret from your OIDC provider (stored encrypted) + } +
+ @if (Model.HasExistingClientSecret) + { +
+ + +
+ } +
+
+ +
Claims & Scopes
+
+
+ + +
+ Space-separated OAuth scopes. Default: openid profile email. + Add additional scopes if your provider requires them for group claims. +
+
+
+ + +
+ The claim name in the ID token that contains group memberships. Common value: groups. + Most providers require additional configuration to include group claims in tokens. Check your provider's documentation for details. +
+
+
+ +
+ +
+
+
+
+
+ +
+
+
+
+ + Setup Guide +
+
    +
  1. Register an application in your OIDC provider.
    2. +
  2. + Set the Redirect URI to: + @($"{ViewContext.HttpContext.Request.Scheme}://{ViewContext.HttpContext.Request.Host}/signin-oidc") +
    3. +
  3. Copy the Client ID and Client Secret here.
    4. +
  4. Enter the Authority URL (issuer) for your provider.
    5. +
  5. Enable OIDC and save.
    6. +
  6. Optionally, configure Group Mappings to auto-assign workspace access.
    7. +
+
+
+
+
diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs new file mode 100644 index 0000000..81b1be6 --- /dev/null +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs @@ -0,0 +1,122 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using SAMA.Web.Constants; +using SAMA.Web.Services; + +namespace SAMA.Web.Pages.Admin.Settings; + +[Authorize(Roles = AuthConstants.AdminRole)] +public class OidcModel( + GlobalSettingsService _globalSettings, + ILogger _logger) : PageModel +{ + [BindProperty] + public OidcInputModel OidcInput { get; set; } = new(); + + public bool HasExistingClientSecret { get; set; } + + public class OidcInputModel + { + [Display(Name = "Enable OIDC")] + public bool Enabled { get; set; } + + [Display(Name = "Authority URL")] + public string Authority { get; set; } = string.Empty; + + [Display(Name = "Client ID")] + public string ClientId { get; set; } = string.Empty; + + [Display(Name = "Client Secret")] + [DataType(DataType.Password)] + public string ClientSecret { get; set; } = string.Empty; + + [Display(Name = "Scopes")] + public string Scopes { get; set; } = "openid profile email"; + + [Display(Name = "Group Claim Type")] + public string GroupClaimType { get; set; } = "groups"; + + [Display(Name = "Provider Name")] + public string ProviderName { get; set; } = "OIDC"; + } + + public void OnGet() + { + LoadCurrentSettings(); + } + + public IActionResult OnPost() + { + if (OidcInput.Enabled) + { + if (string.IsNullOrWhiteSpace(OidcInput.Authority)) + { + TempData["OidcError"] = "Authority URL is required when OIDC is enabled."; + return RedirectToPage(); + } + + if (string.IsNullOrWhiteSpace(OidcInput.ClientId)) + { + TempData["OidcError"] = "Client ID is required when OIDC is enabled."; + return RedirectToPage(); + } + + if (!Uri.TryCreate(OidcInput.Authority, UriKind.Absolute, out var authorityUri) + || (authorityUri.Scheme != "https" && authorityUri.Scheme != "http")) + { + TempData["OidcError"] = "Authority URL must be a valid HTTP or HTTPS URL."; + return RedirectToPage(); + } + } + + try + { + _globalSettings.OidcEnabled = OidcInput.Enabled; + _globalSettings.OidcAuthority = OidcInput.Authority.TrimEnd('/'); + _globalSettings.OidcClientId = OidcInput.ClientId; + _globalSettings.OidcScopes = string.IsNullOrWhiteSpace(OidcInput.Scopes) + ? "openid profile email" + : OidcInput.Scopes; + _globalSettings.OidcGroupClaimType = string.IsNullOrWhiteSpace(OidcInput.GroupClaimType) + ? "groups" + : OidcInput.GroupClaimType; + _globalSettings.OidcProviderName = string.IsNullOrWhiteSpace(OidcInput.ProviderName) + ? "OIDC" + : OidcInput.ProviderName; + + if (!string.IsNullOrEmpty(OidcInput.ClientSecret)) + { + _globalSettings.OidcClientSecret = OidcInput.ClientSecret; + } + else if (Request.Form["ClearClientSecret"].ToString() == "true") + { + _globalSettings.OidcClientSecret = string.Empty; + } + + _logger.LogInformation("OIDC settings updated by {User}", User.Identity?.Name ?? "Unknown"); + + _globalSettings.ClearCache(); + TempData["OidcSuccess"] = "OIDC settings saved successfully."; + } + catch (Exception ex) + { + _logger.LogError(ex, "Error updating OIDC settings"); + TempData["OidcError"] = "An error occurred while saving OIDC settings."; + } + + return RedirectToPage(); + } + + private void LoadCurrentSettings() + { + OidcInput.Enabled = _globalSettings.OidcEnabled; + OidcInput.Authority = _globalSettings.OidcAuthority; + OidcInput.ClientId = _globalSettings.OidcClientId; + OidcInput.Scopes = _globalSettings.OidcScopes; + OidcInput.GroupClaimType = _globalSettings.OidcGroupClaimType; + OidcInput.ProviderName = _globalSettings.OidcProviderName; + HasExistingClientSecret = !string.IsNullOrEmpty(_globalSettings.OidcClientSecret); + } +} diff --git a/SAMA.Web/Pages/Admin/Settings/_SettingsLayout.cshtml b/SAMA.Web/Pages/Admin/Settings/_SettingsLayout.cshtml index 4192de3..6673710 100644 --- a/SAMA.Web/Pages/Admin/Settings/_SettingsLayout.cshtml +++ b/SAMA.Web/Pages/Admin/Settings/_SettingsLayout.cshtml @@ -27,6 +27,13 @@ LDAP +
  • + + + OIDC + +
  • diff --git a/SAMA.Web/Program.cs b/SAMA.Web/Program.cs index 50b8c85..cf4ac50 100644 --- a/SAMA.Web/Program.cs +++ b/SAMA.Web/Program.cs @@ -1,11 +1,13 @@ using System.Reflection; using DnsClient; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Diagnostics.HealthChecks; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Authorization; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; using Quartz; using SAMA.Data; using SAMA.Data.Entities; @@ -91,6 +93,11 @@ options.AccessDeniedPath = "/Account/AccessDenied"; }); +// Configure OIDC authentication (dynamically configured via OidcPostConfigureOptions) +builder.Services.AddAuthentication() + .AddOpenIdConnect(AuthConstants.OidcSource, _ => { }); +builder.Services.AddSingleton, OidcPostConfigureOptions>(); + // Configure ASP.NET Data Protection // When running in Docker, use a volume-mounted directory for keys // Otherwise, use the default ASP.NET location @@ -218,6 +225,8 @@ builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddScoped(); +builder.Services.AddScoped(); // Register CQRS-lite query services builder.Services.AddScoped(); diff --git a/SAMA.Web/SAMA.Web.csproj b/SAMA.Web/SAMA.Web.csproj index 5d7fd29..79ac2ea 100644 --- a/SAMA.Web/SAMA.Web.csproj +++ b/SAMA.Web/SAMA.Web.csproj @@ -17,6 +17,7 @@ + diff --git a/SAMA.Web/Services/GlobalSettingsService.cs b/SAMA.Web/Services/GlobalSettingsService.cs index bdbcbae..eed2826 100644 --- a/SAMA.Web/Services/GlobalSettingsService.cs +++ b/SAMA.Web/Services/GlobalSettingsService.cs @@ -58,6 +58,18 @@ public class GlobalSettingsService(IServiceProvider _serviceProvider, ILogger _cache = new(); @@ -194,6 +206,49 @@ public virtual string LdapCustomRootCa set => SetStringAsync(KeyLdapCustomRootCa, value).GetAwaiter().GetResult(); } + // OIDC + public virtual bool OidcEnabled + { + get => GetBoolAsync(KeyOidcEnabled, false).GetAwaiter().GetResult(); + set => SetBoolAsync(KeyOidcEnabled, value).GetAwaiter().GetResult(); + } + + public virtual string OidcAuthority + { + get => GetStringAsync(KeyOidcAuthority, string.Empty).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcAuthority, value).GetAwaiter().GetResult(); + } + + public virtual string OidcClientId + { + get => GetStringAsync(KeyOidcClientId, string.Empty).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcClientId, value).GetAwaiter().GetResult(); + } + + public virtual string OidcClientSecret + { + get => GetEncryptedStringAsync(KeyOidcClientSecret).GetAwaiter().GetResult(); + set => SetEncryptedStringAsync(KeyOidcClientSecret, value).GetAwaiter().GetResult(); + } + + public virtual string OidcScopes + { + get => GetStringAsync(KeyOidcScopes, DefaultOidcScopes).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcScopes, value).GetAwaiter().GetResult(); + } + + public virtual string OidcGroupClaimType + { + get => GetStringAsync(KeyOidcGroupClaimType, DefaultOidcGroupClaimType).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcGroupClaimType, value).GetAwaiter().GetResult(); + } + + public virtual string OidcProviderName + { + get => GetStringAsync(KeyOidcProviderName, DefaultOidcProviderName).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcProviderName, value).GetAwaiter().GetResult(); + } + /* --- */ private async Task GetIntAsync(string key, int defaultValue) diff --git a/SAMA.Web/Services/GroupMappingSyncService.cs b/SAMA.Web/Services/GroupMappingSyncService.cs new file mode 100644 index 0000000..35a4825 --- /dev/null +++ b/SAMA.Web/Services/GroupMappingSyncService.cs @@ -0,0 +1,133 @@ +using Microsoft.AspNetCore.Identity; +using Microsoft.EntityFrameworkCore; +using SAMA.Data; +using SAMA.Data.Entities; +using SAMA.Web.Constants; + +namespace SAMA.Web.Services; + +public class GroupMappingSyncService(ILogger _logger) +{ + public async Task ApplyGroupMappingsAsync( + SamaDbContext dbContext, + UserManager userManager, + ApplicationUser user, + List groups, + string source) + { + var mappings = await dbContext.WorkspaceGroupMappings + .AsNoTracking() + .Where(m => m.IdentityProvider == source) + .ToListAsync(); + + var normalizedGroups = NormalizeGroups(groups, source); + + var matchedMappings = mappings + .Where(m => normalizedGroups.Contains(m.ExternalGroupId)) + .ToList(); + + await SyncAdminRoleAsync(userManager, user, matchedMappings); + await SyncWorkspaceAssignmentsAsync(dbContext, user, matchedMappings, source); + } + + private static HashSet NormalizeGroups(List groups, string source) + { + var normalized = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var group in groups) + { + normalized.Add(group); + + // For LDAP, also extract CN from full DN for flexible matching + if (source == AuthConstants.LdapSource) + { + var cn = ExtractCnFromDn(group); + if (cn != null) + { + normalized.Add(cn); + } + } + } + + return normalized; + } + + internal static string? ExtractCnFromDn(string dn) + { + if (string.IsNullOrWhiteSpace(dn)) + { + return null; + } + + if (!dn.StartsWith("CN=", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var commaIndex = dn.IndexOf(','); + return commaIndex > 3 ? dn[3..commaIndex] : dn[3..]; + } + + private async Task SyncAdminRoleAsync( + UserManager userManager, + ApplicationUser user, + List matchedMappings) + { + var shouldBeAdmin = matchedMappings.Any(m => m.WorkspaceId == null && m.Role == AuthConstants.AdminRole); + var isAdmin = await userManager.IsInRoleAsync(user, AuthConstants.AdminRole); + + if (shouldBeAdmin && !isAdmin) + { + await userManager.AddToRoleAsync(user, AuthConstants.AdminRole); + _logger.LogInformation("Granted Admin role to user {Email} via group mapping", user.Email); + } + else if (!shouldBeAdmin && isAdmin) + { + await userManager.RemoveFromRoleAsync(user, AuthConstants.AdminRole); + _logger.LogInformation("Revoked Admin role from user {Email} — no longer in an admin group", user.Email); + } + } + + private async Task SyncWorkspaceAssignmentsAsync( + SamaDbContext dbContext, + ApplicationUser user, + List matchedMappings, + string source) + { + var existingAssignments = await dbContext.UserWorkspaces + .Where(uw => uw.UserId == user.Id && uw.Source == source) + .ToListAsync(); + + var desiredAssignments = matchedMappings + .Where(m => m.WorkspaceId != null && m.Role != AuthConstants.AdminRole) + .Select(m => (WorkspaceId: m.WorkspaceId!.Value, m.Role)) + .ToHashSet(); + + var existingSet = existingAssignments + .Select(a => (a.WorkspaceId, a.Role)) + .ToHashSet(); + + var toRemove = existingAssignments + .Where(a => !desiredAssignments.Contains((a.WorkspaceId, a.Role))) + .ToList(); + dbContext.UserWorkspaces.RemoveRange(toRemove); + + var now = DateTimeOffset.UtcNow; + foreach (var (workspaceId, role) in desiredAssignments.Where(d => !existingSet.Contains(d))) + { + dbContext.UserWorkspaces.Add(new UserWorkspace + { + UserId = user.Id, + WorkspaceId = workspaceId, + Role = role, + Source = source, + CreatedAt = now, + UpdatedAt = now, + }); + } + + if (toRemove.Count > 0 || desiredAssignments.Except(existingSet).Any()) + { + await dbContext.SaveChangesAsync(); + } + } +} diff --git a/SAMA.Web/Services/LdapAuthenticationService.cs b/SAMA.Web/Services/LdapAuthenticationService.cs index 5ee1a0e..663828e 100644 --- a/SAMA.Web/Services/LdapAuthenticationService.cs +++ b/SAMA.Web/Services/LdapAuthenticationService.cs @@ -2,7 +2,6 @@ using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using Microsoft.AspNetCore.Identity; -using Microsoft.EntityFrameworkCore; using Novell.Directory.Ldap; using SAMA.Data; using SAMA.Data.Entities; @@ -12,6 +11,7 @@ namespace SAMA.Web.Services; public class LdapAuthenticationService( GlobalSettingsService _globalSettings, + GroupMappingSyncService _groupMappingService, IServiceProvider _serviceProvider, ILogger _logger) { @@ -355,7 +355,7 @@ public virtual async Task ProvisionOrUpdateUserAsync(LdapLoginR } // Apply group mappings - await ApplyGroupMappingsAsync(dbContext, userManager, user, ldapResult.Groups); + await _groupMappingService.ApplyGroupMappingsAsync(dbContext, userManager, user, ldapResult.Groups, AuthConstants.LdapSource); return user; } @@ -446,23 +446,6 @@ private static List GetMultiValuedAttribute(LdapEntry entry, string attr } } - internal static string? ExtractCnFromDn(string dn) - { - if (string.IsNullOrWhiteSpace(dn)) - { - return null; - } - - // Parse "CN=GroupName,OU=Groups,DC=example,DC=com" -> "GroupName" - if (!dn.StartsWith("CN=", StringComparison.OrdinalIgnoreCase)) - { - return null; - } - - var commaIndex = dn.IndexOf(','); - return commaIndex > 3 ? dn[3..commaIndex] : dn[3..]; - } - private static string FormatException(Exception ex) { if (ex is LdapException ldapEx) @@ -514,89 +497,6 @@ private static string EscapeLdapFilter(string input) .Replace("\0", "\\00"); } - private async Task ApplyGroupMappingsAsync( - SamaDbContext dbContext, - UserManager userManager, - ApplicationUser user, - List ldapGroups) - { - var mappings = await dbContext.WorkspaceGroupMappings - .AsNoTracking() - .Where(m => m.IdentityProvider == AuthConstants.LdapSource) - .ToListAsync(); - - // Expand raw DNs to include extracted CNs for flexible matching - var expandedGroups = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var group in ldapGroups) - { - expandedGroups.Add(group); - var cn = ExtractCnFromDn(group); - if (cn != null) - { - expandedGroups.Add(cn); - } - } - - var matchedMappings = mappings - .Where(m => expandedGroups.Contains(m.ExternalGroupId)) - .ToList(); - - // Sync admin role: grant or revoke based on current LDAP groups - var shouldBeAdmin = matchedMappings.Any(m => m.WorkspaceId == null && m.Role == AuthConstants.AdminRole); - var isAdmin = await userManager.IsInRoleAsync(user, AuthConstants.AdminRole); - - if (shouldBeAdmin && !isAdmin) - { - await userManager.AddToRoleAsync(user, AuthConstants.AdminRole); - _logger.LogInformation("Granted Admin role to user {Email} via LDAP group mapping", user.Email); - } - else if (!shouldBeAdmin && isAdmin) - { - await userManager.RemoveFromRoleAsync(user, AuthConstants.AdminRole); - _logger.LogInformation("Revoked Admin role from user {Email} — no longer in an LDAP admin group", user.Email); - } - - // Diff-based workspace assignment sync - var existingAssignments = await dbContext.UserWorkspaces - .Where(uw => uw.UserId == user.Id && uw.Source == AuthConstants.LdapSource) - .ToListAsync(); - - var desiredAssignments = matchedMappings - .Where(m => m.WorkspaceId != null && m.Role != AuthConstants.AdminRole) - .Select(m => (WorkspaceId: m.WorkspaceId!.Value, m.Role)) - .ToHashSet(); - - var existingSet = existingAssignments - .Select(a => (a.WorkspaceId, a.Role)) - .ToHashSet(); - - // Remove assignments that are no longer matched - var toRemove = existingAssignments - .Where(a => !desiredAssignments.Contains((a.WorkspaceId, a.Role))) - .ToList(); - dbContext.UserWorkspaces.RemoveRange(toRemove); - - // Add new assignments that don't exist yet - var now = DateTimeOffset.UtcNow; - foreach (var (workspaceId, role) in desiredAssignments.Where(d => !existingSet.Contains(d))) - { - dbContext.UserWorkspaces.Add(new UserWorkspace - { - UserId = user.Id, - WorkspaceId = workspaceId, - Role = role, - Source = AuthConstants.LdapSource, - CreatedAt = now, - UpdatedAt = now, - }); - } - - if (toRemove.Count > 0 || desiredAssignments.Except(existingSet).Any()) - { - await dbContext.SaveChangesAsync(); - } - } - private async Task> GetGroupMembershipsAsync( string host, int port, diff --git a/SAMA.Web/Services/OidcAuthenticationService.cs b/SAMA.Web/Services/OidcAuthenticationService.cs new file mode 100644 index 0000000..1d6d1a1 --- /dev/null +++ b/SAMA.Web/Services/OidcAuthenticationService.cs @@ -0,0 +1,88 @@ +using System.Security.Claims; +using Microsoft.AspNetCore.Identity; +using SAMA.Data; +using SAMA.Data.Entities; +using SAMA.Web.Constants; + +namespace SAMA.Web.Services; + +public class OidcAuthenticationService( + GlobalSettingsService _globalSettings, + GroupMappingSyncService _groupMappingService, + IServiceProvider _serviceProvider, + ILogger _logger) +{ + public virtual bool IsOidcEnabled => _globalSettings.OidcEnabled + && !string.IsNullOrWhiteSpace(_globalSettings.OidcAuthority) + && !string.IsNullOrWhiteSpace(_globalSettings.OidcClientId); + + public async Task ProvisionOrUpdateUserAsync(ClaimsPrincipal principal) + { + var email = principal.FindFirstValue(ClaimTypes.Email) + ?? principal.FindFirstValue("email"); + + if (string.IsNullOrWhiteSpace(email)) + { + throw new InvalidOperationException("OIDC token does not contain an email claim. Ensure the 'email' scope is requested and the provider is configured to include email."); + } + + var subject = principal.FindFirstValue(ClaimTypes.NameIdentifier) + ?? principal.FindFirstValue("sub") + ?? email; + + using var scope = _serviceProvider.CreateScope(); + var userManager = scope.ServiceProvider.GetRequiredService>(); + var dbContext = scope.ServiceProvider.GetRequiredService(); + + var user = await userManager.FindByEmailAsync(email); + + if (user == null) + { + user = new ApplicationUser + { + UserName = email, + Email = email, + EmailConfirmed = true, + CreatedAt = DateTimeOffset.UtcNow, + }; + + var createResult = await userManager.CreateAsync(user); + if (!createResult.Succeeded) + { + var errors = string.Join(", ", createResult.Errors.Select(e => e.Description)); + _logger.LogError("Failed to create OIDC user {Email}: {Errors}", email, errors); + throw new InvalidOperationException($"Failed to provision OIDC user: {errors}"); + } + + await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + _logger.LogInformation("JIT provisioned new user for OIDC login: {Email}", email); + } + else + { + var logins = await userManager.GetLoginsAsync(user); + if (!logins.Any(l => l.LoginProvider == AuthConstants.OidcSource)) + { + await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + } + } + + var groups = ExtractGroups(principal); + await _groupMappingService.ApplyGroupMappingsAsync(dbContext, userManager, user, groups, AuthConstants.OidcSource); + + return user; + } + + private List ExtractGroups(ClaimsPrincipal principal) + { + var groupClaimType = _globalSettings.OidcGroupClaimType; + if (string.IsNullOrWhiteSpace(groupClaimType)) + { + return []; + } + + return principal.FindAll(groupClaimType) + .Select(c => c.Value) + .Where(v => !string.IsNullOrWhiteSpace(v)) + .ToList(); + } +} diff --git a/SAMA.Web/Services/OidcPostConfigureOptions.cs b/SAMA.Web/Services/OidcPostConfigureOptions.cs new file mode 100644 index 0000000..5e82262 --- /dev/null +++ b/SAMA.Web/Services/OidcPostConfigureOptions.cs @@ -0,0 +1,45 @@ +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; +using SAMA.Web.Constants; + +namespace SAMA.Web.Services; + +public class OidcPostConfigureOptions(GlobalSettingsService _globalSettings) : IPostConfigureOptions +{ + public void PostConfigure(string? name, OpenIdConnectOptions options) + { + if (name != AuthConstants.OidcSource) + { + return; + } + + if (!_globalSettings.OidcEnabled) + { + // When disabled, set authority to a placeholder so the handler doesn't crash, + // but it will never be challenged + options.Authority = "https://localhost"; + options.ClientId = "disabled"; + options.MetadataAddress = string.Empty; + options.Configuration = new OpenIdConnectConfiguration(); + return; + } + + options.Authority = _globalSettings.OidcAuthority; + options.ClientId = _globalSettings.OidcClientId; + options.ClientSecret = _globalSettings.OidcClientSecret; + options.ResponseType = OpenIdConnectResponseType.Code; + options.CallbackPath = "/signin-oidc"; + options.SignedOutCallbackPath = "/signout-callback-oidc"; + options.SaveTokens = true; + options.MapInboundClaims = false; + + options.Scope.Clear(); + foreach (var scope in _globalSettings.OidcScopes.Split(' ', StringSplitOptions.RemoveEmptyEntries)) + { + options.Scope.Add(scope); + } + + options.TokenValidationParameters.NameClaimType = "name"; + } +} From d8a375c1c4810318597e8c8bc4329b1bc0b0d1f4 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Wed, 11 Mar 2026 15:58:46 -0400 Subject: [PATCH 02/17] Fix implementation; add system test --- .github/workflows/ci.yml | 12 +++++ SAMA.Tests.System/OidcTests.cs | 48 +++++++++++++++++++ SAMA.Tests.System/SystemTestBase.cs | 3 ++ .../EmailChannelHandlerTests.cs | 2 +- .../Services/OidcPostConfigureOptionsTests.cs | 3 +- SAMA.Web/Pages/Account/Logout.cshtml.cs | 10 ---- SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs | 4 ++ SAMA.Web/Program.cs | 7 ++- SAMA.Web/Services/OidcPostConfigureOptions.cs | 23 +++++++-- SAMA.Web/docker-compose.yml | 14 ++++++ 10 files changed, 109 insertions(+), 17 deletions(-) create mode 100644 SAMA.Tests.System/OidcTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbb0ca4..0022bd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,18 @@ jobs: ports: - 6467:80 - 6468:25 + oidc: + image: ghcr.io/geigerzaehler/oidc-provider-mock:latest + restart: unless-stopped + command: > + --user-claims '{"sub": "alice", "email": "alice@example.com", "name": "Alice"}' + ports: + - 9400:9400 + ldap: + image: thoteam/slapd-server-mock:latest + restart: unless-stopped + ports: + - 127.0.0.1:389:389 env: POSTGRES_HOST: localhost diff --git a/SAMA.Tests.System/OidcTests.cs b/SAMA.Tests.System/OidcTests.cs new file mode 100644 index 0000000..c932f9f --- /dev/null +++ b/SAMA.Tests.System/OidcTests.cs @@ -0,0 +1,48 @@ +using Microsoft.Playwright; + +using static Microsoft.Playwright.Assertions; + +namespace SAMA.Tests.System; + +[TestClass] +[SystemTestCondition] +public class OidcTests : SystemTestBase +{ + [TestMethod] + public async Task ShouldLoginViaOidc() + { + await SetupInitialAdminAsync(); + await LoginAsync(); + + // Configure OIDC to use the mock provider + await Page.GotoAsync($"{BaseUrl}/Admin/Settings/Oidc"); + await Page.CheckAsync("input[name='OidcInput.Enabled']"); + await Page.FillAsync("input[name='OidcInput.Authority']", "http://localhost:9400"); + await Page.FillAsync("input[name='OidcInput.ProviderName']", "Mock OIDC"); + await Page.FillAsync("input[name='OidcInput.ClientId']", "test-client"); + await Page.FillAsync("input[name='OidcInput.ClientSecret']", "test-secret"); + await Page.ClickAsync("button[type='submit']"); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + await Expect(Page.Locator("text=OIDC settings saved successfully")).ToBeVisibleAsync(); + + await LogoutAsync(); + + // Verify OIDC button appears on login page + await Page.GotoAsync($"{BaseUrl}/Account/Login"); + var oidcButton = Page.Locator("a:has-text('Sign in with Mock OIDC')"); + await Expect(oidcButton).ToBeVisibleAsync(); + + // Initiate OIDC login + await oidcButton.ClickAsync(); + + // On the mock provider, click the predefined "alice" user button + await Page.Locator("button:has-text('alice')").ClickAsync(); + + // Should redirect back to the app, logged in as alice + await Page.WaitForURLAsync($"{BaseUrl}/**"); + + // Alice is a new OIDC-provisioned user with no workspaces + await Expect(Page.Locator("text=You don't have access to any workspaces yet")).ToBeVisibleAsync(); + } +} diff --git a/SAMA.Tests.System/SystemTestBase.cs b/SAMA.Tests.System/SystemTestBase.cs index a0be539..324a91b 100644 --- a/SAMA.Tests.System/SystemTestBase.cs +++ b/SAMA.Tests.System/SystemTestBase.cs @@ -68,6 +68,7 @@ protected async Task LoginAsync(string email = "admin@example.com", string passw await Page.FillAsync("input[name='Input.EmailOrUsername']", email); await Page.FillAsync("input[name='Input.Password']", password); await Page.ClickAsync("button[type='submit']"); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); await Page.WaitForURLAsync($"{BaseUrl}/**"); } @@ -81,6 +82,7 @@ protected async Task SetupInitialAdminAsync(string email = "admin@example.com", await Page.FillAsync("input[name='Input.ConfirmPassword']", password); await Page.ClickAsync("button[type='submit']"); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); await Page.WaitForURLAsync(BaseUrl); } @@ -88,6 +90,7 @@ protected async Task LogoutAsync() { await Page.Locator("#userDropdown").ClickAsync(); await Page.Locator("button:has-text('Logout')").ClickAsync(); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); await Page.WaitForURLAsync(BaseUrl); } diff --git a/SAMA.Tests.Unit/Web/Services/NotificationChannels/EmailChannelHandlerTests.cs b/SAMA.Tests.Unit/Web/Services/NotificationChannels/EmailChannelHandlerTests.cs index 8d0737b..64595ea 100644 --- a/SAMA.Tests.Unit/Web/Services/NotificationChannels/EmailChannelHandlerTests.cs +++ b/SAMA.Tests.Unit/Web/Services/NotificationChannels/EmailChannelHandlerTests.cs @@ -549,7 +549,7 @@ public async Task SendStatusChangeEventAsyncShouldIncludeCorrectSubjectAndBody() { var message = callInfo.ArgAt(0); capturedSubject = message.Subject; - capturedBody = ((TextPart)message.Body).Text; + capturedBody = ((TextPart)message.Body!).Text; return Task.FromResult(string.Empty); }); _mockSmtpClient.DisconnectAsync(Arg.Any(), Arg.Any()) diff --git a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs index 369a656..062531f 100644 --- a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs +++ b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs @@ -61,10 +61,9 @@ public void PostConfigureShouldMapSettingsWhenEnabled() Assert.AreEqual("my-secret", options.ClientSecret); Assert.AreEqual(OpenIdConnectResponseType.Code, options.ResponseType); Assert.AreEqual("/signin-oidc", options.CallbackPath.Value); - Assert.AreEqual("/signout-callback-oidc", options.SignedOutCallbackPath.Value); - Assert.IsTrue(options.SaveTokens); Assert.IsFalse(options.MapInboundClaims); Assert.AreEqual("name", options.TokenValidationParameters.NameClaimType); + Assert.AreEqual("my-client-id", options.TokenValidationParameters.ValidAudience); } [TestMethod] diff --git a/SAMA.Web/Pages/Account/Logout.cshtml.cs b/SAMA.Web/Pages/Account/Logout.cshtml.cs index 3421521..454a874 100644 --- a/SAMA.Web/Pages/Account/Logout.cshtml.cs +++ b/SAMA.Web/Pages/Account/Logout.cshtml.cs @@ -1,18 +1,14 @@ -using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; using SAMA.Data.Entities; -using SAMA.Web.Constants; -using SAMA.Web.Services; namespace SAMA.Web.Pages.Account; [AllowAnonymous] public class LogoutModel( SignInManager signInManager, - OidcAuthenticationService oidcService, ILogger logger) : PageModel { @@ -21,12 +17,6 @@ public async Task OnPostAsync(string? returnUrl = null) await signInManager.SignOutAsync(); logger.LogInformation("User logged out"); - // If OIDC is enabled, also sign out from the OIDC provider - if (oidcService.IsOidcEnabled) - { - await HttpContext.SignOutAsync(AuthConstants.OidcSource); - } - if (returnUrl != null) { return LocalRedirect(returnUrl); diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs index 81b1be6..d7f8612 100644 --- a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs @@ -1,7 +1,9 @@ using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.Extensions.Options; using SAMA.Web.Constants; using SAMA.Web.Services; @@ -10,6 +12,7 @@ namespace SAMA.Web.Pages.Admin.Settings; [Authorize(Roles = AuthConstants.AdminRole)] public class OidcModel( GlobalSettingsService _globalSettings, + IOptionsMonitorCache _oidcOptionsCache, ILogger _logger) : PageModel { [BindProperty] @@ -98,6 +101,7 @@ public IActionResult OnPost() _logger.LogInformation("OIDC settings updated by {User}", User.Identity?.Name ?? "Unknown"); _globalSettings.ClearCache(); + _oidcOptionsCache.TryRemove(AuthConstants.OidcSource); TempData["OidcSuccess"] = "OIDC settings saved successfully."; } catch (Exception ex) diff --git a/SAMA.Web/Program.cs b/SAMA.Web/Program.cs index cf4ac50..0f93e76 100644 --- a/SAMA.Web/Program.cs +++ b/SAMA.Web/Program.cs @@ -95,7 +95,12 @@ // Configure OIDC authentication (dynamically configured via OidcPostConfigureOptions) builder.Services.AddAuthentication() - .AddOpenIdConnect(AuthConstants.OidcSource, _ => { }); + .AddOpenIdConnect(AuthConstants.OidcSource, options => + { + // Set a placeholder configuration so the handler passes startup validation. + // OidcPostConfigureOptions overrides this at runtime with real or dummy settings. + options.Configuration = new Microsoft.IdentityModel.Protocols.OpenIdConnect.OpenIdConnectConfiguration(); + }); builder.Services.AddSingleton, OidcPostConfigureOptions>(); // Configure ASP.NET Data Protection diff --git a/SAMA.Web/Services/OidcPostConfigureOptions.cs b/SAMA.Web/Services/OidcPostConfigureOptions.cs index 5e82262..95c2c1e 100644 --- a/SAMA.Web/Services/OidcPostConfigureOptions.cs +++ b/SAMA.Web/Services/OidcPostConfigureOptions.cs @@ -1,5 +1,6 @@ using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.Extensions.Options; +using Microsoft.IdentityModel.Protocols; using Microsoft.IdentityModel.Protocols.OpenIdConnect; using SAMA.Web.Constants; @@ -25,13 +26,28 @@ public void PostConfigure(string? name, OpenIdConnectOptions options) return; } - options.Authority = _globalSettings.OidcAuthority; + var authority = _globalSettings.OidcAuthority; + var isHttpAuthority = authority.StartsWith("http://", StringComparison.OrdinalIgnoreCase); + + // Clear placeholder configuration and rebuild a real ConfigurationManager from Authority. + // Microsoft's built-in PostConfigure already ran and won't re-create the manager, + // so we must build it ourselves. + var metadataAddress = authority.TrimEnd('/') + "/.well-known/openid-configuration"; + options.Configuration = null; + options.MetadataAddress = metadataAddress; + options.RequireHttpsMetadata = !isHttpAuthority; + var httpClient = options.Backchannel; + var retriever = new HttpDocumentRetriever(httpClient) { RequireHttps = !isHttpAuthority }; + options.ConfigurationManager = new ConfigurationManager( + metadataAddress, + new OpenIdConnectConfigurationRetriever(), + retriever); + + options.Authority = authority; options.ClientId = _globalSettings.OidcClientId; options.ClientSecret = _globalSettings.OidcClientSecret; options.ResponseType = OpenIdConnectResponseType.Code; options.CallbackPath = "/signin-oidc"; - options.SignedOutCallbackPath = "/signout-callback-oidc"; - options.SaveTokens = true; options.MapInboundClaims = false; options.Scope.Clear(); @@ -41,5 +57,6 @@ public void PostConfigure(string? name, OpenIdConnectOptions options) } options.TokenValidationParameters.NameClaimType = "name"; + options.TokenValidationParameters.ValidAudience = _globalSettings.OidcClientId; } } diff --git a/SAMA.Web/docker-compose.yml b/SAMA.Web/docker-compose.yml index 51d68d7..36c200a 100644 --- a/SAMA.Web/docker-compose.yml +++ b/SAMA.Web/docker-compose.yml @@ -23,5 +23,19 @@ services: - 127.0.0.1:6467:80 - 127.0.0.1:6468:25 + oidc: + image: ghcr.io/geigerzaehler/oidc-provider-mock:latest + restart: unless-stopped + command: > + --user-claims '{"sub": "alice", "email": "alice@example.com", "name": "Alice"}' + ports: + - 127.0.0.1:9400:9400 + + ldap: + image: thoteam/slapd-server-mock:latest + restart: unless-stopped + ports: + - 127.0.0.1:389:389 + volumes: pgdata: From d370b78d64bd4b3a7e5231aa6bc57a9c9b32d641 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:05:48 -0400 Subject: [PATCH 03/17] Fix CI syntax --- .github/workflows/ci.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0022bd5..06682c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,16 +89,12 @@ jobs: - 6468:25 oidc: image: ghcr.io/geigerzaehler/oidc-provider-mock:latest - restart: unless-stopped - command: > - --user-claims '{"sub": "alice", "email": "alice@example.com", "name": "Alice"}' ports: - 9400:9400 ldap: image: thoteam/slapd-server-mock:latest - restart: unless-stopped ports: - - 127.0.0.1:389:389 + - 389:389 env: POSTGRES_HOST: localhost @@ -109,6 +105,9 @@ jobs: SAMA_ENCRYPTION_KEY: test-encryption-key-for-ci-only steps: + - name: Prepare OIDC user (this is done via CMD in local dev) + run: | + curl -XPUT localhost:9400/users/alice --json '{"email": "alice@example.com", "name": "Alice"}' - uses: actions/checkout@v4 - name: Set up dotnet uses: actions/setup-dotnet@v4 From 6bce31b1f178ab376602f233ff5069b56e54eeed Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:33:58 -0400 Subject: [PATCH 04/17] Fix unit tests --- .../Services/OidcPostConfigureOptionsTests.cs | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs index 062531f..7d9f70f 100644 --- a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs +++ b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs @@ -11,37 +11,44 @@ public class OidcPostConfigureOptionsTests { private GlobalSettingsService _mockGlobalSettings = null!; private OidcPostConfigureOptions _postConfigure = null!; + private OpenIdConnectOptions _options = null!; [TestInitialize] public void Setup() { _mockGlobalSettings = Substitute.For(null!, null!, null!, null!); _postConfigure = new OidcPostConfigureOptions(_mockGlobalSettings); + _options = new OpenIdConnectOptions(); + _options.Backchannel = new HttpClient(); + } + + [TestCleanup] + public void Cleanup() + { + _options.Backchannel.Dispose(); } [TestMethod] public void PostConfigureShouldIgnoreNonOidcSchemes() { - var options = new OpenIdConnectOptions(); - var originalAuthority = options.Authority; + var originalAuthority = _options.Authority; - _postConfigure.PostConfigure("Cookies", options); + _postConfigure.PostConfigure("Cookies", _options); - Assert.AreEqual(originalAuthority, options.Authority); + Assert.AreEqual(originalAuthority, _options.Authority); } [TestMethod] public void PostConfigureShouldSetDummyConfigWhenDisabled() { _mockGlobalSettings.OidcEnabled.Returns(false); - var options = new OpenIdConnectOptions(); - _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); - Assert.AreEqual("https://localhost", options.Authority); - Assert.AreEqual("disabled", options.ClientId); - Assert.AreEqual(string.Empty, options.MetadataAddress); - Assert.IsNotNull(options.Configuration); + Assert.AreEqual("https://localhost", _options.Authority); + Assert.AreEqual("disabled", _options.ClientId); + Assert.AreEqual(string.Empty, _options.MetadataAddress); + Assert.IsNotNull(_options.Configuration); } [TestMethod] @@ -52,18 +59,17 @@ public void PostConfigureShouldMapSettingsWhenEnabled() _mockGlobalSettings.OidcClientId.Returns("my-client-id"); _mockGlobalSettings.OidcClientSecret.Returns("my-secret"); _mockGlobalSettings.OidcScopes.Returns("openid profile email"); - var options = new OpenIdConnectOptions(); - - _postConfigure.PostConfigure(AuthConstants.OidcSource, options); - - Assert.AreEqual("https://login.example.com/tenant", options.Authority); - Assert.AreEqual("my-client-id", options.ClientId); - Assert.AreEqual("my-secret", options.ClientSecret); - Assert.AreEqual(OpenIdConnectResponseType.Code, options.ResponseType); - Assert.AreEqual("/signin-oidc", options.CallbackPath.Value); - Assert.IsFalse(options.MapInboundClaims); - Assert.AreEqual("name", options.TokenValidationParameters.NameClaimType); - Assert.AreEqual("my-client-id", options.TokenValidationParameters.ValidAudience); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); + + Assert.AreEqual("https://login.example.com/tenant", _options.Authority); + Assert.AreEqual("my-client-id", _options.ClientId); + Assert.AreEqual("my-secret", _options.ClientSecret); + Assert.AreEqual(OpenIdConnectResponseType.Code, _options.ResponseType); + Assert.AreEqual("/signin-oidc", _options.CallbackPath.Value); + Assert.IsFalse(_options.MapInboundClaims); + Assert.AreEqual("name", _options.TokenValidationParameters.NameClaimType); + Assert.AreEqual("my-client-id", _options.TokenValidationParameters.ValidAudience); } [TestMethod] @@ -74,15 +80,14 @@ public void PostConfigureShouldSplitAndSetScopes() _mockGlobalSettings.OidcClientId.Returns("client"); _mockGlobalSettings.OidcClientSecret.Returns(""); _mockGlobalSettings.OidcScopes.Returns("openid profile email groups"); - var options = new OpenIdConnectOptions(); - options.Scope.Add("pre-existing"); + _options.Scope.Add("pre-existing"); - _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); - Assert.AreEqual(4, options.Scope.Count); + Assert.AreEqual(4, _options.Scope.Count); CollectionAssert.AreEquivalent( new[] { "openid", "profile", "email", "groups" }, - options.Scope.ToArray()); + _options.Scope.ToArray()); } [TestMethod] @@ -93,13 +98,12 @@ public void PostConfigureShouldHandleExtraWhitespaceInScopes() _mockGlobalSettings.OidcClientId.Returns("client"); _mockGlobalSettings.OidcClientSecret.Returns(""); _mockGlobalSettings.OidcScopes.Returns("openid profile email"); - var options = new OpenIdConnectOptions(); - _postConfigure.PostConfigure(AuthConstants.OidcSource, options); + _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); - Assert.AreEqual(3, options.Scope.Count); + Assert.AreEqual(3, _options.Scope.Count); CollectionAssert.AreEquivalent( new[] { "openid", "profile", "email" }, - options.Scope.ToArray()); + _options.Scope.ToArray()); } } From 4c289dd4964c14c69d430231c20457fe8f843c6d Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:58:26 -0400 Subject: [PATCH 05/17] Improve system tests --- .github/workflows/ci.yml | 13 +++++++++++++ SAMA.Tests.System/CleanupTests.cs | 18 +++++++++++++++++- SAMA.Tests.System/SystemTestBase.cs | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06682c2..c7ac242 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,11 @@ jobs: - 6468:25 oidc: image: ghcr.io/geigerzaehler/oidc-provider-mock:latest + options: >- + --health-cmd "curl -f http://localhost:9400/.well-known/openid-configuration || exit 1" + --health-interval 10s + --health-timeout 5s + --health-retries 5 ports: - 9400:9400 ldap: @@ -97,6 +102,7 @@ jobs: - 389:389 env: + TMPDIR: /tmp POSTGRES_HOST: localhost POSTGRES_PORT: 5432 POSTGRES_DB: samadb @@ -123,6 +129,13 @@ jobs: run: | cd SAMA.Tests.System dotnet test --logger "console;verbosity=detailed" + - name: Upload Playwright screenshots + if: failure() + uses: actions/upload-artifact@v4 + with: + name: playwright-screenshots + path: /tmp/sama-playwright-screenshots/ + retention-days: 7 docker_build: name: Docker Build diff --git a/SAMA.Tests.System/CleanupTests.cs b/SAMA.Tests.System/CleanupTests.cs index c459969..ebecd3e 100644 --- a/SAMA.Tests.System/CleanupTests.cs +++ b/SAMA.Tests.System/CleanupTests.cs @@ -20,8 +20,9 @@ public async Task CleanupStaleSchemasAndEmails() { var schemasDeleted = await CleanupSchemasAsync(); var emailsDeleted = await CleanupSmtp4DevEmailsAsync(); + var screenshotsDeleted = CleanupScreenshots(); - Console.WriteLine($"Cleanup complete: {schemasDeleted} schemas deleted, {emailsDeleted} emails deleted."); + Console.WriteLine($"Cleanup complete: {schemasDeleted} schemas deleted, {emailsDeleted} emails deleted, {screenshotsDeleted} screenshots deleted."); } private static async Task CleanupSchemasAsync() @@ -98,4 +99,19 @@ private static string GetConnectionString() return $"Host={host};Port={port};Database={database};Username={username};Password={password}"; } + + private static int CleanupScreenshots() + { + var screenshotDir = Path.Combine(Path.GetTempPath(), "sama-playwright-screenshots"); + if (!Directory.Exists(screenshotDir)) + { + return 0; + } + + var files = Directory.GetFiles(screenshotDir); + var count = files.Length; + Directory.Delete(screenshotDir, recursive: true); + Console.WriteLine($"Deleted {count} screenshots from {screenshotDir}"); + return count; + } } diff --git a/SAMA.Tests.System/SystemTestBase.cs b/SAMA.Tests.System/SystemTestBase.cs index 324a91b..71e00a9 100644 --- a/SAMA.Tests.System/SystemTestBase.cs +++ b/SAMA.Tests.System/SystemTestBase.cs @@ -19,6 +19,8 @@ public abstract class SystemTestBase protected static bool IsDebugging => Debugger.IsAttached; + public TestContext TestContext { get; set; } = null!; + protected IPage Page { get; private set; } = null!; protected string BaseUrl => _context?.BaseUrl ?? throw new InvalidOperationException("Test not initialized."); @@ -57,6 +59,18 @@ public virtual async Task TestInitializeAsync() [TestCleanup] public virtual async Task TestCleanupAsync() { + if (TestContext.CurrentTestOutcome != UnitTestOutcome.Passed) + { + var screenshotDir = Path.Combine(Path.GetTempPath(), "sama-playwright-screenshots"); + Directory.CreateDirectory(screenshotDir); + var fileName = $"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}.png"; + await Page.ScreenshotAsync(new PageScreenshotOptions + { + Path = Path.Combine(screenshotDir, fileName), + FullPage = true, + }); + } + await Page.CloseAsync(); await _browser.DisposeAsync(); _playwright.Dispose(); From d41c65cf1c22f6ff1d67603a3680b17c7774fb40 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Wed, 11 Mar 2026 17:10:18 -0400 Subject: [PATCH 06/17] Don't use curl in health check --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7ac242..ffbd9d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,7 +90,7 @@ jobs: oidc: image: ghcr.io/geigerzaehler/oidc-provider-mock:latest options: >- - --health-cmd "curl -f http://localhost:9400/.well-known/openid-configuration || exit 1" + --health-cmd "python -c \"import urllib.request; urllib.request.urlopen('http://localhost:9400/.well-known/openid-configuration')\"" --health-interval 10s --health-timeout 5s --health-retries 5 From 74c678730c6644b5b3604d113a68e97d413def8f Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:08:36 -0400 Subject: [PATCH 07/17] Fix system test --- .github/workflows/ci.yml | 5 +++-- SAMA.Tests.System/SystemTestBase.cs | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffbd9d0..383d154 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,9 +111,10 @@ jobs: SAMA_ENCRYPTION_KEY: test-encryption-key-for-ci-only steps: - - name: Prepare OIDC user (this is done via CMD in local dev) + - name: Prepare OIDC user (this is done via Docker CMD in local dev) run: | - curl -XPUT localhost:9400/users/alice --json '{"email": "alice@example.com", "name": "Alice"}' + curl 'http://localhost:9400/oauth2/authorize?response_type=code&client_id=my-client-id&scope=openid+email&redirect_uri=http://localhost:9400' --data-raw 'sub=alice' + curl -XPUT 'http://localhost:9400/users/alice' --json '{"email": "alice@example.com", "name": "Alice"}' - uses: actions/checkout@v4 - name: Set up dotnet uses: actions/setup-dotnet@v4 diff --git a/SAMA.Tests.System/SystemTestBase.cs b/SAMA.Tests.System/SystemTestBase.cs index 71e00a9..e0b5897 100644 --- a/SAMA.Tests.System/SystemTestBase.cs +++ b/SAMA.Tests.System/SystemTestBase.cs @@ -64,11 +64,13 @@ public virtual async Task TestCleanupAsync() var screenshotDir = Path.Combine(Path.GetTempPath(), "sama-playwright-screenshots"); Directory.CreateDirectory(screenshotDir); var fileName = $"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}.png"; + var screenshotPath = Path.Combine(screenshotDir, fileName); await Page.ScreenshotAsync(new PageScreenshotOptions { - Path = Path.Combine(screenshotDir, fileName), + Path = screenshotPath, FullPage = true, }); + Console.WriteLine($"Screenshot saved: {screenshotPath}"); } await Page.CloseAsync(); From be88396a67d1d3be300346d8de09be9c6cd3336d Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:59:31 -0400 Subject: [PATCH 08/17] Update actions dependencies --- .github/workflows/ci.yml | 16 ++++++++-------- .github/workflows/release-ghcr.yml | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 383d154..a5ad53a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,9 @@ jobs: name: Unit Tests on Linux runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: '10.x' - name: Run unit tests @@ -54,9 +54,9 @@ jobs: SAMA_ENCRYPTION_KEY: test-encryption-key-for-ci-only steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: '10.x' - name: Run integration tests @@ -115,9 +115,9 @@ jobs: run: | curl 'http://localhost:9400/oauth2/authorize?response_type=code&client_id=my-client-id&scope=openid+email&redirect_uri=http://localhost:9400' --data-raw 'sub=alice' curl -XPUT 'http://localhost:9400/users/alice' --json '{"email": "alice@example.com", "name": "Alice"}' - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: '10.x' - name: Build @@ -132,7 +132,7 @@ jobs: dotnet test --logger "console;verbosity=detailed" - name: Upload Playwright screenshots if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: playwright-screenshots path: /tmp/sama-playwright-screenshots/ @@ -142,6 +142,6 @@ jobs: name: Docker Build runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Build Docker image run: docker build -t sama:ci-test . diff --git a/.github/workflows/release-ghcr.yml b/.github/workflows/release-ghcr.yml index 70c673f..c2af362 100644 --- a/.github/workflows/release-ghcr.yml +++ b/.github/workflows/release-ghcr.yml @@ -20,12 +20,12 @@ jobs: outputs: version: ${{ steps.minver.outputs.version }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: fetch-depth: 0 - name: Set up dotnet - uses: actions/setup-dotnet@v4 + uses: actions/setup-dotnet@v5 with: dotnet-version: '10.x' @@ -58,19 +58,19 @@ jobs: description: "Service Availability Monitoring and Alerting - A modern uptime monitoring system (with sudo)" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: fetch-depth: 0 - name: Set up QEMU - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@v4 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@v4 - name: Extract metadata id: meta - uses: docker/metadata-action@v5 + uses: docker/metadata-action@v6 with: images: ${{ env.TAG_NAME }} tags: | @@ -86,14 +86,14 @@ jobs: DOCKER_METADATA_ANNOTATIONS_LEVELS: manifest,index - name: Log in to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@v4 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push - uses: docker/build-push-action@v5 + uses: docker/build-push-action@v7 with: context: . file: ${{ matrix.dockerfile }} From 47e6bbe75eebd067b28fadc58ce19db00263ac94 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:16:27 -0400 Subject: [PATCH 09/17] Improve sign-in logic --- SAMA.Web/Pages/Account/Login.cshtml.cs | 15 ++++++++++++++- SAMA.Web/Services/OidcPostConfigureOptions.cs | 2 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/SAMA.Web/Pages/Account/Login.cshtml.cs b/SAMA.Web/Pages/Account/Login.cshtml.cs index e241253..7f610dc 100644 --- a/SAMA.Web/Pages/Account/Login.cshtml.cs +++ b/SAMA.Web/Pages/Account/Login.cshtml.cs @@ -133,7 +133,7 @@ public async Task OnGetOidcCallbackAsync(string? returnUrl = null { returnUrl ??= Url.Content("~/"); - var authResult = await HttpContext.AuthenticateAsync(AuthConstants.OidcSource); + var authResult = await HttpContext.AuthenticateAsync(IdentityConstants.ExternalScheme); if (!authResult.Succeeded || authResult.Principal == null) { logger.LogWarning("OIDC authentication failed: {Failure}", authResult.Failure?.Message); @@ -142,16 +142,29 @@ public async Task OnGetOidcCallbackAsync(string? returnUrl = null return Page(); } + // Verify the external login came from our OIDC scheme + var scheme = authResult.Properties?.Items[".AuthScheme"]; + if (scheme != AuthConstants.OidcSource) + { + logger.LogWarning("External login scheme mismatch: expected {Expected}, got {Actual}", AuthConstants.OidcSource, scheme); + await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme); + ModelState.AddModelError(string.Empty, "External login failed. Please try again."); + ReturnUrl = returnUrl; + return Page(); + } + try { var user = await oidcService.ProvisionOrUpdateUserAsync(authResult.Principal); await signInManager.SignInAsync(user, isPersistent: false); + await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme); logger.LogInformation("User {Email} logged in via OIDC", user.Email); return LocalRedirect(returnUrl); } catch (Exception ex) { logger.LogError(ex, "Error during OIDC login"); + await HttpContext.SignOutAsync(IdentityConstants.ExternalScheme); ModelState.AddModelError(string.Empty, "An error occurred during login. Please try again."); ReturnUrl = returnUrl; return Page(); diff --git a/SAMA.Web/Services/OidcPostConfigureOptions.cs b/SAMA.Web/Services/OidcPostConfigureOptions.cs index 95c2c1e..bbdd245 100644 --- a/SAMA.Web/Services/OidcPostConfigureOptions.cs +++ b/SAMA.Web/Services/OidcPostConfigureOptions.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Protocols; using Microsoft.IdentityModel.Protocols.OpenIdConnect; @@ -43,6 +44,7 @@ public void PostConfigure(string? name, OpenIdConnectOptions options) new OpenIdConnectConfigurationRetriever(), retriever); + options.SignInScheme = IdentityConstants.ExternalScheme; options.Authority = authority; options.ClientId = _globalSettings.OidcClientId; options.ClientSecret = _globalSettings.OidcClientSecret; From 962625414f76b1328b22be4bdcd54753f21dc1cc Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:18:48 -0400 Subject: [PATCH 10/17] Use proper aspnet helpers --- SAMA.Web/Pages/Account/Login.cshtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SAMA.Web/Pages/Account/Login.cshtml b/SAMA.Web/Pages/Account/Login.cshtml index 9fbee90..f5e5a52 100644 --- a/SAMA.Web/Pages/Account/Login.cshtml +++ b/SAMA.Web/Pages/Account/Login.cshtml @@ -53,7 +53,7 @@ {
    - + Sign in with @Model.OidcProviderName From a4c5a404800da41c05c1cadc30daf7233202cf1e Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:47:08 -0400 Subject: [PATCH 11/17] Add email claim configurability --- .../Services/OidcAuthenticationServiceTests.cs | 18 ++++++++++++++++++ SAMA.Web/Pages/Admin/Settings/Oidc.cshtml | 8 ++++++++ SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs | 7 +++++++ SAMA.Web/Services/GlobalSettingsService.cs | 8 ++++++++ SAMA.Web/Services/OidcAuthenticationService.cs | 6 +++--- 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs b/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs index d4a7e34..1e8b317 100644 --- a/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs +++ b/SAMA.Tests.Integration/Web/Services/OidcAuthenticationServiceTests.cs @@ -124,6 +124,24 @@ public async Task ProvisionOrUpdateUserShouldUseConfiguredGroupClaimType() Assert.IsTrue(await _userManager.IsInRoleAsync(refreshedUser!, AuthConstants.AdminRole)); } + [TestMethod] + public async Task ProvisionOrUpdateUserShouldUseConfiguredEmailClaimType() + { + _globalSettings.OidcEmailClaimType = "preferred_username"; + + var claims = new List + { + new("sub", "sub-custom-email"), + new("preferred_username", "upn-user@example.com"), + }; + var identity = new ClaimsIdentity(claims, AuthConstants.OidcSource); + var principal = new ClaimsPrincipal(identity); + + var user = await _service.ProvisionOrUpdateUserAsync(principal); + + Assert.AreEqual("upn-user@example.com", user.Email); + } + [TestMethod] public async Task ProvisionOrUpdateUserShouldSkipGroupsWhenClaimTypeEmpty() { diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml index 8120040..82ec05e 100644 --- a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml @@ -116,6 +116,14 @@ Add additional scopes if your provider requires them for group claims.
    +
    + + +
    + The claim name in the ID token that contains the user's email address. Default: email. + For Azure AD / Entra ID, you may need preferred_username. +
    +
    diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs index d7f8612..a467166 100644 --- a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml.cs @@ -38,6 +38,9 @@ public class OidcInputModel [Display(Name = "Scopes")] public string Scopes { get; set; } = "openid profile email"; + [Display(Name = "Email Claim Type")] + public string EmailClaimType { get; set; } = "email"; + [Display(Name = "Group Claim Type")] public string GroupClaimType { get; set; } = "groups"; @@ -82,6 +85,9 @@ public IActionResult OnPost() _globalSettings.OidcScopes = string.IsNullOrWhiteSpace(OidcInput.Scopes) ? "openid profile email" : OidcInput.Scopes; + _globalSettings.OidcEmailClaimType = string.IsNullOrWhiteSpace(OidcInput.EmailClaimType) + ? "email" + : OidcInput.EmailClaimType; _globalSettings.OidcGroupClaimType = string.IsNullOrWhiteSpace(OidcInput.GroupClaimType) ? "groups" : OidcInput.GroupClaimType; @@ -119,6 +125,7 @@ private void LoadCurrentSettings() OidcInput.Authority = _globalSettings.OidcAuthority; OidcInput.ClientId = _globalSettings.OidcClientId; OidcInput.Scopes = _globalSettings.OidcScopes; + OidcInput.EmailClaimType = _globalSettings.OidcEmailClaimType; OidcInput.GroupClaimType = _globalSettings.OidcGroupClaimType; OidcInput.ProviderName = _globalSettings.OidcProviderName; HasExistingClientSecret = !string.IsNullOrEmpty(_globalSettings.OidcClientSecret); diff --git a/SAMA.Web/Services/GlobalSettingsService.cs b/SAMA.Web/Services/GlobalSettingsService.cs index eed2826..71bf559 100644 --- a/SAMA.Web/Services/GlobalSettingsService.cs +++ b/SAMA.Web/Services/GlobalSettingsService.cs @@ -65,6 +65,8 @@ public class GlobalSettingsService(IServiceProvider _serviceProvider, ILogger SetStringAsync(KeyOidcScopes, value).GetAwaiter().GetResult(); } + public virtual string OidcEmailClaimType + { + get => GetStringAsync(KeyOidcEmailClaimType, DefaultOidcEmailClaimType).GetAwaiter().GetResult(); + set => SetStringAsync(KeyOidcEmailClaimType, value).GetAwaiter().GetResult(); + } + public virtual string OidcGroupClaimType { get => GetStringAsync(KeyOidcGroupClaimType, DefaultOidcGroupClaimType).GetAwaiter().GetResult(); diff --git a/SAMA.Web/Services/OidcAuthenticationService.cs b/SAMA.Web/Services/OidcAuthenticationService.cs index 1d6d1a1..c954e4b 100644 --- a/SAMA.Web/Services/OidcAuthenticationService.cs +++ b/SAMA.Web/Services/OidcAuthenticationService.cs @@ -18,12 +18,12 @@ public class OidcAuthenticationService( public async Task ProvisionOrUpdateUserAsync(ClaimsPrincipal principal) { - var email = principal.FindFirstValue(ClaimTypes.Email) - ?? principal.FindFirstValue("email"); + var emailClaimType = _globalSettings.OidcEmailClaimType; + var email = principal.FindFirstValue(emailClaimType); if (string.IsNullOrWhiteSpace(email)) { - throw new InvalidOperationException("OIDC token does not contain an email claim. Ensure the 'email' scope is requested and the provider is configured to include email."); + throw new InvalidOperationException($"OIDC token does not contain a '{emailClaimType}' claim. Check the Email Claim Type setting or ensure the provider is configured to include this claim."); } var subject = principal.FindFirstValue(ClaimTypes.NameIdentifier) From 1b94e87d87ae5a998adcd00d79be412f596d0631 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 13:48:42 -0400 Subject: [PATCH 12/17] Treat warnings as errors in this solution --- Directory.Build.props | 1 + 1 file changed, 1 insertion(+) diff --git a/Directory.Build.props b/Directory.Build.props index 3e8a719..012f0eb 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -4,6 +4,7 @@ SAMA Service Availability Monitoring and Alerting Copyright © 2026 SEP + true v minimal true From 2b74a71bb3ddbf29169643654b6d988ca50f049a Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:25:56 -0400 Subject: [PATCH 13/17] Improve resilience --- .editorconfig | 20 +++++++++++++++++++ Directory.Build.props | 3 ++- SAMA.Data/SamaDbContext.cs | 6 +++--- SAMA.Shared/Checks/HttpCheckExecutor.cs | 4 ++-- SAMA.Shared/Wrappers/CustomTlsValidator.cs | 2 +- SAMA.Shared/Wrappers/ProcessWrapper.cs | 2 +- SAMA.Web/Extensions/ScheduleExtensions.cs | 2 +- SAMA.Web/Middleware/HtmxRedirectMiddleware.cs | 2 +- .../Services/LdapAuthenticationService.cs | 17 ++++++++++++++-- .../Services/OidcAuthenticationService.cs | 17 ++++++++++++++-- SAMA.Web/Services/OidcPostConfigureOptions.cs | 6 ++++-- 11 files changed, 65 insertions(+), 16 deletions(-) diff --git a/.editorconfig b/.editorconfig index 4bd21a8..13d1cfe 100644 --- a/.editorconfig +++ b/.editorconfig @@ -88,6 +88,9 @@ dotnet_naming_style.underscore_camel_case.capitalization = camel_case # Code quality rules dotnet_code_quality_unused_parameters = non_public:warning +# IDE0058: Expression value is never used — require explicit discard +csharp_style_unused_value_expression_statement_preference = discard_variable:warning + # CA2000: Dispose objects before losing scope dotnet_diagnostic.CA2000.severity = warning @@ -189,3 +192,20 @@ indent_size = 2 [*.md] trim_trailing_whitespace = false insert_final_newline = false + +# EF Core migrations and configurations use fluent builder patterns +# where return values are intentionally unused +[SAMA.Data/Migrations/**.cs] +csharp_style_unused_value_expression_statement_preference = discard_variable:silent + +[SAMA.Data/Configuration/**.cs] +csharp_style_unused_value_expression_statement_preference = discard_variable:silent + +[SAMA.Tests.Unit/**.cs] +csharp_style_unused_value_expression_statement_preference = discard_variable:silent + +[SAMA.Tests.Integration/**.cs] +csharp_style_unused_value_expression_statement_preference = discard_variable:silent + +[SAMA.Tests.System/**.cs] +csharp_style_unused_value_expression_statement_preference = discard_variable:silent diff --git a/Directory.Build.props b/Directory.Build.props index 012f0eb..e32d099 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -4,7 +4,8 @@ SAMA Service Availability Monitoring and Alerting Copyright © 2026 SEP - true + false + true v minimal true diff --git a/SAMA.Data/SamaDbContext.cs b/SAMA.Data/SamaDbContext.cs index 81eb3eb..0c8c220 100644 --- a/SAMA.Data/SamaDbContext.cs +++ b/SAMA.Data/SamaDbContext.cs @@ -43,7 +43,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) base.OnModelCreating(modelBuilder); // Apply all entity configurations (indexes, relationships, constraints, etc.) - modelBuilder.ApplyConfigurationsFromAssembly(typeof(SamaDbContext).Assembly); + _ = modelBuilder.ApplyConfigurationsFromAssembly(typeof(SamaDbContext).Assembly); // Configure UUIDv7 for all GUID primary keys foreach (var entityType in modelBuilder.Model.GetEntityTypes()) @@ -70,7 +70,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) // Apply encryption converters (runtime-only, not used during migrations) // These convert C# objects to encrypted JSON strings in the database - modelBuilder.Entity() + _ = modelBuilder.Entity() .Property(nc => nc.ConfigurationJson) .HasConversion( v => _encryptionService.Encrypt(JsonSerializer.Serialize(v, (JsonSerializerOptions?)null), _keyProvider.Key), @@ -78,7 +78,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) dictionaryComparer ); - modelBuilder.Entity() + _ = modelBuilder.Entity() .Property(c => c.ConfigurationJson) .HasConversion( v => _encryptionService.Encrypt(JsonSerializer.Serialize(v, (JsonSerializerOptions?)null), _keyProvider.Key), diff --git a/SAMA.Shared/Checks/HttpCheckExecutor.cs b/SAMA.Shared/Checks/HttpCheckExecutor.cs index 4e283ed..7409a17 100644 --- a/SAMA.Shared/Checks/HttpCheckExecutor.cs +++ b/SAMA.Shared/Checks/HttpCheckExecutor.cs @@ -166,11 +166,11 @@ private static void ParseAndAddHeaders(HttpRequestMessage request, string header if (string.Equals(headerName, "Content-Type", StringComparison.OrdinalIgnoreCase) && request.Content != null) { - request.Content.Headers.TryAddWithoutValidation(headerName, headerValue); + _ = request.Content.Headers.TryAddWithoutValidation(headerName, headerValue); } else { - request.Headers.TryAddWithoutValidation(headerName, headerValue); + _ = request.Headers.TryAddWithoutValidation(headerName, headerValue); } } } diff --git a/SAMA.Shared/Wrappers/CustomTlsValidator.cs b/SAMA.Shared/Wrappers/CustomTlsValidator.cs index 15596e9..2272878 100644 --- a/SAMA.Shared/Wrappers/CustomTlsValidator.cs +++ b/SAMA.Shared/Wrappers/CustomTlsValidator.cs @@ -16,7 +16,7 @@ public virtual bool ValidateWithCustomCa(X509Certificate? certificate, X509Chain using var certToValidate = new X509Certificate2(certificate); using var customCaCert = X509Certificate2.CreateFromPem(customCaCertificatePem); chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - chain.ChainPolicy.CustomTrustStore.Add(customCaCert); + _ = chain.ChainPolicy.CustomTrustStore.Add(customCaCert); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; return chain.Build(certToValidate); diff --git a/SAMA.Shared/Wrappers/ProcessWrapper.cs b/SAMA.Shared/Wrappers/ProcessWrapper.cs index 71563c1..570e834 100644 --- a/SAMA.Shared/Wrappers/ProcessWrapper.cs +++ b/SAMA.Shared/Wrappers/ProcessWrapper.cs @@ -18,7 +18,7 @@ public virtual void Start(ProcessStartInfo startInfo) ObjectDisposedException.ThrowIf(_disposedValue, this); _process = new Process { StartInfo = startInfo }; - _process.Start(); + _ = _process.Start(); } /// diff --git a/SAMA.Web/Extensions/ScheduleExtensions.cs b/SAMA.Web/Extensions/ScheduleExtensions.cs index 701dd96..565227a 100644 --- a/SAMA.Web/Extensions/ScheduleExtensions.cs +++ b/SAMA.Web/Extensions/ScheduleExtensions.cs @@ -31,7 +31,7 @@ public static class ScheduleExtensions try { var cron = new CronExpression(schedule); - cron.GetNextValidTimeAfter(DateTimeOffset.UtcNow); + _ = cron.GetNextValidTimeAfter(DateTimeOffset.UtcNow); return null; } catch diff --git a/SAMA.Web/Middleware/HtmxRedirectMiddleware.cs b/SAMA.Web/Middleware/HtmxRedirectMiddleware.cs index 49ce3e5..2369c1a 100644 --- a/SAMA.Web/Middleware/HtmxRedirectMiddleware.cs +++ b/SAMA.Web/Middleware/HtmxRedirectMiddleware.cs @@ -32,7 +32,7 @@ public async Task InvokeAsync(HttpContext context) { // Convert to HTMX-compatible response context.Response.StatusCode = 200; - context.Response.Headers.Remove("Location"); + _ = context.Response.Headers.Remove("Location"); context.Response.Headers["HX-Redirect"] = location; } } diff --git a/SAMA.Web/Services/LdapAuthenticationService.cs b/SAMA.Web/Services/LdapAuthenticationService.cs index 663828e..d08c37d 100644 --- a/SAMA.Web/Services/LdapAuthenticationService.cs +++ b/SAMA.Web/Services/LdapAuthenticationService.cs @@ -341,7 +341,14 @@ public virtual async Task ProvisionOrUpdateUserAsync(LdapLoginR throw new InvalidOperationException($"Failed to provision LDAP user: {errors}"); } - await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.LdapSource, ldapResult.UserDn!, AuthConstants.LdapSource)); + var loginResult = await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.LdapSource, ldapResult.UserDn!, AuthConstants.LdapSource)); + if (!loginResult.Succeeded) + { + var loginErrors = string.Join(", ", loginResult.Errors.Select(e => e.Description)); + _logger.LogError("Failed to add LDAP login for new user {Email}: {Errors}", ldapResult.Email, loginErrors); + throw new InvalidOperationException($"Failed to link LDAP login: {loginErrors}"); + } + _logger.LogInformation("JIT provisioned new user for LDAP login: {Email}", ldapResult.Email); } else @@ -350,7 +357,13 @@ public virtual async Task ProvisionOrUpdateUserAsync(LdapLoginR var logins = await userManager.GetLoginsAsync(user); if (!logins.Any(l => l.LoginProvider == AuthConstants.LdapSource)) { - await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.LdapSource, ldapResult.UserDn!, AuthConstants.LdapSource)); + var loginResult = await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.LdapSource, ldapResult.UserDn!, AuthConstants.LdapSource)); + if (!loginResult.Succeeded) + { + var loginErrors = string.Join(", ", loginResult.Errors.Select(e => e.Description)); + _logger.LogError("Failed to add LDAP login for existing user {Email}: {Errors}", ldapResult.Email, loginErrors); + throw new InvalidOperationException($"Failed to link LDAP login: {loginErrors}"); + } } } diff --git a/SAMA.Web/Services/OidcAuthenticationService.cs b/SAMA.Web/Services/OidcAuthenticationService.cs index c954e4b..5d4c637 100644 --- a/SAMA.Web/Services/OidcAuthenticationService.cs +++ b/SAMA.Web/Services/OidcAuthenticationService.cs @@ -54,7 +54,14 @@ public async Task ProvisionOrUpdateUserAsync(ClaimsPrincipal pr throw new InvalidOperationException($"Failed to provision OIDC user: {errors}"); } - await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + var loginResult = await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + if (!loginResult.Succeeded) + { + var loginErrors = string.Join(", ", loginResult.Errors.Select(e => e.Description)); + _logger.LogError("Failed to add OIDC login for new user {Email}: {Errors}", email, loginErrors); + throw new InvalidOperationException($"Failed to link OIDC login: {loginErrors}"); + } + _logger.LogInformation("JIT provisioned new user for OIDC login: {Email}", email); } else @@ -62,7 +69,13 @@ public async Task ProvisionOrUpdateUserAsync(ClaimsPrincipal pr var logins = await userManager.GetLoginsAsync(user); if (!logins.Any(l => l.LoginProvider == AuthConstants.OidcSource)) { - await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + var loginResult = await userManager.AddLoginAsync(user, new UserLoginInfo(AuthConstants.OidcSource, subject, AuthConstants.OidcSource)); + if (!loginResult.Succeeded) + { + var loginErrors = string.Join(", ", loginResult.Errors.Select(e => e.Description)); + _logger.LogError("Failed to add OIDC login for existing user {Email}: {Errors}", email, loginErrors); + throw new InvalidOperationException($"Failed to link OIDC login: {loginErrors}"); + } } } diff --git a/SAMA.Web/Services/OidcPostConfigureOptions.cs b/SAMA.Web/Services/OidcPostConfigureOptions.cs index bbdd245..1baefd8 100644 --- a/SAMA.Web/Services/OidcPostConfigureOptions.cs +++ b/SAMA.Web/Services/OidcPostConfigureOptions.cs @@ -16,9 +16,11 @@ public void PostConfigure(string? name, OpenIdConnectOptions options) return; } - if (!_globalSettings.OidcEnabled) + if (!_globalSettings.OidcEnabled + || string.IsNullOrWhiteSpace(_globalSettings.OidcAuthority) + || string.IsNullOrWhiteSpace(_globalSettings.OidcClientId)) { - // When disabled, set authority to a placeholder so the handler doesn't crash, + // When disabled or misconfigured, set authority to a placeholder so the handler doesn't crash, // but it will never be challenged options.Authority = "https://localhost"; options.ClientId = "disabled"; From 6cff5528ba66e949fde153752ec4c1891a892261 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Fri, 13 Mar 2026 17:19:42 -0400 Subject: [PATCH 14/17] Update NuGet dependencies --- SAMA.Data/SAMA.Data.csproj | 8 ++++---- SAMA.Shared/SAMA.Shared.csproj | 2 +- SAMA.Tests.Integration/SAMA.Tests.Integration.csproj | 6 +++--- SAMA.Web/SAMA.Web.csproj | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/SAMA.Data/SAMA.Data.csproj b/SAMA.Data/SAMA.Data.csproj index b08b3e1..5e8d97e 100644 --- a/SAMA.Data/SAMA.Data.csproj +++ b/SAMA.Data/SAMA.Data.csproj @@ -7,13 +7,13 @@ - - - + + + runtime; build; native; contentfiles; analyzers; buildtransitive all - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/SAMA.Shared/SAMA.Shared.csproj b/SAMA.Shared/SAMA.Shared.csproj index bf448ae..933ddf7 100644 --- a/SAMA.Shared/SAMA.Shared.csproj +++ b/SAMA.Shared/SAMA.Shared.csproj @@ -8,7 +8,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/SAMA.Tests.Integration/SAMA.Tests.Integration.csproj b/SAMA.Tests.Integration/SAMA.Tests.Integration.csproj index 0bcdb4e..d950d81 100644 --- a/SAMA.Tests.Integration/SAMA.Tests.Integration.csproj +++ b/SAMA.Tests.Integration/SAMA.Tests.Integration.csproj @@ -13,10 +13,10 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - - + + - + all diff --git a/SAMA.Web/SAMA.Web.csproj b/SAMA.Web/SAMA.Web.csproj index 79ac2ea..6e421d5 100644 --- a/SAMA.Web/SAMA.Web.csproj +++ b/SAMA.Web/SAMA.Web.csproj @@ -17,11 +17,11 @@ - - - + + + - + runtime; build; native; contentfiles; analyzers; buildtransitive all From 252625d17f4709551333fc4a3e10ae72fd8d2428 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Mon, 16 Mar 2026 11:09:07 -0400 Subject: [PATCH 15/17] Add group test --- .github/workflows/ci.yml | 4 +- SAMA.Tests.System/OidcGroupTests.cs | 64 +++++++++++++++++++++++++++++ SAMA.Web/docker-compose.yml | 1 + 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 SAMA.Tests.System/OidcGroupTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a5ad53a..f24d8a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,10 +111,12 @@ jobs: SAMA_ENCRYPTION_KEY: test-encryption-key-for-ci-only steps: - - name: Prepare OIDC user (this is done via Docker CMD in local dev) + - name: Prepare OIDC users (this is done via Docker CMD in local dev) run: | curl 'http://localhost:9400/oauth2/authorize?response_type=code&client_id=my-client-id&scope=openid+email&redirect_uri=http://localhost:9400' --data-raw 'sub=alice' curl -XPUT 'http://localhost:9400/users/alice' --json '{"email": "alice@example.com", "name": "Alice"}' + curl 'http://localhost:9400/oauth2/authorize?response_type=code&client_id=my-client-id&scope=openid+email&redirect_uri=http://localhost:9400' --data-raw 'sub=bob' + curl -XPUT 'http://localhost:9400/users/bob' --json '{"email": "bob@example.com", "name": "Bob", "groups": ["test-editors"]}' - uses: actions/checkout@v6 - name: Set up dotnet uses: actions/setup-dotnet@v5 diff --git a/SAMA.Tests.System/OidcGroupTests.cs b/SAMA.Tests.System/OidcGroupTests.cs new file mode 100644 index 0000000..0385ba7 --- /dev/null +++ b/SAMA.Tests.System/OidcGroupTests.cs @@ -0,0 +1,64 @@ +using Microsoft.Playwright; + +using static Microsoft.Playwright.Assertions; + +namespace SAMA.Tests.System; + +[TestClass] +[SystemTestCondition] +public class OidcGroupTests : SystemTestBase +{ + [TestMethod] + public async Task ShouldAssignWorkspaceAccessViaOidcGroupClaim() + { + await SetupInitialAdminAsync(); + await LoginAsync(); + + // Create a workspace for group-based access + await Page.GotoAsync($"{BaseUrl}/Workspaces/Create"); + await Page.FillAsync("input[name='Input.Name']", "OIDC Group Workspace"); + await Page.Locator("button[type='submit']:has-text('Create')").ClickAsync(); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + // Configure OIDC to use the mock provider + await Page.GotoAsync($"{BaseUrl}/Admin/Settings/Oidc"); + await Page.CheckAsync("input[name='OidcInput.Enabled']"); + await Page.FillAsync("input[name='OidcInput.Authority']", "http://localhost:9400"); + await Page.FillAsync("input[name='OidcInput.ProviderName']", "Mock OIDC"); + await Page.FillAsync("input[name='OidcInput.ClientId']", "test-client"); + await Page.FillAsync("input[name='OidcInput.ClientSecret']", "test-secret"); + await Page.ClickAsync("button[type='submit']"); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); + await Expect(Page.Locator("text=OIDC settings saved successfully")).ToBeVisibleAsync(); + + // Add a group mapping: "test-editors" group → OIDC Group Workspace → Editor + await Page.GotoAsync($"{BaseUrl}/Admin/Settings/GroupMappings"); + await Page.FillAsync("input[name='Input.ExternalGroupId']", "test-editors"); + await Page.Locator("select[name='Input.WorkspaceId']").SelectOptionAsync( + new SelectOptionValue { Label = "OIDC Group Workspace" }); + await Page.Locator("select[name='Input.Role']").SelectOptionAsync("Editor"); + await Page.Locator("select[name='Input.IdentityProvider']").SelectOptionAsync("OIDC"); + await Page.Locator("button:has-text('Add Group Mapping')").ClickAsync(); + await Page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + // Verify the mapping was created + await Expect(Page.Locator("text=test-editors")).ToBeVisibleAsync(); + + await LogoutAsync(); + + // Login as bob via OIDC + await Page.GotoAsync($"{BaseUrl}/Account/Login"); + await Page.Locator("a:has-text('Sign in with Mock OIDC')").ClickAsync(); + + // On the mock provider, click the predefined "bob" user button + await Page.Locator("button:has-text('bob')").ClickAsync(); + + // Bob should be redirected back to the app and auto-redirected to the workspace dashboard + // (since he has exactly one accessible workspace via group mapping) + await Page.WaitForURLAsync($"{BaseUrl}/Dashboard**"); + await Expect(Page.Locator("text=OIDC Group Workspace")).ToBeVisibleAsync(); + + // Verify Bob has Editor access (Create Check button is editor-only) + await Expect(Page.Locator("a:has-text('Create Check')")).ToBeVisibleAsync(); + } +} diff --git a/SAMA.Web/docker-compose.yml b/SAMA.Web/docker-compose.yml index 36c200a..69dda5b 100644 --- a/SAMA.Web/docker-compose.yml +++ b/SAMA.Web/docker-compose.yml @@ -28,6 +28,7 @@ services: restart: unless-stopped command: > --user-claims '{"sub": "alice", "email": "alice@example.com", "name": "Alice"}' + --user-claims '{"sub": "bob", "email": "bob@example.com", "name": "Bob", "groups": ["test-editors"]}' ports: - 127.0.0.1:9400:9400 From 0d3c3f6d435425c922255fc0695cc8f155e2aa50 Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:39:35 -0400 Subject: [PATCH 16/17] Update docs --- README.md | 4 ++-- SAMA.Tests.System/README.md | 3 ++- docs/ARCHITECTURE.md | 10 ++++++++-- docs/ROADMAP.md | 11 ++++++----- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 26a9062..458408f 100644 --- a/README.md +++ b/README.md @@ -39,12 +39,12 @@ SAMA is a comprehensive service availability monitoring solution that helps you: ### Authentication & SSO - **LDAP/Active Directory**: Full LDAP authentication with direct bind and search+bind modes, StartTLS, custom Root CA support -- **Group-Based Provisioning**: Just-in-time user provisioning and workspace role assignment via LDAP group mappings +- **OIDC SSO**: OpenID Connect authentication with any OIDC-compliant provider (Azure AD/Entra, Okta, Auth0, Keycloak, etc.) +- **Group-Based Provisioning**: Just-in-time user provisioning and workspace role assignment via LDAP/OIDC group mappings ### Future Enhancements (Phase 2+) - **Geo-Distributed Agents**: Run checks from multiple regions - **Advanced Check Types**: Playwright-based browser automation, Database connectivity checks -- **OIDC SSO**: OpenID Connect authentication (Azure AD/Entra, Okta, Auth0, etc.) - **Audit Logging**: Track all configuration changes ### Security & Compliance diff --git a/SAMA.Tests.System/README.md b/SAMA.Tests.System/README.md index 485d75c..e916c4d 100644 --- a/SAMA.Tests.System/README.md +++ b/SAMA.Tests.System/README.md @@ -13,7 +13,8 @@ System tests are **skipped by default** to avoid running expensive E2E tests dur ## Prerequisites 1. PostgreSQL running (same as development) -2. Build the solution first: `dotnet build` +2. Support services running (from `SAMA.Web/`): `docker compose up -d` (provides SMTP, OIDC mock provider, LDAP mock) +3. Build the solution first: `dotnet build` 3. Install Playwright browsers (one-time): ```powershell # Windows diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 339b93a..1c5d636 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -239,8 +239,14 @@ SAMA provides two complementary notification systems: - Account linking: existing local users matched by email are linked to LDAP; local password login is then disabled - Group membership queries for role/workspace auto-provisioning +**OIDC/OpenID Connect**: +- Authorization Code flow with any OIDC-compliant provider (Azure AD/Entra, Okta, Auth0, Keycloak, etc.) +- Admin UI for provider configuration (authority, client ID/secret, scopes, claim types) +- JIT user provisioning on successful OIDC login +- Configurable group claim extraction for workspace role auto-provisioning +- Account linking: existing local users matched by email are linked to OIDC; local password login is then disabled + **Future**: -- OIDC integration (Azure AD/Entra, Okta, Auth0, etc.) - SAML2 SSO support may be added based on demand ### Authorization @@ -366,10 +372,10 @@ SAMA provides two complementary notification systems: - Automatic migrations on startup - LDAP/Active Directory authentication - Group-based workspace provisioning (LDAP) +- OIDC authentication with group-based workspace provisioning ### Phase 2 - Agent support with advanced checks (Playwright, Database) -- OIDC authentication - Enhanced alerting rules ### Phase 3 diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 43f137c..7a1218b 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -110,10 +110,10 @@ Note, these are all subject to change. - [x] LDAP server configuration UI - [x] Group membership queries - [ ] Testing with Active Directory and OpenLDAP -- [ ] OIDC Integration - - [ ] OIDC authentication handler - - [ ] OIDC provider configuration UI - - [ ] User claim mapping +- [x] OIDC Integration + - [x] OIDC authentication handler + - [x] OIDC provider configuration UI + - [x] User claim mapping - [ ] Testing with common providers (Azure AD/Entra, Okta, Auth0) - [x] Enhanced Authorization (LDAP) - [x] WorkspaceGroupMappings table and UI @@ -135,7 +135,7 @@ Note, these are all subject to change. - [ ] Agent application tests - [ ] Multi-region scenario tests - [x] LDAP/AD authentication tests - - [ ] OIDC flow tests + - [x] OIDC flow tests - [ ] Multi-provider tests - [ ] Documentation - [ ] Agent deployment guide @@ -143,6 +143,7 @@ Note, these are all subject to change. - [ ] API key setup guide - [ ] LDAP/AD setup guide - [ ] OIDC setup guide + - [ ] OIDC group mapping guide - [ ] Common IdP configurations --- From 11fab93baf54cfdd582aaeec5e1139e7961568ac Mon Sep 17 00:00:00 2001 From: Sasha Kotlyar <120513939+arktronic-sep@users.noreply.github.com> Date: Mon, 16 Mar 2026 15:00:07 -0400 Subject: [PATCH 17/17] Always include openid scope --- .../Services/OidcPostConfigureOptionsTests.cs | 21 ++++++++++++++++--- SAMA.Web/Pages/Admin/Settings/Oidc.cshtml | 7 ++++--- SAMA.Web/Services/GlobalSettingsService.cs | 2 +- SAMA.Web/Services/OidcPostConfigureOptions.cs | 1 + 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs index 7d9f70f..94c4709 100644 --- a/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs +++ b/SAMA.Tests.Unit/Web/Services/OidcPostConfigureOptionsTests.cs @@ -58,7 +58,7 @@ public void PostConfigureShouldMapSettingsWhenEnabled() _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com/tenant"); _mockGlobalSettings.OidcClientId.Returns("my-client-id"); _mockGlobalSettings.OidcClientSecret.Returns("my-secret"); - _mockGlobalSettings.OidcScopes.Returns("openid profile email"); + _mockGlobalSettings.OidcScopes.Returns("profile email"); _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); @@ -70,6 +70,7 @@ public void PostConfigureShouldMapSettingsWhenEnabled() Assert.IsFalse(_options.MapInboundClaims); Assert.AreEqual("name", _options.TokenValidationParameters.NameClaimType); Assert.AreEqual("my-client-id", _options.TokenValidationParameters.ValidAudience); + CollectionAssert.Contains(_options.Scope.ToArray(), "openid"); } [TestMethod] @@ -79,7 +80,7 @@ public void PostConfigureShouldSplitAndSetScopes() _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com"); _mockGlobalSettings.OidcClientId.Returns("client"); _mockGlobalSettings.OidcClientSecret.Returns(""); - _mockGlobalSettings.OidcScopes.Returns("openid profile email groups"); + _mockGlobalSettings.OidcScopes.Returns("profile email groups"); _options.Scope.Add("pre-existing"); _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); @@ -97,7 +98,7 @@ public void PostConfigureShouldHandleExtraWhitespaceInScopes() _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com"); _mockGlobalSettings.OidcClientId.Returns("client"); _mockGlobalSettings.OidcClientSecret.Returns(""); - _mockGlobalSettings.OidcScopes.Returns("openid profile email"); + _mockGlobalSettings.OidcScopes.Returns("profile email"); _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); @@ -106,4 +107,18 @@ public void PostConfigureShouldHandleExtraWhitespaceInScopes() new[] { "openid", "profile", "email" }, _options.Scope.ToArray()); } + + [TestMethod] + public void PostConfigureShouldAlwaysIncludeOpenidScope() + { + _mockGlobalSettings.OidcEnabled.Returns(true); + _mockGlobalSettings.OidcAuthority.Returns("https://login.example.com"); + _mockGlobalSettings.OidcClientId.Returns("client"); + _mockGlobalSettings.OidcClientSecret.Returns(""); + _mockGlobalSettings.OidcScopes.Returns("profile email"); + + _postConfigure.PostConfigure(AuthConstants.OidcSource, _options); + + CollectionAssert.Contains(_options.Scope.ToArray(), "openid"); + } } diff --git a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml index 82ec05e..1810a6b 100644 --- a/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml +++ b/SAMA.Web/Pages/Admin/Settings/Oidc.cshtml @@ -110,10 +110,11 @@
    - +
    - Space-separated OAuth scopes. Default: openid profile email. - Add additional scopes if your provider requires them for group claims. + Additional OAuth scopes to request. The openid scope is always included automatically. + Default: profile email. + Add extra scopes if your provider requires them for group claims.
    diff --git a/SAMA.Web/Services/GlobalSettingsService.cs b/SAMA.Web/Services/GlobalSettingsService.cs index 71bf559..4079a2e 100644 --- a/SAMA.Web/Services/GlobalSettingsService.cs +++ b/SAMA.Web/Services/GlobalSettingsService.cs @@ -64,7 +64,7 @@ public class GlobalSettingsService(IServiceProvider _serviceProvider, ILogger