rebase and implement nonce for returnUrl

Zonnex · Zonnex · commit d9c066b88135 · 2025-03-26T16:19:02.000+01:00
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiApiControllerBase.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiApiControllerBase.cs
@@ -8,6 +8,7 @@
 using ActiveLogin.Authentication.BankId.AspNetCore.DataProtection;
 using ActiveLogin.Authentication.BankId.AspNetCore.Helpers;
 using ActiveLogin.Authentication.BankId.AspNetCore.Models;
+using ActiveLogin.Authentication.BankId.Core;
 using ActiveLogin.Authentication.BankId.Core.Flow;
 using ActiveLogin.Authentication.BankId.Core.UserMessage;
 
@@ -25,6 +26,7 @@ public abstract class BankIdUiApiControllerBase : ControllerBase
     protected readonly IBankIdUiOrderRefProtector OrderRefProtector;
     protected readonly IBankIdQrStartStateProtector QrStartStateProtector;
     protected readonly IBankIdUiOptionsProtector UiOptionsProtector;
+    protected readonly IStateStorage _stateStorage;
 
     private readonly IBankIdUserMessage _bankIdUserMessage;
     private readonly IBankIdUserMessageLocalizer _bankIdUserMessageLocalizer;
@@ -38,7 +40,8 @@ protected BankIdUiApiControllerBase(
 
         IBankIdUserMessage bankIdUserMessage,
         IBankIdUserMessageLocalizer bankIdUserMessageLocalizer,
-        IBankIdUiResultProtector uiAuthResultProtector
+        IBankIdUiResultProtector uiAuthResultProtector,
+        IStateStorage stateStorage
 
     )
     {
@@ -50,6 +53,7 @@ IBankIdUiResultProtector uiAuthResultProtector
         _bankIdUserMessage = bankIdUserMessage;
         _bankIdUserMessageLocalizer = bankIdUserMessageLocalizer;
         _uiAuthResultProtector = uiAuthResultProtector;
+        _stateStorage = stateStorage;
     }
 
     [ValidateAntiForgeryToken]
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiAuthApiController.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiAuthApiController.cs
@@ -4,6 +4,7 @@
 using ActiveLogin.Authentication.BankId.AspNetCore.DataProtection;
 using ActiveLogin.Authentication.BankId.AspNetCore.Helpers;
 using ActiveLogin.Authentication.BankId.AspNetCore.Models;
+using ActiveLogin.Authentication.BankId.Core;
 using ActiveLogin.Authentication.BankId.Core.Flow;
 using ActiveLogin.Authentication.BankId.Core.UserMessage;
 
@@ -26,8 +27,10 @@ public BankIdUiAuthApiController(
         IBankIdUiOptionsProtector uiOptionsProtector,
         IBankIdUserMessage bankIdUserMessage,
         IBankIdUserMessageLocalizer bankIdUserMessageLocalizer,
-        IBankIdUiResultProtector uiAuthResultProtector)
-        : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector)
+        IBankIdUiResultProtector uiAuthResultProtector,
+        IStateStorage stateStorage
+        )
+        : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector, stateStorage)
     {
     }
 
@@ -43,13 +46,17 @@ public async Task<ActionResult<BankIdUiApiInitializeResponse>> Initialize(BankId
         BankIdFlowInitializeResult bankIdFlowInitializeResult;
         try
         {
+            var nonce = Guid.NewGuid().ToString();
             var returnRedirectUrl = Url.Action(BankIdConstants.Routes.BankIdAuthInitActionName, BankIdConstants.Routes.BankIdAuthControllerName, new
             {
                 returnUrl = request.ReturnUrl,
-                uiOptions = request.UiOptions
+                uiOptions = request.UiOptions,
+                nonce
             }, protocol: Request.Scheme) ?? throw new Exception(BankIdConstants.ErrorMessages.CouldNotGetUrlFor(BankIdConstants.Routes.BankIdAuthControllerName, BankIdConstants.Routes.BankIdAuthInitActionName));
 
             bankIdFlowInitializeResult = await BankIdFlowService.InitializeAuth(uiOptions.ToBankIdFlowOptions(), returnRedirectUrl);
+
+            await _stateStorage.SetAsync(new(nonce), returnRedirectUrl);
         }
         catch (BankIdApiException bankIdApiException)
         {
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiAuthController.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiAuthController.cs
@@ -27,8 +27,8 @@ IStateStorage stateStorage
 {
     [HttpGet]
     [Route($"/[area]/{BankIdConstants.Routes.BankIdPathName}/{BankIdConstants.Routes.BankIdAuthControllerPath}")]
-    public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions)
+    public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string uiOptions, [FromQuery(Name = "nonce")] string nonce)
     {
-        return Initialize(returnUrl, BankIdConstants.Routes.BankIdAuthApiControllerName, protectedUiOptions, "Init");
+        return Initialize(returnUrl, BankIdConstants.Routes.BankIdAuthApiControllerName, uiOptions, nonce, "Init");
     }
 }
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiControllerBase.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiControllerBase.cs
@@ -53,11 +53,18 @@ IStateStorage stateStorage
         return await stateStorage.GetAsync<T>(stateKey);
     }
 
-    protected async Task<ActionResult> Initialize(string returnUrl, string apiControllerName, string protectedUiOptions, string viewName)
+    protected async Task<ActionResult> Initialize(string returnUrl, string apiControllerName, string protectedUiOptions, string nonce, string viewName)
     {
         Validators.ThrowIfNullOrWhitespace(returnUrl);
         Validators.ThrowIfNullOrWhitespace(protectedUiOptions, BankIdConstants.QueryStringParameters.UiOptions);
 
+        if (await stateStorage.TryGetAsync<string>(new StateKey(nonce), out var rru)) {
+            // if we have a saved url, it should be perfect match with returnUrl
+            if (rru != returnUrl) {
+                throw new ArgumentException("ReturnUrl mismatch");
+            }
+        }
+
         if (!Url.IsLocalUrl(returnUrl))
         {
             throw new ArgumentException(BankIdConstants.ErrorMessages.InvalidReturnUrl);
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiSignApiController.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiSignApiController.cs
@@ -22,8 +22,6 @@ namespace ActiveLogin.Authentication.BankId.AspNetCore.Areas.ActiveLogin.Control
 [NonController]
 public class BankIdUiSignApiController : BankIdUiApiControllerBase
 {
-    private readonly IStateStorage stateStorage;
-
     public BankIdUiSignApiController(
         IBankIdFlowService bankIdFlowService,
         IBankIdUiOrderRefProtector orderRefProtector,
@@ -33,9 +31,8 @@ public BankIdUiSignApiController(
         IBankIdUserMessageLocalizer bankIdUserMessageLocalizer,
         IBankIdUiResultProtector uiAuthResultProtector,
         IStateStorage stateStorage
-    ) : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector)
+    ) : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector, stateStorage)
     {
-        this.stateStorage = stateStorage;
     }
 
     [ValidateAntiForgeryToken]
@@ -115,6 +112,6 @@ public async Task<ActionResult<BankIdUiApiInitializeResponse>> Initialize(BankId
         }
 
         var stateKey = new StateKey(cookie);
-        return stateStorage.GetAsync<BankIdUiSignState>(stateKey);
+        return _stateStorage.GetAsync<BankIdUiSignState>(stateKey);
     }
 }
diff --git a/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiSignController.cs b/src/ActiveLogin.Authentication.BankId.AspNetCore/Areas/ActiveLogin/Controllers/BankIdUiSignController.cs
@@ -33,8 +33,8 @@ IStateStorage stateStorage
     [HttpGet]
     [AllowAnonymous]
     [Route($"/[area]/{BankIdConstants.Routes.BankIdPathName}/{BankIdConstants.Routes.BankIdSignControllerPath}")]
-    public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions)
+    public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions, [FromQuery(Name = "nonce")] string nonce)
     {
-        return Initialize(returnUrl, BankIdConstants.Routes.BankIdSignApiControllerName, protectedUiOptions, "Init");
+        return Initialize(returnUrl, BankIdConstants.Routes.BankIdSignApiControllerName, protectedUiOptions, nonce, "Init");
     }
 }
diff --git a/src/ActiveLogin.Authentication.BankId.Core/IStateStorage.cs b/src/ActiveLogin.Authentication.BankId.Core/IStateStorage.cs
@@ -14,6 +14,7 @@ public interface IStateStorage
     Task<T?> GetAsync<T>(StateKey key);
     Task<bool> TryGetAsync<T>(StateKey key, [NotNullWhen(true)] out T? value);
     Task<StateKey> SetAsync<T>(T value);
+    Task<StateKey> SetAsync<T>(StateKey key, T value);
 }
 
 public static class BankIdBuilderStateStorageExtensions
diff --git a/src/ActiveLogin.Authentication.BankId.Core/InMemoryStateStorage.cs b/src/ActiveLogin.Authentication.BankId.Core/InMemoryStateStorage.cs
@@ -34,4 +34,10 @@ public Task<StateKey> SetAsync<T>(T value)
         cache.Set(stateKey, value, _memoryCacheEntryOptions);
         return Task.FromResult(stateKey);
     }
+
+    public Task<StateKey> SetAsync<T>(StateKey key, T value)
+    {
+        cache.Set(key, value, _memoryCacheEntryOptions);
+        return Task.FromResult(key);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -27,8 +27,8 @@ IStateStorage stateStorage`
`27`	`27`	`{`
`28`	`28`	`[HttpGet]`
`29`	`29`	`[Route($"/[area]/{BankIdConstants.Routes.BankIdPathName}/{BankIdConstants.Routes.BankIdAuthControllerPath}")]`
`30`		`- public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions)`
	`30`	`+ public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string uiOptions, [FromQuery(Name = "nonce")] string nonce)`
`31`	`31`	`{`
`32`		`- return Initialize(returnUrl, BankIdConstants.Routes.BankIdAuthApiControllerName, protectedUiOptions, "Init");`
	`32`	`+ return Initialize(returnUrl, BankIdConstants.Routes.BankIdAuthApiControllerName, uiOptions, nonce, "Init");`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,11 +53,18 @@ IStateStorage stateStorage`
`53`	`53`	`return await stateStorage.GetAsync<T>(stateKey);`
`54`	`54`	`}`
`55`	`55`
`56`		`- protected async Task<ActionResult> Initialize(string returnUrl, string apiControllerName, string protectedUiOptions, string viewName)`
	`56`	`+ protected async Task<ActionResult> Initialize(string returnUrl, string apiControllerName, string protectedUiOptions, string nonce, string viewName)`
`57`	`57`	`{`
`58`	`58`	`Validators.ThrowIfNullOrWhitespace(returnUrl);`
`59`	`59`	`Validators.ThrowIfNullOrWhitespace(protectedUiOptions, BankIdConstants.QueryStringParameters.UiOptions);`
`60`	`60`
	`61`	`+ if (await stateStorage.TryGetAsync<string>(new StateKey(nonce), out var rru)) {`
	`62`	`+ // if we have a saved url, it should be perfect match with returnUrl`
	`63`	`+ if (rru != returnUrl) {`
	`64`	`+ throw new ArgumentException("ReturnUrl mismatch");`
	`65`	`+ }`
	`66`	`+ }`
	`67`	`+`
`61`	`68`	`if (!Url.IsLocalUrl(returnUrl))`
`62`	`69`	`{`
`63`	`70`	`throw new ArgumentException(BankIdConstants.ErrorMessages.InvalidReturnUrl);`
Original file line number	Diff line number	Diff line change
`@@ -22,8 +22,6 @@ namespace ActiveLogin.Authentication.BankId.AspNetCore.Areas.ActiveLogin.Control`
`22`	`22`	`[NonController]`
`23`	`23`	`public class BankIdUiSignApiController : BankIdUiApiControllerBase`
`24`	`24`	`{`
`25`		`- private readonly IStateStorage stateStorage;`
`26`		`-`
`27`	`25`	`public BankIdUiSignApiController(`
`28`	`26`	`IBankIdFlowService bankIdFlowService,`
`29`	`27`	`IBankIdUiOrderRefProtector orderRefProtector,`
`@@ -33,9 +31,8 @@ public BankIdUiSignApiController(`
`33`	`31`	`IBankIdUserMessageLocalizer bankIdUserMessageLocalizer,`
`34`	`32`	`IBankIdUiResultProtector uiAuthResultProtector,`
`35`	`33`	`IStateStorage stateStorage`
`36`		`- ) : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector)`
	`34`	`+ ) : base(bankIdFlowService, orderRefProtector, qrStartStateProtector, uiOptionsProtector, bankIdUserMessage, bankIdUserMessageLocalizer, uiAuthResultProtector, stateStorage)`
`37`	`35`	`{`
`38`		`- this.stateStorage = stateStorage;`
`39`	`36`	`}`
`40`	`37`
`41`	`38`	`[ValidateAntiForgeryToken]`
`@@ -115,6 +112,6 @@ public async Task<ActionResult<BankIdUiApiInitializeResponse>> Initialize(BankId`
`115`	`112`	`}`
`116`	`113`
`117`	`114`	`var stateKey = new StateKey(cookie);`
`118`		`- return stateStorage.GetAsync<BankIdUiSignState>(stateKey);`
	`115`	`+ return _stateStorage.GetAsync<BankIdUiSignState>(stateKey);`
`119`	`116`	`}`
`120`	`117`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,8 +33,8 @@ IStateStorage stateStorage`
`33`	`33`	`[HttpGet]`
`34`	`34`	`[AllowAnonymous]`
`35`	`35`	`[Route($"/[area]/{BankIdConstants.Routes.BankIdPathName}/{BankIdConstants.Routes.BankIdSignControllerPath}")]`
`36`		`- public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions)`
	`36`	`+ public Task<ActionResult> Init(string returnUrl, [FromQuery(Name = BankIdConstants.QueryStringParameters.UiOptions)] string protectedUiOptions, [FromQuery(Name = "nonce")] string nonce)`
`37`	`37`	`{`
`38`		`- return Initialize(returnUrl, BankIdConstants.Routes.BankIdSignApiControllerName, protectedUiOptions, "Init");`
	`38`	`+ return Initialize(returnUrl, BankIdConstants.Routes.BankIdSignApiControllerName, protectedUiOptions, nonce, "Init");`
`39`	`39`	`}`
`40`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,7 @@ public interface IStateStorage`
`14`	`14`	`Task<T?> GetAsync<T>(StateKey key);`
`15`	`15`	`Task<bool> TryGetAsync<T>(StateKey key, [NotNullWhen(true)] out T? value);`
`16`	`16`	`Task<StateKey> SetAsync<T>(T value);`
	`17`	`+ Task<StateKey> SetAsync<T>(StateKey key, T value);`
`17`	`18`	`}`
`18`	`19`
`19`	`20`	`public static class BankIdBuilderStateStorageExtensions`
Original file line number	Diff line number	Diff line change
`@@ -34,4 +34,10 @@ public Task<StateKey> SetAsync<T>(T value)`
`34`	`34`	`cache.Set(stateKey, value, _memoryCacheEntryOptions);`
`35`	`35`	`return Task.FromResult(stateKey);`
`36`	`36`	`}`
	`37`	`+`
	`38`	`+ public Task<StateKey> SetAsync<T>(StateKey key, T value)`
	`39`	`+ {`
	`40`	`+ cache.Set(key, value, _memoryCacheEntryOptions);`
	`41`	`+ return Task.FromResult(key);`
	`42`	`+ }`
`37`	`43`	`}`