Conversation
| These instructions use [Shim Sangmin](https://hogangnono.com)'s [qrcode.js: Cross-browser QRCode generator for JavaScript](https://davidshimjs.github.io/qrcodejs/) ([`davidshimjs/qrcodejs` GitHub repository](https://github.com/davidshimjs/qrcodejs)). | ||
| A QR code generated by the app to set up 2FA with an TOTP authenticator app must be generated by a QR code library. | ||
|
|
||
| The guidance in this article uses [`nimiq/qr-creator`](https://github.com/nimiq/qr-creator), but you can use any QR code generation library. |
There was a problem hiding this comment.
@Rick-Anderson Can we update the corresponding content for MVC & Razor Pages as well for consistency? https://learn.microsoft.com/aspnet/core/security/authentication/identity-enable-qrcodes
There was a problem hiding this comment.
@guardrex Since this is for Blazor, is there a reason we can't use a .NET QR code generator for this? Maybe https://github.com/manuelbl/QrCodeGenerator?
There was a problem hiding this comment.
I'll have that swapped in tomorrow morning.
There was a problem hiding this comment.
@danroth27 @Rick-Anderson ... Done! 👍 I rolled the new QR code lib into both the BWA 2FA/TOTP article and the new WASM article here. It SUPER cleaned up the insanity on calling the JS lib in the BWA article to generate the QR code. I've also updated the WASM sample app, and you can see the revised Manage2fa component here ...
@danroth27 ... Rick asked me offline about showing the Login and Manage2fa components here versus cross-linking to them in sample code, so they wouldn't appear in the article. Readers would need to follow a link to reach them.
I agree that they're long, but I don't think it matters all that much. Yeah, they have to scroll, but they don't need to click away from the text, which is something that I hate having to do myself when reading an article.
It would rope devs into taking that code when they go for a basic nuts-'n-bolts experience with the WASM+Identity sample app. However, hacking is so crazy bad these days that perhaps we should force the code on them. Then, should we also place the account confirmation and PW recovery code into the sample app? ... but don't require a confirmed account to log in because the dev then must set up an email provider just to get the basic experiences working as a demo.
Would placing all of these features into the sample app lead us to .... brace yourself ... lead us to the Music Store of Identity sample apps!!?? 🙈😆 ... i.e., ... it just adds so much stuff that it scares the poor new dev or new-to-Blazor dev to death. There are pros and cons to moving the code out of the article and into the sample app. It's your call 👨⚖️🏛️. I'm 👂 for your decision.
Co-authored-by: Rick Anderson <3605364+Rick-Anderson@users.noreply.github.com>
🦖
|
|
|
||
| ```diff | ||
| - <div class="alert alert-info"> | ||
| - Learn how to <a href="https://go.microsoft.com/fwlink/?Linkid=852423">enable |
There was a problem hiding this comment.
Can we get a new go.microsoft.com link that points to this Blazor QR code generation docs? It would be nice to update the Blazor Identity project template and scaffolding to point to this instead. Maybe we could remove <div></div><div data-url="@authenticatorUri"></div> while we're at it.
There was a problem hiding this comment.
There was a problem hiding this comment.
Unless we need to change the current filename/URL, the GO link destination will be ...
| } | ||
| } | ||
| } | ||
| catch { } |
There was a problem hiding this comment.
Can we remove the catch { } blocks from everywhere we have them? If there's an exception, it's better to let it bubble up for debugging/logging purposes.
There was a problem hiding this comment.
But this is mirroring your approach ... from the code that you submitted ...
If we remove the try-catch, then how can it return a FormResult on an error with Succeeded of false and the error message for the user?
Can we log the exception there and leave the try-catch in place?
UPDATE: I'm having it log the exceptions now.
There was a problem hiding this comment.
UPDATE (12/5): I've worked further on a couple of items, and I've updated this comment to reflect the latest bits.
WRT that question ☝️ on the try-catch situation
I left the try-catch statements in place but I'm logging the exceptions now.
There's going to be a instruction in the new article to inject the logger (manually add it to the code). In the future, I'll work dotnet/blazor-samples#401 to have the main sample app inject the logger, and then I'll remove the manual instruction on doing it from the article.
Forgetting the machine on a 2FA disable
I had to add ...
await Acct.TwoFactorRequestAsync(new() { ForgetMachine = true });... to the Disable2fa method because if it isn't there and 2FA is disabled by the user and re-enabled, it remembers them on the next login attempt and doesn't ask for a 2FA code. By forgetting the machine when 2FA is disabled, it does request a new 2FA/recovery code on the next login after they've re-enabled 2FA. I think that's a better UE.
Give new recovery codes every time 2FA is enabled, not just the first time
If the user disables 2FA and then re-enables it, I would kind'a expect a new set of recovery codes to get generated. Apparently, if they're still on the user's account from the last time that they had them established, the API doesn't generate a new set. I feel that a better UE is if they disable and re-enable that they get a new set of codes. I'll take care of that in the morning. I'm off to 🛌💤 shortly. Done! 👍
Display of the number of remaining recovery codes
I had a world of trouble 😈 trying to get the count to appear in the UI after the user successfully logs in with a recovery code. I've pulled that code out now.
Instead, I have the Manage2fa component display the number of recovery codes left anytime they visit the Manage2fa page after enabling 2FA. Let's roll with that approach.
|
Going ahead with this. Of course, I can patch whatever needs more work on a follow-up PR. @danroth27 ... One open item mentioned here is Halter's remark at #34189 (comment) for ...
When that link is generated, it should point to ... Second, there was Rick's question on perhaps using a sample app for the code and pulling it in from there at #34189 (comment). I think it's best for now not to pull from a sample app; however, I'm making a tracking note to discuss it further in 25Q1. The question will be if the WASM+Identity sample app should have account confirmation with PW recovery and this 2FA/TOTP stuff all OOB ... and then, yes, the code can be pulled over into the article from the sample app. Let's take that discussion up next year. |
Fixes #33772
On the PR
There are changes here to a few articles, including some touches to the account confirmation and PW recovery article. Those changes started out with just a few NITs but kind'a snowballed on me. They aren't really germane to the thrust of the PR on 2FA/TOTP.
The main article for review is the NEW WASM+Identity 2FA article at the bottom of the diff ...
aspnetcore/blazor/security/webassembly/standalone-with-identity/qrcodes-for-authenticator-apps.md
If you want to review all of the changes, that's great, of course, but you don't have to look at the rest of this if you're busy. I feel good about just taking on-going reader feedback on the rest of it.
Single login component
I see what was done for server-based Identity components ... three login components; but thus far, I like the approach I've taken here for a single
Logincomponent that can manage non-2FA, 2FA TOTP, and 2FA recovery code logins. I don't think it's too complicated, and it's economical to just have a single component. It also makes state management from the email/PW step to the 2FA step a snap.New QR code generator
I'm going with https://github.com/manuelbl/QrCodeGenerator per DR's suggestion.
Sample app
You can pull down ...
https://github.com/guardrex/BlazorWebAssemblyStandaloneWithIdentity
... and use the seeded "Leela" account (
leela@contoso.com,Passw0rd!), which is preloaded into theLogincomponent. That account is confirmed by seeding code, so you don't need to implement account confirmation.All of the 2FA features light up OOB ... no app config or additional steps required.
Revamp of
TwoFactorRequestAsyncHalter, took your advice from the offline convo and added the serializer option for ...
... to shorten the
TwoFactorRequestAsyncmethod nicely by only passing the relevant non-nullparams to the POST.AFAICT, all good! 🍻
Requiring a TOTP every login
Offline, we discussed setting
useSessionCookiestotrue. However, that didn't work. It only required a TOTP for one more login. After that, it went back to remembering the user. BTW: I noticed thatuseSessionCookieswasn't in the main doc set article, so I opened #34255. We link to the main doc set article; so if we don't need to mention it here for any common use case, that's fine.I wondered if calling the manage endpoint with
ForgetMachine(true) after login would work. Yes! That approach appears to work 🎉, but I don't know if doing it that way is a supported scenario. I simply did this for after a 2FA login is successful ...MIA rendering
I built the
Logincomponent so that it would display the number of remaining recovery codes after a successful recovery code login. However, it's just NOT working ... not rendering the value of myrecoveryCodesRemainingMessage. The message string is assigned to the variable, but the string just won't display for some strange 👽 rendering reason.Halter, you said you'll debug that and see why it isn't working. I did remove the silly
StateHasChangedcall from the sample code. I just had that there on a lark, and it didn't work.Added a loading check
This morning (Monday, 12/2), I added a
loadingcheck to theManage2facomponent (and a check in the companion JS to stoptrimfrom a 💥 when the QR code element isn't rendered) because the page was flashing the enable-2FA UI pieces before rendering the correct UI when 2FA is already enabled. The loading UI seems to be working ok here.Internal previews