Repository navigation
fix(identity): make the two-factor issuer name configurable - #1443
Open
marcelo-maciel wants to merge 3 commits into
Open
marcelo-maciel wants to merge 3 commits into
marcelo-maciel wants to merge 3 commits into
Conversation
…isories Advisories published on 2026-10-13 cover every 10.0.x up to 10.0.12 of System.Security.Cryptography.Xml, System.Security.Cryptography.Pkcs and Microsoft.AspNetCore.SignalR.Common, and NuGetAudit under TreatWarningsAsErrors fails restore for every project, main included. SignalR.Common 10.0.13 requires Microsoft.Extensions.* 10.0.13, so a transitive pin alone does not resolve; the whole 10.0.12 line moves to 10.0.13 and Xml/Pkcs stay pinned.
EnrollTwoFactorCommandHandler hardcoded "FullStackHero" as the issuer of the otpauth URI, so every product built on the kit showed that name in the user's authenticator app. The issuer now comes from TwoFactorOptions:Issuer, defaulting to "FullStackHero" and validated on start so a blank value fails fast. It stays percent-encoded in both the label and the issuer parameter. Only new enrollments pick up a changed issuer; existing enrollments keep working because the shared secret does not change. Fixes fullstackhero#1440
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The template replaces "FullStackHero" with the project's display name, which can contain spaces, so a literal expectation breaks the scaffolded build. The default-issuer test now derives the expected label from TwoFactorOptions, and the integration assertion goes back to the prefix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1440
What
The issuer of the two-factor
otpauth://URI was a hardcoded"FullStackHero"inEnrollTwoFactorCommandHandler, so every product built on the kit appeared under that name in the user's authenticator app. It now comes from configuration:TwoFactorOptions(Identity module root, next toTenantGraceOptions), bound withAddOptions<TwoFactorOptions>().BindConfiguration(nameof(TwoFactorOptions)).ValidateDataAnnotations().ValidateOnStart(), the same idiom asJwtOptions. No existing Identity options class covers TOTP (JwtOptions:Issueris the token issuer,fsh.local, a different concept), so a small dedicated class seemed cleaner than overloading one.Issuerdefaults toFullStackHero, so nothing changes for current deployments. It is[Required]: a blank issuer would produceotpauth://totp/:user?...&issuer=, which authenticator apps reject, so it fails at startup instead.UrlEncoder.Defaultin both the label and theissuer=parameter, soCanal CorretobecomesCanal%20Correto,AçãobecomesA%C3%A7%C3%A3o, and a:or&in the name cannot break the label or the query.appsettings.jsondocuments the key next toPasswordPolicy.fsh new/dotnet new fshalready got their own display name here, because the template replacesFullStackHerowith it; that still applies to both the default and theappsettings.jsonvalue. The default-issuer test derives its expectation fromTwoFactorOptionsfor that reason (the brand can contain a space, as in the scaffold smoke job'sSmoke Api). What this PR adds is the ability to change it after scaffolding, and for clones and forks of the kit.Existing enrollments
Only new enrollments get the new issuer. Users who enrolled earlier keep the old label in their app, and their codes keep working because the shared secret is not touched. No migration is needed. A user who wants the new label disables and enrolls again.
Tests
Identity.Tests/Handlers/EnrollTwoFactorCommandHandlerTests(new): the default isFullStackHero; exact URI with the default issuer, with a custom issuer, and withCanal Correto,AçãoandA:B & C(encoded in both positions, andUri.TryCreateaccepts it); unauthenticated caller throwsUnauthorizedException; a blank or whitespace issuer fails validation."FullStackHero"hardcoded again, field still read), 4 tests fail (custom issuer + the three encoding cases); with the issuer passed unencoded, the 3 encoding cases fail. Both green with the fix.Note on the first commit
The first commit (
build(deps): move the .NET 10 packages to 10.0.13 for the October advisories) is byte-for-byte the same asfd6e9b0bin #1426.maincurrently fails restore on NuGet audit (System.Security.Cryptography.Pkcs/Xml and SignalR.Common 10.0.12 advisories underTreatWarningsAsErrors), so this PR carries it to build. It drops out on rebase once #1426 merges.Docs: fullstackhero/docs#271