Skip to content

broker: support Entra passwordless and FIDO/security-key MFA - #1723

Open
nooreldeenmansour wants to merge 11 commits into
mainfrom
fido-passkeys-mfa
Open

broker: support Entra passwordless and FIDO/security-key MFA #1723
nooreldeenmansour wants to merge 11 commits into
mainfrom
fido-passkeys-mfa

Conversation

@nooreldeenmansour

@nooreldeenmansour nooreldeenmansour commented Jul 10, 2026

Copy link
Copy Markdown
Member

Augment the Entra ID authentication flows to support both password/passwordless MFA methods by probing for a passwordless method first and prompt for a password only when Entra reports one is required.

Security-key challenges are completed locally with libfido2 behind new entra_mfa_fido and entra_mfa_fido_pin modes.

Tested with Yubikey MFA, and Passwordless MS Authenticator MFA

UDENG-10773
UDENG-10774

Relevant Issues and Discussions (WIP)

Closes #832

#595

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.02439% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.85%. Comparing base (9905a37) to head (9760d97).

Files with missing lines Patch % Lines
authd-oidc-brokers/internal/broker/broker.go 87.95% 23 Missing ⚠️
authd-oidc-brokers/internal/fido/assertion.go 93.87% 3 Missing ⚠️
...nternal/providers/msentraid/himmelblau/entrapwd.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1723      +/-   ##
==========================================
+ Coverage   84.97%   85.85%   +0.88%     
==========================================
  Files          25       26       +1     
  Lines        1943     2107     +164     
==========================================
+ Hits         1651     1809     +158     
- Misses        292      298       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Microsoft Entra ID broker to support passwordless authentication (probing passwordless methods first and only prompting for a password when required) and adds local security-key (FIDO/WebAuthn) MFA completion using libfido2, integrating new follow-up auth modes into the broker flow.

Changes:

  • Add passwordless-first initiation for entra_password, with explicit fallback to password+MFA when Entra reports PASSWORD_REQUIRED.
  • Implement local FIDO/WebAuthn MFA completion (entra_mfa_fido, entra_mfa_fido_pin) using a new internal/fido package and enhanced libhimmelblau challenge metadata.
  • Update build/CI/snap packaging to include libfido2 headers/runtime and add extensive broker/unit test coverage for the new flows.

Reviewed changes

Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
snap/variants/msentraid/snapcraft.yaml Adds snap interface + libfido2 build/runtime dependencies for security-key MFA.
authd-oidc-brokers/internal/providers/msentraid/msentraid.go Plumbs optional auth options through provider initiation.
authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go Adds passwordless constraints and extracts richer MFA challenge metadata (incl. FIDO).
authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go Adds C-option mapping and exposes FIDO challenge/allow-list accessors.
authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c_test.go Tests new MSAL error mapping and C-auth-option translation.
authd-oidc-brokers/internal/providers/msentraid/himmelblau/generate.sh Uses cargo install --locked for reproducible tool installs.
authd-oidc-brokers/internal/providers/msentraid/himmelblau/entrapwd.go Defines AuthOption + enriches MFAChallengeInfo (FIDO fields) and error classification.
authd-oidc-brokers/internal/fido/authenticator.go Implements libfido2-backed authenticator and assertion ceremony handling.
authd-oidc-brokers/internal/fido/authenticator_test.go Unit tests for mapping libfido2 errors to broker-actionable sentinels.
authd-oidc-brokers/internal/fido/assertion.go Builds Entra-specific assertion JSON and helper decoding/parsing utilities.
authd-oidc-brokers/internal/fido/assertion_test.go Unit tests for assertion JSON, allow-list decoding, and CBOR authData unwrap.
authd-oidc-brokers/internal/broker/options_test.go Exposes test-only option hook to inject a mock FIDO authenticator.
authd-oidc-brokers/internal/broker/helper_test.go Enables FIDO authenticator injection in broker test harness.
authd-oidc-brokers/internal/broker/fido_stub.go Disables FIDO modes in non-withmsentraid builds.
authd-oidc-brokers/internal/broker/fido_msentraid.go Enables default libfido2 authenticator in withmsentraid builds.
authd-oidc-brokers/internal/broker/export_test.go Test hook to shorten the “wait for key insertion” timeout.
authd-oidc-brokers/internal/broker/broker.go Implements passwordless probing, FIDO routing, and new FIDO follow-up modes.
authd-oidc-brokers/internal/broker/broker_test.go Adds comprehensive tests for passwordless + FIDO routing and edge cases.
authd-oidc-brokers/internal/broker/authmodes/consts.go Adds new auth mode IDs/labels for security-key MFA.
authd-oidc-brokers/go.sum Adds go-libfido2 and transitive dependency checksums.
authd-oidc-brokers/go.mod Adds go-libfido2 dependency.
authd-oidc-brokers/conf/variants/msentraid/broker.conf Documents passwordless negotiation behavior for entra_password.
.gitmodules Updates libhimmelblau submodule tracking configuration.
.github/workflows/tics-run.yaml Installs libfido2 headers in TICS workflow environment.
.github/workflows/brokers-qa.yaml Installs libfido2 headers for broker QA/coverage workflows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .gitmodules
Comment thread authd-oidc-brokers/internal/broker/broker.go
@nooreldeenmansour
nooreldeenmansour marked this pull request as ready for review July 10, 2026 10:04
Comment thread .github/workflows/brokers-qa.yaml
NewPassword = "newpassword"

// EntraPassword is the ID of the Entra ID password + MFA authentication method.
// EntraPassword is the ID of the Entra ID password/passwordless MFA method.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an e2e-test for Entra ID passwordless authentication?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will look into it. If we can make TOTP MFA (similar to current test) passwordless, it should be feasible. Otherwise, I'm not sure if there is a way to automate a passwordless MFA, but I'll do some research and report back here

@nooreldeenmansour nooreldeenmansour Jul 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documenting for reference, A potential solution which I need to experiment with is https://learn.microsoft.com/en-us/entra/identity/authentication/howto-authentication-temporary-access-pass, which can work as a passwordless method if there is a way to auotmate its generation in the e2e tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does TOTP not work?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to be the case, unfortunately. https://learn.microsoft.com/en-us/entra/identity/authentication/overview-authentication

Software OATH Tokens is listed as a secondary method only

Comment thread authd-oidc-brokers/internal/broker/authmodes/consts.go Outdated
Comment thread authd-oidc-brokers/internal/broker/broker.go Outdated
Comment thread authd-oidc-brokers/internal/providers/msentraid/msentraid.go Outdated
Comment thread authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go Outdated
Comment thread authd-oidc-brokers/internal/broker/broker.go Outdated
Comment thread authd-oidc-brokers/internal/fido/authenticator.go
@nooreldeenmansour
nooreldeenmansour force-pushed the fido-passkeys-mfa branch 3 times, most recently from 156e815 to 7bf6a34 Compare July 10, 2026 14:41
@adombeck

Copy link
Copy Markdown
Contributor

@nooreldeenmansour is this ready for another round of review?

@adombeck adombeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just missing some renames and the e2e-test.

@nooreldeenmansour
nooreldeenmansour force-pushed the fido-passkeys-mfa branch 3 times, most recently from a8e5120 to df16ab0 Compare July 13, 2026 13:56
Comment on lines +114 to +115
## Note: If both device_code and entra_auth are disabled, no
## authentication will be available and users will not be able to log in.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we treat that as invalid config and let the broker fail to start?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I've updated the comment, making it more explicit

Comment on lines +45 to +46
EntraAuthWait: "Waiting for MFA approval",
EntraAuthCode: "Enter your MFA code",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these two actually about MFA? If so, we can keep naming them EntraMFAWait / EntraMFACode. If not, we should consider changing the display label.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were about MFA indeed, I've changed those back to their older names, and update their references as well

@adombeck adombeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just missing the e2e-test now

@nooreldeenmansour
nooreldeenmansour force-pushed the fido-passkeys-mfa branch 2 times, most recently from 2ca583b to d373efb Compare July 15, 2026 22:57
@nooreldeenmansour nooreldeenmansour added the e2e-tests This issue is related to end-to-end tests / Run end-to-end tests on this pull request label Jul 19, 2026
@nooreldeenmansour
nooreldeenmansour force-pushed the fido-passkeys-mfa branch 3 times, most recently from 47c74d0 to a8d9794 Compare July 19, 2026 20:07
nooreldeenmansour and others added 2 commits July 20, 2026 19:25
Entra accounts increasingly authenticate without a password
(Authenticator number-matching, TAP, security keys). Drive this from
the existing Entra auth flow: probe for a passwordless method first and
prompt for a password only when Entra reports one is required.

Security-key challenges are completed locally with libfido2
(internal/fido) behind new entra_auth_fido and entra_auth_fido_pin
modes. A key cannot be attached to a headless or SSH host, so the
security-key step waits a bounded time and then falls back to the
device code flow instead of blocking the login; FIDO is still
advertised to Entra so passwordless FIDO-only accounts are discovered
before a key is inserted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Entra direct-auth flow now handles both password and passwordless
authentication, so the old entra_password name is misleading. Because
the feature has not shipped yet, hard rename it instead of preserving
compatibility aliases.

Rename the config key, auth mode constant, cached-token marker, e2e
suite names, and the remaining Go identifiers consistently to the
entra_auth family.
nooreldeenmansour and others added 7 commits July 20, 2026 22:01
To prevent CI failures until the changes are reviewed and merged
upstream.
A returning passwordless user whose first login already used the
app-only Graph fallback is locked out of every later online login when
device registration keeps failing: the cached token carries no device
registration data, which also disqualifies the local password mode
online, so the user is forced back into entra_auth where the fallback
was gated on "first login" (oldAuthInfo == nil) and skipped.

Gate the fallback on the absence of device registration data instead,
which is the condition the surrounding comment always described: the
fallback exists for logins that fail registration before any local
device state exists, regardless of whether it is the first one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The initial entra_auth layout is a wait-only form (the passwordless
probe screen), but the mode was offered to any client whose form layout
supports chars_password. authd validates the broker's layout against
the client's declared capabilities and rejects undeclared fields, so a
client without wait support would see entra_auth offered yet fail every
attempt to select it.

All in-repo PAM clients declare wait, so this only affects clients that
self-declare their capabilities (GDM's JSON protocol, third-party D-Bus
consumers). The shared test fixture now declares wait like the real
clients do.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EndSession freed the MFA flow synchronously, so an in-flight
AcquireTokenByMFAFlow call could hold MFAFlowState.mu and stall the
D-Bus reply, and with it PAM teardown, for the broker HTTP timeout.
Release the flow asynchronously while authentication is still running;
the idempotent, mutex-guarded free remains synchronous otherwise.

Use denyAndClearMFA for terminal denial paths so the flow, password hash,
and FIDO PIN are cleared consistently. Clarify that completed-flow replay
also covers failure redirects, not only successful MFA transitions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cAuthOptions silently drops unknown AuthOption values, so it can return
an empty slice even for non-empty input, and indexing &options[0]
unconditionally would panic inside the broker daemon. Unreachable today
only because InitiateMFAFlow always prepends NoDAGFallback; use
unsafe.SliceData, which yields nil for a nil slice, and the C API
accepts NULL with length 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The poll-based and code-entry follow-up modes are only used for MFA
challenges, so calling them entra_auth_wait/entra_auth_code was
misleading after the main mode was renamed to entra_auth. Restore the
MFA-specific naming for these two modes.
parseFlowsConfig already treats having both device_code and entra_auth
disabled as an invalid configuration and makes the broker fail to start.
Update the comment to match that behavior.
@nooreldeenmansour

nooreldeenmansour commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

Just missing the e2e-test now

Automated an e2e test for TAP MFA login, as discussed in #1723 (comment). Review the last commit for details (Had to add an application permission to automate the generation of the TAPs)

Previously it always fell back to the Entra password prompt; a small libhimmelblau patch was needed to enable TAP MFA, and now it works correctly (Users with a temporary access pass are shown MFA method directly, rather than "Enter your Entra ID password"

Passed locally; waiting on CI.

Edit: The new test is passing in the last runs, first, second run

Latest forced push is to trim the comments verbosity, and to ensure that the temporary access passwords are cleared for the tests that do not require them (if not already cleared) to prevent failures

Test passwordless Entra sign-in via a Temporary Access Pass (TAP)
instead of a regular password.

Mint and revoke the TAP per-test through the Microsoft Graph API
instead of provisioning a dedicated passwordless account, so the
existing shared test user can be reused without affecting the
password-based tests.
Other Entra e2e tests that share the same test account started
failing intermittently, timing out waiting for the Entra password
prompt.

Entra was showing a Temporary Access Pass challenge instead: once a
TAP exists for an account, Entra offers it to any interactive
sign-in for that account, regardless of which flow started the
session. Since the passwordless test's TAP can outlive its own
teardown (or a stray one can come from unrelated manual testing),
any other test using the same account can hit it unexpectedly.

None of the other Entra tests know about TAP, so add a defensive,
best-effort cleanup at the shared entry points that perform a real
interactive Entra sign-in, rather than relying solely on the
passwordless test's own teardown to keep the account clean.

The three release channels run concurrently against the same
account, so also gate the cleanup on TAP age: a very fresh TAP may
belong to a passwordless test still in flight in another channel,
and deleting it out from under that test would trade one kind of
cross-test breakage for another.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e-tests This issue is related to end-to-end tests / Run end-to-end tests on this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Link to Authenticator Apps

3 participants