Feature/risk level ip verification (#465)

* Add possibility to set BankID requirement risk. * Removed ip verification for same device. Since it is sometimes blocking legitimate transactions. Can be implemented outside ActiveLogin. * Update documentation. * Update documet for breaking changes. * Update version number. --------- Co-authored-by: Elin Fokine <[email protected]>
ActiveLogin · Aug 23, 2024 · 7f50fa3 · 7f50fa3
1 parent 88b1a53
commit 7f50fa3
Show file tree

Hide file tree

Showing 18 changed files with 78 additions and 205 deletions.
diff --git a/BREAKINGCHANGES.md b/BREAKINGCHANGES.md
@@ -25,7 +25,8 @@ Breaking changes between version 10.0.0 and 9.0.0
 * Remove policy handling from API library into Core
 * Fall back to mobile BankID policy for OtherDevice flow
 * Enum for setting policy instead of string
-
+* Allow only low risk level transactions by default.
+* Replaced expired embedded BankID certificate for test environment FPTestcert4_20220818.p12 with new certificate from BankID FPTestcert5_20240610.p12.
 
 ---
 

diff --git a/docs/articles/bankid.md b/docs/articles/bankid.md
@@ -64,7 +64,7 @@ BankID requires you to use a client certificate and trust a specific root CA-cer
 
 Read through the [BankID Relying Party Guidelines](https://www.bankid.com/utvecklare/guider). This ensures you have a basic understanding of the terminology as well as how the flow and security works.
 
-_For test:_ We have (with the permission from BankID) embedded the SSL certificate ([FPTestcert3_20200618.pfx](https://www.bankid.com/utvecklare/guider)) in the library.
+_For test:_ We have (with the permission from BankID) embedded the SSL certificate ([FPTestcert5_20240610.p12](https://www.bankid.com/utvecklare/guider)) in the library.
 _For production:_ Contact a [reseller](https://www.bankid.com/foretag/anslut-foeretag) to get your very own client certificate for production. This will probably take a few business days to get sorted. Please ask for "Direktupphandlad BankID" as they otherwise might refer you to a broker/partner. If you haven't decided on using BankID, but want to try it out anyway there are test- and simulation possibilities. See Environments below.
 
 The root CA-certificates specified in _BankID Relying Party Guidelines_ (#7 for Production and #8 for Test environment) needs to be trusted at the computer where the app will run. We have (with the permission from BankID) embedded these in the library for easy verification.
@@ -450,6 +450,11 @@ BankId options allows you to set and override some options such as these.
     // Limit possible login methods to, for example, only allow BankID on smartcard.
     // If no policy is set, it will fall back to require mobile BankID for OtherDevice flow
     options.BankIdCertificatePolicies = [ BankIdCertificatePolicy.BankIdOnFile, BankIdCertificatePolicy.BankIdOnSmartCard ];
+
+    // Set the acceptable risk level for the transaction. If the risk is higher than the specified level,
+    // the transaction will be blocked. The risk indication requires that the endUserIp is correct.
+    // An incorrect IP-address will result in legitimate transactions being blocked.
+    options.BankIdAllowedRiskLevel = BankIdAllowedRiskLevel.Low;
 });
 ```
 

diff --git a/src/ActiveLogin.Authentication.BankId.Api/Models/Requirement.cs b/src/ActiveLogin.Authentication.BankId.Api/Models/Requirement.cs
@@ -29,14 +29,13 @@ public class Requirement
     /// A personal number to be used to complete the transaction. If a BankID with another personal number attempts to sign the transaction, it fails.
     /// </param>
     /// <param name="risk">Set the acceptable risk level for the transaction.</param>
-    public Requirement(List<string>? certificatePolicies = null, bool? pinCode = null, bool? mrtd = null, string? personalNumber = null, RiskLevel? risk = null)
+    public Requirement(List<string>? certificatePolicies = null, string? risk = null, bool? pinCode = null, bool? mrtd = null, string? personalNumber = null)
     {
         CertificatePolicies = certificatePolicies;
+        Risk = risk;
         PinCode = pinCode;
         Mrtd = mrtd;
         PersonalNumber = personalNumber;
-
-        Risk = ParseRiskLevel(risk);
     }
 
     /// <summary>
@@ -77,19 +76,4 @@ public Requirement(List<string>? certificatePolicies = null, bool? pinCode = nul
     [JsonPropertyName("risk"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
     public string? Risk { get; }
 
-    private static string? ParseRiskLevel(RiskLevel? risk)
-    {
-        return risk switch
-        {
-            null => null,
-
-            RiskLevel.Low => RiskLevel.Low.ToString().ToLower(),
-            RiskLevel.Moderate => RiskLevel.Moderate.ToString().ToLower(),
-            RiskLevel.High => RiskLevel.High.ToString().ToLower(),
-
-            _ => throw new ArgumentException(
-                $"Risk must be {RiskLevel.Low}, {RiskLevel.Moderate} or {RiskLevel.High}",
-                nameof(risk))
-        };
-    }
 }
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Auth/BankIdAuthHandler.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Auth/BankIdAuthHandler.cs
@@ -159,6 +159,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
 
         var uiOptions = new BankIdUiOptions(
             Options.BankIdCertificatePolicies,
+            Options.BankIdAllowedRiskLevel,
             Options.BankIdSameDevice,
             Options.BankIdRequirePinCode,
             Options.BankIdRequireMrtd,

diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Auth/BankIdAuthOptions.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Auth/BankIdAuthOptions.cs
@@ -1,4 +1,5 @@
 using ActiveLogin.Authentication.BankId.Core.CertificatePolicies;
+using ActiveLogin.Authentication.BankId.Core.Risk;
 
 using Microsoft.AspNetCore.Authentication;
 using Microsoft.AspNetCore.Http;
@@ -31,6 +32,13 @@ public class BankIdAuthOptions : RemoteAuthenticationOptions
     /// </summary>
     public bool BankIdRequireMrtd { get; set; } = false;
 
+    /// <summary>
+    /// Set the acceptable risk level for the transaction. If the risk is higher than the specified level,
+    /// the transaction will be blocked. The risk indication requires that the endUserIp is correct.
+    /// An incorrect IP-address will result in legitimate transactions being blocked.
+    /// </summary>
+    public BankIdAllowedRiskLevel BankIdAllowedRiskLevel { get; set; } = BankIdAllowedRiskLevel.Low;
+
     /// <summary>
     /// Auto launch the BankID app on the current device.
     /// </summary>

diff --git a/...uthentication.BankId.AspNetCore/DataProtection/Serialization/BankIdUiOptionsSerializer.cs b/...uthentication.BankId.AspNetCore/DataProtection/Serialization/BankIdUiOptionsSerializer.cs
@@ -1,5 +1,6 @@
 using ActiveLogin.Authentication.BankId.AspNetCore.Models;
 using ActiveLogin.Authentication.BankId.Core.CertificatePolicies;
+using ActiveLogin.Authentication.BankId.Core.Risk;
 
 namespace ActiveLogin.Authentication.BankId.AspNetCore.DataProtection.Serialization;
 
@@ -10,6 +11,7 @@ internal class BankIdUiOptionsSerializer : BankIdDataSerializer<BankIdUiOptions>
     protected override void Write(BinaryWriter writer, BankIdUiOptions model)
     {
         writer.Write(string.Join(CertificatePoliciesSeparator.ToString(), model.CertificatePolicies));
+        writer.Write(model.AllowedRiskLevel.ToString());
         writer.Write(model.SameDevice);
         writer.Write(model.RequirePinCode);
         writer.Write(model.RequireMrtd);
@@ -28,8 +30,11 @@ protected override BankIdUiOptions Read(BinaryReader reader)
                                     .Select(Enum.Parse<BankIdCertificatePolicy>)
                                     .ToList();
 
+        var riskLevel = Enum.TryParse<BankIdAllowedRiskLevel>(reader.ReadString(), out var allowedRiskLevel) ? allowedRiskLevel : BankIdAllowedRiskLevel.Low;
+
         return new BankIdUiOptions(
             certificatePolicies,
+            riskLevel,
             reader.ReadBoolean(),
             reader.ReadBoolean(),
             reader.ReadBoolean(),

diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Models/BankIdUiOptions.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Models/BankIdUiOptions.cs
@@ -1,18 +1,21 @@
 using ActiveLogin.Authentication.BankId.Core.CertificatePolicies;
+using ActiveLogin.Authentication.BankId.Core.Risk;
 
 namespace ActiveLogin.Authentication.BankId.AspNetCore.Models;
 
 public class BankIdUiOptions
 {
     public BankIdUiOptions(
         List<BankIdCertificatePolicy> certificatePolicies,
+        BankIdAllowedRiskLevel allowedRiskLevel,
         bool sameDevice,
         bool requirePinCode,
         bool requireMrtd,
         string cancelReturnUrl,
         string stateCookieName)
     {
         CertificatePolicies = certificatePolicies;
+        AllowedRiskLevel = allowedRiskLevel;
         SameDevice = sameDevice;
         RequirePinCode = requirePinCode;
         RequireMrtd = requireMrtd;
@@ -28,6 +31,8 @@ public BankIdUiOptions(
 
     public bool RequireMrtd { get; }
 
+    public BankIdAllowedRiskLevel AllowedRiskLevel { get; }
+
     public string CancelReturnUrl { get; }
 
     public string StateCookieName { get; }

diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Models/BankIdUiOptionsExtensions.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Models/BankIdUiOptionsExtensions.cs
@@ -6,6 +6,7 @@ public static class BankIdUiOptionsExtensions
 {
     public static BankIdFlowOptions ToBankIdFlowOptions(this BankIdUiOptions options) => new(
         options.CertificatePolicies,
+        options.AllowedRiskLevel,
         options.SameDevice,
         options.RequirePinCode,
         options.RequireMrtd

diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Sign/BankIdSignOptions.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Sign/BankIdSignOptions.cs
@@ -1,4 +1,5 @@
 using ActiveLogin.Authentication.BankId.Core.CertificatePolicies;
+using ActiveLogin.Authentication.BankId.Core.Risk;
 
 using Microsoft.AspNetCore.Http;
 
@@ -22,6 +23,13 @@ public class BankIdSignOptions
     /// </summary>
     public bool BankIdRequireMrtd { get; set; } = false;
 
+    /// <summary>
+    /// Set the acceptable risk level for the transaction. If the risk is higher than the specified level,
+    /// the transaction will be blocked. The risk indication requires that the endUserIp is correct.
+    /// An incorrect IP-address will result in legitimate transactions being blocked.
+    /// </summary>
+    public BankIdAllowedRiskLevel BankIdAllowedRiskLevel { get; set; } = BankIdAllowedRiskLevel.Low;
+
     /// <summary>
     /// Auto launch the BankID app on the current device.
     /// </summary>

diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Sign/BankIdSignService.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Sign/BankIdSignService.cs
@@ -63,6 +63,7 @@ public Task InitiateSignAsync(BankIdSignProperties properties, string callbackPa
 
         var uiOptions = new BankIdUiOptions(
             options.BankIdCertificatePolicies,
+            options.BankIdAllowedRiskLevel,
             options.BankIdSameDevice,
             options.BankIdRequirePinCode,
             options.BankIdRequireMrtd,

diff --git a/src/ActiveLogin.Authentication.BankId.Core/Flow/BankIdFlowService.cs b/src/ActiveLogin.Authentication.BankId.Core/Flow/BankIdFlowService.cs
@@ -100,8 +100,9 @@ private async Task<AuthRequest> GetAuthRequest(BankIdFlowOptions flowOptions)
     {
         var endUserIp = _bankIdEndUserIpResolver.GetEndUserIp();
         var resolvedCertificatePolicies = GetResolvedCertificatePolicies(flowOptions);
+        var resolvedRiskLevel = flowOptions.AllowedRiskLevel.ToString().ToLower();
 
-        var authRequestRequirement = new Requirement(resolvedCertificatePolicies, flowOptions.RequirePinCode, flowOptions.RequireMrtd);
+        var authRequestRequirement = new Requirement(resolvedCertificatePolicies, resolvedRiskLevel, flowOptions.RequirePinCode, flowOptions.RequireMrtd);
 
         var authRequestContext = new BankIdAuthRequestContext(endUserIp, authRequestRequirement);
         var userData = await _bankIdAuthUserDataResolver.GetUserDataAsync(authRequestContext);
@@ -158,7 +159,9 @@ private SignRequest GetSignRequest(BankIdFlowOptions flowOptions, BankIdSignData
     {
         var endUserIp = _bankIdEndUserIpResolver.GetEndUserIp();
         var resolvedCertificatePolicies = GetResolvedCertificatePolicies(flowOptions);
-        var requestRequirement = new Requirement(resolvedCertificatePolicies, flowOptions.RequirePinCode, flowOptions.RequireMrtd, flowOptions.RequiredPersonalIdentityNumber?.To12DigitString());
+        var resolvedRiskLevel = flowOptions.AllowedRiskLevel.ToString().ToLower();
+
+        var requestRequirement = new Requirement(resolvedCertificatePolicies, resolvedRiskLevel, flowOptions.RequirePinCode, flowOptions.RequireMrtd, flowOptions.RequiredPersonalIdentityNumber?.To12DigitString());
 
         return new SignRequest(
             endUserIp,
@@ -216,17 +219,6 @@ public async Task<BankIdFlowCollectResult> Collect(string orderRef, int autoStar
                 {
                     throw new InvalidOperationException("Missing CompletionData from BankID API");
                 }
-
-                // Verify that it is the same device that started the BankID flow
-                if (flowOptions.SameDevice)
-                {
-                    var deviceIp = collectResponse.CompletionData.Device.IpAddress;
-                    var endUserIp = _bankIdEndUserIpResolver.GetEndUserIp();
-                    if (!deviceIp.Equals(endUserIp))
-                    {
-                        throw new InvalidOperationException("The device that completed the BankID flow is not the same as the device that started the flow");
-                    }
-                }
 
                 await _bankIdEventTrigger.TriggerAsync(new BankIdCollectCompletedEvent(collectResponse.OrderRef, collectResponse.CompletionData, detectedUserDevice, flowOptions));
                 return new BankIdFlowCollectResultComplete(collectResponse.CompletionData);

diff --git a/src/ActiveLogin.Authentication.BankId.Core/Models/BankIdFlowOptions.cs b/src/ActiveLogin.Authentication.BankId.Core/Models/BankIdFlowOptions.cs
@@ -1,4 +1,5 @@
 using ActiveLogin.Authentication.BankId.Core.CertificatePolicies;
+using ActiveLogin.Authentication.BankId.Core.Risk;
 using ActiveLogin.Identity.Swedish;
 
 namespace ActiveLogin.Authentication.BankId.Core.Models;
@@ -7,12 +8,14 @@ public class BankIdFlowOptions
 {
     public BankIdFlowOptions(
         List<BankIdCertificatePolicy> certificatePolicies,
+        BankIdAllowedRiskLevel allowedRiskLevel,
         bool sameDevice,
         bool requirePinCode,
         bool requireMrtd,
         PersonalIdentityNumber? requiredPersonalIdentityNumber = null)
     {
         CertificatePolicies = certificatePolicies;
+        AllowedRiskLevel = allowedRiskLevel;
         SameDevice = sameDevice;
         RequirePinCode = requirePinCode;
         RequireMrtd = requireMrtd;
@@ -27,6 +30,8 @@ public BankIdFlowOptions(
 
     public bool RequireMrtd { get; }
 
+    public BankIdAllowedRiskLevel AllowedRiskLevel { get; }
+
     public PersonalIdentityNumber? RequiredPersonalIdentityNumber { get; }
 
 }
diff --git a/src/ActiveLogin.Authentication.BankId.Core/Risk/BankIdAllowedRiskLevel.cs b/src/ActiveLogin.Authentication.BankId.Core/Risk/BankIdAllowedRiskLevel.cs
@@ -0,0 +1,14 @@
+namespace ActiveLogin.Authentication.BankId.Core.Risk;
+
+public enum BankIdAllowedRiskLevel
+{
+    /// <summary>
+    /// Only accept low risk orders.
+    /// </summary>
+    Low,
+
+    /// <summary>
+    /// Accept low and moderate risk orders
+    /// </summary>
+    Moderate
+}
diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -15,7 +15,7 @@
         <PackageId>$(AssemblyName)</PackageId>
 
         <VersionPrefix>10.0.0</VersionPrefix>
-        <VersionSuffix>beta-1</VersionSuffix>
+        <VersionSuffix>rc-1</VersionSuffix>
         <AssemblyVersion>10.0.0.0</AssemblyVersion>
         <FileVersion Condition="'$(BUILD_BUILDNUMBER)' == ''">$(VersionPrefix).0</FileVersion>
         <FileVersion Condition="'$(BUILD_BUILDNUMBER)' != ''">$(VersionPrefix).$(BUILD_BUILDNUMBER)</FileVersion>

diff --git a/test/ActiveLogin.Authentication.BankId.Api.Test/BankIdAppApiClient_Tests.cs b/test/ActiveLogin.Authentication.BankId.Api.Test/BankIdAppApiClient_Tests.cs
@@ -106,7 +106,7 @@ public async Task AuthAsync_WithRequirements__ShouldPostJsonPayload_WithRequirem
         // Arrange
 
         // Act
-        await _bankIdAppApiClient.AuthAsync(new AuthRequest("1.1.1.1", new Requirement(new List<string> { "req1", "req2" }, true, true, "190001010101")));
+        await _bankIdAppApiClient.AuthAsync(new AuthRequest("1.1.1.1", new Requirement(new List<string> { "req1", "req2" }, "low", true, true, "190001010101")));
 
         // Assert
         var request = _messageHandlerMock.GetFirstArgumentOfFirstInvocation<HttpMessageHandler, HttpRequestMessage>();
@@ -292,7 +292,7 @@ public async Task SignAsync_WithRequirements__ShouldPostJsonPayload_WithRequirem
         // Arrange
 
         // Act
-        await _bankIdAppApiClient.SignAsync(new SignRequest("1.1.1.1", "userVisibleData", Encoding.UTF8.GetBytes("userNonVisibleData"), new Requirement(new List<string> { "req1", "req2" }, true, true, "190001010101")));
+        await _bankIdAppApiClient.SignAsync(new SignRequest("1.1.1.1", "userVisibleData", Encoding.UTF8.GetBytes("userNonVisibleData"), new Requirement(new List<string> { "req1", "req2" }, "low", true, true, "190001010101")));
 
         // Assert
         var request = _messageHandlerMock.GetFirstArgumentOfFirstInvocation<HttpMessageHandler, HttpRequestMessage>();

diff --git a/test/ActiveLogin.Authentication.BankId.AspNetCore.Test/BankId_UiAuth_Tests.cs b/test/ActiveLogin.Authentication.BankId.AspNetCore.Test/BankId_UiAuth_Tests.cs
@@ -46,7 +46,7 @@ public BankId_UiAuth_Tests()
         _bankIdUiOptionsProtector = new Mock<IBankIdUiOptionsProtector>();
         _bankIdUiOptionsProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
-            .Returns(new BankIdUiOptions(new List<BankIdCertificatePolicy>(), false, false, false, "/", DefaultStateCookieName));
+            .Returns(new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, false, false, false, "/", DefaultStateCookieName));
         _bankIdUiOptionsProtector
             .Setup(protector => protector.Protect(It.IsAny<BankIdUiOptions>()))
             .Returns("Ignored");
@@ -220,7 +220,7 @@ public async Task Authentication_UI_Should_Be_Accessible_Even_When_Site_Requires
     public async Task Init_Returns_Ui_With_Resolved_Cancel_Url()
     {
         // Arrange
-        var options = new BankIdUiOptions(new List<BankIdCertificatePolicy>(), true, false, false, "~/cru", DefaultStateCookieName);
+        var options = new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, true, false, false, "~/cru", DefaultStateCookieName);
         _bankIdUiOptionsProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
             .Returns(options);
@@ -368,7 +368,7 @@ public async Task AutoLaunch_Sets_Correct_RedirectUri()
     {
         // Arrange mocks
         var autoLaunchOptions =
-            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), true, false, false, string.Empty, DefaultStateCookieName);
+            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, true, false, false, string.Empty, DefaultStateCookieName);
         var mockProtector = new Mock<IBankIdUiOptionsProtector>();
         mockProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
@@ -424,7 +424,7 @@ public async Task Api_Always_Returns_CamelCase_Json_For_Http200Ok()
     {
         // Arrange mocks
         var autoLaunchOptions =
-            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), false, false, false, string.Empty, DefaultStateCookieName);
+            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, false, false, false, string.Empty, DefaultStateCookieName);
         var mockProtector = new Mock<IBankIdUiOptionsProtector>();
         mockProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
@@ -480,7 +480,7 @@ public async Task Api_Always_Returns_CamelCase_Json_For_Http400BadRequest()
     {
         // Arrange mocks
         var autoLaunchOptions =
-            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), false, false, false, string.Empty, DefaultStateCookieName);
+            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, false, false, false, string.Empty, DefaultStateCookieName);
         _bankIdUiOptionsProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
             .Returns(autoLaunchOptions);
@@ -526,7 +526,7 @@ public async Task Cancel_Calls_CancelApi()
     {
         // Arrange mocks
         var autoLaunchOptions =
-            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), false, false, false, string.Empty, DefaultStateCookieName);
+            new BankIdUiOptions(new List<BankIdCertificatePolicy>(), Core.Risk.BankIdAllowedRiskLevel.Low, false, false, false, string.Empty, DefaultStateCookieName);
         _bankIdUiOptionsProtector
             .Setup(protector => protector.Unprotect(It.IsAny<string>()))
             .Returns(autoLaunchOptions);