ActiveLogin / ActiveLogin.Authentication

Support Swedish BankID (svenskt BankID) authentication in .NET. Unofficial package, not created by BankID.
https://activelogin.net
MIT License
216 stars 75 forks source link

Unexpected handling of sign requests with a large UserNonVisibleData property #425

Open henkealg opened 1 year ago

henkealg commented 1 year ago

Describe the bug When using the sign flow with a (BankIdSignProperties)UserNonVisibleData byte array based on a string longer than 2830 characters, the sign request is returned back to the url provided in (BankIdSignProperties)Items.returnUrl without any exceptions or other error messages.

According to the /sign documentation the max length for the userNonVisibleData property is 200k so a non visible data of this size should not cause issues.

What area is it related to Sign API

To Reproduce This can easily be reproduced using the Standalone.MvcSample project by making the following edits to the SignController:

// var userNonVisibleData = BitConverter.GetBytes(model.FileToSign.GetHashCode());
var userNonVisibleData = Encoding.UTF8.GetBytes("add-long-string-over-2830-chars-here");

Expected behavior The sign flow working as expected, or if the value is to long, a relevant exception/error message.

NuGet package version 7.0.0

Runtime version Are you using .NET Core / .NET Framework? What version? 7.0.10

Additional context Thank you for this awesome library. 👍

PeterOrneholm commented 1 year ago

Thanks for reporting this! Will have a look. Can you provide slightly more context? How did you come up with the number 2830? Any logs? What happens when you are returned to that URL?

I've created a fiddle to show you what happens under "the hood", as that byte array is Base64 encoded the length will be slightly larger, but not going from 2830 to 200 000.

https://dotnetfiddle.net/iKSCzF

henkealg commented 1 year ago

@PeterOrneholm Thank you for looking into this.

I came up with ~2830 simply by trying to subtract from the original (json) string causing the reported issue in production. The sign flow worked with a string shorter than ~2830 characters. The original content causing the issue in prod. is ~7000 characters.

It is true that the base64 encoded string is longer than the unencoded version, but no where near the cap. I am guessing that it has something to to with the byte array conversion.

I cannot find anything in the debug logs when this issue is forced in my lde. There is only the redirect back to the provided returnUrl value.

I have had no problem re-creating the issue using your sample project in my lde using current master/main.

We have shipped a workaround for the issue but I still wanted to report it for possible future improvement/fix.

PeterOrneholm commented 12 months ago

Great, thanks for reporting! We will look into this later on. Good to know this is a known issue at the moment.