Support Swedish BankID (svenskt BankID) authentication in .NET. Unofficial package, not created by BankID.
Home Page: https://activelogin.net
License: MIT License
We don't have a usecase for using our own Json serializer so we should remove the interface for now so that people don't start taking dependencies on our Common package.
Even thought that ActiveLogin.Identity allows different formats of the Personal Identity Number and all of them are valid, in Authentication it still fails with Unknown error. Please try again. and as a response
{
"personalIdentityNumber": "Invalid PersonalIdentityNumber."
}
if user provides it with the delimiter.
That becomes a bit confusing for users since our mask kind of demands it:
Without the delimiter everything is good
At the moment BankID uses English texts and GranID - Swedish:
We use the latest <partial name="" /> tag helper, but somehow it does not work when you override it in the actual MVC project, so the browser renders exactly the tag which means Razor does not recognise it as a valid partial wrapper.
One of the solutions could be to rollback to the old implementation of rendering partials, like @await Html.PartialAsync("_partial.cshtml")
Or we can completely get rid of the partials and start using components instead (https://docs.microsoft.com/en-us/aspnet/core/mvc/views/view-components?view=aspnetcore-2.2). I like this option more, but needs to be tested if Razor engine on the top project can recognise it.
As suggested in #79, if we would convert our controllers to Razor Pages, we would make it possible to override our partials in more places than now.
Currently there seems to be only 2 folders where our partials can be overridden, and it doesn't match the folder structure you use with Razor pages.
Staying with controllers, the users have to match the old folder style.
If we convert, we support traditional MVC folder structure and Razor pages folder structure.
GrandID (BankID) allows you to prepopulate the Personal Identity Number so that you can have your own UI for this. We could implement it similar to how it's done for native BankID (Razor library) to have a unified UI experience.
Inspired by Microsoft Open-source library guidance we should strong name the assemblies to support beeing referenced from other strong named assemblies.
CONSIDER strong naming your library's assemblies.
Is your feature request related to a problem? Please describe.
Considering we are open source, it seems to me like a good idea to include an .editorconfig file so it's easier for us and other contributors to adhere to coding style. I'm opening this issue to start the discussion regarding what conventions we want in our project. Here is one we could start the discussions around. What do we like, what do we want to change?
What area is it related to
Source
Describe the solution you'd like
Include an EditorConfig in the solution
Describe alternatives you've considered
Resharper config. EditorConfig doesn't depend on Resharper, and there is a VSCode extension available
Additional context
Add any other context or screenshots about the feature request here.
Use inline SVG instead if possible.
Hi Team,
It seems the live demo doesn't work.
After I enter the "Personal Identity Number", it goes to the start the bank id app.
Then I open the bankid app on my mobile, but it doesn't let me enter the password.
It got stuck at the step "Start your BankID app", and then failed.
"The BankID app is not responding. Please check that the program is started and that you have internet access. If you don’t have a valid BankID you can get one from your bank. Try again.".
Can you kindly confirm?
Thanks.
Is your feature request related to a problem? Please describe.
To support users running latest version of ASP.NET Core, we should upgrade our package.
What area is it related to
BankID
Describe the solution you'd like
Upgrade to ASP.NET Core 3 and make sure it works. Find any changes done to the pipeline.
I'd suggest that we implement an BankIdCertificatePoliciyBuilder, with an API similar to this:
var policies = BankIdCertificatePoliciyBuilder
.Add("1.2.752.78.1.1") // Custom
.Add(BankIdCertificatePolicy.BankIdOnFile) // Mapped known policies
.Add(BankIdCertificatePolicy.MobileBankId) // Mapped known policies
.Build(BankIdEnvironment.Prod);.Build() would take an argument of what environment to build it for (defaults to Prod), as BankId has different Policy numbers per environment.
The enum BankIdCertificatePolicy would contain:
BankIdCertificatePolicy.BankIdOnFileBankIdCertificatePolicy.BankIdOnSmartCardBankIdCertificatePolicy.MobileBankIdBankIdCertificatePolicy.NordeaEidOnFileAndOnSmartCardBankIdCertificatePolicy.TestBankIdThe name(s) returned from BankID and GrandID seems to be returned as all capital letters in some cases. For me personally, it does not, but fore some colleagues it does.
So, for me it's returned as this: Peter Örneholm
But for some other, it's returned like this: PETER ÖRNEHOLM
Should we, and if so how, implement a way of normalizing this in any way? Would be nice to be able to consume it and trust that it's written not capitalized.
If we decide to implement it, I'd suggest it's behind a flag so you can disable it from *AuthenticationOptions.
To get a quick visual indication on how the UI might look for the user, we should add some screenshots.
Is your feature request related to a problem? Please describe.
BankID supports Sign in addition to Login. At the moment we do have support in the API wrapper, but no UI support. By providing UI support users could sign things more easy.
What area is it related to
BankID
Describe the solution you'd like
As signing is not as standarized as login, I'm not sure. Let's use this issue to discuss possible implementations.
To make testing and development faster and easier, it would be nice to have logout buttons both on the MVC Client and the actual Identity Server.
Right now, one needs to delete cookies manually to be able to make a new login attempt (after a successful one).
Adding this as a feature request and as a personal reminder that it would be nice to have. It's good to start the discussion.
BankID does have it's advantages with it's user base, but Freja EID appears to gain traction. It is also a cheaper alternative compared to BankID for smaller projects.
For more information and documentation of the flow and api: https://frejaeid.com/for-utvecklare
In September 2018, support for QR codes was introduced in BankId. This is described under 4 in the BankID Relaying Party Guidelines.
We intend to implement support for this and this issue will keep track of the progress and suggestions on how to implement it.
Is your feature request related to a problem? Please describe.
BankID does support a "Cancel"-feature, which we on an API-level, have support for. But at the moment, we don't expose any UI for that so a user can't abort an ongoing login.
What area is it related to
BankID
Describe the solution you'd like
Implement a cancel button during the login flow.
Would be great to be able to track logins in Azure Monitor (Application Insights and/or Azure Log Analytics). We do log to the ILogger today, but they end up as traces using the default connector in ASP.NET Core.
I'd suggest an extension method in an *.Azure package that would add tighter integration and would log custom events and metrics to be able to setup dashboards and alerts on succeed / failed logins. As each login does cost, it would be nice to have such metrics available.
It would be implemented for both BankID and GrandID and suggestion is to start with loggint the same events as we do in ILoggerExtensions (GrandID > ILoggerExtensions and BankID > ILoggerExtensions).
I think there is a need for an MVC sample that doesn't use IdentityServer since it could simplify the understanding of the setup in that scenario. I've created one based on the standard ASP.NET Core MVC 2.1 template (added the same UI for showing Claims as in the MVCClient sample). What do you think - worth adding to the solution?
Inspired by Microsoft Open-source library guidance we should add another target for net461 in addition to the existing netstandard20.
CONSIDER adding a target for net461 when you're offering a netstandard2.0 target.
At the moment, you can't access the BankID UI if you use authentication for all site as a default policy.
If user use development environment and select BankID same device with turned on option AutoLogin, then the JS method Initialize() triggers two times which lead immediately to the error An identification or signing for this personal number is already started. Please try again.
Changes from issue #58 could lead to the that. Needs to be investigated.
Describe the bug
I found out that when using the ASP.NET Core package for BankID from .NET Core 3.0 invalid URLs where resolved for the API endpoint. Turns out it's related to a breaking change in how URLs for actions with the Async suffix is calculated, see:
dotnet/aspnetcore#4849
What area is it related to
BankID
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The url should resolve to: /BankIdAuthentication/Api/* but resolved to /BankIdApi/*
NuGet package version
ActiveLogin.Authentication.BankId.AspNetCore 2.0.0
Runtime version
.NET Core 2.2.0 and .NET Core 3.0.0 preview 9
Is your feature request related to a problem? Please describe.
Swagger 2.0 requires all actions to specify HTTP methods. One action is missing that.
What area is it related to
BankID
Describe the solution you'd like
Adding a HttpGet on the action ActiveLogin.Authentication.BankId.AspNetCore.Areas.BankIdAuthentication.Controllers.BankIdController.Login.
Today builds are done using Azure Pipelines, which we love. Though, all build configuration is defined as tasks using the UI. I'd suggest we rewrite the builds using Cake and define the parts we need in Azure Pipelines as YAML. This would make it easier to build on your local machine and the build process would be more transparent.
Is your feature request related to a problem? Please describe.
At the moment, if the user is logging in from another device, he/she needs to fill in the Personal Identity Number on every new sign in. We could persist the personal identity number for that user.
What area is it related to
BankID
Describe the solution you'd like
For security and integrity reasons, I don't want to store the PIN on the local computer. Instead I'm proposing to use some server side store and store a token in a cookie that references the PIN. Potentially we can store the PIN encrypted using .NET Core Data Protector, depending on how long lived that data can be.
It should be opt in to enable such feature for the user.
During the development of the GrandID authentication handler a thorough work went into making sure the API, especially the public API, being developer friendly as well ass following common conventions.
We aim to refactor the BankID authentication handler before reaching stable 1.0.0 to use the same API design.
This issue will keep track of that work and, before implementing it, will have a suggested API design described here.
GrandID supports SITHS in addition to BankID. https://e-identitet.se/tjanster/inloggningsmetoder/grandid-api/
Implementing this will require changes in ActiveLogin.Authentication.GrandId.Api and ActiveLogin.Authentication.GrandId.AspNetCore.
From what I understand It's up to the one implementing it do decide whether to use the dekstop or mobile key. From what I understand, you can't use the mobile app to authenticate a login on your desktop. Therefore, from the consumption point of view, it would make the most sense to just have .AddSiths(), supply both keys, and then figure out which one to use depending on what device the user has. We have an implementation for this is the BankID implementation. If you only support one of them, you only supply one key.
The final implementation should then have a public facing API like this:
services.AddAuthentication()
.AddGrandId(builder =>
{
builder
.UseDevelopmentEnvironment()
.AddBankIdChooseDevice(options =>
{
....
})
.AddSiths(options =>
{
options.GrandIdDesktopDeviceAuthenticateServiceKey = "UIO";
options.GrandIdMobileDeviceAuthenticateServiceKey = "ABC";
});
});The implementation includes (but is probably not limited to) these files:
SithsFederatedLoginAsync to/IGrandIdApiClient.cs and implement in derived classesSithsGetSessionAsync to /IGrandIdApiClient.cs and implement in derived classes.AddSiths() in /GrandIdAuthenticationBuilderSchemeExtensions.csIE11 don't have the required window.fetch and a polyfill is needed for those that wants to support it.
What area is it related to
Source
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Page shows input for Personal Number and Sign In Button
Screenshots
Desktop (please complete the following information):
Would be great if the samples (especially the one with Identity Server) used Docker to expand the scenarios to where they can be deployed.
I'd propose we change our sample for Identity Server to be deployed as a container to Azure App Service (Linux?).
Inspired by #83
On iOS, you are required to set a returnUrl that the BankID iOS app will return the user to once signed in. When iOS always launches a URL it will always be launched in the built in browser, Safari. If the session is initiated in a third party browser like Chrome or Firefox, it will not succeed as the login has to be finihsed in the same browser (and that) that it was initiated in.
On Android this is not an issue as when returnUrl is set to null, documented in "3.1.2 Parameters in the Start URL" of the BankID documentation it will automatically launch the previously used app, so it would handle Chrome, Firefox or even custom apps.
We could detect (IBankIdSupportedDeviceDetector) when the user is running iOS and a known third party browser, and then use a browser prefix.
Any other browsers that we can and should support?
If you need to support a custom scheme, like for your own app, I'd recommend to implement a custom IBankIdLauncher that would handle that scenario. If this is a common scenario, we could build it in and let it be configured.
Thanks for making a great effort to streamline the implementation of BankID!
We have stumbled upon an unexpected behaviour in the login flow using ActiveLogin with the GrandID BankID implementation in our Identity Server project.
UNEXPECTED BEHAVIOUR
BankID application redirects the user to the wrong browser (always default system browser) if the BankID app is opened via the GrandID login page from a native app (IABT) OR non default system web browser (e.g. Chrome on iOS). This unexpected behaviour occurs on Android as well as iOS.
REPRODUCTION
0) The device being used to reproduce this behaviour must have a working BankID installed.
POSSIBLE SOLUTION
Apologies if I’m barking up the wrong tree. I’m not as well versed in C# .NET as I am in JS. Perhaps there’s a way to pass a redirectURL to the GrandID API that will instruct the BankID application to open the initial application once signing is complete? E.g. using custom URL schemes, returnURL=se.activesolution.AnAwesomeApp:// to re-open the initial application to complete the login flow?
Happy to contribute on this!
/Christofer
The Grand ID API (Svensk e-identitet) supports adding pnr (short for personnummer (Swedish personal number)) property to authentication request to them.
When the pnr property is included in the request, the client will not have to fill out the Swedish personal number from Svensk e-identitets UI, but can choose to use a customized form for that on its own site or app.
Today, the Azure extension require you to supply the secret api key, we should support managed identity as that is best practise. Samples can be found here:
https://azure.microsoft.com/en-us/resources/samples/app-service-msi-keyvault-dotnet/
Is your feature request related to a problem? Please describe.
Currently we inline / embed styles and scripts as ASP.NET Core 2.X does not provide a good way to reference static assets
What area is it related to
BankID
Describe the solution you'd like
In ASP.NET Core 3, preview 6 the feature to reference Static assets (like styles and scripts) in Razor class libraries was introduced. We should try this solution.
Additional context
.NET Core 3 also introduces a new, faster JSON implementation, but this will require our API wrappers to target .NET Standard 2.1 and leave out .NET Framework etc. We won't use this feature now.
To be able to run the samples in dev mode, there are a couple of manual steps that need to be fixed before it can run properly.
Missing certificates
The IdentityServerSample project expects two certificates to exist in the /Certificates folder even though you don't need them in dev mode.
Authority URL mismatch
The port that the IdentityServerSample project will start on does not match the authority URL that the MvcClientSample expects.
We might want to look into making some better defaults to make the samples work out of the box (pure f5 experience ✌).
Inspired by Microsoft Open-source library guidance we should probably remove the common NuGet-package as a package and instead use a NuGet shared source packages
CONSIDER referencing shared source packages for small, internal pieces of functionality.
SonarCloud has found a few code smells (https://sonarcloud.io/dashboard?id=ActiveLogin_ActiveLogin.Authentication), I don't think all should be fixed as they might be opiniated, but some are low hanging fruits.
Fix these:
Add a 'protected' constructor or the 'static' keyword to the class declaration. 'System.Exception should not be thrown by user code.Update this implementation of 'ISerializable' to conform to the recommended serialization pattern.Fix this implementation of 'IDisposable' to conform to the dispose pattern.Merge this if statement with the enclosing one.'TResult' is not used in the method.Use a constructor overloads that allows a more meaningful exception message to be provided.Maybe:
Remove this commented out code. Can we handle the different scenarios in other ways?Not:
Refactor your code not to use hardcoded absolute paths or URIs. I'd say we should have these hard coded in this case, they are constants. Maybe we should split the constant of URL and the URI object though?Add a <legend> tag to this fieldset. Legend is optional and does not bring value here.Constructor has X parameters, which is greater than the 7 authorized. Don't fix now, is breaking change.I think DevelopmentEnvironment sounds like an actual environment that GrandID/BankID has.
I'd suggest we rename it into InMemoryEnvironment to make it more clear what it actually is.
When you choose "BankID - Same device" on mobile the flow looks correct and works, but it does open a new window in the browser when going back from the BankID app to the browser.
I think it reused the same tab before we implemented support for same device on desktop.
We need to rethink the insertion of Styles section in our Razor Area.
For example in Login.cshtml within BankIdAuthentication area we assume that people use section Style in their _Layout.cshtml, so the code @section Styles { <style type="text/css"> ... </style> } will throw an exception if there is no @RenderSection("Styles", required: false) in _Layout.cshtml master page.
The solution could be either to insert it in the header directly (which is not recommended since we use Areas) or do the smart-check: if it defined - then we use as it is, otherwise via the header.
The BankId API client only expose the "auth" method but not the similar "sign" with the extra properties in the request as described in 14.1.2 Additional Parameters for sign.
I know the package is focused on Authentication, but I think it should expose the sign-functionality as well since it's almost there.
We aim to support BankID not only natively but also through GrandID (Svensk E-Identitet) as that has advantages such as being quicker on getting started with.
The GrandID implementation will follow the same principles as for the BankID, such as that the API wrapper will be a separate project and NuGet package,
Before moving to 1.0.0 I've been looking through our default config and would like to make a "braking change" by not issuing the birthday and gender claims by default. They are based on the hints extracted from the personal identity number and issued as standardized JWT-token claims. Though, for reasons we describe here:
https://github.com/ActiveLogin/ActiveLogin.Identity#hints
Those hints are not 100% and I'd therefore like the consumer (programmer) to make an active choice to use them.
Do you agree @rfolkes and @nikolaykrondev?
A PR would set the defaults to false:
https://github.com/ActiveLogin/ActiveLogin.Authentication/blob/master/src/ActiveLogin.Authentication.BankId.AspNetCore/BankIdAuthenticationOptions.cs#L54
https://github.com/ActiveLogin/ActiveLogin.Authentication/blob/master/src/ActiveLogin.Authentication.BankId.AspNetCore/BankIdAuthenticationOptions.cs#L59
https://github.com/ActiveLogin/ActiveLogin.Authentication/blob/master/src/ActiveLogin.Authentication.GrandId.AspNetCore/GrandIdBankIdAuthenticationOptions.cs#L17
https://github.com/ActiveLogin/ActiveLogin.Authentication/blob/master/src/ActiveLogin.Authentication.GrandId.AspNetCore/GrandIdBankIdAuthenticationOptions.cs#L22
And to achieve the current behavior, require to set them to true during setup:
.AddSameDevice(options =>
{
options.IssueGenderClaim = true;
options.IssueBirthdateClaim = true;
})
.AddOtherDevice(options =>
{
options.IssueGenderClaim = true;
options.IssueBirthdateClaim = true;
});Or, if you want to apply it to all:
services.Configure<BankIdAuthenticationOptions>(options => {
options.IssueGenderClaim = true;
options.IssueBirthdateClaim = true;
});There are a few places where we have typos on "magic" strings, for example "UnsuportedBrowser_ErrorMessage" and "SingIn_Title".
BankID supports policies to better secure the implementation by restricting to only the auth methods you allow. This is described under 14.5 in the BankID Relaying Party Guidelines.
Our IBankIdApiClient implementation does support this by setting AuthRequest.Requirements.CertificatePolicies, but we never do this in our BankIdAuthenticationHandler.
We intend to implement support for this and this issue will keep track of the progress and suggestions on how to implement it.
Preparing for supporting SITHS through GrandID (#45) inspired by the PR #44 by @hematmedhelp I realized it might be a good thing to rename the .AddGrandId() method into something more specific for BankID.
BankID and SITHS do share some parts in the underlying API (like name etc) but differs in what other data is returned (Personal Identty number, email etc). Also, concepts like .AddSameDevice() and .AddOtherDeivce() probably differs enough that it should be two implementations, that might (and will) share code under the covers.
I see two potential patterns:
Pattern 1:
.AddGrandIdBankId().AddGrandIdSiths()Pattern 2:
An alternative would be to keep the top level .AddGrandId() but split it further down, like this:
services
.AddAuthentication()
.AddGrandId(builder =>
{
builder
.UseDevelopmentEnvironment()
.AddBankIdSameDevice(options => { })
.AddBankIdOtherDevice(options => { })
.AddSiths(options => { });
});Any input or other ideas?
Congratulations and thank you for this release.
I was hoping to get a README showing basic usage when you like me, don't use the ASP.Net Core platform. What is the "core" library's API and what is a simple usage of it? Or alternatively (after seeing that there's a lot of code specific to ASP.Net) perhaps this can be deployed as a k8s/docker container and/or be part of an authentication flow being an OpenId Connect IdP? In that case, could my app could be RP and receive a JWT?
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
Django
The Web framework for perfectionists with deadlines.
Laravel
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
Microsoft
Open source projects and samples from Microsoft.
Google
Google ❤️ Open Source for everyone.
Alibaba
Alibaba Open Source for everyone
D3
Data-Driven Documents codes.
Tencent
China tencent open source team.