diff --git a/docs/oidc-resource-server.md b/docs/oidc-resource-server.md index fb5e95c8e..fc5594f23 100644 --- a/docs/oidc-resource-server.md +++ b/docs/oidc-resource-server.md @@ -126,20 +126,30 @@ customer in this order. 1. No `sub` claim: 401. 2. A stored link for `sub` (provider `ExternalAuth.OpenIdConnect`, exact, case-sensitive match) selects the customer. The customer must be active, not deleted, registered, and not a system account; otherwise 403. An inactive customer - is never reactivated. + is never reactivated. A link made by e-mail (point 4) is checked again on every use: once the account has + back-office access or GrandNode two-factor authentication, the link no longer serves it (403) until the account is + connected with its password. 3. Without a link the e-mail claim is needed (missing: 401). With `RequireEmailVerified = true` it must be verified (otherwise 401). -4. A customer with the same e-mail address is linked on first use, and the link is stored. - * With `RequireEmailVerified = true` this **includes back-office accounts** (administrators, vendors, store staff): - the identity provider is trusted for verified e-mail addresses. Secure the provider accordingly (verify e-mail - addresses, restrict self-registration and e-mail changes). - * With `RequireEmailVerified = false` back-office accounts are **never** linked by e-mail. +4. A customer with the same e-mail address is linked on first use, and the link is stored, marked as made by e-mail. + * **Back-office accounts** (admin panel access, vendors, store staff) and accounts that use **GrandNode two-factor + authentication** (enabled for the customer and in the store) are **never** linked by e-mail, verified or not + (403 without a link). A provider where someone can mark an address as verified without owning it (an + administrator of another organization or realm creating a user) must not be enough to take over such an account. + They are connected once with the account's password: through the browser sign-in plugin (section 11), which asks + for the store password and two-factor code; their tokens then use that link. + * Ordinary customers are linked by a verified e-mail. With `RequireEmailVerified = false` they are linked by an + unverified one too: only for providers that never send `email_verified` and do not let users choose an address. + The browser sign-in does not take over such a link as it is: it asks for the password whenever the provider + does not report the e-mail as verified. 5. System accounts and guests are never mapped. 6. One identity per customer: when the customer is already linked to another `sub`, the new subject is refused (403). One identity per customer is enforced by the link's identifier, which is derived from the customer, so two concurrent requests with different subjects cannot both link the same customer. - Links do not record the provider instance, so after switching `Authority` to another provider the customers - linked by the previous one are refused until their link is removed. + Links do not record the provider instance, and a `sub` is unique only within one provider. Before switching + `Authority` to another provider (or realm), remove the `ExternalAuth.OpenIdConnect` links: a subject of the new + provider equal to a stored one would select the old customer, and customers linked by the previous provider would be + refused as linked to another identity. 7. With `RegisterCustomersPerStore` enabled an account of another store is refused (403) and lookups are store-scoped. **Per-store identity.** With `RegisterCustomersPerStore` enabled the same person can have a separate customer in each @@ -169,6 +179,16 @@ A token alone grants no rights. To let a user call the API: 2. in GrandNode add the (auto-registered or existing) customer to a **customer group** whose permissions include *Access admin panel* and the permissions the calls need (or use a group created for API access). +The customer must be **linked** to the provider identity, and a link made by e-mail does not serve an account with +admin panel access, a vendor or a staff store (section 3.1). Either: + +* connect the account once with the browser sign-in plugin (section 11), which asks for its password; or +* without that plugin, let the token register the account (`AutoRegister = true`, section 3.2) and grant the access + to that account: a link made by the registration is not a link made by e-mail. + +Granting back-office access to a customer linked by e-mail stops its tokens (403) until the account is connected with +its password. + ### 3.4 Resource conditions After the mapping the resource's conditions apply; a refusal is 403 without `WWW-Authenticate`: @@ -528,7 +548,8 @@ After the provider signed the user in, the result depends on the GrandNode side: | Situation | Result | |---|---| -| A link for the provider's `sub` exists | The linked customer is signed in. Staff included; the admin panel access follows their existing permissions. The customer must be active, not deleted and registered. GrandNode two-factor authentication is not asked for; the provider's multi-factor policy applies. | +| A link for the provider's `sub` made by e-mail (by an access token, or by this sign-in without a password), and the account is now **staff** or uses **GrandNode two-factor authentication**, or the provider did **not** verify the e-mail | "Connect your account" (as below); the password sign-in confirms the existing link. | +| A link for the provider's `sub` exists (other cases) | The linked customer is signed in. Staff included; the admin panel access follows their existing permissions. The customer must be active, not deleted and registered. GrandNode two-factor authentication is not asked for; the provider's multi-factor policy applies. | | No link; an account with the same e-mail exists, the provider **verified** the e-mail, the account is **not staff** and does not use GrandNode two-factor authentication | The account is linked and signed in at once. | | No link; the account is **staff** (admin panel access, vendor, store staff), the account uses **GrandNode two-factor authentication** (enabled for the customer and in the store), or the provider did **not** verify the e-mail | "Connect your account": the page ends any current store session, and the visitor signs in **once** with the store password (and the two-factor code, when enabled). The connection is then completed (`/oidc/link/complete`); the signed-in account must have the same e-mail address. | | No account with that e-mail | The regular registration form opens with the e-mail taken from the provider (read-only). When the provider **verified** the e-mail, the form has **no password fields** and the new account is connected to the provider identity. When it did **not**, the customer chooses a password as in any registration and the account is **not** connected; the customer connects it later with "Connect your account" (row above). Store registration type "Standard" becomes e-mail validation when the provider did not verify the e-mail; "E-mail validation" becomes standard when it did; "Admin approval" stays admin approval either way. New accounts get the **Registered** group only. | @@ -645,9 +666,10 @@ Use the project of section 7. Create a **separate** application for the browser staff account always needs the store password (and its 2FA). * The button never replaces another signed-in account: a visitor signed in to the store as someone else gets the failure page and stays signed in. -* A staff account is never linked by e-mail alone, but the one who completes the password sign-in and holds the - provider identity owns the link; keep the provider's e-mail verification and e-mail change policy strict (same advice - as section 3.1). +* A staff account is never linked by e-mail alone, neither here nor by an access token (section 3.1), and a link + made by e-mail before the account became staff asks for the password again; but the one who + completes the password sign-in and holds the provider identity owns the link; keep the provider's e-mail + verification and e-mail change policy strict. * **Logout is local.** Signing out of GrandNode does not end the provider session; the next click on the button signs in again without a prompt while the provider session lasts. * Provider tokens are not stored. The only data kept from the provider is the `sub`/e-mail link, plus the pending diff --git a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs index 97e590f13..c013a3fa1 100644 --- a/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs +++ b/src/Business/Grand.Business.Authentication/Services/OidcCustomerResolver.cs @@ -22,7 +22,9 @@ namespace Grand.Business.Authentication.Services; /// /// Maps a validated access token or a browser sign-in to a customer: stored subject link; customer with the verified e-mail (link -/// stored); registration with the Registered group only. Only active, registered, non-system accounts are used, +/// stored and marked as made by e-mail; never a back-office account or an account with two-factor authentication, which connect +/// only with their password); registration with the Registered group only. A link made by e-mail is re-checked on every use +/// against the rules of a first link. Only active, registered, non-system accounts are used, /// and one customer holds at most one identity (enforced by the link's identifier, see ). /// With per-store identity a subject has one link per store (plus the link of a customer of no store, valid in /// every store). A unique index on (provider, identifier, store) makes the first concurrent requests of one user @@ -32,6 +34,9 @@ public class OidcCustomerResolver : IOidcCustomerResolver { public const string ProviderSystemName = "ExternalAuth.OpenIdConnect"; + public const string AccountConnectionRequiredDescription = + "The account must be connected to the identity with its password first"; + private readonly IRepository _externalAuthenticationRepository; private readonly ICustomerService _customerService; private readonly ICustomerManagerService _customerManagerService; @@ -77,7 +82,15 @@ public virtual async Task Resolve(ClaimsPrincipal principal, var link = await GetLink(sub, storeId); if (link != null) - return await Usable(await _customerService.GetCustomerById(link.CustomerId), sub, storeId); + { + var linkedCustomer = await _customerService.GetCustomerById(link.CustomerId); + var linkedResult = await Usable(linkedCustomer, sub, storeId); + //a link made by e-mail proves no more than the e-mail: it stops serving an account that has since + //become one that connects only with its password + if (linkedResult.Succeeded && link.LinkedByEmail && await RequiresPassword(linkedCustomer, storeId)) + return AccountConnectionRequired(linkedCustomer, sub); + return linkedResult; + } var email = principal.FindFirst(_config.Claims.Email)?.Value; if (string.IsNullOrWhiteSpace(email)) @@ -98,20 +111,17 @@ public virtual async Task Resolve(ClaimsPrincipal principal, if (!result.Succeeded) return result; - //an e-mail nobody verified could take over a back-office account, so those are never linked by it - if (!_config.RequireEmailVerified && await IsBackOfficeAccount(customer)) - { - _logger.LogWarning("Access token subject {Subject}: customer {CustomerId} is a back-office account and the e-mail is not verified", - sub, customer.Id); - return OidcCustomerResult.Forbidden("Back-office accounts can only be linked with a verified e-mail"); - } - //one identity per customer - a second subject with the same e-mail must not take over the account; the //link's identifier enforces it against concurrent requests, this check gives the answer in the normal case if (await HasOtherLink(customer, sub)) return LinkedToOtherIdentity(customer, sub); - var (linked, other) = await LinkOrGetWinner(customer, sub, email, storeId); + //a back-office account and an account protected by two-factor authentication are never linked by e-mail, + //verified or not; they connect with the account's password (browser sign-in), and tokens then use that link + if (await RequiresPassword(customer, storeId)) + return AccountConnectionRequired(customer, sub); + + var (linked, other) = await LinkOrGetWinner(customer, sub, email, storeId, true); if (other != null) return LinkedToOtherIdentity(customer, sub); if (linked?.Id != customer.Id) @@ -132,7 +142,14 @@ public virtual async Task ResolveForLogin(string sub, string em { var linked = await _customerService.GetCustomerById(link.CustomerId); var reason = await Check(linked, sub, storeId); - return reason == Unusable.None ? OidcLoginResult.SignIn(linked) : OidcLoginResult.Refused(ToRefusal(reason)); + if (reason != Unusable.None) + return OidcLoginResult.Refused(ToRefusal(reason)); + + //a link made by e-mail (by an access token or an earlier sign-in) is held to the rules of a first link + if (link.LinkedByEmail && (!emailVerified || await RequiresPassword(linked, storeId))) + return OidcLoginResult.LinkRequired(linked); + + return OidcLoginResult.SignIn(linked); } if (string.IsNullOrWhiteSpace(email)) @@ -164,16 +181,19 @@ public virtual async Task ResolveForLogin(string sub, string em //the first link of a back-office account, of an account protected by two-factor authentication, and of any account //whose e-mail nobody verified, needs the account's password (and its second factor) - if (!emailVerified || await IsBackOfficeAccount(customer) || await HasTwoFactor(customer, storeId)) + if (!emailVerified || await RequiresPassword(customer, storeId)) return OidcLoginResult.LinkRequired(customer); - return await LinkIdentity(customer, sub, email, storeId) switch { + return await Link(customer, sub, email, storeId, true) switch { OidcLinkResult.Linked => OidcLoginResult.SignIn(customer), _ => OidcLoginResult.Refused(OidcLoginRefusal.LinkedToOtherIdentity) }; } - public virtual async Task LinkIdentity(Customer customer, string sub, string email, string storeId) + public virtual Task LinkIdentity(Customer customer, string sub, string email, string storeId) => + Link(customer, sub, email, storeId, false); + + private async Task Link(Customer customer, string sub, string email, string storeId, bool byEmail) { ArgumentNullException.ThrowIfNull(customer); ArgumentException.ThrowIfNullOrEmpty(sub); @@ -181,12 +201,20 @@ public virtual async Task LinkIdentity(Customer customer, string //the link this store would use - with per-store identity a link of another store does not block this one var existing = await GetLink(sub, storeId); if (existing != null) - return existing.CustomerId == customer.Id ? OidcLinkResult.Linked : OidcLinkResult.SubjectLinkedToOtherCustomer; + { + if (existing.CustomerId != customer.Id) + return OidcLinkResult.SubjectLinkedToOtherCustomer; + + //the account's password now stands behind a link made by e-mail + if (!byEmail && existing.LinkedByEmail) + await _externalAuthenticationRepository.UpdateField(existing.Id, x => x.LinkedByEmail, false); + return OidcLinkResult.Linked; + } if (await HasOtherLink(customer, sub)) return OidcLinkResult.CustomerLinkedToOtherSubject; - var (winner, other) = await LinkOrGetWinner(customer, sub, email ?? customer.Email ?? "", storeId); + var (winner, other) = await LinkOrGetWinner(customer, sub, email ?? customer.Email ?? "", storeId, byEmail); if (other != null) return OidcLinkResult.CustomerLinkedToOtherSubject; if (winner?.Id != customer.Id) @@ -237,7 +265,7 @@ private async Task Register(string sub, string email, string await _customerManagerService.RegisterCustomer(new RegistrationRequest(customer, normalizedEmail, normalizedEmail, CommonHelper.GenerateRandomDigitCode(20), PasswordFormat.Hashed, storeId)); - var (linked, other) = await LinkOrGetWinner(customer, sub, email, storeId); + var (linked, other) = await LinkOrGetWinner(customer, sub, email, storeId, false); if (other != null || linked?.Id != customer.Id) { //lost the race: drop the account just created and use the one the winner linked @@ -300,13 +328,20 @@ private OidcCustomerResult LinkedToOtherIdentity(Customer customer, string sub) return OidcCustomerResult.Forbidden("The customer account is linked to another identity"); } + private OidcCustomerResult AccountConnectionRequired(Customer customer, string sub) + { + _logger.LogWarning("Access token subject {Subject}: customer {CustomerId} must be connected with its password", + sub, customer.Id); + return OidcCustomerResult.Forbidden(AccountConnectionRequiredDescription); + } + /// /// Stores the link under the customer's . When a concurrent request stored a link /// first: the link of another subject now holding this customer (OtherLink), or the customer the subject's /// winning link points to (Customer). Rethrows when the insert failed for another reason. /// private async Task<(Customer Customer, ExternalAuthentication OtherLink)> LinkOrGetWinner(Customer customer, - string sub, string email, string storeId) + string sub, string email, string storeId, bool byEmail) { var linkId = OidcLinkId(customer.Id); try @@ -318,7 +353,8 @@ await _externalAuthenticationRepository.InsertAsync(new ExternalAuthentication { ExternalIdentifier = sub, ExternalDisplayIdentifier = email, ProviderSystemName = ProviderSystemName, - StoreId = LinkStoreId(customer) + StoreId = LinkStoreId(customer), + LinkedByEmail = byEmail }); return (customer, null); } @@ -411,6 +447,10 @@ private async Task HasTwoFactor(Customer customer, string storeId) => customer.GetUserFieldFromEntity(SystemCustomerFieldNames.TwoFactorEnabled) && (await _settingService.LoadSetting(storeId)).TwoFactorAuthenticationEnabled; + //accounts that are never linked by e-mail alone, and whose links made by e-mail stop serving them + private async Task RequiresPassword(Customer customer, string storeId) => + await IsBackOfficeAccount(customer) || await HasTwoFactor(customer, storeId); + private async Task IsBackOfficeAccount(Customer customer) => !string.IsNullOrEmpty(customer.VendorId) || !string.IsNullOrEmpty(customer.StaffStoreId) || await _permissionService.Authorize(StandardPermission.ManageAccessAdminPanel, customer); diff --git a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcCustomerResolver.cs b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcCustomerResolver.cs index 532648bcd..38f70af1d 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcCustomerResolver.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Authentication/IOidcCustomerResolver.cs @@ -6,7 +6,8 @@ namespace Grand.Business.Core.Interfaces.Authentication; /// /// Maps a validated access token or a browser sign-in of the identity provider to a customer: by the stored link to the token -/// subject, on first use by the verified e-mail address, otherwise by registering one +/// subject, on first use by the verified e-mail address (never a back-office account or an account with two-factor +/// authentication: those are connected with their password), otherwise by registering one /// public interface IOidcCustomerResolver { @@ -15,7 +16,8 @@ public interface IOidcCustomerResolver Task Resolve(ClaimsPrincipal principal, string storeId); /// - /// Browser sign-in: never registers and never links a back-office account or an unverified e-mail by itself + /// Browser sign-in: never registers, and never links (or signs in through a link made by e-mail) a back-office + /// account, an account with two-factor authentication or an unverified e-mail by itself /// /// Subject of the provider identity /// E-mail of the provider identity @@ -25,7 +27,8 @@ public interface IOidcCustomerResolver /// /// Stores the link between the provider subject and the customer (one subject per customer, one customer per subject; - /// with per-store identity one customer per subject and store) + /// with per-store identity one customer per subject and store). Call it only once the account is proven (password + /// sign-in, registration): the link counts as made with the password, also when it was first made by e-mail /// /// Customer to link /// Subject of the provider identity diff --git a/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs b/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs index 30647d7b0..d657c2c5c 100644 --- a/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs +++ b/src/Core/Grand.Domain/Customers/ExternalAuthentication.cs @@ -39,4 +39,9 @@ public class ExternalAuthentication : BaseEntity /// Gets or sets the store of the linked customer when customers are registered per store; empty otherwise /// public string StoreId { get; set; } + + /// + /// Gets or sets a value indicating whether the link was made by the e-mail address alone, without the account's password + /// + public bool LinkedByEmail { get; set; } } \ No newline at end of file diff --git a/src/Core/Grand.Infrastructure/Configuration/OidcResourceServerConfig.cs b/src/Core/Grand.Infrastructure/Configuration/OidcResourceServerConfig.cs index 85e4dc718..6269a0dae 100644 --- a/src/Core/Grand.Infrastructure/Configuration/OidcResourceServerConfig.cs +++ b/src/Core/Grand.Infrastructure/Configuration/OidcResourceServerConfig.cs @@ -19,8 +19,8 @@ public class OidcResourceServerConfig /// /// True: a token whose e-mail is not verified never links to an account and never registers one. False: the - /// e-mail in the token is trusted as is, except that back-office accounts (admin panel access, vendor, store - /// staff) are never linked by it + /// e-mail in the token is trusted as is. In both modes back-office accounts (admin panel access, vendor, store + /// staff) and accounts with two-factor authentication are never linked by e-mail /// public bool RequireEmailVerified { get; set; } = true; diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverLoginTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverLoginTests.cs index 7e76798bf..aa4006dba 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverLoginTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverLoginTests.cs @@ -59,9 +59,10 @@ private OidcCustomerResolver Resolver(IRepository links _groupService.Object, _permissionService.Object, _mediator.Object, _config, _customerConfig, NullLogger.Instance); private Task Link(string customerId, string sub = "sub-1", string provider = OidcCustomerResolver.ProviderSystemName, - string storeId = null) => + string storeId = null, bool byEmail = false) => _links.InsertAsync(new ExternalAuthentication { - CustomerId = customerId, ProviderSystemName = provider, ExternalIdentifier = sub, StoreId = storeId + CustomerId = customerId, ProviderSystemName = provider, ExternalIdentifier = sub, StoreId = storeId, + LinkedByEmail = byEmail }); private List LinksOf(string sub) => @@ -107,6 +108,7 @@ public async Task ExistingCustomer_Verified_LinkedAndSignIn() var link = await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1"); Assert.AreEqual("c1", link.CustomerId); Assert.AreEqual(OidcCustomerResolver.ProviderSystemName, link.ProviderSystemName); + Assert.IsTrue(link.LinkedByEmail); } [TestMethod] @@ -179,6 +181,50 @@ public async Task LinkedTwoFactorCustomer_SignIn() Assert.AreEqual(OidcLoginOutcome.SignIn, (await Resolver().ResolveForLogin("sub-1", "a@b.c", true, Store)).Outcome); } + [TestMethod] + public async Task LinkMadeByEmail_PlainCustomer_Verified_SignIn() + { + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(new Customer { Id = "c1", Active = true }); + await Link("c1", byEmail: true); + Assert.AreEqual(OidcLoginOutcome.SignIn, (await Resolver().ResolveForLogin("sub-1", "a@b.c", true, Store)).Outcome); + } + + [TestMethod] + public async Task LinkMadeByEmail_Unverified_LinkRequired() + { + //an access token linked the account by an unverified e-mail (RequireEmailVerified off) + var customer = new Customer { Id = "c1", Active = true }; + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(customer); + await Link("c1", byEmail: true); + var result = await Resolver().ResolveForLogin("sub-1", "a@b.c", false, Store); + Assert.AreEqual(OidcLoginOutcome.LinkRequired, result.Outcome); + Assert.AreSame(customer, result.Customer); + } + + [TestMethod] + public async Task LinkMadeByEmail_TwoFactorCustomer_LinkRequired() + { + _customerSettings.TwoFactorAuthenticationEnabled = true; + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(TwoFactorCustomer()); + await Link("c1", byEmail: true); + Assert.AreEqual(OidcLoginOutcome.LinkRequired, (await Resolver().ResolveForLogin("sub-1", "a@b.c", true, Store)).Outcome); + } + + [TestMethod] + [DataRow(null, null)] + [DataRow("vendor1", null)] + [DataRow(null, "store1")] + public async Task LinkMadeByEmail_BackOfficeAccount_LinkRequired(string vendorId, string staffStoreId) + { + //linked while it was an ordinary customer, given back-office access later + var customer = new Customer { Id = "c1", Active = true, VendorId = vendorId, StaffStoreId = staffStoreId }; + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(customer); + if (vendorId == null && staffStoreId == null) + _permissionService.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, customer)).ReturnsAsync(true); + await Link("c1", byEmail: true); + Assert.AreEqual(OidcLoginOutcome.LinkRequired, (await Resolver().ResolveForLogin("sub-1", "a@b.c", true, Store)).Outcome); + } + [TestMethod] public async Task LinkedAdmin_SignIn() { @@ -239,6 +285,22 @@ public async Task LinkIdentity_StoresLink() Assert.AreEqual("a@b.c", link.Email); } + [TestMethod] + public async Task LinkIdentity_NotMadeByEmail() + { + await Resolver().LinkIdentity(new Customer { Id = "c1" }, "sub-1", "a@b.c", Store); + Assert.IsFalse((await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")).LinkedByEmail); + } + + [TestMethod] + public async Task LinkIdentity_LinkMadeByEmail_NowMadeWithPassword() + { + //the password sign-in that LinkRequired asked for stands behind the existing link + await Link("c1", byEmail: true); + Assert.AreEqual(OidcLinkResult.Linked, await Resolver().LinkIdentity(new Customer { Id = "c1" }, "sub-1", "a@b.c", Store)); + Assert.IsFalse((await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")).LinkedByEmail); + } + [TestMethod] public async Task LinkIdentity_SameCustomerTwice_Linked() { diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverTests.cs index 5015ecd76..96ad7dbf3 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/OidcCustomerResolverTests.cs @@ -8,6 +8,7 @@ using Grand.Business.Core.Utilities.Customers; using Grand.Data; using Grand.Data.Tests.MongoDb; +using Grand.Domain.Common; using Grand.Domain.Customers; using Grand.Domain.Permissions; using Grand.Infrastructure.Configuration; @@ -68,9 +69,10 @@ private static ClaimsPrincipal Principal(string sub = "sub-1", string email = "J } private Task Link(string customerId, string sub = "sub-1", string provider = OidcCustomerResolver.ProviderSystemName, - string storeId = null) => + string storeId = null, bool byEmail = false) => _links.InsertAsync(new ExternalAuthentication { - CustomerId = customerId, ProviderSystemName = provider, ExternalIdentifier = sub, StoreId = storeId + CustomerId = customerId, ProviderSystemName = provider, ExternalIdentifier = sub, StoreId = storeId, + LinkedByEmail = byEmail }); private List LinksOf(string sub) => @@ -334,50 +336,132 @@ public async Task EmailMatchesCustomerLinkedToAnotherSubject_Forbidden_NotLinked Assert.IsNull(await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")); } + private static Customer TwoFactorCustomer(string id = "c1") + { + var customer = new Customer { Id = id, Active = true }; + customer.UserFields.Add(new UserField { Key = SystemCustomerFieldNames.TwoFactorEnabled, Value = "True", StoreId = "" }); + return customer; + } + + //admin panel access, vendor, store staff, two-factor authentication + private Customer PasswordOnlyCustomer(string kind) + { + var customer = kind switch { + "vendor" => new Customer { Id = "c1", Active = true, VendorId = "vendor1" }, + "staff" => new Customer { Id = "c1", Active = true, StaffStoreId = "store1" }, + "twoFactor" => TwoFactorCustomer(), + _ => new Customer { Id = "c1", Active = true } + }; + if (kind == "admin") + _permissionService.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, customer)).ReturnsAsync(true); + _customerSettings.TwoFactorAuthenticationEnabled = true; + return customer; + } + [TestMethod] - public async Task UnverifiedEmail_AdminPanelAccount_Forbidden_NotLinked() + [DataRow("admin", true)] + [DataRow("admin", false)] + [DataRow("vendor", true)] + [DataRow("vendor", false)] + [DataRow("staff", true)] + [DataRow("staff", false)] + [DataRow("twoFactor", true)] + [DataRow("twoFactor", false)] + public async Task PasswordOnlyAccount_NotLinkedByEmail(string kind, bool emailVerified) { - _config.RequireEmailVerified = false; - var admin = new Customer { Id = "c1", Active = true }; - _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(admin); - _permissionService.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, admin)).ReturnsAsync(true); + _config.RequireEmailVerified = emailVerified; + _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(PasswordOnlyCustomer(kind)); + + var result = await Resolver().Resolve(Principal(emailVerified: emailVerified ? "true" : null), "store1"); - Assert.AreEqual(OidcFailureKind.Forbidden, (await Resolver().Resolve(Principal(emailVerified: null), "store1")).Failure); + Assert.AreEqual(OidcFailureKind.Forbidden, result.Failure); + Assert.AreEqual(OidcCustomerResolver.AccountConnectionRequiredDescription, result.Description); Assert.IsNull(await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")); } [TestMethod] - [DataRow("vendor1", null)] - [DataRow(null, "store1")] - public async Task UnverifiedEmail_VendorOrStaffAccount_Forbidden_NotLinked(string vendorId, string staffStoreId) + public async Task TwoFactorCustomer_StoreTwoFactorOff_LinkedByEmail() { - _config.RequireEmailVerified = false; - _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(new Customer { - Id = "c1", Active = true, VendorId = vendorId, StaffStoreId = staffStoreId - }); + _customerSettings.TwoFactorAuthenticationEnabled = false; + _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(TwoFactorCustomer()); - Assert.AreEqual(OidcFailureKind.Forbidden, (await Resolver().Resolve(Principal(emailVerified: null), "store1")).Failure); - Assert.IsNull(await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")); + Assert.IsTrue((await Resolver().Resolve(Principal(), "store1")).Succeeded); + Assert.IsTrue((await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")).LinkedByEmail); } [TestMethod] - public async Task UnverifiedEmail_PlainRegisteredCustomer_Linked() + public async Task PasswordOnlyAccount_LinkedToOtherSubject_LinkedToOtherIdentity() { - _config.RequireEmailVerified = false; + await Link("c1", sub: "other-sub"); + _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(PasswordOnlyCustomer("admin")); + + var result = await Resolver().Resolve(Principal(), "store1"); + + Assert.AreEqual(OidcFailureKind.Forbidden, result.Failure); + Assert.AreEqual("The customer account is linked to another identity", result.Description); + } + + [TestMethod] + [DataRow("true")] + [DataRow(null)] + public async Task PlainRegisteredCustomer_LinkedByEmail(string emailVerified) + { + _config.RequireEmailVerified = emailVerified != null; _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(new Customer { Id = "c1", Active = true }); - Assert.IsTrue((await Resolver().Resolve(Principal(emailVerified: null), "store1")).Succeeded); - Assert.AreEqual("c1", (await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")).CustomerId); + Assert.IsTrue((await Resolver().Resolve(Principal(emailVerified: emailVerified), "store1")).Succeeded); + var link = await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1"); + Assert.AreEqual("c1", link.CustomerId); + Assert.IsTrue(link.LinkedByEmail); + } + + [TestMethod] + [DataRow("admin")] + [DataRow("vendor")] + [DataRow("staff")] + [DataRow("twoFactor")] + public async Task PasswordOnlyAccount_LinkMadeWithPassword_Succeeds(string kind) + { + //connected once with the account's password (browser sign-in); the link then serves tokens + await Link("c1"); + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(PasswordOnlyCustomer(kind)); + + Assert.IsTrue((await Resolver().Resolve(Principal(), "store1")).Succeeded); + } + + [TestMethod] + [DataRow("admin")] + [DataRow("vendor")] + [DataRow("staff")] + [DataRow("twoFactor")] + public async Task LinkMadeByEmail_AccountNowPasswordOnly_Forbidden(string kind) + { + //linked by e-mail while it was an ordinary customer, given back-office access or two-factor later + await Link("c1", byEmail: true); + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(PasswordOnlyCustomer(kind)); + + var result = await Resolver().Resolve(Principal(), "store1"); + + Assert.AreEqual(OidcFailureKind.Forbidden, result.Failure); + Assert.AreEqual(OidcCustomerResolver.AccountConnectionRequiredDescription, result.Description); } [TestMethod] - public async Task VerifiedEmail_BackOfficeAccount_Linked() + public async Task LinkMadeByEmail_PlainCustomer_Succeeds() { - var admin = new Customer { Id = "c1", Active = true, StaffStoreId = "store1" }; - _customerService.Setup(x => x.GetCustomerByEmail("John@Example.com", "")).ReturnsAsync(admin); - _permissionService.Setup(x => x.Authorize(StandardPermission.ManageAccessAdminPanel, admin)).ReturnsAsync(true); + await Link("c1", byEmail: true); + _customerService.Setup(x => x.GetCustomerById("c1")).ReturnsAsync(new Customer { Id = "c1", Active = true }); + + Assert.IsTrue((await Resolver().Resolve(Principal(), "store1")).Succeeded); + } + + [TestMethod] + public async Task Registration_LinkNotMadeByEmail() + { + _config.AutoRegister = true; Assert.IsTrue((await Resolver().Resolve(Principal(), "store1")).Succeeded); + Assert.IsFalse((await _links.GetOneAsync(x => x.ExternalIdentifier == "sub-1")).LinkedByEmail); } [TestMethod]