Implement phone API support

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

Implement phone API support #447

Closed nordeke closed 5 months ago

nordeke commented 7 months ago

This PR fixes #404 .

High level overview of this PR:

[x] Implemented the 2 (auth/sign) phone API endpoints
[x] Added tests and some documentation (did I miss anything?)
[x] Followed same structure as original auth/sign for implementation, request-/response models and tests
[x] Renamed enum CollectCallInitiator to more general CallInitiator
[x] Phone auth/sign are tested locally against BankID as well as simulated environment

These things have been implemented (when relevant):

[x] Code written
[x] Test added
[x] Documentation updated / written

PeterOrneholm commented 6 months ago

Great work @nordeke! Just had a quick look and looks great :) Will have to do a deeper review though before we can merge it. Can't promise when we can prioritize that at the moment, but let us know if it is urgent for you and we can find a way.

The one thing I noticed right away is that the API takes a dependency on the Identity library. We have tried to ensure that the API package is very "low level" and does not do to much of validation. We might change this later, but would like to take that away from the API part.

nordeke commented 6 months ago

Thank you, @PeterOrneholm!

About prioritization, the old functionality with the ability to provide personal identity number will not be supported from BankID after may 1 2024, since "secure start" will be mandatory. (https://www.bankid.com/en/tekniska-uppdateringar/secure-start-will-be-mandatory-from-1-may-2024) It would therefore be appropriate if this functionality could be released some time before that.

I added the reference to Identity just to validate the personal identity number, but I understand your mindset about "low level" and will push a commit removing it.

PeterOrneholm commented 6 months ago

Thanks!

I can't make any promises at the moment, we usually encourage companies to sign up for an support agreement if they need it to be looked at quick.

ghost commented 6 months ago

Hi, we have been in contact with ActiveSolution (I guess you know already @PeterOrneholm) about letting them charge us for some consultancy hours to get this in the next release and from the information I received this is scheduled for merge during next week. Please correct me if I am wrong Peter.

Also thank you @nordeke for the PR implementation. Perhaps we can continue in a discussion thread regarding the actual client implementation in order to make the best of the new functionality.

PeterOrneholm commented 5 months ago

Hi @vijng and @nordeke! We have scheduled time this wednesday to review, fix any final things, merge and push a new version. Expect a version to be available at latest thursday if we don't find anything blocking us,

ghost commented 5 months ago

Great!

Från: Peter Örneholm @.> Skickat: den 15 april 2024 16:39 Till: ActiveLogin/ActiveLogin.Authentication @.> Kopia: Jonas Norberg @.>; Mention @.> Ämne: Re: [ActiveLogin/ActiveLogin.Authentication] Implement phone API support (PR #447)

Hi @vijnghttps://github.com/vijng and @nordekehttps://github.com/nordeke! We have scheduled time this wednesday to review, fix any final things, merge and push a new version. Expect a version to be available at latest thursday if we don't find anything blocking us,

— Reply to this email directly, view it on GitHubhttps://github.com/ActiveLogin/ActiveLogin.Authentication/pull/447#issuecomment-2057024033, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A7YKR6J5RT42PF3VAXPSVMLY5PRCHAVCNFSM6AAAAABEQDM5A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJXGAZDIMBTGM. You are receiving this because you were mentioned.Message ID: @.***>

PeterOrneholm commented 5 months ago

We've merged this now. Will wrap up a few things for a v9 release soon.

PeterOrneholm commented 5 months ago

Hi @vijng and @nordeke! Thanks again for the PR and the sponsoring of this feature. We have now launched ActiveLogin.Authentication 9.0.0 RC 1 on NuGet. It has no known issues so is "go live" ready.

A mini console sample, to get a look and feel for how to use the API can be found here: https://github.com/ActiveLogin/ActiveLogin.Authentication/tree/main/samples/Phone.ConsoleSample

ghost commented 5 months ago

[celebrate] Jonas Norberg reacted to your message:

From: Peter Örneholm @.> Sent: Thursday, April 18, 2024 1:43:31 PM To: ActiveLogin/ActiveLogin.Authentication @.> Cc: Jonas Norberg @.>; Mention @.> Subject: Re: [ActiveLogin/ActiveLogin.Authentication] Implement phone API support (PR #447)

Hi @vijnghttps://github.com/vijng and @nordekehttps://github.com/nordeke! Thanks again for the PR and the sponsoring of this feature. We have now launched ActiveLogin.Authentication 9.0.0 RC 1 on NuGet. It has no known issues so is "go live" ready.

A mini console sample, to get a look and feel for how to use the API can be found here: https://github.com/ActiveLogin/ActiveLogin.Authentication/tree/main/samples/Phone.ConsoleSample

— Reply to this email directly, view it on GitHubhttps://github.com/ActiveLogin/ActiveLogin.Authentication/pull/447#issuecomment-2063901486, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A7YKR6NMMLBGUJ2OVGFJRDDY57EYHAVCNFSM6AAAAABEQDM5A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRTHEYDCNBYGY. You are receiving this because you were mentioned.Message ID: @.***>