From 84b138f4314143ce8142ad5357c2bdc5e2397224 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Fri, 12 Dec 2025 10:26:29 -0800 Subject: [PATCH] (Auth) [PM-27108] Add OrgId checks in SSO Process (#6710) --- .../src/Sso/Controllers/AccountController.cs | 30 ++++- .../Controllers/AccountControllerTest.cs | 127 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs index bc26fb270a..7141f8429d 100644 --- a/bitwarden_license/src/Sso/Controllers/AccountController.cs +++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs @@ -201,12 +201,15 @@ public class AccountController : Controller returnUrl, state = context.Parameters["state"], userIdentifier = context.Parameters["session_state"], + ssoToken }); } [HttpGet] - public IActionResult ExternalChallenge(string scheme, string returnUrl, string state, string userIdentifier) + public IActionResult ExternalChallenge(string scheme, string returnUrl, string state, string userIdentifier, string ssoToken) { + ValidateSchemeAgainstSsoToken(scheme, ssoToken); + if (string.IsNullOrEmpty(returnUrl)) { returnUrl = "~/"; @@ -235,6 +238,31 @@ public class AccountController : Controller return Challenge(props, scheme); } + /// + /// Validates the scheme (organization ID) against the organization ID found in the ssoToken. + /// + /// The authentication scheme (organization ID) to validate. + /// The SSO token to validate against. + /// Thrown if the scheme (organization ID) does not match the organization ID found in the ssoToken. + private void ValidateSchemeAgainstSsoToken(string scheme, string ssoToken) + { + SsoTokenable tokenable; + + try + { + tokenable = _dataProtector.Unprotect(ssoToken); + } + catch + { + throw new Exception(_i18nService.T("InvalidSsoToken")); + } + + if (!Guid.TryParse(scheme, out var schemeOrgId) || tokenable.OrganizationId != schemeOrgId) + { + throw new Exception(_i18nService.T("SsoOrganizationIdMismatch")); + } + } + [HttpGet] public async Task ExternalCallback() { diff --git a/bitwarden_license/test/SSO.Test/Controllers/AccountControllerTest.cs b/bitwarden_license/test/SSO.Test/Controllers/AccountControllerTest.cs index c04948e21f..b276174814 100644 --- a/bitwarden_license/test/SSO.Test/Controllers/AccountControllerTest.cs +++ b/bitwarden_license/test/SSO.Test/Controllers/AccountControllerTest.cs @@ -3,6 +3,7 @@ using System.Security.Claims; using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.Registration; @@ -10,6 +11,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.Sso.Controllers; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -1137,4 +1139,129 @@ public class AccountControllerTest Assert.NotNull(result.user); Assert.Equal(email, result.user.Email); } + + [Theory, BitAutoData] + public void ExternalChallenge_WithMatchingOrgId_Succeeds( + SutProvider sutProvider, + Organization organization) + { + // Arrange + var orgId = organization.Id; + var scheme = orgId.ToString(); + var returnUrl = "~/vault"; + var state = "test-state"; + var userIdentifier = "user-123"; + var ssoToken = "valid-sso-token"; + + // Mock the data protector to return a tokenable with matching org ID + var dataProtector = sutProvider.GetDependency>(); + var tokenable = new SsoTokenable(organization, 3600); + dataProtector.Unprotect(ssoToken).Returns(tokenable); + + // Mock URL helper for IsLocalUrl check + var urlHelper = Substitute.For(); + urlHelper.IsLocalUrl(returnUrl).Returns(true); + sutProvider.Sut.Url = urlHelper; + + // Mock interaction service for IsValidReturnUrl check + var interactionService = sutProvider.GetDependency(); + interactionService.IsValidReturnUrl(returnUrl).Returns(true); + + // Act + var result = sutProvider.Sut.ExternalChallenge(scheme, returnUrl, state, userIdentifier, ssoToken); + + // Assert + var challengeResult = Assert.IsType(result); + Assert.Contains(scheme, challengeResult.AuthenticationSchemes); + Assert.NotNull(challengeResult.Properties); + Assert.Equal(scheme, challengeResult.Properties.Items["scheme"]); + Assert.Equal(returnUrl, challengeResult.Properties.Items["return_url"]); + Assert.Equal(state, challengeResult.Properties.Items["state"]); + Assert.Equal(userIdentifier, challengeResult.Properties.Items["user_identifier"]); + } + + [Theory, BitAutoData] + public void ExternalChallenge_WithMismatchedOrgId_ThrowsSsoOrganizationIdMismatch( + SutProvider sutProvider, + Organization organization) + { + // Arrange + var correctOrgId = organization.Id; + var wrongOrgId = Guid.NewGuid(); + var scheme = wrongOrgId.ToString(); // Different from tokenable's org ID + var returnUrl = "~/vault"; + var state = "test-state"; + var userIdentifier = "user-123"; + var ssoToken = "valid-sso-token"; + + // Mock the data protector to return a tokenable with different org ID + var dataProtector = sutProvider.GetDependency>(); + var tokenable = new SsoTokenable(organization, 3600); // Contains correctOrgId + dataProtector.Unprotect(ssoToken).Returns(tokenable); + + // Mock i18n service to return the key + sutProvider.GetDependency() + .T(Arg.Any()) + .Returns(ci => (string)ci[0]!); + + // Act & Assert + var ex = Assert.Throws(() => + sutProvider.Sut.ExternalChallenge(scheme, returnUrl, state, userIdentifier, ssoToken)); + Assert.Equal("SsoOrganizationIdMismatch", ex.Message); + } + + [Theory, BitAutoData] + public void ExternalChallenge_WithInvalidSchemeFormat_ThrowsSsoOrganizationIdMismatch( + SutProvider sutProvider, + Organization organization) + { + // Arrange + var scheme = "not-a-valid-guid"; + var returnUrl = "~/vault"; + var state = "test-state"; + var userIdentifier = "user-123"; + var ssoToken = "valid-sso-token"; + + // Mock the data protector to return a valid tokenable + var dataProtector = sutProvider.GetDependency>(); + var tokenable = new SsoTokenable(organization, 3600); + dataProtector.Unprotect(ssoToken).Returns(tokenable); + + // Mock i18n service to return the key + sutProvider.GetDependency() + .T(Arg.Any()) + .Returns(ci => (string)ci[0]!); + + // Act & Assert + var ex = Assert.Throws(() => + sutProvider.Sut.ExternalChallenge(scheme, returnUrl, state, userIdentifier, ssoToken)); + Assert.Equal("SsoOrganizationIdMismatch", ex.Message); + } + + [Theory, BitAutoData] + public void ExternalChallenge_WithInvalidSsoToken_ThrowsInvalidSsoToken( + SutProvider sutProvider) + { + // Arrange + var orgId = Guid.NewGuid(); + var scheme = orgId.ToString(); + var returnUrl = "~/vault"; + var state = "test-state"; + var userIdentifier = "user-123"; + var ssoToken = "invalid-corrupted-token"; + + // Mock the data protector to throw when trying to unprotect + var dataProtector = sutProvider.GetDependency>(); + dataProtector.Unprotect(ssoToken).Returns(_ => throw new Exception("Token validation failed")); + + // Mock i18n service to return the key + sutProvider.GetDependency() + .T(Arg.Any()) + .Returns(ci => (string)ci[0]!); + + // Act & Assert + var ex = Assert.Throws(() => + sutProvider.Sut.ExternalChallenge(scheme, returnUrl, state, userIdentifier, ssoToken)); + Assert.Equal("InvalidSsoToken", ex.Message); + } }