Skip to content

Direct Password and MFA Authentication with MS Entra ID#1520

Open
nooreldeenmansour wants to merge 17 commits into
mainfrom
002-entra-password-mfa-auth
Open

Direct Password and MFA Authentication with MS Entra ID#1520
nooreldeenmansour wants to merge 17 commits into
mainfrom
002-entra-password-mfa-auth

Conversation

@nooreldeenmansour

@nooreldeenmansour nooreldeenmansour commented May 18, 2026

Copy link
Copy Markdown
Member

Adds a direct password + MFA authentication flow to the MS Entra ID broker as an additional bootstrap authentication method alongside device authentication. Users can authenticate with entra_password, complete MFA via push notification (entra_mfa_wait) or OTP/SMS code (entra_mfa_code), then authd caches the OAuth2 token and stores an Argon2id password hash for offline re-authentication.

Changes Summary

  • Add broker support for direct Entra password authentication, MFA follow-up steps, and user-facing handling for common AADSTS failures such as expired passwords, account lockouts, and required MFA enrollment.
  • Split password hashing from persistence so the plaintext is hashed immediately and written to disk only after MFA succeeds.
  • Extend the MS Entra ID provider and Himmelblau bindings with the APIs needed to start and complete the Entra password + MFA flow.
  • Add a Graph fallback for group lookup: fall back to an app-only client-credentials token when the delegated token lacks GroupMember.Read.All and a client_secret is configured.
  • Replace the single cached Himmelblau broker app with a cache keyed by client configuration so device registration and Entra password login can reuse separate apps safely.
  • Add [flows] toggles for entra_password and device_auth, and validate the prerequisites for entra_password at startup. When neither register_device = true nor a configured client_secret can support group lookup, the flow is disabled or rejected if it would leave no bootstrap login method.
  • Document the new flow, its prerequisites, and the cases that fall back to device authentication.

Notes

  • libhimmelblau is temporarily pinned to a fork branch that carries the MFA C API pending upstream review.
  • entra_password remains disabled in the e2e provisioning script until e2e coverage is added.

Related Issues and Discussions (WIP)

UDENG-6756

@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 4 times, most recently from 71977ed to d44496f Compare May 18, 2026 13:22
@codecov

codecov Bot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.51254% with 92 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.21%. Comparing base (31f8478) to head (fb41ec0).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
authd-oidc-brokers/internal/broker/broker.go 83.20% 66 Missing ⚠️
...internal/providers/msentraid/tokenverify/keyset.go 75.00% 10 Missing ⚠️
...nal/providers/msentraid/tokenverify/tokenverify.go 82.85% 6 Missing ⚠️
authd-oidc-brokers/internal/broker/config.go 85.71% 3 Missing ⚠️
authd-oidc-brokers/internal/password/password.go 77.77% 2 Missing ⚠️
internal/services/pam/pam.go 80.00% 2 Missing ⚠️
pam/internal/adapter/nativemodel.go 33.33% 2 Missing ⚠️
internal/brokers/broker.go 94.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1520      +/-   ##
==========================================
+ Coverage   84.24%   87.21%   +2.96%     
==========================================
  Files          21      124     +103     
  Lines        1168     8463    +7295     
  Branches        0      111     +111     
==========================================
+ Hits          984     7381    +6397     
- Misses        184     1026     +842     
- Partials        0       56      +56     

☔ 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.

@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 8 times, most recently from 95281f3 to c593968 Compare May 20, 2026 12:18
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 12 times, most recently from 552221e to 4b2a458 Compare June 5, 2026 12:18
@nooreldeenmansour nooreldeenmansour marked this pull request as ready for review June 5, 2026 12:19
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 2 times, most recently from 7f4a985 to e3a0ec1 Compare June 8, 2026 17:51
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 2 times, most recently from 509900b to a1e0379 Compare June 24, 2026 13:04
nooreldeenmansour added a commit that referenced this pull request Jun 24, 2026
After the Entra password+MFA flow caches the user's password for offline
login, the user had no indication that their local password was set to
their Entra password (#1520 discussion).

Attach a broker-owned notice to the granted response so it surfaces
through the PAM conversation. The notice lives in the broker — the
component that knows when caching occurred — rather than being hardcoded
in authd.

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-Authored-By: GitHub Copilot <noreply@github.com>
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 2 times, most recently from 2639346 to b91ee52 Compare June 24, 2026 14:07
nooreldeenmansour added a commit that referenced this pull request Jun 24, 2026
libhimmelblau now returns dedicated MsalError variants —
AuthorizationDenied, MFAInvalidCode, MFADAGFallbackDisabled — with
matching MSAL_ERROR_CODE C enum values for the three MFA outcomes
that authd previously classified by matching on error message text.

Replace all three string-matching blocks in newMFAError with
code-based classification in mfaErrorCategory, removing the fragile
dependency on libhimmelblau's error wording. The strings import is
retained for AADSTS prefix matching in acquireTokenByRefreshToken.

Extend TestMFAErrorCategoryMapping to pin the three new code ->
category mappings alongside the existing enum-drift checks.

Bump the libhimmelblau submodule to the revision carrying the new
structured error variants.

See PR #1520 discussions r3453231791 and r3453255404.
@adombeck adombeck mentioned this pull request Jun 24, 2026
@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 Jun 25, 2026
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch 3 times, most recently from 18d1001 to 9140aa9 Compare June 25, 2026 10:52
Move `DeviceRegistrationData` and its validation helper into an
untagged file so cached device-registration JSON can be validated
without the `libhimmelblau` cgo build.
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch from b67c2f3 to de9431b Compare June 25, 2026 13:56
nooreldeenmansour and others added 11 commits June 25, 2026 17:41
Add the Go-side types and Cgo bindings needed to start and continue the
`libhimmelblau` MFA flow from the broker.

Also add helpers to extract user identity fields from access token
claims, since the Entra password flow returns an access token rather
than a standard OIDC ID token.

Bump `libhimmelblau` to the revision that exposes the MFA C API.
The previous `sync.Once` singleton permanently reused the first client
configuration that initialized the broker app.

Replace it with a keyed cache so device registration and Entra password
login can reuse separate broker apps without interfering with each
other.
When the delegated token cannot call Microsoft Graph directly, let
`GetGroups` fall back to an app-only client-credentials token derived
from the configured OIDC client secret.

Also keep the Graph-token requirement in cached auth state instead of
provider-global mutable state.
Implement the provider methods that start the Entra password flow and
complete its MFA challenge. Also advertise `entra_password` in the
supported auth modes.
Split `HashAndStorePassword` into separate hashing and persistence
steps so callers can hash the plaintext immediately and write the
result to disk only after MFA succeeds.
Add the broker-side login flow for direct Entra password authentication
and its MFA follow-up modes, including challenge routing, `AADSTS`
error handling, and finalization of cached tokens plus the local
password hash.

Also add `[flows]` configuration for bootstrap auth mode selection and
validate the Entra password prerequisites, disabling the flow when
neither device registration nor a configured client secret can support
group lookup after login.
Point the `libhimmelblau` submodule at a temporary fork that carries
the MFA C API needed by this branch until the upstream changes land.
The current e2e environment relies on device auth flow, so keep the new
flow disabled there for now to avoid breaking the current expected
flow, until e2e tests for `entra_password` are added
The Entra password + MFA flow reads the user's identity from the access
token's claims (decoded by libhimmelblau) rather than from a verified
OIDC ID token, so the trust previously rested on the TLS connection to
Entra alone: a man-in-the-middle able to present a valid certificate for
the Microsoft endpoint could have forged the identity claims.

Verify the access token's RS256 signature against the tenant's published
JWKS, and its tenant, before trusting any claim from it; deny the login
on any verification failure. Microsoft first-party (e.g. Graph-scoped)
tokens carry a header nonce that is SHA256-rewritten before signing, so
the verifier reproduces that rewrite; all login variants then validate.

The signature, nonce-rewrite and JWKS logic lives in a cgo-free
tokenverify package so it is unit-tested directly; the provider supplies
the tenant JWKS via the EntraPasswordProvider interface.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When MFA setup fails (authenticator not registered, interactive auth
required), libhimmelblau silently fell back to its own Device
Authorization Grant flow, showing a plain browser-URL prompt that
conflicts with authd's QR-code Device Authentication screen and
bypasses the auth state machine entirely.

AADSTS 50203 and 16000 surface the same MFA-not-configured condition
from a different server path and are routed to Device Authentication
for the same reason.

Based on https://gitlab.com/nooreldeensalah/libhimmelblau/-/tree/capi-mfa-auth-options
libhimmelblau now returns dedicated error variants for the three MFA
outcomes that authd previously classified by matching on error message
text. Remove the string-matching blocks from newMFAError and map the
new C enum codes directly in mfaErrorCategory.
@nooreldeenmansour nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch from de9431b to 7a68890 Compare June 25, 2026 14:44
nooreldeenmansour and others added 5 commits June 25, 2026 18:18
The Entra password+MFA login failed with "invalid character 'Y' looking
for beginning of value" after a successful grant. The broker emitted the
success message as a bare string while the consumer (dataToMsg) expects
a {"message": ...} envelope, so the PAM client's parse aborted an
already-granted login.

Forward a consistent {userinfo, message} envelope from IsAuthenticated
so consumers always parse the same shape regardless of whether the broker
attached a notice. Encode IAResponse.Msg as {"message": ...} matching
the format used for non-granted replies. Non-string values in the broker's
message field are treated as absent rather than rejected, so a malformed
cosmetic field never blocks an already-granted login at the broker boundary.

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
A cosmetic notice attached to a granted IAResponse must never abort an
already-granted login. Treat a parse error on the Msg field as a
warning and fall back to showing no notice.

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
After the Entra password+MFA flow caches the user's password for offline
login, the user had no indication that their local password was set to
their Entra password

Attach a broker-owned notice to the granted response so it surfaces
through the PAM conversation. The notice lives in the broker — the
component that knows when caching occurred — rather than being hardcoded
in authd.

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Native (SSH, non-TTY) clients receive the granted message twice: once
through nativeModel.sendInfo and again through the PAM TextInfo
conversation echo in sendReturnMessageToPam.  GDM and interactive-
terminal clients do not go through the native sendInfo path, so they
must keep receiving the echo.

Suppress the redundant PAM-conversation echo only for Native clients by
returning false from shouldSendAuthMessage when clientType == Native and
the response is a success.

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
UDENG-10797


After the Entra password+MFA flow caches the user's password for offline
login,
users had no indication that their local password was set to their Entra
password
([#1520
discussion](#1520 (comment))).

## Changes

- **`broker`**: Attaches a broker-owned caching notice to the granted
response so it
surfaces through the PAM conversation. The notice lives in the broker —
the component
  that knows when caching occurred.

- **`broker/pam`**: Normalizes the granted-message envelope. Granted
responses were
emitting the notice as a bare string instead of the `{"message": ...}`
envelope every
consumer expects, aborting already-granted logins with `invalid
character 'Y'`. Non-string
  message values are treated as absent rather than rejecting the login.

- **`pam`**: Fixes a double-echo on native clients (SSH). `Native`
clients were printing
the notice twice — once via `nativeModel.sendInfo` and again via
`sendReturnMessageToPam`.
GDM and interactive-terminal clients do not go through the native
sendInfo path and
  continue to receive the notice through the PAM conversation.
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.

4 participants