diff --git a/.github/workflows/brokers-qa.yaml b/.github/workflows/brokers-qa.yaml index 7877dbe993..94d7443e47 100644 --- a/.github/workflows/brokers-qa.yaml +++ b/.github/workflows/brokers-qa.yaml @@ -45,6 +45,14 @@ jobs: path: ~/.cargo/bin/cargo-cbuild key: ${{ runner.os }}-cargo-c-${{ hashFiles('authd-oidc-brokers/tools/install-cargo-c') }} + - uses: canonical/desktop-engineering/gh-actions/common/dpkg-install-speedup@main + - name: Install dependencies + run: | + set -eu + sudo apt-get update + # Headers for the cgo bindings to libfido2 (internal/fido). + sudo apt-get install -y libfido2-dev + - name: Build libhimmelblau # The code sanity check fails if himmelblau.h does not exist, so we generate it first. run: go generate --tags withmsentraid ./internal/providers/msentraid/... @@ -87,7 +95,9 @@ jobs: run: | set -eu sudo apt-get update - sudo apt-get install -y git-delta + # libfido2-dev provides the headers for the cgo bindings to libfido2 + # (internal/fido). + sudo apt-get install -y git-delta libfido2-dev - name: Install coverage collection dependencies if: matrix.test == 'coverage' diff --git a/.github/workflows/e2e-tests-run.yaml b/.github/workflows/e2e-tests-run.yaml index e8ee15dfe3..cafc8ac527 100644 --- a/.github/workflows/e2e-tests-run.yaml +++ b/.github/workflows/e2e-tests-run.yaml @@ -233,6 +233,8 @@ jobs: export E2E_USER="${{ secrets.E2E_MSENTRA_USERNAME }}" export E2E_PASSWORD="${{ secrets.E2E_MSENTRA_PASSWORD }}" export TOTP_SECRET="${{ secrets.E2E_MSENTRA_TOTP_SECRET }}" + export AUTHD_MSENTRAID_ISSUER_ID="${{ secrets.E2E_MSENTRA_ISSUER_ID }}" + export AUTHD_MSENTRAID_CLIENT_ID="${{ secrets.E2E_MSENTRA_CLIENT_ID }}" export AUTHD_MSENTRAID_CLIENT_SECRET="${{ secrets.E2E_MSENTRA_CLIENT_SECRET }}" elif [ "${{ inputs.broker }}" = "authd-google" ]; then export E2E_USER="${{ secrets.E2E_GOOGLE_USERNAME }}" diff --git a/.github/workflows/tics-run.yaml b/.github/workflows/tics-run.yaml index 6273afffb8..f8767c51ed 100644 --- a/.github/workflows/tics-run.yaml +++ b/.github/workflows/tics-run.yaml @@ -12,6 +12,7 @@ env: clang-tools clang dotnet8 + libfido2-dev libglib2.0-dev libpam-dev libpwquality-dev diff --git a/authd-oidc-brokers/cmd/authd-oidc/daemon/export_test.go b/authd-oidc-brokers/cmd/authd-oidc/daemon/export_test.go index c9098d6cc4..741df35781 100644 --- a/authd-oidc-brokers/cmd/authd-oidc/daemon/export_test.go +++ b/authd-oidc-brokers/cmd/authd-oidc/daemon/export_test.go @@ -75,10 +75,10 @@ issuer = %s client_id = client_id [flows] -# These tests don't exercise the entra_password flow, and the default +# These tests don't exercise the entra_auth flow, and the default # (enabled) would fail startup validation under the withmsentraid tag # because no client_secret or register_device is configured here. -entra_password = false +entra_auth = false `, providerURL) err = os.WriteFile(p, []byte(brokerCfg), 0600) require.NoError(t, err, "Setup: could not create broker configuration for tests") diff --git a/authd-oidc-brokers/conf/variants/msentraid/broker.conf b/authd-oidc-brokers/conf/variants/msentraid/broker.conf index 4ef7636097..3794a29c71 100644 --- a/authd-oidc-brokers/conf/variants/msentraid/broker.conf +++ b/authd-oidc-brokers/conf/variants/msentraid/broker.conf @@ -105,9 +105,12 @@ client_id = ## a code). #device_code = true -## entra_password: When true (default), users can authenticate by entering -## their Microsoft Entra ID password directly, followed by MFA verification. +## entra_auth: When true (default), users can authenticate with the +## Microsoft Entra ID direct-auth flow. +## When the user has enrolled passwordless methods (FIDO2 security keys, +## Microsoft Authenticator, or a Temporary Access Pass), the flow negotiates +## those automatically; otherwise it falls back to password + MFA. ## -## Note: If both flows are disabled, no authentication will be available -## and users will not be able to log in. -#entra_password = true +## Note: If both device_code and entra_auth are disabled, the broker +## considers the configuration invalid and will fail to start. +#entra_auth = true diff --git a/authd-oidc-brokers/go.mod b/authd-oidc-brokers/go.mod index 8590e0209e..86d460601e 100644 --- a/authd-oidc-brokers/go.mod +++ b/authd-oidc-brokers/go.mod @@ -14,6 +14,7 @@ require ( github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/uuid v1.6.0 github.com/k0kubun/pp v3.0.1+incompatible + github.com/keys-pub/go-libfido2 v1.5.3 github.com/microsoftgraph/msgraph-sdk-go v1.100.0 github.com/microsoftgraph/msgraph-sdk-go-core v1.4.1 github.com/mitchellh/mapstructure v1.5.0 @@ -52,6 +53,7 @@ require ( github.com/otiai10/copy v1.14.1 // indirect github.com/otiai10/mint v1.6.3 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/sagikazarmark/locafero v0.11.0 // indirect github.com/sirupsen/logrus v1.9.3 // indirect diff --git a/authd-oidc-brokers/go.sum b/authd-oidc-brokers/go.sum index f6a2bc3d1f..1b38670e27 100644 --- a/authd-oidc-brokers/go.sum +++ b/authd-oidc-brokers/go.sum @@ -44,6 +44,8 @@ github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88 h1:uC1QfSlInpQ github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k= github.com/k0kubun/pp v3.0.1+incompatible h1:3tqvf7QgUnZ5tXO6pNAZlrvHgl6DvifjDrd9g2S9Z40= github.com/k0kubun/pp v3.0.1+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg= +github.com/keys-pub/go-libfido2 v1.5.3 h1:vtgHxlSB43u6lj0TSuA3VvT6z3E7VI+L1a2hvMFdECk= +github.com/keys-pub/go-libfido2 v1.5.3/go.mod h1:P0V19qHwJNY0htZwZDe9Ilvs/nokGhdFX7faKFyZ6+U= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -80,6 +82,8 @@ github.com/otiai10/mint v1.6.3 h1:87qsV/aw1F5as1eH1zS/yqHY85ANKVMgkDrf9rcxbQs= github.com/otiai10/mint v1.6.3/go.mod h1:MJm72SBthJjz8qhefc4z1PYEieWmy8Bku7CjcAqyUSM= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -109,6 +113,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= @@ -145,6 +150,7 @@ golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= @@ -174,6 +180,7 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/ini.v1 v1.67.3 h1:iM9Lhz5MRSGhHVGGwCuzG9KO8PoirCXj/m/qTmOJJQw= gopkg.in/ini.v1 v1.67.3/go.mod h1:x/cyOwCgZqOkJoDIJ3c1KNHMo10+nLGAhh+kn3Zizss= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/authd-oidc-brokers/internal/broker/authmodes/consts.go b/authd-oidc-brokers/internal/broker/authmodes/consts.go index e8f4532955..1dc219bb7c 100644 --- a/authd-oidc-brokers/internal/broker/authmodes/consts.go +++ b/authd-oidc-brokers/internal/broker/authmodes/consts.go @@ -16,26 +16,35 @@ const ( // NewPassword is the ID of the new password configuration method. NewPassword = "newpassword" - // EntraPassword is the ID of the Entra ID password + MFA authentication flow. - EntraPassword = "entra_password" + // EntraAuth is the ID of the Entra ID password/passwordless authentication method. + EntraAuth = "entra_auth" // EntraMFAWait is the ID of the poll-based MFA follow-up mode. EntraMFAWait = "entra_mfa_wait" // EntraMFACode is the ID of the code-entry MFA follow-up mode. EntraMFACode = "entra_mfa_code" + + // EntraAuthFido is the ID of the security-key MFA follow-up mode, which + // performs the WebAuthn assertion with a locally connected FIDO2 device. + EntraAuthFido = "entra_auth_fido" + + // EntraAuthFidoPin is the ID of the security-key PIN entry mode, chained + // before EntraAuthFido when the connected device requires a client PIN. + EntraAuthFidoPin = "entra_auth_fido_pin" ) var ( // Label is a map of auth mode IDs to their display labels. - //nolint:gosec // G101: These are auth mode display labels, not credentials. Label = map[string]string{ - Password: "Local password", - Device: "Device code flow", - DeviceQr: "Device code flow", - NewPassword: "Define your local password", - EntraPassword: "Entra ID password + MFA", - EntraMFAWait: "Waiting for MFA approval", - EntraMFACode: "Enter your MFA code", + Password: "Local password", + Device: "Device code flow", + DeviceQr: "Device code flow", + NewPassword: "Define your local password", + EntraAuth: "Entra ID authentication", + EntraMFAWait: "Waiting for MFA approval", + EntraMFACode: "Enter your MFA code", + EntraAuthFido: "Use your security key", + EntraAuthFidoPin: "Enter your security key PIN", } ) diff --git a/authd-oidc-brokers/internal/broker/broker.go b/authd-oidc-brokers/internal/broker/broker.go index e748b41659..2548418a0e 100644 --- a/authd-oidc-brokers/internal/broker/broker.go +++ b/authd-oidc-brokers/internal/broker/broker.go @@ -21,6 +21,7 @@ import ( "github.com/canonical/authd/authd-oidc-brokers/internal/broker/authmodes" "github.com/canonical/authd/authd-oidc-brokers/internal/broker/sessionmode" "github.com/canonical/authd/authd-oidc-brokers/internal/consts" + "github.com/canonical/authd/authd-oidc-brokers/internal/fido" "github.com/canonical/authd/authd-oidc-brokers/internal/fileutils" "github.com/canonical/authd/authd-oidc-brokers/internal/password" "github.com/canonical/authd/authd-oidc-brokers/internal/providers" @@ -50,7 +51,7 @@ const ( // reauthModes is the set of auth modes offered when the user must re-authenticate // (e.g. after token revocation, expiry, or password change). -var reauthModes = []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr} +var reauthModes = []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr} // Config is the configuration for the broker. type Config struct { @@ -60,6 +61,21 @@ type Config struct { userConfig } +// fidoAuthenticator performs WebAuthn assertions with a locally connected +// FIDO2 security key. It is implemented by fido.Authenticator in withmsentraid +// builds; in other builds there is no implementation, the broker's field stays +// nil and the FIDO auth modes are disabled. +type fidoAuthenticator interface { + // DevicePresent reports whether a FIDO device is connected to this machine. + DevicePresent() bool + // DeviceRequiresPIN reports whether the device needs a client PIN for + // user verification. + DeviceRequiresPIN() (bool, error) + // Assert performs the WebAuthn Get ceremony and returns the assertion + // JSON to pass back to the MFA flow as auth data. + Assert(ctx context.Context, challenge string, allowList []string, pin string) (string, error) +} + // Broker is the real implementation of the broker to track sessions and process oidc calls. type Broker struct { cfg Config @@ -68,6 +84,7 @@ type Broker struct { provider providers.Provider oidcCfg oidc.Config oidcClientSecret string + fido fidoAuthenticator currentSessions map[string]session currentSessionsMu sync.RWMutex @@ -95,11 +112,13 @@ type session struct { tokenPath string // Data to pass from one request to another. - deviceAuthResponse *oauth2.DeviceAuthResponse - authInfo *token.AuthCachedInfo - mfaFlowActive *himmelblau.MFAFlowState - mfaChallengeInfo *himmelblau.MFAChallengeInfo - entraPasswordHash string // pre-computed hash (not plaintext) for offline use + deviceAuthResponse *oauth2.DeviceAuthResponse + authInfo *token.AuthCachedInfo + mfaFlowActive *himmelblau.MFAFlowState + mfaChallengeInfo *himmelblau.MFAChallengeInfo + entraAuthPasswordHash string // pre-computed hash (not plaintext) for offline use + entraAuthPasswordRequired bool + fidoPIN string // security key PIN, kept in memory only while the FIDO exchange runs isAuthenticating *isAuthenticatedCtx } @@ -109,14 +128,14 @@ type isAuthenticatedCtx struct { cancelFunc context.CancelFunc } -// verifyAndExtractEntraUserInfo verifies the Entra MFA access token's RS256 +// verifyAndExtractEntraUserInfo verifies the Entra auth access token's RS256 // signature against the tenant JWKS and extracts user info from that verified // access token. It does NOT cross-check the username against the session; first // login does that via userInfoFromTokenExtras. func (b *Broker) verifyAndExtractEntraUserInfo(ctx context.Context, token *oauth2.Token) (info.User, error) { - ep, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider) + ep, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) if !ok { - return info.User{}, errors.New("provider does not support Entra password authentication") + return info.User{}, errors.New("provider does not support Entra authentication") } if err := ep.VerifyAccessToken(ctx, b.cfg.issuerURL, token.AccessToken); err != nil { return info.User{}, fmt.Errorf("access token verification failed: %w", err) @@ -196,6 +215,7 @@ func (b *Broker) populateAuthInfo(ctx context.Context, session *session, t *oaut type option struct { provider providers.Provider + fido fidoAuthenticator } // Option is a func that allows to override some of the broker default settings. @@ -214,6 +234,7 @@ func New(cfg Config, apiVersion uint, args ...Option) (b *Broker, err error) { opts := option{ provider: p, + fido: defaultFIDOAuthenticator(), } for _, arg := range args { arg(&opts) @@ -228,17 +249,17 @@ func New(cfg Config, apiVersion uint, args ...Option) (b *Broker, err error) { if cfg.clientID == "" { err = errors.Join(err, errors.New("client ID is required and was not provided")) } - // The entra_password flow can only retrieve groups from Microsoft Graph when + // The entra_auth flow can only retrieve groups from Microsoft Graph when // device registration or a client secret is available (see the matching check - // in isAuthModeAvailable). If neither is configured, the flow is unusable, so - // fail here rather than silently falling back at login time: a startup failure - // is far more visible to the administrator than a per-login denial. - if cfg.flows.EntraPassword && !cfg.registerDevice && cfg.clientSecret == "" { - if _, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](opts.provider); ok { + // in authModeIsAvailable). If neither is configured, the flow is unusable, so + // fail at startup rather than silently falling back at login time: a startup + // failure is far more visible to the administrator than a per-login denial. + if cfg.flows.EntraAuth && !cfg.registerDevice && cfg.clientSecret == "" { + if _, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](opts.provider); ok { err = errors.Join(err, fmt.Errorf( "invalid configuration: the %[1]q flow is enabled in [%[2]s], but it cannot retrieve group memberships from Microsoft Graph without %[3]q enabled or a %[4]q configured; "+ "fix this by either disabling %[1]q, enabling %[3]q, or granting the app the GroupMember.Read.All application permission and configuring a %[4]q", - flowsEntraPasswordKey, flowsSection, registerDeviceKey, clientSecret)) + flowsEntraAuthKey, flowsSection, registerDeviceKey, clientSecret)) } } if err != nil { @@ -275,6 +296,7 @@ func New(cfg Config, apiVersion uint, args ...Option) (b *Broker, err error) { provider: opts.provider, oidcCfg: oidc.Config{ClientID: clientID}, oidcClientSecret: oidcClientSecret, + fido: opts.fido, privateKey: privateKey, currentSessions: make(map[string]session), @@ -920,31 +942,31 @@ func (b *Broker) authModeIsAvailable(session session, authMode string) bool { return false } return true - case authmodes.EntraPassword: - if _, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider); !ok { + case authmodes.EntraAuth: + if _, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider); !ok { return false } - if !b.cfg.flows.EntraPassword { - log.Debugf(context.Background(), "The %q flow is disabled in the [flows] config, so it is not available", authmodes.EntraPassword) + if !b.cfg.flows.EntraAuth { + log.Debugf(context.Background(), "The %q flow is disabled in the [flows] config, so it is not available", authmodes.EntraAuth) return false } if session.isOffline { - log.Debugf(context.Background(), "Session is in offline mode, so Entra password authentication is not available") + log.Debugf(context.Background(), "Session is in offline mode, so Entra %s authentication is not available", authMode) return false } - // The entra_password flow can only retrieve groups from Microsoft Graph - // when device registration (PRT-based token exchange) or a client secret - // (app-only client credentials) is available. Without either, every - // entra_password login would fail at the group-fetch step, so don't offer - // the mode rather than letting users hit an undiagnosable denial. New() - // already rejects that configuration for real broker startup, so this is a - // defensive guard for tests or manually constructed brokers. + // The entra_auth flow can only retrieve groups from Microsoft Graph + // when device registration (PRT-based token exchange) or a client + // secret (app-only client credentials) is available. Without either, + // every login would fail at the group-fetch step, so don't offer the + // mode rather than letting users hit an undiagnosable denial. New() + // rejects that configuration at startup, so this is a defensive guard + // for tests or manually constructed brokers. if !b.cfg.registerDevice && b.cfg.clientSecret == "" { - log.Debugf(context.Background(), "The %q flow requires %q to be enabled or a client secret to be configured to retrieve groups from Microsoft Graph, so it is not available", flowsEntraPasswordKey, registerDeviceKey) + log.Debugf(context.Background(), "The %q flow requires %q to be enabled or a client secret to be configured to retrieve groups from Microsoft Graph, so it is not available", flowsEntraAuthKey, registerDeviceKey) return false } return true - case authmodes.EntraMFAWait, authmodes.EntraMFACode: + case authmodes.EntraMFAWait, authmodes.EntraMFACode, authmodes.EntraAuthFido, authmodes.EntraAuthFidoPin: // MFA follow-up modes are always available when offered via AuthNext. return true } @@ -989,11 +1011,20 @@ func (b *Broker) supportedAuthModesFromLayout(layout map[string]string) []string case "form": var modes []string + supportsWait := strings.Contains(layout["wait"], "true") if slices.Contains(supportedEntries, "chars_password") { - modes = append(modes, authmodes.Password, authmodes.EntraPassword) + modes = append(modes, authmodes.Password, authmodes.EntraAuthFidoPin) + // The entra_auth mode needs both entry capabilities: its initial + // passwordless-probe layout is a wait-only form, and its password + // layout is a chars_password form. Offering it to a client that + // cannot render one of them would make the mode unselectable + // (authd rejects layouts with undeclared fields). + if supportsWait { + modes = append(modes, authmodes.EntraAuth) + } } - if strings.Contains(layout["wait"], "true") { - modes = append(modes, authmodes.EntraMFAWait) + if supportsWait { + modes = append(modes, authmodes.EntraMFAWait, authmodes.EntraAuthFido) } if slices.Contains(supportedEntries, "chars") { modes = append(modes, authmodes.EntraMFACode) @@ -1091,11 +1122,19 @@ func (b *Broker) generateUILayout(session *session, authModeID string) (map[stri "entry": "chars_password", } - case authmodes.EntraPassword: - uiLayout = map[string]string{ - "type": "form", - "label": "Enter your Entra ID password", - "entry": "chars_password", + case authmodes.EntraAuth: + if session.entraAuthPasswordRequired { + uiLayout = map[string]string{ + "type": "form", + "label": "Enter your Entra ID password", + "entry": "chars_password", + } + } else { + uiLayout = map[string]string{ + "type": "form", + "label": "Checking available Entra ID authentication methods...", + "wait": "true", + } } case authmodes.EntraMFAWait: @@ -1116,6 +1155,20 @@ func (b *Broker) generateUILayout(session *session, authModeID string) (map[stri "label": "Enter your MFA code", } + case authmodes.EntraAuthFido: + uiLayout = map[string]string{ + "type": "form", + "label": "Insert your security key and touch it", + "wait": "true", + } + + case authmodes.EntraAuthFidoPin: + uiLayout = map[string]string{ + "type": "form", + "entry": "chars_password", + "label": "Enter your security key PIN", + } + case authmodes.NewPassword: label := "Create a local password" if session.mode == sessionmode.ChangePassword || session.mode == sessionmode.ChangePasswordOld { @@ -1180,8 +1233,8 @@ func (b *Broker) IsAuthenticated(sessionID, authenticationData string) (string, iadResponse = errorMessage{Message: "Maximum number of authentication attempts reached"} // Free any in-progress MFA flow immediately rather than waiting for // EndSession — consistent with all other terminal paths. - session.entraPasswordHash = "" - clearEntraMFAState(&session) + session.entraAuthPasswordHash = "" + clearEntraAuthState(&session) } } @@ -1225,12 +1278,16 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, session *session, au return b.passwordAuth(ctx, session, secret) case authmodes.NewPassword: return b.newPassword(session, secret) - case authmodes.EntraPassword: - return b.entraPasswordAuth(ctx, session, secret) + case authmodes.EntraAuth: + return b.entraAuth(ctx, session, secret) case authmodes.EntraMFAWait: return b.entraMFAWaitAuth(ctx, session) case authmodes.EntraMFACode: return b.entraMFACodeAuth(ctx, session, secret) + case authmodes.EntraAuthFido: + return b.entraAuthFidoAuth(ctx, session) + case authmodes.EntraAuthFidoPin: + return b.entraAuthFidoPinAuth(session, secret) default: log.Errorf(context.Background(), "unknown authentication mode %q", session.selectedMode) return AuthDenied, unexpectedErrMsg("unknown authentication mode") @@ -1340,7 +1397,7 @@ func (b *Broker) passwordAuth(ctx context.Context, session *session, secret stri // Refresh the token on every online login (even if it has not expired) to // re-verify the account with the provider. This refresh is also the live - // disabled/revoked-user check. Entra password + MFA tokens are issued by the + // disabled/revoked-user check. Tokens obtained via the Entra auth flow are issued by the // Microsoft Broker App and are refreshed as a public client (no client_secret) // via the provider; all other tokens use the OIDC app refresh. Both paths feed // the same error classification below. @@ -1353,8 +1410,8 @@ func (b *Broker) passwordAuth(ctx context.Context, session *session, secret stri session.nextAuthModes = reauthModes return AuthNext, errorMessage{Message: "Remote authentication failed: No refresh token. Please contact your administrator."} } - if authInfo.ObtainedViaEntraPasswordAuth { - authInfo, err = b.refreshEntraPasswordToken(ctx, session, authInfo) + if authInfo.ObtainedViaEntraAuth { + authInfo, err = b.refreshEntraToken(ctx, session, authInfo) } else { authInfo, err = b.refreshToken(ctx, session, authInfo) } @@ -1475,17 +1532,21 @@ func (b *Broker) passwordAuth(ctx context.Context, session *session, secret stri return b.finishAuth(session, authInfo) } -func (b *Broker) entraPasswordAuth(ctx context.Context, session *session, userPassword string) (string, isAuthenticatedDataResponse) { - entraProvider, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider) +func (b *Broker) entraAuth(ctx context.Context, session *session, userPassword string) (string, isAuthenticatedDataResponse) { + entraProvider, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) if !ok { - log.Error(context.Background(), "entra_password mode selected but provider does not support it") - return AuthDenied, unexpectedErrMsg("provider does not support Entra password authentication") + log.Error(context.Background(), "entra_auth mode selected but provider does not support it") + return AuthDenied, unexpectedErrMsg("provider does not support Entra authentication") + } + passwordSubmitted := userPassword != "" + if session.entraAuthPasswordRequired && !passwordSubmitted { + return AuthRetry, errorMessage{Message: "Please enter your Entra ID password."} } // A prior MFA flow may still be active if the password step is restarted // (e.g. the user navigates back to re-enter the password). Release it before // starting a new one so the libhimmelblau continuation it owns is not leaked. - clearEntraMFAState(session) + clearEntraAuthState(session) // Load the cached auth info once at the start of the flow and stash it on the // session, so the second step (entra_mfa_wait/entra_mfa_code → finishEntraAuth) @@ -1501,15 +1562,30 @@ func (b *Broker) entraPasswordAuth(ctx context.Context, session *session, userPa } session.authInfo = cachedAuthInfo - // Existing device registration data for the MFA flow (from the cached info). + // Existing device registration data for the Entra auth flow (from the cached info). deviceRegistrationData := b.cachedDeviceRegistrationData(session) - // Use device-scoped MFA flow when we expect to register the device or - // already have valid device data for PRT-based token exchange. The || - // short-circuits so we skip parsing the data when registration is enabled. - withDeviceScope := b.cfg.registerDevice || himmelblau.ValidDeviceRegistrationDataJSON(deviceRegistrationData) - - flow, challengeInfo, err := entraProvider.InitiateEntraPasswordAuth(ctx, b.cfg.clientID, b.cfg.issuerURL, session.username, userPassword, deviceRegistrationData, withDeviceScope) + // Use device-scoped MFA only after a real password has been submitted. + // Passwordless probing passes a NULL password to libhimmelblau, and the + // native device/enrollment flow cannot operate without password-derived + // material. + withDeviceScope := passwordSubmitted && (b.cfg.registerDevice || himmelblau.ValidDeviceRegistrationDataJSON(deviceRegistrationData)) + + // Advertise FIDO assertion capability whenever this build can perform a + // local WebAuthn assertion at all, even if no key is currently plugged in. + // libhimmelblau forwards this as isFidoSupported to Entra, which controls + // whether it fetches the WebAuthn challenge; without it, Entra reports + // PASSWORD_REQUIRED for unplugged-key sessions and passwordless FIDO-only + // accounts are misrouted to a password prompt. The actual local-vs-remote + // gate is entraAuthFidoAuth, which waits for a key up to fidoDeviceWaitTimeout + // and then falls back to the device code flow, so a headless/SSH session + // where no key can appear is not stranded. + var authOpts []himmelblau.AuthOption + if b.fido != nil { + authOpts = append(authOpts, himmelblau.AuthOptionFido) + } + + flow, challengeInfo, err := entraProvider.InitiateEntraAuth(ctx, b.cfg.clientID, b.cfg.issuerURL, session.username, userPassword, deviceRegistrationData, withDeviceScope, authOpts...) if err != nil { var mfaErr *himmelblau.MFAError if errors.As(err, &mfaErr) { @@ -1517,16 +1593,16 @@ func (b *Broker) entraPasswordAuth(ctx context.Context, session *session, userPa } // A non-MFAError here is unexpected (the provider should classify expected // failures as MFAError); surface it as a reportable bug. - log.Errorf(context.Background(), "Entra password authentication failed: %v", err) - return AuthDenied, unexpectedErrMsg("failed to initiate Entra password flow") + log.Errorf(context.Background(), "Entra authentication failed: %v", err) + return AuthDenied, unexpectedErrMsg("failed to initiate Entra authentication flow") } if flow == nil || challengeInfo == nil { himmelblau.FreeMFAFlowState(flow) - log.Error(context.Background(), "Entra password authentication did not return a complete MFA challenge") + log.Error(context.Background(), "Entra authentication did not return a complete MFA challenge") return AuthDenied, unexpectedErrMsg("provider returned incomplete MFA challenge") } - // InitiateEntraPasswordAuth is a non-preemptible cgo call; if the request was + // InitiateEntraAuth is a non-preemptible cgo call; if the request was // cancelled while it was in flight, IsAuthenticated already returned via its // ctx.Done() branch without persisting this session update. Stashing the flow // on session at that point would make it unreachable, leaking the native @@ -1534,40 +1610,41 @@ func (b *Broker) entraPasswordAuth(ctx context.Context, session *session, userPa // as the equivalent check in finishEntraAuth). if ctx.Err() != nil { himmelblau.FreeMFAFlowState(flow) - log.Noticef(context.Background(), "Entra password authentication succeeded but the request was cancelled; discarding MFA flow for user %q", session.username) + log.Noticef(context.Background(), "Entra authentication succeeded but the request was cancelled; discarding MFA flow for user %q", session.username) return AuthCancelled, nil } session.mfaFlowActive = flow session.mfaChallengeInfo = challengeInfo + session.entraAuthPasswordRequired = false + session.entraAuthPasswordHash = "" - // Hash the password immediately to narrow the plaintext memory window. - // The hash is written to disk in finishEntraAuth after MFA succeeds. - passwordHash, hashErr := password.HashPassword(userPassword) - if hashErr != nil { - log.Errorf(context.Background(), "Failed to hash password: %v", hashErr) - clearEntraMFAState(session) - return AuthDenied, unexpectedErrMsg("failed to process password") + if passwordSubmitted { + // Hash the password immediately to narrow the plaintext memory window. + // The hash is written to disk in finishEntraAuth after MFA succeeds. + passwordHash, hashErr := password.HashPassword(userPassword) + if hashErr != nil { + log.Errorf(context.Background(), "Failed to hash password: %v", hashErr) + clearEntraAuthState(session) + return AuthDenied, unexpectedErrMsg("failed to process password") + } + session.entraAuthPasswordHash = passwordHash } - session.entraPasswordHash = passwordHash - // Determine MFA challenge type. + return b.routeMFAChallenge(session, challengeInfo) +} + +// routeMFAChallenge inspects the MFA challenge negotiated by the password +// entry mode and sets the session's next auth modes to the matching +// follow-up: the local security-key ceremony (or its Device +// Authentication fallback) for FIDO methods, code entry for prompt methods, +// and the out-of-band poll for the rest. +func (b *Broker) routeMFAChallenge(session *session, challengeInfo *himmelblau.MFAChallengeInfo) (string, isAuthenticatedDataResponse) { mfaMethod := challengeInfo.Method pollingInterval := challengeInfo.PollingIntervalMs - // FIDO/security-key MFA is not yet wired up in this terminal-based flow. - // This is an implementation gap, not a fundamental limitation: libhimmelblau - // can do FIDO (see https://github.com/himmelblau-idm/himmelblau/blob/main/src/common/src/auth.rs). - // TODO: support FIDO MFA directly without redirecting to the device code flow. if isFIDOMethod(mfaMethod) { - log.Noticef(context.Background(), "FIDO MFA method %q detected for user %q; redirecting to the device code flow", mfaMethod, session.username) - session.entraPasswordHash = "" - clearEntraMFAState(session) - if b.cfg.flows.DeviceAuth { - session.nextAuthModes = []string{authmodes.Device, authmodes.DeviceQr} - return AuthNext, errorMessage{Message: "This account requires FIDO/security key authentication. Please complete authentication using the device code flow."} - } - return AuthDenied, errorMessage{Message: "This account requires FIDO/security key authentication, which is not yet supported in this mode. The device code flow is also unavailable. Please contact your administrator."} + return b.routeFIDOChallenge(session, challengeInfo) } switch { @@ -1591,15 +1668,57 @@ func (b *Broker) entraPasswordAuth(ctx context.Context, session *session, userPa return AuthNext, nil } -func clearEntraMFAState(session *session) { +func clearEntraAuthState(session *session) { himmelblau.FreeMFAFlowState(session.mfaFlowActive) session.mfaFlowActive = nil session.mfaChallengeInfo = nil + session.fidoPIN = "" +} + +// restartFromEntraAuth handles a terminal MFA-step failure: it clears the +// now-dead MFA state (including the cached password hash) and directs the +// client back to entra_auth so it can restart the flow rather than +// re-entering a dead follow-up mode, showing msg to the user. +func restartFromEntraAuth(session *session, msg string) (string, isAuthenticatedDataResponse) { + needsPassword := session.entraAuthPasswordRequired || session.entraAuthPasswordHash != "" + session.entraAuthPasswordHash = "" + session.entraAuthPasswordRequired = needsPassword + clearEntraAuthState(session) + session.nextAuthModes = []string{authmodes.EntraAuth} + return AuthNext, errorMessage{Message: msg} +} + +// replayCompletedMFA handles a stray IsAuthenticated call for an MFA follow-up +// mode that arrives once mfaFlowActive is already nil: the previous call +// finished this step — successfully (chaining to the next mode) or with a +// failure that redirected elsewhere (restartFromEntraAuth, +// redirectFIDOToDeviceAuth) — and stored its transition in nextAuthModes. It +// replays that stored transition (AuthNext) instead of denying, steering the +// duplicate call to wherever the original outcome pointed (the message of a +// failed original was already delivered with that response); with no pending +// transition it denies. These flows are single-use, so replaying the stored +// transition is the only safe response to a duplicate call. +func replayCompletedMFA(session *session, mode string) (string, isAuthenticatedDataResponse) { + if len(session.nextAuthModes) > 0 && !slices.Contains(session.nextAuthModes, mode) { + log.Debugf(context.Background(), "%q mode selected again for user %q after already completing; replaying transition to %v", mode, session.username, session.nextAuthModes) + return AuthNext, nil + } + log.Errorf(context.Background(), "%q mode selected but no active MFA flow", mode) + return AuthDenied, unexpectedErrMsg("no active MFA flow") +} + +// denyAndClearMFA frees the MFA flow and wipes the cached password hash, then +// denies with data. Terminal FIDO/MFA failures use it; success paths keep the +// hash for offline caching, so clearEntraAuthState alone must not wipe it. +func denyAndClearMFA(session *session, data isAuthenticatedDataResponse) (string, isAuthenticatedDataResponse) { + session.entraAuthPasswordHash = "" + clearEntraAuthState(session) + return AuthDenied, data } // cachedDeviceRegistrationData returns the device registration data from the // session's cached auth info (loaded once at the start of the flow by -// entraPasswordAuth), or nil if there is no cached token or it carries none. +// entraAuth), or nil if there is no cached token or it carries none. func (b *Broker) cachedDeviceRegistrationData(session *session) []byte { if session.authInfo != nil { return session.authInfo.DeviceRegistrationData @@ -1608,15 +1727,14 @@ func (b *Broker) cachedDeviceRegistrationData(session *session) []byte { } func (b *Broker) entraMFAWaitAuth(ctx context.Context, session *session) (string, isAuthenticatedDataResponse) { - entraProvider, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider) + entraProvider, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) if !ok { log.Error(context.Background(), "entra_mfa_wait mode selected but provider does not support it") return AuthDenied, unexpectedErrMsg("provider does not support Entra MFA") } if session.mfaFlowActive == nil { - log.Error(context.Background(), "MFA wait mode selected but no active MFA flow") - return AuthDenied, unexpectedErrMsg("no active MFA flow") + return replayCompletedMFA(session, authmodes.EntraMFAWait) } if session.mfaChallengeInfo == nil { log.Error(context.Background(), "MFA wait mode selected but no MFA challenge metadata is available") @@ -1660,10 +1778,8 @@ func (b *Broker) entraMFAWaitAuth(ctx context.Context, session *session) (string // A user denial is terminal — handle it first, even if our poll // deadline happened to elapse during this (non-preemptible) call. if errors.As(err, &mfaErr) && mfaErr.IsMFADenied() { - session.entraPasswordHash = "" - clearEntraMFAState(session) log.Noticef(context.Background(), "MFA authentication denied for user %q", session.username) - return AuthDenied, errorMessage{Message: "MFA authentication was denied."} + return denyAndClearMFA(session, errorMessage{Message: "MFA authentication was denied."}) } // AcquireTokenByMFAFlow is a non-preemptible CGo call: our poll // deadline (or the caller's cancellation) can elapse while it is in @@ -1675,17 +1791,12 @@ func (b *Broker) entraMFAWaitAuth(ctx context.Context, session *session) (string return b.endExpiredMFAPoll(ctx, session) } // Genuine MFA failure. - session.entraPasswordHash = "" - clearEntraMFAState(session) log.Errorf(context.Background(), "MFA poll failed: %v", err) - // MFA flow state was cleared; direct the client back to entra_password - // so it can restart the flow rather than re-entering a dead MFA mode. - session.nextAuthModes = []string{authmodes.EntraPassword} - return AuthNext, errorMessage{Message: "MFA authentication failed. Please try again."} + return restartFromEntraAuth(session, "MFA authentication failed. Please try again.") } // MFA approved — finish auth. - clearEntraMFAState(session) + clearEntraAuthState(session) return b.finishEntraAuth(ctx, session, oauthToken) } @@ -1696,31 +1807,36 @@ func (b *Broker) entraMFAWaitAuth(ctx context.Context, session *session) (string // endExpiredMFAPoll handles a poll-loop exit caused by the internal poll // deadline elapsing, the caller cancelling the request, or the maximum number // of poll attempts being exhausted. It clears the now-dead MFA state and directs -// the client back to entra_password so it can restart the flow, distinguishing a +// the client back to entra_auth so it can restart the flow, distinguishing a // caller cancellation (AuthCancelled) from a wall-clock timeout (AuthNext). func (b *Broker) endExpiredMFAPoll(ctx context.Context, session *session) (string, isAuthenticatedDataResponse) { - session.entraPasswordHash = "" - clearEntraMFAState(session) - session.nextAuthModes = []string{authmodes.EntraPassword} if ctx.Err() != nil { - // The whole IsAuthenticated request was cancelled by the caller. + // The whole IsAuthenticated request was cancelled by the caller, which + // is usually transient (e.g. GDM re-selecting the wait mode). Like the + // FIDO assertion path, do NOT free the flow here: IsAuthenticated + // returns via its ctx.Done() branch and skips updateSession, so the + // stored session keeps its mfaFlowActive pointer. A cancelled poll does + // not consume the flow (libhimmelblau only advances it on success), so + // leaving it intact lets the resumed poll keep waiting for the same MFA + // approval instead of dead-ending on a released flow and looping back to + // the password probe. A genuinely terminal cancel is handled by + // EndSession, which frees the flow. log.Noticef(context.Background(), "MFA poll cancelled for user %q", session.username) return AuthCancelled, nil } log.Noticef(context.Background(), "MFA poll timed out for user %q", session.username) - return AuthNext, errorMessage{Message: "MFA approval timed out. Please try again."} + return restartFromEntraAuth(session, "MFA approval timed out. Please try again.") } func (b *Broker) entraMFACodeAuth(ctx context.Context, session *session, code string) (string, isAuthenticatedDataResponse) { - entraProvider, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider) + entraProvider, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) if !ok { log.Error(context.Background(), "entra_mfa_code mode selected but provider does not support it") return AuthDenied, unexpectedErrMsg("provider does not support Entra MFA") } if session.mfaFlowActive == nil { - log.Error(context.Background(), "MFA code mode selected but no active MFA flow") - return AuthDenied, unexpectedErrMsg("no active MFA flow") + return replayCompletedMFA(session, authmodes.EntraMFACode) } deviceRegistrationData := b.cachedDeviceRegistrationData(session) @@ -1734,9 +1850,7 @@ func (b *Broker) entraMFACodeAuth(ctx context.Context, session *session, code st var mfaErr *himmelblau.MFAError if errors.As(err, &mfaErr) && mfaErr.IsMFADenied() { log.Noticef(context.Background(), "MFA code verification denied for user %q", session.username) - session.entraPasswordHash = "" - clearEntraMFAState(session) - return AuthDenied, errorMessage{Message: "MFA authentication was denied."} + return denyAndClearMFA(session, errorMessage{Message: "MFA authentication was denied."}) } if errors.As(err, &mfaErr) && mfaErr.IsMFARetryableCode() { // An incorrect or expired one-time code: re-prompt for the code @@ -1750,21 +1864,231 @@ func (b *Broker) entraMFACodeAuth(ctx context.Context, session *session, code st return AuthRetry, errorMessage{Message: "Incorrect or expired code. Please try again."} } log.Noticef(context.Background(), "MFA code verification failed for user %q: %v", session.username, err) - session.entraPasswordHash = "" - clearEntraMFAState(session) - // MFA flow state was cleared; direct the client back to entra_password - // so it can restart the flow rather than re-entering the dead code mode. - session.nextAuthModes = []string{authmodes.EntraPassword} - return AuthNext, errorMessage{Message: "MFA authentication failed. Please try again."} + return restartFromEntraAuth(session, "MFA authentication failed. Please try again.") + } + + clearEntraAuthState(session) + return b.finishEntraAuth(ctx, session, oauthToken) +} + +// routeFIDOChallenge decides how to continue when Entra ID selected a +// FIDO/security-key method. When this build can perform the assertion and the +// challenge carries the WebAuthn data, continue with the local FIDO modes even +// if no key is plugged in yet: the entra_auth_fido step waits (bounded by +// fidoDeviceWaitTimeout) for the user to insert and touch it, then falls back +// to the device code flow on a machine where no key appears. Redirect to the +// device code flow immediately only when local FIDO is impossible at all (no +// authenticator in this build, or Entra sent no WebAuthn challenge). +func (b *Broker) routeFIDOChallenge(session *session, challengeInfo *himmelblau.MFAChallengeInfo) (string, isAuthenticatedDataResponse) { + if challengeInfo.FidoChallenge == "" || b.fido == nil { + log.Noticef(context.Background(), "FIDO MFA method %q for user %q cannot be completed locally; redirecting to the device code flow", challengeInfo.Method, session.username) + return b.redirectFIDOToDeviceAuth(session) + } + + // When a key is already connected and needs a PIN, collect it before the + // touch. When none is connected yet, go straight to the assertion step: it + // waits for insertion, and a PIN is requested reactively (ErrPINRequired) + // if the key the user eventually inserts needs one. + if b.fido.DevicePresent() { + pinRequired, err := b.fido.DeviceRequiresPIN() + if err != nil { + log.Warningf(context.Background(), "Could not determine whether the security key requires a PIN: %v", err) + } + if pinRequired && session.fidoPIN == "" { + session.nextAuthModes = []string{authmodes.EntraAuthFidoPin} + return AuthNext, nil + } + } + session.nextAuthModes = []string{authmodes.EntraAuthFido} + return AuthNext, nil +} + +// redirectFIDOToDeviceAuth clears the MFA state and directs the client to +// the device code flow (or denies when that flow is disabled). It is the +// fallback for FIDO challenges that cannot be completed on this machine. +func (b *Broker) redirectFIDOToDeviceAuth(session *session) (string, isAuthenticatedDataResponse) { + session.entraAuthPasswordHash = "" + clearEntraAuthState(session) + if b.cfg.flows.DeviceAuth { + session.nextAuthModes = []string{authmodes.Device, authmodes.DeviceQr} + return AuthNext, errorMessage{Message: "This account requires FIDO/security key authentication. Please complete authentication using the device code flow."} + } + return AuthDenied, errorMessage{Message: "This account requires FIDO/security key authentication, which is not available in this mode. The device code flow is also unavailable. Please contact your administrator."} +} + +// failFIDOAssertion handles a local ceremony that could not be completed. A +// passwordless session has no validated password to restart from, and +// re-probing would just loop back to the same failing local assertion, so it +// falls back to the device code flow where Entra runs the ceremony in a +// browser; a password session restarts the password flow instead. +func (b *Broker) failFIDOAssertion(session *session) (string, isAuthenticatedDataResponse) { + if session.entraAuthPasswordHash == "" { + return b.redirectFIDOToDeviceAuth(session) + } + return restartFromEntraAuth(session, "Security key authentication failed. Please try again.") +} + +// entraAuthFidoPinAuth stores the security key PIN on the session and advances +// to the assertion mode. The PIN never leaves broker memory: it is passed to +// the local WebAuthn ceremony and cleared with the MFA state. +func (b *Broker) entraAuthFidoPinAuth(session *session, pin string) (string, isAuthenticatedDataResponse) { + if session.mfaFlowActive == nil { + return replayCompletedMFA(session, authmodes.EntraAuthFidoPin) + } + if pin == "" { + return AuthRetry, errorMessage{Message: "Please enter your security key PIN."} } - clearEntraMFAState(session) + session.fidoPIN = pin + session.nextAuthModes = []string{authmodes.EntraAuthFido} + return AuthNext, nil +} + +// fidoDevicePollInterval is how often entraAuthFidoAuth re-checks for an +// inserted security key while showing the "insert your security key" screen. +const fidoDevicePollInterval = 500 * time.Millisecond + +// fidoDeviceWaitTimeout bounds how long entraAuthFidoAuth waits for a key to be +// inserted before falling back to the device code flow. A headless or SSH +// session cannot attach a security key, so without a bound the "insert your +// security key" screen would block forever; the timeout gives an interactive +// user time to plug the key in while still failing over on a machine where none +// can appear. It is a var so tests can shorten it. +var fidoDeviceWaitTimeout = 60 * time.Second + +// entraAuthFidoAuth performs the WebAuthn assertion with the local security +// key and completes the MFA flow with the resulting assertion. +func (b *Broker) entraAuthFidoAuth(ctx context.Context, session *session) (string, isAuthenticatedDataResponse) { + entraProvider, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) + if !ok { + log.Error(context.Background(), "entra_auth_fido mode selected but provider does not support it") + return denyAndClearMFA(session, unexpectedErrMsg("provider does not support Entra MFA")) + } + if b.fido == nil { + log.Error(context.Background(), "entra_auth_fido mode selected but this build has no FIDO support") + return denyAndClearMFA(session, unexpectedErrMsg("FIDO authentication is not available")) + } + if session.mfaFlowActive == nil { + return replayCompletedMFA(session, authmodes.EntraAuthFido) + } + if session.mfaChallengeInfo == nil || session.mfaChallengeInfo.FidoChallenge == "" { + log.Error(context.Background(), "FIDO mode selected but no FIDO challenge is available") + return denyAndClearMFA(session, unexpectedErrMsg("no active FIDO challenge")) + } + + // The entra_auth_fido screen says "Insert your security key and touch it", so + // wait here for a key to be connected rather than failing over immediately + // when none is plugged in yet. Give up after fidoDeviceWaitTimeout and fall + // back to the device code flow: a headless/SSH session can never attach a + // key, and blocking forever would strand the login. Cancellation (user abort + // or session end) unwinds like a cancelled assertion: return without freeing + // the flow, so a resumed attempt reuses it (see the ErrCanceled note in + // routeFIDOAssertionError). + if !b.fido.DevicePresent() { + waitDeadline := time.NewTimer(fidoDeviceWaitTimeout) + defer waitDeadline.Stop() + pollTicker := time.NewTicker(fidoDevicePollInterval) + defer pollTicker.Stop() + for !b.fido.DevicePresent() { + select { + case <-ctx.Done(): + log.Noticef(context.Background(), "Security key wait cancelled for user %q", session.username) + return AuthCancelled, nil + case <-waitDeadline.C: + log.Noticef(context.Background(), "No security key inserted for user %q within %s; falling back to the device code flow", session.username, fidoDeviceWaitTimeout) + return b.redirectFIDOToDeviceAuth(session) + case <-pollTicker.C: + } + } + } + + assertion, err := b.fido.Assert(ctx, session.mfaChallengeInfo.FidoChallenge, session.mfaChallengeInfo.FidoAllowList, session.fidoPIN) + if err != nil { + return b.routeFIDOAssertionError(ctx, session, err) + } + + deviceRegistrationData := b.cachedDeviceRegistrationData(session) + + oauthToken, err := entraProvider.AcquireTokenByMFAFlow( + ctx, b.cfg.clientID, b.cfg.issuerURL, session.username, + session.mfaFlowActive, assertion, 0, + deviceRegistrationData, + ) + if err != nil { + var mfaErr *himmelblau.MFAError + if errors.As(err, &mfaErr) && mfaErr.IsMFADenied() { + log.Noticef(context.Background(), "FIDO MFA denied for user %q", session.username) + return denyAndClearMFA(session, errorMessage{Message: "MFA authentication was denied."}) + } + log.Noticef(context.Background(), "FIDO assertion was rejected for user %q: %v", session.username, err) + return b.failFIDOAssertion(session) + } + + clearEntraAuthState(session) return b.finishEntraAuth(ctx, session, oauthToken) } +// routeFIDOAssertionError maps a failed local WebAuthn ceremony to the next +// broker step. PIN problems route to the PIN mode (bounded by the device, +// which hard-blocks its PIN after a few consecutive failures), a missed touch +// is retriable on the same mode, and everything else restarts the flow. +func (b *Broker) routeFIDOAssertionError(ctx context.Context, session *session, err error) (string, isAuthenticatedDataResponse) { + switch { + case errors.Is(err, fido.ErrPINRequired): + session.fidoPIN = "" + session.nextAuthModes = []string{authmodes.EntraAuthFidoPin} + return AuthNext, errorMessage{Message: "Your security key requires a PIN."} + case errors.Is(err, fido.ErrPINInvalid): + log.Noticef(context.Background(), "Incorrect security key PIN for user %q, re-prompting", session.username) + session.fidoPIN = "" + session.nextAuthModes = []string{authmodes.EntraAuthFidoPin} + return AuthNext, errorMessage{Message: "Incorrect security key PIN. Please try again."} + case errors.Is(err, fido.ErrPINBlocked): + log.Noticef(context.Background(), "Security key PIN blocked for user %q", session.username) + if session.entraAuthPasswordHash == "" { + return b.redirectFIDOToDeviceAuth(session) + } + return denyAndClearMFA(session, errorMessage{Message: "The security key PIN is blocked. Remove and reinsert the key, then try again."}) + case errors.Is(err, fido.ErrTimeout): + // The MFA flow is still valid (nothing was sent to Entra ID), so the + // user can retry the touch on the same mode. AuthRetry is capped by + // maxAuthAttempts, so a never-touched key still ends in denial. + return AuthRetry, errorMessage{Message: "The security key was not touched in time. Please try again."} + case errors.Is(err, fido.ErrCanceled) || ctx.Err() != nil: + // A cancelled assertion is almost always transient: the PAM client + // dropped the current conversation (e.g. GDM re-selecting the mode) and + // resumes the SAME session. The local WebAuthn ceremony does not consume + // the MFA flow, so leave it intact for the resumed attempt to reuse. + // + // Crucially, do NOT free the flow here. IsAuthenticated returns via its + // ctx.Done() branch on cancellation and skips updateSession, so any + // clearing done on this session copy is discarded: the stored session + // keeps its mfaFlowActive pointer. Freeing the underlying flow would + // then strand that stored pointer as a released flow, so the resumed + // assertion fails with "MFA flow state has been released", which + // restarts from the password probe and loops the user back to FIDO + // indefinitely. A genuinely terminal cancel is handled by EndSession, + // which frees the flow itself. + log.Noticef(context.Background(), "FIDO assertion cancelled for user %q", session.username) + return AuthCancelled, nil + case errors.Is(err, fido.ErrNoDevice): + // The key was unplugged mid-ceremony. Retry on the same mode, which + // waits for the user to reinsert it, rather than failing over to the + // device code flow. AuthRetry is capped by maxAuthAttempts. + log.Noticef(context.Background(), "Security key removed mid-ceremony for user %q; waiting for reinsertion", session.username) + return AuthRetry, errorMessage{Message: "The security key was removed. Please reinsert it and try again."} + case errors.Is(err, fido.ErrNoCredentials): + log.Noticef(context.Background(), "Connected security key has no matching credential for user %q; redirecting to the device code flow", session.username) + return b.redirectFIDOToDeviceAuth(session) + default: + log.Errorf(context.Background(), "FIDO assertion failed for user %q: %v", session.username, err) + return b.failFIDOAssertion(session) + } +} + func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken *oauth2.Token) (string, isAuthenticatedDataResponse) { // Ensure any cached password hash is cleared from memory on all exit paths. - defer func() { session.entraPasswordHash = "" }() + defer func() { session.entraAuthPasswordHash = "" }() // AcquireTokenByMFAFlow returns (nil, nil) only on a provider contract // violation, but this is the trust boundary into the generic broker: guard @@ -1786,7 +2110,7 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken } t := mfaToken - // Reuse the auth info loaded once at the start of the flow (entraPasswordAuth) + // Reuse the auth info loaded once at the start of the flow (entraAuth) // rather than re-reading the token from disk. oldAuthInfo := session.authInfo @@ -1812,11 +2136,11 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken return access, data } - // Mark this token as having been obtained via the entra_password MFA flow so + // Mark this token as having been obtained via the entra_auth flow so // that returning logins refresh it through the Microsoft Broker App public // refresh path (the liveness/revocation check) rather than the OIDC app // refresh. - authInfo.ObtainedViaEntraPasswordAuth = true + authInfo.ObtainedViaEntraAuth = true // Carry over device registration data from a previous login when we are not // (re-)registering the device in this one. authInfo is built fresh from the @@ -1831,10 +2155,26 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken if oldAuthInfo != nil { deviceRegistrationData = oldAuthInfo.DeviceRegistrationData } + // A successful passwordless MFA flow can still yield a token that is valid + // for first-time device registration, so do not gate registration on an + // entered Entra password here. cleanup, access, data := b.maybeRegisterDevice(ctx, session, authInfo, t, deviceRegistrationData) defer cleanup() if access != "" { - return access, data + // Keep the existing client-secret group-fetch fallback for passwordless + // Entra logins in mixed configs: register_device=true should prefer + // registration, but if it fails before any local device state exists and + // an app-only Graph path is configured, continue without registration + // instead of denying the login. Keyed on the absence of device data (not + // on "first login"): a returning user whose first login already used + // this fallback has a cached token without device data, and denying them + // on a repeated registration failure would lock out an account that + // could log in before. + if session.entraAuthPasswordHash == "" && len(deviceRegistrationData) == 0 && b.cfg.clientSecret != "" { + log.Warningf(context.Background(), "Device registration failed for passwordless Entra login for user %q; falling back to app-only Graph lookup", session.username) + } else { + return access, data + } } // Fetch groups. The MFA flow just performed a live provider verification, so a @@ -1853,6 +2193,17 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken authInfo.UserInfo.Groups = groups } + // A passwordless login has no Entra password to cache for offline + // authentication. When the user has no local password yet, chain into the + // newpassword step (like the device-auth flow does) so offline logins + // keep working; an existing local password stays valid, so returning + // users are not asked to redefine one on every login. + if session.entraAuthPasswordHash == "" && !passwordFileExists(*session) { + session.authInfo = authInfo + session.nextAuthModes = []string{authmodes.NewPassword} + return AuthNext, nil + } + access, data = b.finishAuth(session, authInfo) if access != AuthGranted { return access, data @@ -1862,12 +2213,12 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken // after finishAuth so that a denial there cannot leave a password file on // disk without a cached token (token-then-password matches the ordering of // the device-auth flow). - if session.entraPasswordHash != "" { - if hashErr := password.StoreHashedPassword(session.entraPasswordHash, session.passwordPath); hashErr != nil { + if session.entraAuthPasswordHash != "" { + if hashErr := password.StoreHashedPassword(session.entraAuthPasswordHash, session.passwordPath); hashErr != nil { log.Errorf(context.Background(), "Failed to store password hash: %v", hashErr) return AuthDenied, unexpectedErrMsg("failed to store password") } - session.entraPasswordHash = "" + session.entraAuthPasswordHash = "" if msg, ok := data.(userInfoMessage); ok { msg.Message = cachedPasswordMessage @@ -1878,9 +2229,22 @@ func (b *Broker) finishEntraAuth(ctx context.Context, session *session, mfaToken return access, data } -// routeMFAInitError routes the AADSTS errors returned by InitiateEntraPasswordAuth +// routeMFAInitError routes the AADSTS errors returned by InitiateEntraAuth // (the MFA init step) to appropriate broker responses. func (b *Broker) routeMFAInitError(mfaErr *himmelblau.MFAError, session *session) (string, isAuthenticatedDataResponse) { + if mfaErr.IsMFAPasswordRequired() { + log.Debugf(context.Background(), "Passwordless Entra authentication for user %q requires password entry", session.username) + session.entraAuthPasswordRequired = true + session.nextAuthModes = []string{authmodes.EntraAuth} + // Do not send an intermediate AuthNext message here: GDM shows any + // auth.Next message as a transient challenge state before it requests + // the next layout, which creates a redundant spinner-only screen with + // the same label as the real password form. The next selected layout + // already prompts for the Entra password, so returning AuthNext with no + // message moves straight to that form. + return AuthNext, nil + } + switch mfaErr.AADSTS { case 50053: log.Noticef(context.Background(), "Account locked for user %q (AADSTS50053)", session.username) @@ -1915,7 +2279,11 @@ func (b *Broker) routeMFAInitError(mfaErr *himmelblau.MFAError, session *session return AuthNext, errorMessage{Message: "Your password was changed remotely. Please re-authenticate."} case 53003: log.Noticef(context.Background(), "Conditional Access blocked sign-in for user %q (AADSTS53003)", session.username) - return AuthDenied, errorMessage{Message: "Access was blocked by your organization's Conditional Access policies. Please contact your administrator."} + if b.cfg.flows.DeviceAuth { + session.nextAuthModes = []string{authmodes.Device, authmodes.DeviceQr} + return AuthNext, errorMessage{Message: "Access was blocked by your organization's Conditional Access policies. Please complete authentication using the device code flow."} + } + return AuthDenied, errorMessage{Message: "Access was blocked by your organization's Conditional Access policies and the device code flow is disabled. Please contact your administrator."} default: if mfaErr.IsMFARequired() { // The native password MFA flow could not be set up; redirect to Device @@ -2116,16 +2484,28 @@ func (b *Broker) EndSession(sessionID string) error { } // Checks if there is a isAuthenticated call running for this session and cancels it before ending the session. - // When a poll is in flight, cancelling lets that goroutine free the MFA flow - // as it unwinds; otherwise we free it here. These two paths can race (the - // finishing goroutine may nil isAuthenticating via CancelIsAuthenticated just - // as we read our own session copy), so both could call FreeMFAFlowState on the - // same pointer. That is safe: FreeMFAFlowState takes MFAFlowState.mu and nils - // its release callback, so the underlying C free runs exactly once and a - // second call is a no-op. Sessions are stored by value, so there is no shared + // Cancelling asks any in-flight goroutine to unwind; we then free the MFA + // flow ourselves rather than relying on that goroutine to do it. Some + // cancellation paths intentionally leave the flow intact (e.g. the FIDO + // assertion, whose cancel is usually a transient re-select and must not + // strand the resumed session with a released flow), so freeing here is what + // guarantees the flow is not leaked on a genuinely terminal cancel. + // + // Freeing is safe even when a goroutine is mid-flight: FreeMFAFlowState + // takes MFAFlowState.mu and nils its release callback, so it waits for any + // concurrent AcquireTokenByMFAFlow to finish, runs the underlying C free + // exactly once, and is a no-op if that goroutine also frees the flow on its + // own terminal path. Sessions are stored by value, so there is no shared // write to mfaFlowActive itself (confirmed race-clean under `go test -race`). + // if session.isAuthenticating != nil { b.CancelIsAuthenticated(sessionID) + // Free on a separate goroutine: waiting on MFAFlowState.mu can block + // for the remainder of an in-flight cgo network call (cancellation is + // only observed between poll iterations), and EndSession answers a + // D-Bus call on the PAM teardown path, which must not stall for + // seconds. Nothing after this depends on the free having completed. + go himmelblau.FreeMFAFlowState(session.mfaFlowActive) } else { himmelblau.FreeMFAFlowState(session.mfaFlowActive) } @@ -2266,26 +2646,26 @@ func (b *Broker) updateSession(sessionID string, session session) error { return nil } -// refreshEntraPasswordToken refreshes an Entra password + MFA token for the +// refreshEntraToken refreshes a token obtained via the Entra auth flow for the // liveness/revocation check on a returning login. The provider performs a public // refresh (no client_secret) as the Microsoft Broker App; on success the rotated // refresh token replaces the cached one (kept fresh on each login, like the // device-auth refresh). Errors are returned unwrapped so the caller classifies them // with the same checks it uses for device-auth (IsUserDisabledError → AADSTS50057, // IsTokenExpiredError → AADSTS50173, isAADSTSGrantRevokedError, net.Error → offline). -func (b *Broker) refreshEntraPasswordToken(ctx context.Context, session *session, oldToken *token.AuthCachedInfo) (*token.AuthCachedInfo, error) { - ep, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](b.provider) +func (b *Broker) refreshEntraToken(ctx context.Context, session *session, oldToken *token.AuthCachedInfo) (*token.AuthCachedInfo, error) { + ep, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](b.provider) if !ok { - // The token was obtained via the entra_password flow, so the provider that - // issued it must implement EntraPasswordProvider. If it no longer does, the + // The token was obtained via the entra_auth flow, so the provider that + // issued it must implement EntraAuthProvider. If it no longer does, the // deployment is misconfigured: fail the login rather than skipping the // liveness/revocation check, which would let a deleted/disabled user keep // logging in with the cached token. - return nil, fmt.Errorf("provider does not implement EntraPasswordProvider; cannot refresh entra_password token for user %q", oldToken.UserInfo.Name) + return nil, fmt.Errorf("provider does not implement EntraAuthProvider; cannot refresh entra_auth token for user %q", oldToken.UserInfo.Name) } refreshCtx, cancel := context.WithTimeout(ctx, maxRequestDuration) defer cancel() - newTok, err := ep.RefreshEntraPasswordToken(refreshCtx, b.cfg.issuerURL, oldToken.Token.RefreshToken) + newTok, err := ep.RefreshEntraToken(refreshCtx, b.cfg.issuerURL, oldToken.Token.RefreshToken) if err != nil { return oldToken, err } @@ -2342,7 +2722,7 @@ func (b *Broker) refreshEntraPasswordToken(ctx context.Context, session *session func (b *Broker) refreshToken(ctx context.Context, session *session, oldToken *token.AuthCachedInfo) (*token.AuthCachedInfo, error) { timeoutCtx, cancel := context.WithTimeout(ctx, maxRequestDuration) defer cancel() - // Build a token carrying only the refresh token, like refreshEntraPasswordToken + // Build a token carrying only the refresh token, like refreshEntraToken // does: oauth2.Token.Valid() requires a non-empty AccessToken, so omitting it // forces TokenSource to hit the token endpoint even if the cached token has not // actually expired, without mutating the caller's cached oldToken. diff --git a/authd-oidc-brokers/internal/broker/broker_test.go b/authd-oidc-brokers/internal/broker/broker_test.go index 1bb7811248..97f3f91ef8 100644 --- a/authd-oidc-brokers/internal/broker/broker_test.go +++ b/authd-oidc-brokers/internal/broker/broker_test.go @@ -20,6 +20,7 @@ import ( "github.com/canonical/authd/authd-oidc-brokers/internal/broker/authmodes" "github.com/canonical/authd/authd-oidc-brokers/internal/broker/sessionmode" "github.com/canonical/authd/authd-oidc-brokers/internal/consts" + "github.com/canonical/authd/authd-oidc-brokers/internal/fido" "github.com/canonical/authd/authd-oidc-brokers/internal/password" providerErrors "github.com/canonical/authd/authd-oidc-brokers/internal/providers/errors" "github.com/canonical/authd/authd-oidc-brokers/internal/providers/info" @@ -43,7 +44,21 @@ func newTrackedMFAFlowState(release func()) *himmelblau.MFAFlowState { return flow } -type mockEntraPasswordProvider struct { +func lockMFAFlowStateForTests(flow *himmelblau.MFAFlowState) func() { + muField := reflect.ValueOf(flow).Elem().FieldByName("mu") + //nolint:gosec // G103: unsafe pointer required to lock the unexported MFA flow mutex for testing purposes only. + mu, ok := reflect.NewAt(muField.Type(), unsafe.Pointer(muField.UnsafeAddr())).Interface().(interface { + Lock() + Unlock() + }) + if !ok { + panic("MFA flow mutex has an unexpected type") + } + mu.Lock() + return mu.Unlock +} + +type mockEntraAuthProvider struct { *testutils.MockProvider flowState *himmelblau.MFAFlowState challengeInfo *himmelblau.MFAChallengeInfo @@ -51,8 +66,12 @@ type mockEntraPasswordProvider struct { initErr error recordedPollAttempts []int recordedChallengeData []string - refreshResult *oauth2.Token // returned by RefreshEntraPasswordToken (defaults to a rotated token) - refreshErr error // when set, RefreshEntraPasswordToken returns it (e.g. AADSTS50057) + recordedInitAuthOpts [][]himmelblau.AuthOption + recordedInitPasswords []string + recordedInitDevScopes []bool + registerDeviceCalls int + refreshResult *oauth2.Token // returned by RefreshEntraToken (defaults to a rotated token) + refreshErr error // when set, RefreshEntraToken returns it (e.g. AADSTS50057) refreshDelay time.Duration refreshCtxDeadline time.Time verifyCtxDeadline time.Time @@ -62,14 +81,28 @@ type mockEntraPasswordProvider struct { userInfoFromTokenErr error // when set, UserInfoFromAccessToken returns this error } -func (p *mockEntraPasswordProvider) VerifyAccessToken(ctx context.Context, _, _ string) error { +type blockingMFAProvider struct { + *mockEntraAuthProvider + started chan struct{} + unblock chan struct{} + finished chan struct{} +} + +func (p *blockingMFAProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, _ string, _ int, _ []byte) (*oauth2.Token, error) { + p.started <- struct{}{} + <-p.unblock + close(p.finished) + return nil, context.Canceled +} + +func (p *mockEntraAuthProvider) VerifyAccessToken(ctx context.Context, _, _ string) error { if deadline, ok := ctx.Deadline(); ok { p.verifyCtxDeadline = deadline } return p.verifyAccessTokenErr } -func (p *mockEntraPasswordProvider) UserInfoFromAccessToken(_ string) (info.User, error) { +func (p *mockEntraAuthProvider) UserInfoFromAccessToken(_ string) (info.User, error) { if p.userInfoFromTokenErr != nil { return info.User{}, p.userInfoFromTokenErr } @@ -84,7 +117,7 @@ type mockProviderWithEntraModes struct { } func (p *mockProviderWithEntraModes) SupportedOnlineAuthModes() []string { - return []string{authmodes.Device, authmodes.DeviceQr, authmodes.EntraPassword} + return []string{authmodes.Device, authmodes.DeviceQr, authmodes.EntraAuth} } type mockGrantRevokedProvider struct { @@ -97,14 +130,17 @@ func (p *mockGrantRevokedProvider) IsTokenExpiredError(err *oauth2.RetrieveError var mockDeviceRegistrationData = []byte(`{"device_id":"test-device-id","cert_key":"Y2VydA==","transport_key":"dHJhbnNwb3J0","auth_value":"test-auth-value","tpm_machine_key":"dHBtLW1hY2hpbmUta2V5"}`) -func (p *mockEntraPasswordProvider) InitiateEntraPasswordAuth(_ context.Context, _, _ string, _, _ string, _ []byte, _ bool) (*himmelblau.MFAFlowState, *himmelblau.MFAChallengeInfo, error) { +func (p *mockEntraAuthProvider) InitiateEntraAuth(_ context.Context, _, _ string, _, password string, _ []byte, withDeviceScope bool, authOpts ...himmelblau.AuthOption) (*himmelblau.MFAFlowState, *himmelblau.MFAChallengeInfo, error) { + p.recordedInitAuthOpts = append(p.recordedInitAuthOpts, authOpts) + p.recordedInitPasswords = append(p.recordedInitPasswords, password) + p.recordedInitDevScopes = append(p.recordedInitDevScopes, withDeviceScope) if p.initErr != nil { return nil, nil, p.initErr } return p.flowState, p.challengeInfo, nil } -func (p *mockEntraPasswordProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, authData string, pollAttempt int, _ []byte) (*oauth2.Token, error) { +func (p *mockEntraAuthProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, authData string, pollAttempt int, _ []byte) (*oauth2.Token, error) { p.recordedPollAttempts = append(p.recordedPollAttempts, pollAttempt) p.recordedChallengeData = append(p.recordedChallengeData, authData) if p.mfaTokenResult == nil { @@ -113,7 +149,7 @@ func (p *mockEntraPasswordProvider) AcquireTokenByMFAFlow(_ context.Context, _, return p.mfaTokenResult, nil } -func (p *mockEntraPasswordProvider) RefreshEntraPasswordToken(ctx context.Context, _, _ string) (*oauth2.Token, error) { +func (p *mockEntraAuthProvider) RefreshEntraToken(ctx context.Context, _, _ string) (*oauth2.Token, error) { if deadline, ok := ctx.Deadline(); ok { p.refreshCtxDeadline = deadline } @@ -141,15 +177,16 @@ func (p *mockEntraPasswordProvider) RefreshEntraPasswordToken(ctx context.Contex // broker tests can exercise the refresh-rejection classification. It matches on a // sentinel error code, mirroring testutils.MockUserDisabledCheckerProvider; the real // AADSTS50057 detection is covered by the provider-level tests. -func (p *mockEntraPasswordProvider) IsUserDisabledError(err *oauth2.RetrieveError) bool { +func (p *mockEntraAuthProvider) IsUserDisabledError(err *oauth2.RetrieveError) bool { return p.userDisabledErrorCode != "" && err != nil && err.ErrorCode == p.userDisabledErrorCode } -func (p *mockEntraPasswordProvider) IsTokenForDeviceRegistration(authInfo *token.AuthCachedInfo) bool { +func (p *mockEntraAuthProvider) IsTokenForDeviceRegistration(authInfo *token.AuthCachedInfo) bool { return authInfo != nil && len(authInfo.DeviceRegistrationData) > 0 } -func (p *mockEntraPasswordProvider) MaybeRegisterDevice(_ context.Context, _ *oauth2.Token, _ string, _ string, oldData []byte) ([]byte, func(), error) { +func (p *mockEntraAuthProvider) MaybeRegisterDevice(_ context.Context, _ *oauth2.Token, _ string, _ string, oldData []byte) ([]byte, func(), error) { + p.registerDeviceCalls++ if len(oldData) > 0 { return oldData, func() {}, nil } @@ -158,17 +195,18 @@ func (p *mockEntraPasswordProvider) MaybeRegisterDevice(_ context.Context, _ *oa // mockMFADeniedProvider simulates MFA push notification being denied by the user. type mockMFADeniedProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider } // mockDeviceRegistrationFailProvider simulates a first-time login where device // registration fails at the network level (e.g. no connectivity to // enterpriseregistration.windows.net). type mockDeviceRegistrationFailProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider } func (p *mockDeviceRegistrationFailProvider) MaybeRegisterDevice(_ context.Context, _ *oauth2.Token, _ string, _ string, oldData []byte) ([]byte, func(), error) { + p.registerDeviceCalls++ if len(oldData) > 0 { // Re-use existing registration — failure is only on first registration. return oldData, func() {}, nil @@ -183,7 +221,7 @@ func (p *mockMFADeniedProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ st // mockMFATimeoutProvider simulates MFA poll continuing until max attempts are exhausted. type mockMFATimeoutProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider } func (p *mockMFATimeoutProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, _ string, _ int, _ []byte) (*oauth2.Token, error) { @@ -191,13 +229,33 @@ func (p *mockMFATimeoutProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ s return nil, &himmelblau.MFAError{Category: himmelblau.MFAErrorPollContinue, Message: "MFA poll continue"} } +// mockPasswordRequiredThenSuccessProvider simulates a passwordless probe that +// discovers the account needs a password, followed by a successful password+MFA +// initialization. +type mockPasswordRequiredThenSuccessProvider struct { + *mockEntraAuthProvider +} + +func (p *mockPasswordRequiredThenSuccessProvider) InitiateEntraAuth(_ context.Context, _, _ string, _, password string, _ []byte, withDeviceScope bool, authOpts ...himmelblau.AuthOption) (*himmelblau.MFAFlowState, *himmelblau.MFAChallengeInfo, error) { + p.recordedInitAuthOpts = append(p.recordedInitAuthOpts, authOpts) + p.recordedInitPasswords = append(p.recordedInitPasswords, password) + p.recordedInitDevScopes = append(p.recordedInitDevScopes, withDeviceScope) + if len(p.recordedInitPasswords) == 1 { + return nil, nil, &himmelblau.MFAError{ + Category: himmelblau.MFAErrorPasswordRequired, + Message: "password required", + } + } + return p.flowState, p.challengeInfo, nil +} + // mockMFAWrongCodeThenSuccessProvider simulates an incorrect or expired // one-time code on the first code submission followed by a correct code on the // second. libhimmelblau reports a wrong code as an MFAInvalidCode error (which // authd maps to MFAErrorRetryableCode via the C enum code), while leaving the // flow intact. This is what production consumers see. type mockMFAWrongCodeThenSuccessProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider codeAttempts int } @@ -217,20 +275,35 @@ func (p *mockMFAWrongCodeThenSuccessProvider) AcquireTokenByMFAFlow(_ context.Co // from AcquireTokenByMFAFlow, exercising the broker's defensive nil-token guard // (a misbehaving provider must deny, not panic the broker). type mockMFANilTokenProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider } func (p *mockMFANilTokenProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, _ string, _ int, _ []byte) (*oauth2.Token, error) { return nil, nil } +// mockInvalidFIDOAssertionProvider simulates Entra rejecting the local FIDO +// assertion during the MFA continuation step. +type mockInvalidFIDOAssertionProvider struct { + *mockEntraAuthProvider +} + +func (p *mockInvalidFIDOAssertionProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, authData string, pollAttempt int, _ []byte) (*oauth2.Token, error) { + p.recordedChallengeData = append(p.recordedChallengeData, authData) + p.recordedPollAttempts = append(p.recordedPollAttempts, pollAttempt) + return nil, &himmelblau.MFAError{ + AADSTS: 135020, + Message: "AADSTS135020: Invalid Fido assertion.", + } +} + // mockMFAAlwaysWrongCodeProvider simulates every submitted one-time code being // incorrect or expired (MFAErrorRetryableCode), while the MFA flow itself stays // valid. This is used to exercise the maxAuthAttempts lockout path: repeated // retryable wrong codes must eventually return AuthDeniedMaxTries and release // the in-progress MFA flow. type mockMFAAlwaysWrongCodeProvider struct { - *mockEntraPasswordProvider + *mockEntraAuthProvider } func (p *mockMFAAlwaysWrongCodeProvider) AcquireTokenByMFAFlow(_ context.Context, _, _ string, _ string, _ *himmelblau.MFAFlowState, authData string, _ int, _ []byte) (*oauth2.Token, error) { @@ -354,11 +427,12 @@ func TestNew(t *testing.T) { } } -// TestNewRejectsUnusableEntraPasswordWithoutGroupSource verifies that New fails -// fast when entra_password is enabled but can't retrieve groups from Microsoft -// Graph (no device registration, no client secret) — rather than starting -// successfully and only failing once a user logs in. -func TestNewRejectsUnusableEntraPasswordWithoutGroupSource(t *testing.T) { +// TestNewRejectsUnusableEntraAuthWithoutGroupSource verifies that New fails +// fast whenever entra_auth is enabled but can't retrieve groups from +// Microsoft Graph (no device registration, no client secret). The failure is +// unconditional: having other flows (e.g. device_code) enabled does not suppress +// it, because a silent runtime fallback would hide a misconfiguration from admins. +func TestNewRejectsUnusableEntraAuthWithoutGroupSource(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -368,13 +442,10 @@ func TestNewRejectsUnusableEntraPasswordWithoutGroupSource(t *testing.T) { wantErr bool }{ - "Error_when_entra_password_is_the_only_flow_and_unusable": {wantErr: true}, - "Error_when_device_code_is_also_enabled_but_entra_password_is_still_unusable": { - deviceCodeFlowEnabled: true, - wantErr: true, - }, - "No_error_when_device_registration_makes_it_usable": {registerDevice: true}, - "No_error_when_a_client_secret_makes_it_usable": {clientSecret: "test-client-secret"}, + "Error_when_entra_auth_enabled_without_group_source": {wantErr: true}, + "Error_even_when_device_code_is_also_enabled": {deviceCodeFlowEnabled: true, wantErr: true}, + "No_error_when_device_registration_makes_it_usable": {registerDevice: true}, + "No_error_when_a_client_secret_makes_it_usable": {clientSecret: "test-client-secret"}, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -388,7 +459,7 @@ func TestNewRejectsUnusableEntraPasswordWithoutGroupSource(t *testing.T) { bCfg.SetRegisterDevice(tc.registerDevice) bCfg.SetClientSecret(tc.clientSecret) - provider := &mockEntraPasswordProvider{MockProvider: &testutils.MockProvider{}} + provider := &mockEntraAuthProvider{MockProvider: &testutils.MockProvider{}} b, err := broker.New(*bCfg, broker.LatestAPIVersion, broker.WithCustomProvider(provider)) if tc.wantErr { require.Error(t, err, "New should have returned an error") @@ -616,9 +687,16 @@ func TestNewSessionRemovesUnsafeCacheSymlink(t *testing.T) { } var supportedUILayouts = map[string]map[string]string{ + // All real authd PAM clients declare wait on their form layout; entra_auth + // requires it for the passwordless-probe screen. "form": { "type": "form", "entry": "chars_password", + "wait": "true", + }, + "form-without-wait": { + "type": "form", + "entry": "chars_password", }, "form-without-entry": { "type": "form", @@ -1163,28 +1241,28 @@ func TestIsAuthenticated(t *testing.T) { "Authenticating_with_password_when_refresh_token_is_expired_results_in_device_auth_as_next_mode": { firstMode: authmodes.Password, token: &tokenOptions{refreshTokenExpired: true}, - wantNextAuthModes: []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, + wantNextAuthModes: []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, wantSecondCall: true, secondMode: authmodes.DeviceQr, }, "Authenticating_with_password_when_refresh_token_is_expired_due_to_inactivity_results_in_device_auth_as_next_mode": { firstMode: authmodes.Password, token: &tokenOptions{refreshTokenInactiveExpired: true}, - wantNextAuthModes: []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, + wantNextAuthModes: []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, wantSecondCall: true, secondMode: authmodes.DeviceQr, }, "Authenticating_with_password_when_refresh_token_is_expired_due_to_ca_sign_in_frequency_results_in_device_auth_as_next_mode": { firstMode: authmodes.Password, token: &tokenOptions{refreshTokenStale: true}, - wantNextAuthModes: []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, + wantNextAuthModes: []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, wantSecondCall: true, secondMode: authmodes.DeviceQr, }, "Authenticating_with_password_when_no_refresh_token_results_in_device_auth_as_next_mode": { firstMode: authmodes.Password, token: &tokenOptions{noRefreshToken: true}, - wantNextAuthModes: []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, + wantNextAuthModes: []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, wantSecondCall: true, secondMode: authmodes.DeviceQr, }, @@ -1201,11 +1279,11 @@ func TestIsAuthenticated(t *testing.T) { token: &tokenOptions{}, forceAccessCheckWithProvider: true, }, - // Note: the entra_password group-fetch fallback (a returning login whose + // Note: the entra_auth group-fetch fallback (a returning login whose // liveness refresh succeeds but whose group fetch fails must use cached // groups, not deny) is covered by the dedicated // TestIsAuthenticatedPasswordEntraTokenFallsBackToCachedGroupsOnGroupFetchError, - // which uses a provider that implements EntraPasswordProvider so the refresh + // which uses a provider that implements EntraAuthProvider so the refresh // path is actually exercised rather than the misconfiguration no-op. "Extra_groups_configured": { firstMode: authmodes.Password, @@ -1488,7 +1566,7 @@ func TestIsAuthenticated(t *testing.T) { getGroupsFunc: func() ([]info.Group, error) { return nil, &providerErrors.RetryWithDeviceAuthError{Err: errors.New("token acquisition failed")} }, - wantNextAuthModes: []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, + wantNextAuthModes: []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, }, } for name, tc := range tests { @@ -2124,7 +2202,7 @@ func TestEndSessionReleasesPendingMFAFlow(t *testing.T) { t.Parallel() released := 0 - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: newTrackedMFAFlowState(func() { released++ }), challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -2144,7 +2222,7 @@ func TestEndSessionReleasesPendingMFAFlow(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2157,12 +2235,279 @@ func TestEndSessionReleasesPendingMFAFlow(t *testing.T) { require.Equal(t, 1, released, "EndSession should release any pending MFA flow state") } +func TestEndSessionDoesNotBlockOnInFlightMFA(t *testing.T) { + t.Parallel() + + released := make(chan struct{}, 1) + flow := newTrackedMFAFlowState(func() { released <- struct{}{} }) + provider := &blockingMFAProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: flow, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 5000, + MaxPollAttempts: 10, + }, + }, + started: make(chan struct{}), + unblock: make(chan struct{}), + finished: make(chan struct{}), + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + }) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + + unlockFlow := lockMFAFlowStateForTests(flow) + updateAuthModes(t, b, sessionID, authmodes.EntraMFAWait) + + authDone := make(chan error, 1) + go func() { + _, _, err := b.IsAuthenticated(sessionID, `{}`) + authDone <- err + }() + + select { + case <-provider.started: + case <-time.After(time.Second): + t.Fatal("MFA continuation did not start") + } + + endSessionDone := make(chan error, 1) + go func() { + endSessionDone <- b.EndSession(sessionID) + }() + + select { + case err := <-endSessionDone: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("EndSession waited for the in-flight MFA continuation") + } + + select { + case <-released: + t.Fatal("MFA flow released while its continuation still held the lock") + default: + } + + close(provider.unblock) + select { + case <-provider.finished: + case <-time.After(time.Second): + t.Fatal("MFA continuation did not finish") + } + + unlockFlow() + select { + case <-released: + case <-time.After(time.Second): + t.Fatal("MFA flow was not released after the continuation lock became available") + } + + select { + case err := <-authDone: + require.ErrorIs(t, err, context.Canceled) + case <-time.After(time.Second): + t.Fatal("cancelled authentication did not return") + } +} + +func TestEntraAuthProbePromptsForPasswordWhenRequired(t *testing.T) { + t.Parallel() + + provider := &mockPasswordRequiredThenSuccessProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 1, + MaxPollAttempts: 1, + }, + }, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + + layout, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + require.Equal(t, "true", layout["wait"], "the initial Entra password selection should probe without asking for a password") + require.Empty(t, layout["entry"], "the passwordless probe must not expose a password field") + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuth}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the first call should be a passwordless probe") + require.Equal(t, []bool{false}, provider.recordedInitDevScopes, + "passwordless probing must never request device-scoped auth") + require.Equal(t, "{}", data, + "PASSWORD_REQUIRED should transition directly to the real password form, not an intermediate message-only next state") + + layout, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + require.Equal(t, "chars_password", layout["entry"], + "PASSWORD_REQUIRED should turn the same auth mode into a password prompt") + require.Empty(t, layout["wait"], "the password prompt must not be an auto-submit wait layout") + + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + access, _, err = b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraMFAWait}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{"", "password"}, provider.recordedInitPasswords, + "the second call should submit the user-entered password") + require.Equal(t, []bool{false, true}, provider.recordedInitDevScopes, + "device-scoped auth should be used only after a password was submitted") + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "the offline password must not be cached until MFA succeeds") +} + +func TestEntraAuthPasswordlessSuccessDoesNotCacheOfflinePassword(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Open your Authenticator app and approve the sign-in request", + Method: "PhoneAppNotification", + PollingIntervalMs: 1, + MaxPollAttempts: 1, + }, + mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, username, sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraMFAWait}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "passwordless initiation should not submit a password") + require.Equal(t, []bool{false}, provider.recordedInitDevScopes, + "passwordless initiation must not request device-scoped auth even when device registration is enabled") + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraMFAWait)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + + access, _, err = b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access, + "first-time passwordless logins should chain to local password creation") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) + _, err = os.Stat(b.TokenPathForSession(sessionID)) + require.NoError(t, err, "passwordless auth should cache the token once device registration succeeds") + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "passwordless auth has no verified Entra password to cache for offline login") + require.Equal(t, 1, provider.registerDeviceCalls, + "passwordless auth should attempt first-time device registration after MFA succeeds") +} + +func TestEntraAuthPasswordlessTimeoutRestartsPasswordlessProbe(t *testing.T) { + t.Parallel() + + provider := &mockMFATimeoutProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Open your Authenticator app and approve the sign-in request", + Method: "PhoneAppNotification", + PollingIntervalMs: 1, + MaxPollAttempts: 1, + }, + }, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraMFAWait}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the first step should be a passwordless probe") + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraMFAWait)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Contains(t, data, "timed out") + require.Equal(t, []string{authmodes.EntraAuth}, b.GetNextAuthModes(sessionID)) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + layout, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + require.Equal(t, "true", layout["wait"], + "a passwordless MFA timeout should restart discovery, not assume the user has an Entra password") + require.Empty(t, layout["entry"]) +} + func TestIsAuthenticatedEntraMFAWaitStartsPollingAtOne(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -2183,7 +2528,7 @@ func TestIsAuthenticatedEntraMFAWaitStartsPollingAtOne(t *testing.T) { sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, data, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2224,7 +2569,7 @@ func TestIsAuthenticatedEntraMFAWaitStartsPollingAtOne(t *testing.T) { func advanceToEntraMFAWait(t *testing.T, b *broker.Broker, sessionID, key string) { t.Helper() - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2237,15 +2582,15 @@ func advanceToEntraMFAWait(t *testing.T, b *broker.Broker, sessionID, key string require.NoError(t, err) } -// TestIsAuthenticatedEntraMFADeniesOnAccessTokenVerificationFailure verifies that +// TestIsAuthenticatedEntraAuthDeniesOnAccessTokenVerificationFailure verifies that // when the MFA access token fails signature verification (the TLS-MITM defense), // the login is denied rather than trusting the token's identity claims. -func TestIsAuthenticatedEntraMFADeniesOnAccessTokenVerificationFailure(t *testing.T) { +func TestIsAuthenticatedEntraAuthDeniesOnAccessTokenVerificationFailure(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -2283,7 +2628,7 @@ func TestIsAuthenticatedEntraMFAWaitPollsWhenMaxPollAttemptsZero(t *testing.T) { username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{Message: "Approve the sign-in request", PollingIntervalMs: 1, MaxPollAttempts: 0}, @@ -2307,15 +2652,15 @@ func TestIsAuthenticatedEntraMFAWaitPollsWhenMaxPollAttemptsZero(t *testing.T) { require.Equal(t, []int{1}, provider.recordedPollAttempts, "the poll loop must run at least once when MaxPollAttempts==0") } -// TestIsAuthenticatedEntraMFADeniesOnNilToken verifies the defensive nil-token +// TestIsAuthenticatedEntraAuthDeniesOnNilToken verifies the defensive nil-token // guard: a provider returning (nil, nil) from AcquireTokenByMFAFlow must deny // rather than panic the broker on the token dereference in finishEntraAuth. -func TestIsAuthenticatedEntraMFADeniesOnNilToken(t *testing.T) { +func TestIsAuthenticatedEntraAuthDeniesOnNilToken(t *testing.T) { t.Parallel() username := "test-user@email.com" provider := &mockMFANilTokenProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{Message: "Approve the sign-in request", PollingIntervalMs: 1, MaxPollAttempts: 1}, @@ -2350,7 +2695,7 @@ func TestIsAuthenticatedEntraMFAWaitNumberMatchingLabelShown(t *testing.T) { // Simulate the message libhimmelblau returns for PhoneAppNotification with number // matching: "Open your Authenticator app, and enter the number '60' to sign in." numberMatchingMsg := "Open your Authenticator app, and enter the number '60' to sign in." - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -2373,7 +2718,7 @@ func TestIsAuthenticatedEntraMFAWaitNumberMatchingLabelShown(t *testing.T) { sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) // Submit password – broker should offer entra_mfa_wait for PhoneAppNotification. - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2400,7 +2745,7 @@ func TestIsAuthenticatedEntraMFAWaitDeniedWhenDeviceRegistrationFails(t *testing username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) provider := &mockDeviceRegistrationFailProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -2425,7 +2770,7 @@ func TestIsAuthenticatedEntraMFAWaitDeniedWhenDeviceRegistrationFails(t *testing sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) // Step 1: Submit password — should initiate MFA. - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2442,21 +2787,23 @@ func TestIsAuthenticatedEntraMFAWaitDeniedWhenDeviceRegistrationFails(t *testing require.True(t, json.Valid([]byte(data)), "IsAuthenticated returned data must be valid JSON") } -func TestIsAuthenticatedEntraMFADeniedWhenInitialGroupFetchFails(t *testing.T) { +func TestIsAuthenticatedEntraMFAWaitFallsBackToClientSecretWhenPasswordlessRegistrationFails(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ - MockProvider: &testutils.MockProvider{GetGroupsFails: true}, - flowState: &himmelblau.MFAFlowState{}, - challengeInfo: &himmelblau.MFAChallengeInfo{ - Message: "Approve the sign-in request in Microsoft Authenticator", - Method: "PhoneAppNotification", - PollingIntervalMs: 1, - MaxPollAttempts: 1, + provider := &mockDeviceRegistrationFailProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 5000, + MaxPollAttempts: 10, + }, + mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), }, - mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), } b := newBrokerForTests(t, &brokerForTestConfig{ @@ -2465,49 +2812,56 @@ func TestIsAuthenticatedEntraMFADeniedWhenInitialGroupFetchFails(t *testing.T) { firstUserBecomesOwner: true, provider: provider, issuerURL: defaultIssuerURL, + clientSecret: "test-client-secret", + registerDevice: true, }) - sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) - passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + sessionID, _ := newSessionForTests(t, b, username, sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) - access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + access, _, err := b.IsAuthenticated(sessionID, "{}") require.NoError(t, err) require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraMFAWait}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the mixed-config path should still start with a passwordless probe") - err = b.SetAvailableMode(sessionID, authmodes.EntraMFAWait) - require.NoError(t, err) - _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFAWait) - require.NoError(t, err) - - access, data, err := b.IsAuthenticated(sessionID, "{}") + updateAuthModes(t, b, sessionID, authmodes.EntraMFAWait) + access, _, err = b.IsAuthenticated(sessionID, "{}") require.NoError(t, err) - require.Equal(t, broker.AuthDenied, access, "initial Entra MFA logins must be denied when groups cannot be resolved") - - var payload struct { - Message string `json:"message"` - } - require.NoError(t, json.Unmarshal([]byte(data), &payload)) - require.Contains(t, payload.Message, "Failed to retrieve groups") + require.Equal(t, broker.AuthNext, access, + "first-time passwordless MFA should fall back to app-only Graph lookup when registration fails but a client secret is configured") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) + require.Equal(t, 1, provider.registerDeviceCalls, + "device registration should still be attempted before falling back to the client-secret path") + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "passwordless fallback should still defer offline password caching until the local password step") } -func TestIsAuthenticatedEntraMFAUsesCachedGroupsWhenRefreshFails(t *testing.T) { +// TestIsAuthenticatedEntraMFAWaitFallbackAppliesToReturningPasswordlessLogins +// covers the returning variant of the client-secret fallback: the user's first +// passwordless login already used it (so the cached token carries no device +// registration data) and device registration keeps failing. The fallback must +// be keyed on the missing device data, not on "first login" — otherwise a user +// who could log in yesterday is denied on every later online login, since the +// device-data-less token also disqualifies the local password mode online. +func TestIsAuthenticatedEntraMFAWaitFallbackAppliesToReturningPasswordlessLogins(t *testing.T) { t.Parallel() username := "test-user@email.com" - oldAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - oldAuthInfo.UserInfo.Groups = []info.Group{{Name: "cached-group", UGID: "cached-id"}} mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ - MockProvider: &testutils.MockProvider{GetGroupsFails: true}, - flowState: &himmelblau.MFAFlowState{}, - challengeInfo: &himmelblau.MFAChallengeInfo{ - Message: "Approve the sign-in request in Microsoft Authenticator", - Method: "PhoneAppNotification", - PollingIntervalMs: 1, - MaxPollAttempts: 1, + provider := &mockDeviceRegistrationFailProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 5000, + MaxPollAttempts: 10, + }, + mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), }, - mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), } b := newBrokerForTests(t, &brokerForTestConfig{ @@ -2516,12 +2870,113 @@ func TestIsAuthenticatedEntraMFAUsesCachedGroupsWhenRefreshFails(t *testing.T) { firstUserBecomesOwner: true, provider: provider, issuerURL: defaultIssuerURL, + clientSecret: "test-client-secret", + registerDevice: true, }) - sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - require.NoError(t, token.CacheAuthInfo(b.TokenPathForSession(sessionID), oldAuthInfo)) + sessionID, _ := newSessionForTests(t, b, username, sessionmode.Login) + // State left behind by the first fallback login: a cached token without + // device registration data and a local password. + generateAndStoreCachedInfo(t, + tokenOptions{username: username, issuer: defaultIssuerURL, obtainedViaEntraAuth: true}, + b.TokenPathForSession(sessionID)) + require.NoError(t, + password.HashAndStorePassword("password", b.PasswordFilepathForSession(sessionID)), + "Setup: storing the local password should succeed") + + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraMFAWait}, b.GetNextAuthModes(sessionID)) + + updateAuthModes(t, b, sessionID, authmodes.EntraMFAWait) + access, _, err = b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthGranted, access, + "a returning passwordless login whose cached token has no device data must keep using the app-only Graph fallback when registration fails again") +} + +func TestIsAuthenticatedEntraAuthDeniedWhenInitialGroupFetchFails(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{GetGroupsFails: true}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 1, + MaxPollAttempts: 1, + }, + mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + }) + + sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + + err = b.SetAvailableMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthDenied, access, "initial Entra MFA logins must be denied when groups cannot be resolved") + + var payload struct { + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(data), &payload)) + require.Contains(t, payload.Message, "Failed to retrieve groups") +} + +func TestIsAuthenticatedEntraAuthUsesCachedGroupsWhenRefreshFails(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + oldAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + oldAuthInfo.UserInfo.Groups = []info.Group{{Name: "cached-group", UGID: "cached-id"}} + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{GetGroupsFails: true}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Approve the sign-in request in Microsoft Authenticator", + Method: "PhoneAppNotification", + PollingIntervalMs: 1, + MaxPollAttempts: 1, + }, + mfaTokenResult: newMFATokenResult(mfaAuthInfo.Token), + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + }) + + sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) + require.NoError(t, token.CacheAuthInfo(b.TokenPathForSession(sessionID), oldAuthInfo)) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2546,20 +3001,20 @@ func TestIsAuthenticatedEntraMFAUsesCachedGroupsWhenRefreshFails(t *testing.T) { require.Equal(t, []info.Group{{Name: "cached-group", UGID: "cached-id"}}, payload.UserInfo.Groups) } -// TestIsAuthenticatedEntraMFASurfacesForDisplayErrorOnFirstLogin verifies that on a +// TestIsAuthenticatedEntraAuthSurfacesForDisplayErrorOnFirstLogin verifies that on a // first Entra MFA login (no cached groups to fall back to) a group fetch that fails // with a user-displayable ForDisplayError (e.g. a missing GroupMember.Read.All // permission — a configuration problem) is surfaced verbatim by finishEntraAuth // instead of being replaced by a misleading generic network hint. This is // independent of force_access_check_with_provider (left unset here on purpose): the // surfacing is driven by there being no cached groups, not by the forced check. -func TestIsAuthenticatedEntraMFASurfacesForDisplayErrorOnFirstLogin(t *testing.T) { +func TestIsAuthenticatedEntraAuthSurfacesForDisplayErrorOnFirstLogin(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) const graphPermMsg = "Error: the Microsoft Entra ID app is missing the GroupMember.Read.All permission" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{ GetGroupsFunc: func() ([]info.Group, error) { return nil, &providerErrors.ForDisplayError{Message: graphPermMsg} @@ -2584,7 +3039,7 @@ func TestIsAuthenticatedEntraMFASurfacesForDisplayErrorOnFirstLogin(t *testing.T }) sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -2612,9 +3067,9 @@ func TestIsAuthenticatedEntraMFASurfacesForDisplayErrorOnFirstLogin(t *testing.T func TestGetAuthenticationModesFiltersNextAuthModesByFlows(t *testing.T) { t.Parallel() - // Use a provider that implements EntraPasswordProvider so that + // Use a provider that implements EntraAuthProvider so that // authModeIsAvailable can confirm the capability before offering the mode. - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{}, @@ -2626,13 +3081,13 @@ func TestGetAuthenticationModesFiltersNextAuthModesByFlows(t *testing.T) { ownerAllowed: true, firstUserBecomesOwner: true, deviceAuthFlowDisabled: true, - // Provide a group source (device registration) so the entra_password + // Provide a group source (device registration) so the entra_auth // flow passes the group-lookup availability check in authModeIsAvailable. registerDevice: true, }) sessionID, _ := newSessionForTests(t, b, "", sessionmode.Login) - b.SetNextAuthModes(sessionID, []string{authmodes.EntraPassword, authmodes.DeviceQr}) + b.SetNextAuthModes(sessionID, []string{authmodes.EntraAuth, authmodes.DeviceQr}) modes, err := b.GetAuthenticationModes(sessionID, []map[string]string{ supportedUILayouts["form"], @@ -2640,16 +3095,16 @@ func TestGetAuthenticationModesFiltersNextAuthModesByFlows(t *testing.T) { }) require.NoError(t, err) require.Equal(t, []map[string]string{{ - "id": authmodes.EntraPassword, - "label": authmodes.Label[authmodes.EntraPassword], + "id": authmodes.EntraAuth, + "label": authmodes.Label[authmodes.EntraAuth], }}, modes) } -// TestGetAuthenticationModesEntraPasswordRequiresGroupSource verifies that once -// the broker has started successfully, the entra_password mode is offered only +// TestGetAuthenticationModesEntraAuthRequiresGroupSource verifies that once +// the broker has started successfully, the entra_auth mode is offered only // when a Microsoft Graph group source is available, i.e. device registration or // a client secret. The missing-group-source case is rejected earlier by New(). -func TestGetAuthenticationModesEntraPasswordRequiresGroupSource(t *testing.T) { +func TestGetAuthenticationModesEntraAuthRequiresGroupSource(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -2664,7 +3119,7 @@ func TestGetAuthenticationModesEntraPasswordRequiresGroupSource(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{}, @@ -2680,7 +3135,7 @@ func TestGetAuthenticationModesEntraPasswordRequiresGroupSource(t *testing.T) { }) sessionID, _ := newSessionForTests(t, b, "", sessionmode.Login) - b.SetNextAuthModes(sessionID, []string{authmodes.EntraPassword, authmodes.DeviceQr}) + b.SetNextAuthModes(sessionID, []string{authmodes.EntraAuth, authmodes.DeviceQr}) modes, err := b.GetAuthenticationModes(sessionID, []map[string]string{ supportedUILayouts["form"], @@ -2692,11 +3147,52 @@ func TestGetAuthenticationModesEntraPasswordRequiresGroupSource(t *testing.T) { for _, m := range modes { ids = append(ids, m["id"]) } - require.Contains(t, ids, authmodes.EntraPassword, "entra_password should be offered when a group source is available") + require.Contains(t, ids, authmodes.EntraAuth, "entra_auth should be offered when a group source is available") }) } } +// TestGetAuthenticationModesEntraAuthRequiresWaitCapability verifies that +// entra_auth is not offered to a client whose form layout cannot render the +// wait-based passwordless probe screen: authd rejects layouts carrying fields +// the client did not declare, so offering the mode would make it permanently +// unselectable for such a client. +func TestGetAuthenticationModesEntraAuthRequiresWaitCapability(t *testing.T) { + t.Parallel() + + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{}, + } + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + provider: provider, + issuerURL: defaultIssuerURL, + ownerAllowed: true, + firstUserBecomesOwner: true, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "", sessionmode.Login) + b.SetNextAuthModes(sessionID, []string{authmodes.EntraAuth, authmodes.DeviceQr}) + + modes, err := b.GetAuthenticationModes(sessionID, []map[string]string{ + supportedUILayouts["form-without-wait"], + supportedUILayouts["qrcode"], + }) + require.NoError(t, err) + + var ids []string + for _, m := range modes { + ids = append(ids, m["id"]) + } + require.NotContains(t, ids, authmodes.EntraAuth, + "entra_auth must not be offered when the form layout does not support wait") + require.Contains(t, ids, authmodes.DeviceQr, + "other modes must remain offered") +} + func TestIsAuthenticatedPasswordGrantRevokedInvalidatesCachedCredentials(t *testing.T) { t.Parallel() @@ -2730,10 +3226,10 @@ func TestIsAuthenticatedPasswordGrantRevokedInvalidatesCachedCredentials(t *test require.NoError(t, err) require.Equal(t, broker.AuthNext, access) require.True(t, json.Valid([]byte(data)), "IsAuthenticated returned data must be valid JSON") - // reauthModes includes EntraPassword, but the provider does not implement - // EntraPasswordProvider, so authModeIsAvailable filters it out — only + // reauthModes includes EntraAuth, but the provider does not implement + // EntraAuthProvider, so authModeIsAvailable filters it out — only // Device/DeviceQr survive into the actual offer. - require.Equal(t, []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr}, b.GetNextAuthModes(sessionID)) _, err = os.Stat(b.PasswordFilepathForSession(sessionID)) require.ErrorIs(t, err, os.ErrNotExist) @@ -2751,7 +3247,7 @@ func TestIsAuthenticatedPasswordGrantRevokedInvalidatesCachedCredentials(t *test for _, mode := range modes { modeIDs = append(modeIDs, mode["id"]) } - // entra_password is in reauthModes but filtered by the capability check; only device modes offered. + // entra_auth is in reauthModes but filtered by the capability check; only device modes offered. require.ElementsMatch(t, []string{authmodes.DeviceQr}, modeIDs) } @@ -2760,10 +3256,10 @@ func TestIsAuthenticatedPasswordGrantRevokedInvalidatesCachedCredentials(t *test // group-fetch failure — even a user-displayable ForDisplayError such as a missing // GroupMember.Read.All permission — falls back to the cached groups instead of // denying, exactly like the device-auth flow. The live provider check now happens -// at the token refresh (see refreshEntraPasswordToken), so the group fetch is no +// at the token refresh (see refreshEntraToken), so the group fetch is no // longer a liveness signal. The ForDisplayError is still surfaced on a *first* // login that has no cached groups (see -// TestIsAuthenticatedEntraMFASurfacesForDisplayErrorOnFirstLogin). +// TestIsAuthenticatedEntraAuthSurfacesForDisplayErrorOnFirstLogin). func TestIsAuthenticatedPasswordEntraTokenFallsBackToCachedGroupsOnGroupFetchError(t *testing.T) { t.Parallel() @@ -2771,11 +3267,11 @@ func TestIsAuthenticatedPasswordEntraTokenFallsBackToCachedGroupsOnGroupFetchErr const graphPermMsg = "Error: the Microsoft Entra ID app is missing the GroupMember.Read.All permission" cachedGroups := []info.Group{{Name: "cached-group", UGID: "cached-id"}} - // The token was obtained via the entra_password flow, so the provider must - // implement EntraPasswordProvider for the returning-login liveness refresh. + // The token was obtained via the entra_auth flow, so the provider must + // implement EntraAuthProvider for the returning-login liveness refresh. // The refresh succeeds (active user); the subsequent group fetch fails, which // must fall back to cached groups rather than deny. - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{ GetGroupsFunc: func() ([]info.Group, error) { return nil, &providerErrors.ForDisplayError{Message: graphPermMsg} @@ -2792,7 +3288,7 @@ func TestIsAuthenticatedPasswordEntraTokenFallsBackToCachedGroupsOnGroupFetchErr }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true, groups: cachedGroups}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true, groups: cachedGroups}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -2813,7 +3309,7 @@ func TestIsAuthenticatedPasswordEntraTokenFallsBackToCachedGroupsOnGroupFetchErr } // TestIsAuthenticatedPasswordEntraTokenRefreshDetectsDisabledUser verifies that on a -// returning login the Entra password token refresh (refreshEntraPasswordToken) is the +// returning login the Entra password token refresh (refreshEntraToken) is the // live disabled-user check: an AADSTS50057-class rejection is classified exactly like // the device-auth flow — login is denied and UserIsDisabled is cached so later offline // attempts are denied too. @@ -2821,7 +3317,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshDetectsDisabledUser(t *testing. t.Parallel() const correctPassword = "password" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, userDisabledErrorCode: "user_disabled", refreshErr: &oauth2.RetrieveError{ @@ -2839,7 +3335,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshDetectsDisabledUser(t *testing. }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -2924,7 +3420,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshRotatesRefreshToken(t *testing. const correctPassword = "password" const rotatedRefreshToken = "rotated-refresh-token" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, refreshResult: &oauth2.Token{AccessToken: "new-access-token", RefreshToken: rotatedRefreshToken}, } @@ -2938,7 +3434,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshRotatesRefreshToken(t *testing. }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true, groups: []info.Group{{Name: "remote-group"}}}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true, groups: []info.Group{{Name: "remote-group"}}}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -2951,7 +3447,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshRotatesRefreshToken(t *testing. cached, err := token.LoadAuthInfo(b.TokenPathForSession(sessionID)) require.NoError(t, err) require.Equal(t, rotatedRefreshToken, cached.Token.RefreshToken, - "the rotated refresh token from refreshEntraPasswordToken must be persisted") + "the rotated refresh token from refreshEntraToken must be persisted") } // TestIsAuthenticatedPasswordEntraTokenRefreshUpdatesUserInfo verifies that a @@ -2963,7 +3459,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshUpdatesUserInfo(t *testing.T) { t.Parallel() const correctPassword = "password" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, @@ -2983,9 +3479,9 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshUpdatesUserInfo(t *testing.T) { // Seed a stale cached token with a different gecos and the groups that should // survive the refresh. generateAndStoreCachedInfo(t, tokenOptions{ - obtainedViaEntraPasswordAuth: true, - gecos: "stale gecos", - groups: []info.Group{{Name: "remote-group"}}, + obtainedViaEntraAuth: true, + gecos: "stale gecos", + groups: []info.Group{{Name: "remote-group"}}, }, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) @@ -3006,7 +3502,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshUpdatesUserInfo(t *testing.T) { "groups must be preserved from the cached token, not overwritten by the refresh") } -func runReturningEntraPasswordLogin(t *testing.T, provider *mockEntraPasswordProvider) (*broker.Broker, string, string) { +func runReturningEntraAuthLogin(t *testing.T, provider *mockEntraAuthProvider) (*broker.Broker, string, string) { t.Helper() const correctPassword = "password" @@ -3019,7 +3515,7 @@ func runReturningEntraPasswordLogin(t *testing.T, provider *mockEntraPasswordPro }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -3033,11 +3529,11 @@ func runReturningEntraPasswordLogin(t *testing.T, provider *mockEntraPasswordPro // TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnVerificationFailure verifies // that if the refreshed Entra password token fails signature verification the // returning login is denied — mirroring the first-login deny path in -// TestIsAuthenticatedEntraMFADeniesOnAccessTokenVerificationFailure. +// TestIsAuthenticatedEntraAuthDeniesOnAccessTokenVerificationFailure. func TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnVerificationFailure(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, @@ -3045,7 +3541,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnVerificationFailure(t * verifyAccessTokenErr: errors.New("token signature verification failed"), } - b, sessionID, access := runReturningEntraPasswordLogin(t, provider) + b, sessionID, access := runReturningEntraAuthLogin(t, provider) require.Equal(t, broker.AuthDenied, access, "a refreshed token that fails signature verification must deny the returning login") @@ -3064,7 +3560,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshVerificationHasOwnTimeout(t *te const correctPassword = "password" const refreshDelay = 50 * time.Millisecond - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, @@ -3081,7 +3577,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshVerificationHasOwnTimeout(t *te }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -3103,7 +3599,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshVerificationHasOwnTimeout(t *te func TestIsAuthenticatedPasswordEntraTokenRefreshPreservesRotationOnUserInfoError(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, @@ -3111,7 +3607,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshPreservesRotationOnUserInfoErro userInfoFromTokenErr: errors.New("missing preferred_username claim"), } - b, sessionID, access := runReturningEntraPasswordLogin(t, provider) + b, sessionID, access := runReturningEntraAuthLogin(t, provider) require.Equal(t, broker.AuthDenied, access, "a refreshed token whose user info cannot be extracted must deny the returning login") @@ -3130,7 +3626,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnUsernameMismatch(t *tes t.Parallel() const correctPassword = "password" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFunc: func() ([]info.Group, error) { return []info.Group{{Name: "remote-group"}}, nil }}, @@ -3147,7 +3643,7 @@ func TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnUsernameMismatch(t *tes }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraPasswordAuth: true}, b.TokenPathForSession(sessionID)) + generateAndStoreCachedInfo(t, tokenOptions{obtainedViaEntraAuth: true}, b.TokenPathForSession(sessionID)) require.NoError(t, password.HashAndStorePassword(correctPassword, b.PasswordFilepathForSession(sessionID))) updateAuthModes(t, b, sessionID, authmodes.Password) @@ -3221,7 +3717,7 @@ func TestIsAuthenticatedPhoneAppOTPRoutesToMFACode(t *testing.T) { username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3244,7 +3740,7 @@ func TestIsAuthenticatedPhoneAppOTPRoutesToMFACode(t *testing.T) { sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) // Step 1: Submit password — broker should recognise PhoneAppOTP and offer entra_mfa_code. - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, data, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3291,7 +3787,7 @@ func TestIsAuthenticatedEntraMFACodeWrongCodeRetries(t *testing.T) { mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) released := 0 provider := &mockMFAWrongCodeThenSuccessProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: newTrackedMFAFlowState(func() { released++ }), challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3315,7 +3811,7 @@ func TestIsAuthenticatedEntraMFACodeWrongCodeRetries(t *testing.T) { sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) // Step 1: Submit password — routed to entra_mfa_code (PhoneAppOTP). - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) require.NoError(t, err) @@ -3353,7 +3849,7 @@ func TestIsAuthenticatedEntraMFAWaitDenialReturnsAuthDenied(t *testing.T) { t.Parallel() provider := &mockMFADeniedProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3373,7 +3869,7 @@ func TestIsAuthenticatedEntraMFAWaitDenialReturnsAuthDenied(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3402,7 +3898,7 @@ func TestIsAuthenticatedEntraMFAWaitTimeoutReturnsAuthNext(t *testing.T) { t.Parallel() provider := &mockMFATimeoutProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3422,7 +3918,7 @@ func TestIsAuthenticatedEntraMFAWaitTimeoutReturnsAuthNext(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3435,11 +3931,11 @@ func TestIsAuthenticatedEntraMFAWaitTimeoutReturnsAuthNext(t *testing.T) { require.NoError(t, err) // Poll - the mock always returns MFA_POLL_CONTINUE, so max attempts will be exhausted. - // After timeout the broker should redirect back to entra_password rather than + // After timeout the broker should redirect back to entra_auth rather than // asking the client to retry a dead MFA wait mode. access, data, err := b.IsAuthenticated(sessionID, "{}") require.NoError(t, err) - require.Equal(t, broker.AuthNext, access, "MFA timeout should return AuthNext to restart from entra_password") + require.Equal(t, broker.AuthNext, access, "MFA timeout should return AuthNext to restart from entra_auth") var payload struct { Message string `json:"message"` @@ -3448,9 +3944,9 @@ func TestIsAuthenticatedEntraMFAWaitTimeoutReturnsAuthNext(t *testing.T) { require.Contains(t, payload.Message, "timed out") } -// TestEntraPasswordRoutesAADSTSErrors verifies that an AADSTS error raised while +// TestEntraAuthRoutesAADSTSErrors verifies that an AADSTS error raised while // initiating the password+MFA flow is mapped to the right broker outcome. -func TestEntraPasswordRoutesAADSTSErrors(t *testing.T) { +func TestEntraAuthRoutesAADSTSErrors(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -3465,7 +3961,7 @@ func TestEntraPasswordRoutesAADSTSErrors(t *testing.T) { "Account_locked": {aadsts: 50053, wantAccess: broker.AuthDenied, wantMsg: "locked"}, "Password_expired": {aadsts: 50055, wantAccess: broker.AuthDenied, wantMsg: "expired"}, "Invalid_credentials_retry": {aadsts: 50126, wantAccess: broker.AuthRetry, wantMsg: "Incorrect password"}, - "Conditional_access_blocked": {aadsts: 53003, wantAccess: broker.AuthDenied, wantMsg: "Conditional Access"}, + "Conditional_access_blocked": {aadsts: 53003, wantAccess: broker.AuthNext, wantNextModes: []string{authmodes.Device, authmodes.DeviceQr}, wantMsg: "Conditional Access"}, "Interactive_auth_to_device": {aadsts: 16000, wantAccess: broker.AuthNext, wantNextModes: []string{authmodes.Device, authmodes.DeviceQr}, wantMsg: "MFA registration required"}, "Interactive_auth_denied_when_device_disabled": {aadsts: 16000, deviceAuthDisabled: true, wantAccess: broker.AuthDenied, wantMsg: "disabled"}, "MFA_enrollment_to_device": {aadsts: 50072, wantAccess: broker.AuthNext, wantNextModes: []string{authmodes.Device, authmodes.DeviceQr}, wantMsg: "MFA registration required"}, @@ -3480,7 +3976,7 @@ func TestEntraPasswordRoutesAADSTSErrors(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, initErr: &himmelblau.MFAError{ AADSTS: tc.aadsts, @@ -3497,13 +3993,13 @@ func TestEntraPasswordRoutesAADSTSErrors(t *testing.T) { issuerURL: defaultIssuerURL, deviceAuthFlowDisabled: tc.deviceAuthDisabled, // Provide a group source (device registration) so a broker with - // device_code disabled still satisfies the entra_password + // device_code disabled still satisfies the entra_auth // only-enabled-flow startup check in New(). registerDevice: true, }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, data, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3587,14 +4083,14 @@ func TestIsAuthenticatedPasswordDeviceRegistrationRefreshDoesNotSendClientSecret "the Microsoft Broker App is a public client, so refresh must not send the configured OIDC client secret") } -// TestEntraPasswordInvalidatesCachedCredentialsOnRemotePasswordChange verifies +// TestEntraAuthInvalidatesCachedCredentialsOnRemotePasswordChange verifies // that an AADSTS50173 (grant revoked by a remote password change) wipes the // cached token and password files and offers re-authentication. -func TestEntraPasswordInvalidatesCachedCredentialsOnRemotePasswordChange(t *testing.T) { +func TestEntraAuthInvalidatesCachedCredentialsOnRemotePasswordChange(t *testing.T) { t.Parallel() username := "test-user@email.com" - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, initErr: &himmelblau.MFAError{AADSTS: 50173, Message: "grant revoked"}, } @@ -3614,7 +4110,7 @@ func TestEntraPasswordInvalidatesCachedCredentialsOnRemotePasswordChange(t *test require.NoError(t, token.CacheAuthInfo(b.TokenPathForSession(sessionID), cached)) require.NoError(t, password.HashAndStorePassword("password", b.PasswordFilepathForSession(sessionID))) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, data, err := b.IsAuthenticated(sessionID, passwordAuthData) require.NoError(t, err) @@ -3650,7 +4146,7 @@ func TestIsAuthenticatedFIDOMethodRoutesToDevice(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3667,13 +4163,13 @@ func TestIsAuthenticatedFIDOMethodRoutesToDevice(t *testing.T) { issuerURL: defaultIssuerURL, deviceAuthFlowDisabled: tc.deviceAuthDisabled, // Provide a group source (device registration) so a broker with - // device_code disabled still satisfies the entra_password + // device_code disabled still satisfies the entra_auth // only-enabled-flow startup check in New(). registerDevice: true, }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, data, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3701,7 +4197,7 @@ func TestIsAuthenticatedEntraMFACodeDenied(t *testing.T) { t.Parallel() provider := &mockMFADeniedProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3720,7 +4216,7 @@ func TestIsAuthenticatedEntraMFACodeDenied(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3743,11 +4239,11 @@ func TestIsAuthenticatedEntraMFACodeDenied(t *testing.T) { // TestIsAuthenticatedEntraMFACodeFailureRoutesBack verifies that a non-denial // failure during code verification clears the dead MFA state and routes the -// client back to entra_password rather than the now-dead code mode. +// client back to entra_auth rather than the now-dead code mode. func TestIsAuthenticatedEntraMFACodeFailureRoutesBack(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3766,7 +4262,7 @@ func TestIsAuthenticatedEntraMFACodeFailureRoutesBack(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -3778,8 +4274,8 @@ func TestIsAuthenticatedEntraMFACodeFailureRoutesBack(t *testing.T) { access, data, err := b.IsAuthenticated(sessionID, codeAuthData) require.NoError(t, err) require.Equal(t, broker.AuthNext, access) - require.Equal(t, []string{authmodes.EntraPassword}, b.GetNextAuthModes(sessionID), - "a failed code submission should route back to entra_password") + require.Equal(t, []string{authmodes.EntraAuth}, b.GetNextAuthModes(sessionID), + "a failed code submission should route back to entra_auth") var payload struct { Message string `json:"message"` @@ -3788,15 +4284,15 @@ func TestIsAuthenticatedEntraMFACodeFailureRoutesBack(t *testing.T) { require.Contains(t, payload.Message, "failed") } -// TestIsAuthenticatedEntraMFAFallsBackToEmailClaim verifies that when the MFA +// TestIsAuthenticatedEntraAuthFallsBackToEmailClaim verifies that when the MFA // token carries no preferred_username, the user identity is recovered from the // email extra instead. -func TestIsAuthenticatedEntraMFAFallsBackToEmailClaim(t *testing.T) { +func TestIsAuthenticatedEntraAuthFallsBackToEmailClaim(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -3819,7 +4315,7 @@ func TestIsAuthenticatedEntraMFAFallsBackToEmailClaim(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -4745,41 +5241,41 @@ func TestIsPromptMethod(t *testing.T) { } } -// TestEntraPasswordAuthProviderNotSupported verifies that entraPasswordAuth +// TestEntraAuthProviderNotSupported verifies that entraAuth // returns AuthDenied when the broker's provider does not implement -// EntraPasswordProvider (defensive guard against misconfiguration). -func TestEntraPasswordAuthProviderNotSupported(t *testing.T) { +// EntraAuthProvider (defensive guard against misconfiguration). +func TestEntraAuthProviderNotSupported(t *testing.T) { t.Parallel() b := newBrokerForTests(t, &brokerForTestConfig{ ownerAllowed: true, firstUserBecomesOwner: true, issuerURL: defaultIssuerURL, - // Default MockProvider — does NOT implement EntraPasswordProvider. + // Default MockProvider — does NOT implement EntraAuthProvider. }) sessionID, _ := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) - // Force the session into entra_password mode without going through the + // Force the session into entra_auth mode without going through the // normal availability check (which would reject a provider that lacks support). - err := b.SetAvailableMode(sessionID, authmodes.EntraPassword) + err := b.SetAvailableMode(sessionID, authmodes.EntraAuth) require.NoError(t, err) - _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraPassword) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) require.NoError(t, err) // Empty auth data (no secret) is fine: ProviderAs check fires before any // password is consumed. access, _, err := b.IsAuthenticated(sessionID, "{}") require.NoError(t, err) - require.Equal(t, broker.AuthDenied, access, "entra_password with unsupported provider must deny") + require.Equal(t, broker.AuthDenied, access, "entra_auth with unsupported provider must deny") } -// TestEntraPasswordAuthNonMFAError verifies that a non-MFAError from -// InitiateEntraPasswordAuth (e.g. a network failure) returns AuthDenied. -func TestEntraPasswordAuthNonMFAError(t *testing.T) { +// TestEntraAuthNonMFAError verifies that a non-MFAError from +// InitiateEntraAuth (e.g. a network failure) returns AuthDenied. +func TestEntraAuthNonMFAError(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, initErr: errors.New("simulated network failure"), } @@ -4792,22 +5288,22 @@ func TestEntraPasswordAuthNonMFAError(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) authData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, authData) require.NoError(t, err) - require.Equal(t, broker.AuthDenied, access, "non-MFAError from InitiateEntraPasswordAuth must deny") + require.Equal(t, broker.AuthDenied, access, "non-MFAError from InitiateEntraAuth must deny") } -// TestEntraPasswordAuthNilFlowOrChallenge verifies that a nil flow/challenge -// returned by InitiateEntraPasswordAuth (provider contract violation) returns +// TestEntraAuthNilFlowOrChallenge verifies that a nil flow/challenge +// returned by InitiateEntraAuth (provider contract violation) returns // AuthDenied. -func TestEntraPasswordAuthNilFlowOrChallenge(t *testing.T) { +func TestEntraAuthNilFlowOrChallenge(t *testing.T) { t.Parallel() // initErr is nil but both flowState and challengeInfo are nil (default zero values). - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, // flowState and challengeInfo left nil. } @@ -4820,7 +5316,7 @@ func TestEntraPasswordAuthNilFlowOrChallenge(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) authData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, authData) @@ -4830,7 +5326,7 @@ func TestEntraPasswordAuthNilFlowOrChallenge(t *testing.T) { // TestEntraMFAWaitAuthProviderNotSupported verifies that entraMFAWaitAuth // returns AuthDenied when the broker's provider does not implement -// EntraPasswordProvider. +// EntraAuthProvider. func TestEntraMFAWaitAuthProviderNotSupported(t *testing.T) { t.Parallel() @@ -4858,7 +5354,7 @@ func TestEntraMFAWaitAuthProviderNotSupported(t *testing.T) { func TestEntraMFAWaitAuthNoActiveMFAFlow(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{PollingIntervalMs: 1, MaxPollAttempts: 1}, @@ -4873,7 +5369,7 @@ func TestEntraMFAWaitAuthNoActiveMFAFlow(t *testing.T) { sessionID, _ := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) - // Jump straight to entra_mfa_wait without running entra_password first, + // Jump straight to entra_mfa_wait without running entra_auth first, // so session.mfaFlowActive remains nil. err := b.SetAvailableMode(sessionID, authmodes.EntraMFAWait) require.NoError(t, err) @@ -4885,13 +5381,54 @@ func TestEntraMFAWaitAuthNoActiveMFAFlow(t *testing.T) { require.Equal(t, broker.AuthDenied, access, "entra_mfa_wait with no active MFA flow must deny") } +// TestEntraMFAWaitAuthReplaysStaleDuplicateCall verifies that a duplicate +// IsAuthenticated call for entra_mfa_wait, arriving after the session already +// completed successfully and moved on to a different mode, replays that +// transition instead of denying an already-successful login. This is the +// broker-level guard for a client sending a stray repeat call after success +// (observed in practice with entra_auth_fido, whose success path also frees +// mfaFlowActive and switches nextAuthModes in one step). +func TestEntraMFAWaitAuthReplaysStaleDuplicateCall(t *testing.T) { + t.Parallel() + + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{PollingIntervalMs: 1, MaxPollAttempts: 1}, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) + + err := b.SetAvailableMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFAWait) + require.NoError(t, err) + + // Simulate a prior successful call already having freed mfaFlowActive and + // chained to a different mode. + b.SetNextAuthModes(sessionID, []string{authmodes.NewPassword}) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access, + "a stray duplicate call after success must replay the transition, not deny") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) +} + // TestEntraMFAWaitAuthNoChallengeMeta verifies that entraMFAWaitAuth returns // AuthDenied when the session has an active MFA flow but no challenge metadata // (another provider contract violation guard). func TestEntraMFAWaitAuthNoChallengeMeta(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{PollingIntervalMs: 1, MaxPollAttempts: 1}, @@ -4921,7 +5458,7 @@ func TestEntraMFAWaitAuthNoChallengeMeta(t *testing.T) { } // TestEntraMFACodeAuthProviderNotSupported verifies that entraMFACodeAuth -// returns AuthDenied when the provider does not implement EntraPasswordProvider. +// returns AuthDenied when the provider does not implement EntraAuthProvider. func TestEntraMFACodeAuthProviderNotSupported(t *testing.T) { t.Parallel() @@ -4948,7 +5485,7 @@ func TestEntraMFACodeAuthProviderNotSupported(t *testing.T) { func TestEntraMFACodeAuthNoActiveMFAFlow(t *testing.T) { t.Parallel() - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{}, @@ -4963,7 +5500,7 @@ func TestEntraMFACodeAuthNoActiveMFAFlow(t *testing.T) { sessionID, _ := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) - // Jump straight to entra_mfa_code without running entra_password first. + // Jump straight to entra_mfa_code without running entra_auth first. err := b.SetAvailableMode(sessionID, authmodes.EntraMFACode) require.NoError(t, err) _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFACode) @@ -4974,16 +5511,50 @@ func TestEntraMFACodeAuthNoActiveMFAFlow(t *testing.T) { require.Equal(t, broker.AuthDenied, access, "entra_mfa_code with no active MFA flow must deny") } -// TestIsAuthenticatedEntraMFAUsesVerifiedAccessTokenIdentity verifies that +// TestEntraMFACodeAuthReplaysStaleDuplicateCall mirrors +// TestEntraMFAWaitAuthReplaysStaleDuplicateCall for entra_mfa_code. +func TestEntraMFACodeAuthReplaysStaleDuplicateCall(t *testing.T) { + t.Parallel() + + provider := &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{}, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + issuerURL: defaultIssuerURL, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@example.com", sessionmode.Login) + + err := b.SetAvailableMode(sessionID, authmodes.EntraMFACode) + require.NoError(t, err) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraMFACode) + require.NoError(t, err) + + b.SetNextAuthModes(sessionID, []string{authmodes.NewPassword}) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access, + "a stray duplicate call after success must replay the transition, not deny") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) +} + +// TestIsAuthenticatedEntraAuthUsesVerifiedAccessTokenIdentity verifies that // first-login identity comes from UserInfoFromAccessToken after VerifyAccessToken, // not from OAuth token extras that may have been sourced from an unverified // id_token by libhimmelblau. -func TestIsAuthenticatedEntraMFAUsesVerifiedAccessTokenIdentity(t *testing.T) { +func TestIsAuthenticatedEntraAuthUsesVerifiedAccessTokenIdentity(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -5020,17 +5591,17 @@ func TestIsAuthenticatedEntraMFAUsesVerifiedAccessTokenIdentity(t *testing.T) { require.Equal(t, "verified-access-token-user-id", cached.UserInfo.ProviderID) } -// TestIsAuthenticatedEntraMFADeniesOnUsernameMismatch verifies the first-login +// TestIsAuthenticatedEntraAuthDeniesOnUsernameMismatch verifies the first-login // identity cross-check: when the verified MFA access token identity does not // match the username the user authenticated as, VerifyUsername fails and the // login is denied. This is the first-login counterpart to the refresh-path // mismatch test (TestIsAuthenticatedPasswordEntraTokenRefreshDeniesOnUsernameMismatch). -func TestIsAuthenticatedEntraMFADeniesOnUsernameMismatch(t *testing.T) { +func TestIsAuthenticatedEntraAuthDeniesOnUsernameMismatch(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -5059,18 +5630,18 @@ func TestIsAuthenticatedEntraMFADeniesOnUsernameMismatch(t *testing.T) { "a first-login MFA access token whose identity does not match the session username must be denied") } -// TestIsAuthenticatedEntraMFADenialsDoNotCachePassword verifies that an Entra MFA +// TestIsAuthenticatedEntraAuthDenialsDoNotCachePassword verifies that an Entra MFA // denial does not persist an offline password file. A successful first login // caches the password for offline use; a denied one must leave no such artifact, // otherwise a later offline login could grant access to a user who never // authenticated. This complements the existing denial tests, which assert the // AuthDenied reply but not the absence of the password file. -func TestIsAuthenticatedEntraMFADenialsDoNotCachePassword(t *testing.T) { +func TestIsAuthenticatedEntraAuthDenialsDoNotCachePassword(t *testing.T) { t.Parallel() username := "test-user@email.com" mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) - provider := &mockEntraPasswordProvider{ + provider := &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{GetGroupsFails: true}, flowState: &himmelblau.MFAFlowState{}, challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -5091,7 +5662,7 @@ func TestIsAuthenticatedEntraMFADenialsDoNotCachePassword(t *testing.T) { }) sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) @@ -5122,7 +5693,7 @@ func TestIsAuthenticatedEntraMFACodeMaxAttemptsLockout(t *testing.T) { mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) released := 0 provider := &mockMFAAlwaysWrongCodeProvider{ - mockEntraPasswordProvider: &mockEntraPasswordProvider{ + mockEntraAuthProvider: &mockEntraAuthProvider{ MockProvider: &testutils.MockProvider{}, flowState: newTrackedMFAFlowState(func() { released++ }), challengeInfo: &himmelblau.MFAChallengeInfo{ @@ -5146,7 +5717,7 @@ func TestIsAuthenticatedEntraMFACodeMaxAttemptsLockout(t *testing.T) { sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) // Step 1: Submit password — routed to entra_mfa_code (PhoneAppOTP). - updateAuthModes(t, b, sessionID, authmodes.EntraPassword) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) require.NoError(t, err) @@ -5189,3 +5760,731 @@ func TestMain(m *testing.M) { m.Run() } + +// mockFIDOAuthenticator implements broker.FIDOAuthenticator for tests. +type mockFIDOAuthenticator struct { + devicePresent bool + requiresPIN bool + requiresPINErr error + assertion string + assertErrs []error // consumed one per Assert call; a nil entry means success + + assertCalls int + recordedChallenges []string + recordedAllowLists [][]string + recordedPINs []string +} + +func (m *mockFIDOAuthenticator) DevicePresent() bool { return m.devicePresent } + +func (m *mockFIDOAuthenticator) DeviceRequiresPIN() (bool, error) { + return m.requiresPIN, m.requiresPINErr +} + +func (m *mockFIDOAuthenticator) Assert(_ context.Context, challenge string, allowList []string, pin string) (string, error) { + m.recordedChallenges = append(m.recordedChallenges, challenge) + m.recordedAllowLists = append(m.recordedAllowLists, allowList) + m.recordedPINs = append(m.recordedPINs, pin) + call := m.assertCalls + m.assertCalls++ + if call < len(m.assertErrs) && m.assertErrs[call] != nil { + return "", m.assertErrs[call] + } + return m.assertion, nil +} + +// newFIDOChallengeProvider returns a provider whose password step yields a +// FidoKey challenge carrying WebAuthn data. +func newFIDOChallengeProvider(mfaTokenResult *oauth2.Token) *mockEntraAuthProvider { + return &mockEntraAuthProvider{ + MockProvider: &testutils.MockProvider{}, + flowState: &himmelblau.MFAFlowState{}, + challengeInfo: &himmelblau.MFAChallengeInfo{ + Message: "Use your security key", + Method: "FidoKey", + FidoChallenge: "fido-challenge", + FidoAllowList: []string{"Y3JlZA=="}, + }, + mfaTokenResult: mfaTokenResult, + } +} + +// TestIsAuthenticatedFIDOChallengeRouting verifies how a FidoKey challenge is +// routed after the password step: to the local FIDO modes whenever this build +// can perform the assertion (PIN mode first when a connected key requires one, +// otherwise the assertion mode, which waits for a key to be inserted), and to +// Device Authentication only when Entra sent no WebAuthn challenge. +func TestIsAuthenticatedFIDOChallengeRouting(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + fido *mockFIDOAuthenticator + noChallengeData bool + + wantNextModes []string + wantFidoInitOpts bool + }{ + "Routes_to_fido_mode": { + fido: &mockFIDOAuthenticator{devicePresent: true}, + wantNextModes: []string{authmodes.EntraAuthFido}, + wantFidoInitOpts: true, + }, + "Routes_to_pin_mode_when_key_requires_pin": { + fido: &mockFIDOAuthenticator{devicePresent: true, requiresPIN: true}, + wantNextModes: []string{authmodes.EntraAuthFidoPin}, + wantFidoInitOpts: true, + }, + "Routes_to_fido_mode_when_pin_check_fails": { + fido: &mockFIDOAuthenticator{devicePresent: true, requiresPINErr: errors.New("device error")}, + wantNextModes: []string{authmodes.EntraAuthFido}, + wantFidoInitOpts: true, + }, + "Waits_on_fido_mode_when_no_device_is_present_yet": { + fido: &mockFIDOAuthenticator{devicePresent: false}, + wantNextModes: []string{authmodes.EntraAuthFido}, + wantFidoInitOpts: true, + }, + "Redirects_to_device_when_challenge_has_no_webauthn_data": { + fido: &mockFIDOAuthenticator{devicePresent: true}, + noChallengeData: true, + wantNextModes: []string{authmodes.Device, authmodes.DeviceQr}, + wantFidoInitOpts: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + if tc.noChallengeData { + provider.challengeInfo.FidoChallenge = "" + provider.challengeInfo.FidoAllowList = nil + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: tc.fido, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, tc.wantNextModes, b.GetNextAuthModes(sessionID)) + + wantOpts := []himmelblau.AuthOption(nil) + if tc.wantFidoInitOpts { + wantOpts = []himmelblau.AuthOption{himmelblau.AuthOptionFido} + } + require.Equal(t, [][]himmelblau.AuthOption{wantOpts}, provider.recordedInitAuthOpts, + "the FIDO auth option must be passed exactly when local FIDO support exists, so passwordless discovery still finds FIDO-only accounts even without a plugged-in key") + }) + } +} + +// TestIsAuthenticatedEntraAuthFidoSucceeds walks the full security-key MFA +// chain: password -> PIN entry -> assertion -> granted. +func TestIsAuthenticatedEntraAuthFidoSucceeds(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := newFIDOChallengeProvider(newMFATokenResult(mfaAuthInfo.Token)) + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + requiresPIN: true, + assertion: `{"id":"assertion"}`, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + }) + + sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFidoPin}, b.GetNextAuthModes(sessionID)) + + // PIN entry step. + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFidoPin)) + layout, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFidoPin) + require.NoError(t, err) + require.Equal(t, "chars_password", layout["entry"], "the PIN prompt must be a hidden entry") + + pinAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "123456", key)) + access, _, err = b.IsAuthenticated(sessionID, pinAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + + // Assertion step. + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + layout, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + require.Equal(t, "true", layout["wait"], "the assertion step must be a wait layout") + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthGranted, access) + require.True(t, json.Valid([]byte(data)), "IsAuthenticated returned data must be valid JSON") + + require.Equal(t, []string{"fido-challenge"}, fidoMock.recordedChallenges) + require.Equal(t, [][]string{{"Y3JlZA=="}}, fidoMock.recordedAllowLists) + require.Equal(t, []string{"123456"}, fidoMock.recordedPINs, + "the assertion must run with the PIN collected in the PIN step") + require.Equal(t, []string{`{"id":"assertion"}`}, provider.recordedChallengeData, + "the assertion JSON must be submitted to the MFA flow as auth data") + require.Equal(t, []int{0}, provider.recordedPollAttempts) + + _, err = os.Stat(b.PasswordFilepathForSession(sessionID)) + require.NoError(t, err, "FIDO MFA completion should cache the offline password") + _, err = os.Stat(b.TokenPathForSession(sessionID)) + require.NoError(t, err, "FIDO MFA completion should cache the refreshed token") +} + +// TestIsAuthenticatedEntraAuthFidoResumesAfterTransientCancel is a regression +// test for the security-key login loop: a transient PAM cancel during the +// WebAuthn assertion (e.g. GDM re-selecting the "touch your key" wait mode) +// must leave the MFA flow intact so the resumed assertion completes. Tearing +// the flow down on cancel stranded the session without an active MFA flow, so +// the next assertion dead-ended and looped the user back through the password +// probe. +func TestIsAuthenticatedEntraAuthFidoResumesAfterTransientCancel(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := newFIDOChallengeProvider(newMFATokenResult(mfaAuthInfo.Token)) + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + assertion: `{"id":"assertion"}`, + // The first ceremony is cancelled; the second (nil entry) succeeds. + assertErrs: []error{fido.ErrCanceled}, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, key := newSessionForTests(t, b, username, sessionmode.Login) + advanceToEntraAuthFido(t, b, sessionID, key) + + // First assertion is cancelled mid-ceremony. + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthCancelled, access, "a cancelled assertion must report cancellation") + + // The resumed assertion on the same session must complete, rather than + // dead-ending on a torn-down flow and looping back to the password probe. + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthGranted, access, "the resumed security-key assertion must succeed") + require.True(t, json.Valid([]byte(data)), "IsAuthenticated returned data must be valid JSON") +} + +func TestPasswordlessFIDOFailureFallsBackToDeviceAuth(t *testing.T) { + t.Parallel() + + tests := map[string]error{ + "Generic_failure": errors.New("credential mismatch"), + "Blocked_PIN": fido.ErrPINBlocked, + } + for name, assertErr := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + assertErrs: []error{assertErr}, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + layout, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + require.Equal(t, "true", layout["wait"], "initial Entra Password selection should probe passwordless methods") + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the FIDO challenge should come from the passwordless probe") + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + access, _, err = b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.Device, authmodes.DeviceQr}, b.GetNextAuthModes(sessionID)) + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "a failed passwordless FIDO probe must not cache an offline password") + }) + } +} + +// TestPasswordlessProbeWithoutLocalKeyWaitsForInsertion verifies that a +// passwordless FIDO-only account still routes to the local security-key step +// when no key is plugged in yet: that step waits for insertion rather than +// falling back to the device code flow. +func TestPasswordlessProbeWithoutLocalKeyWaitsForInsertion(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{devicePresent: false} + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + layout, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + require.Equal(t, "true", layout["wait"], "the passwordless probe must auto-submit") + require.Empty(t, layout["entry"], "the passwordless probe must not prompt for an Entra password") + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID), + "a passwordless FIDO-only account without a plugged-in key must wait on the security-key step, not fall back to device auth") + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the unplugged-key path must still start with a passwordless probe") + require.Equal(t, [][]himmelblau.AuthOption{{himmelblau.AuthOptionFido}}, provider.recordedInitAuthOpts, + "the passwordless probe must still advertise FIDO capability so Entra reveals the FIDO-only path") +} + +// TestFIDOWaitTimesOutToDeviceAuth verifies that the security-key step does not +// block forever when no key is ever inserted (a headless or SSH session): it +// waits up to fidoDeviceWaitTimeout and then falls back to the device code flow. +func TestFIDOWaitTimesOutToDeviceAuth(t *testing.T) { + restore := broker.SetFIDODeviceWaitTimeout(20 * time.Millisecond) + defer restore() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{devicePresent: false} + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + access, _, err = b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.Device, authmodes.DeviceQr}, b.GetNextAuthModes(sessionID), + "a security-key step with no key ever inserted must fall back to the device code flow") +} + +func TestPasswordlessFIDOSuccessRegistersDeviceAndChainsToNewPassword(t *testing.T) { + t.Parallel() + + username := "test-user@email.com" + mfaAuthInfo := generateCachedInfo(t, tokenOptions{username: username, issuer: defaultIssuerURL}) + provider := newFIDOChallengeProvider(newMFATokenResult(mfaAuthInfo.Token)) + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + assertion: `{"id":"assertion"}`, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, username, sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + require.Equal(t, []string{""}, provider.recordedInitPasswords, + "the FIDO challenge should come from the passwordless probe") + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + access, _, err = b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access, + "first-time passwordless FIDO should chain to local password creation after registering the device") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) + require.Equal(t, 1, provider.registerDeviceCalls, + "passwordless FIDO should attempt first-time device registration after MFA succeeds") + require.Equal(t, []string{`{"id":"assertion"}`}, provider.recordedChallengeData, + "the assertion JSON must be submitted to the MFA flow as auth data") + _, err = os.Stat(b.TokenPathForSession(sessionID)) + require.NoError(t, err, "passwordless FIDO should cache the token once device registration succeeds") + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "passwordless FIDO should still wait for the local password step before caching an offline password") +} + +func TestPasswordlessServerRejectedFIDOFallsBackToDeviceAuth(t *testing.T) { + t.Parallel() + + provider := &mockInvalidFIDOAssertionProvider{mockEntraAuthProvider: newFIDOChallengeProvider(nil)} + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + assertion: `{"id":"assertion"}`, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuth)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuth) + require.NoError(t, err) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.Device, authmodes.DeviceQr}, b.GetNextAuthModes(sessionID), + "a passwordless FIDO assertion rejected by Entra must fall back to device auth instead of re-probing FIDO forever") + require.Contains(t, data, "device code flow") + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID), + "a passwordless FIDO rejection must not cache an offline password") +} + +// TestEntraAuthFidoAuthReplaysStaleDuplicateCall verifies that a duplicate +// IsAuthenticated call for entra_auth_fido, arriving after the session already +// completed the WebAuthn assertion and chained to a different mode (freeing +// mfaFlowActive in the process), replays that transition instead of denying +// an already-successful login. This is the case observed in practice: GDM +// sent a second IsAuthenticated for entra_auth_fido right after a successful +// assertion had already moved the session to newpassword. +func TestEntraAuthFidoAuthReplaysStaleDuplicateCall(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{devicePresent: true} + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + }) + + sessionID, _ := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err := b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) + + // Simulate a prior successful call already having freed mfaFlowActive and + // chained to a different mode, without ever running the assertion here. + b.SetNextAuthModes(sessionID, []string{authmodes.NewPassword}) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access, + "a stray duplicate call after success must replay the transition, not deny") + require.Equal(t, []string{authmodes.NewPassword}, b.GetNextAuthModes(sessionID)) + require.Empty(t, fidoMock.recordedChallenges, "a replayed call must not re-run the WebAuthn assertion") +} + +// advanceToEntraAuthFido submits the Entra password for the session and selects +// the entra_auth_fido mode, leaving the session ready for the assertion +// IsAuthenticated("{}"). +func advanceToEntraAuthFido(t *testing.T, b *broker.Broker, sessionID, key string) { + t.Helper() + + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFido}, b.GetNextAuthModes(sessionID)) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFido)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFido) + require.NoError(t, err) +} + +// TestIsAuthenticatedEntraAuthFidoAssertionErrors verifies the routing of +// local WebAuthn ceremony failures. +func TestIsAuthenticatedEntraAuthFidoAssertionErrors(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + assertErr error + + wantAccess string + wantNextModes []string + wantMsgContains string + }{ + "PIN_required_routes_to_pin_mode": { + assertErr: fido.ErrPINRequired, + wantAccess: broker.AuthNext, + wantNextModes: []string{authmodes.EntraAuthFidoPin}, + wantMsgContains: "PIN", + }, + "Invalid_PIN_reprompts_for_pin": { + assertErr: fido.ErrPINInvalid, + wantAccess: broker.AuthNext, + wantNextModes: []string{authmodes.EntraAuthFidoPin}, + wantMsgContains: "Incorrect", + }, + "Blocked_PIN_denies": { + assertErr: fido.ErrPINBlocked, + wantAccess: broker.AuthDenied, + wantMsgContains: "blocked", + }, + "Touch_timeout_retries_the_same_mode": { + assertErr: fido.ErrTimeout, + wantAccess: broker.AuthRetry, + wantMsgContains: "not touched in time", + }, + "Cancellation_cancels": { + assertErr: fido.ErrCanceled, + wantAccess: broker.AuthCancelled, + }, + "Unplugged_device_retries_and_waits_for_reinsertion": { + assertErr: fido.ErrNoDevice, + wantAccess: broker.AuthRetry, + wantMsgContains: "reinsert", + }, + "Other_failures_restart_from_password": { + assertErr: errors.New("assertion exploded"), + wantAccess: broker.AuthNext, + wantNextModes: []string{authmodes.EntraAuth}, + wantMsgContains: "failed", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{ + devicePresent: true, + assertErrs: []error{tc.assertErr}, + } + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + advanceToEntraAuthFido(t, b, sessionID, key) + + access, data, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, tc.wantAccess, access) + + if tc.wantNextModes != nil { + require.Equal(t, tc.wantNextModes, b.GetNextAuthModes(sessionID)) + } + if tc.wantMsgContains != "" { + var payload struct { + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(data), &payload)) + require.Contains(t, payload.Message, tc.wantMsgContains) + } + + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID)) + }) + } +} + +// TestIsAuthenticatedEntraAuthFidoPinValidation verifies the PIN entry mode's +// input handling. +func TestIsAuthenticatedEntraAuthFidoPinValidation(t *testing.T) { + t.Parallel() + + provider := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{devicePresent: true, requiresPIN: true} + + b := newBrokerForTests(t, &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: provider, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + }) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + updateAuthModes(t, b, sessionID, authmodes.EntraAuth) + + passwordAuthData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "password", key)) + access, _, err := b.IsAuthenticated(sessionID, passwordAuthData) + require.NoError(t, err) + require.Equal(t, broker.AuthNext, access) + require.Equal(t, []string{authmodes.EntraAuthFidoPin}, b.GetNextAuthModes(sessionID)) + + require.NoError(t, b.SetAvailableMode(sessionID, authmodes.EntraAuthFidoPin)) + _, err = b.SelectAuthenticationMode(sessionID, authmodes.EntraAuthFidoPin) + require.NoError(t, err) + + // An empty PIN must re-prompt rather than run the ceremony without one. + emptyPinData := fmt.Sprintf(`{"%s":"%s"}`, broker.AuthDataSecret, encryptSecret(t, "", key)) + access, data, err := b.IsAuthenticated(sessionID, emptyPinData) + require.NoError(t, err) + require.Equal(t, broker.AuthRetry, access) + + var payload struct { + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(data), &payload)) + require.Contains(t, payload.Message, "PIN") + require.Zero(t, fidoMock.assertCalls, "no assertion may run without the PIN") +} + +// TestIsAuthenticatedEntraAuthFidoRejectedByEntra verifies the routing when the +// local ceremony succeeds but Entra ID rejects the assertion. +func TestIsAuthenticatedEntraAuthFidoRejectedByEntra(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + denied bool + + wantAccess string + wantNextModes []string + }{ + "Denied_assertion_denies": {denied: true, wantAccess: broker.AuthDenied}, + "Other_errors_restart_from_password": {wantAccess: broker.AuthNext, wantNextModes: []string{authmodes.EntraAuth}}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + // A nil mfaTokenResult makes the mock's AcquireTokenByMFAFlow fail + // with a generic error; the denied variant wraps it to return the + // MFA-denied classification instead. + base := newFIDOChallengeProvider(nil) + fidoMock := &mockFIDOAuthenticator{devicePresent: true, assertion: `{"id":"assertion"}`} + + cfg := &brokerForTestConfig{ + Config: broker.Config{DataDir: t.TempDir()}, + ownerAllowed: true, + firstUserBecomesOwner: true, + provider: base, + fidoAuthenticator: fidoMock, + issuerURL: defaultIssuerURL, + registerDevice: true, + } + if tc.denied { + cfg.provider = &mockMFADeniedProvider{mockEntraAuthProvider: base} + } + b := newBrokerForTests(t, cfg) + + sessionID, key := newSessionForTests(t, b, "test-user@email.com", sessionmode.Login) + advanceToEntraAuthFido(t, b, sessionID, key) + + access, _, err := b.IsAuthenticated(sessionID, "{}") + require.NoError(t, err) + require.Equal(t, tc.wantAccess, access) + if tc.wantNextModes != nil { + require.Equal(t, tc.wantNextModes, b.GetNextAuthModes(sessionID)) + } + require.NoFileExists(t, b.PasswordFilepathForSession(sessionID)) + }) + } +} diff --git a/authd-oidc-brokers/internal/broker/config.go b/authd-oidc-brokers/internal/broker/config.go index 22d2601136..5d6e5a0d77 100644 --- a/authd-oidc-brokers/internal/broker/config.go +++ b/authd-oidc-brokers/internal/broker/config.go @@ -65,8 +65,8 @@ const ( flowsSection = "flows" // flowsDeviceAuthKey controls whether the device_auth and device_auth_qr modes are enabled. flowsDeviceAuthKey = "device_code" - // flowsEntraPasswordKey controls whether entra_password mode is enabled. - flowsEntraPasswordKey = "entra_password" + // flowsEntraAuthKey controls whether entra_auth mode is enabled. + flowsEntraAuthKey = "entra_auth" // ownerAutoRegistrationConfigPath is the name of the file that will be auto-generated to register the owner. ownerAutoRegistrationConfigPath = "20-owner-autoregistration.conf" @@ -100,8 +100,8 @@ var ( ownerExtraGroupsKey: {}, }, flowsSection: { - flowsDeviceAuthKey: {}, - flowsEntraPasswordKey: {}, + flowsDeviceAuthKey: {}, + flowsEntraAuthKey: {}, }, } ) @@ -147,15 +147,15 @@ type userConfig struct { // flowsConfig holds the parsed [flows] section configuration. type flowsConfig struct { - DeviceAuth bool - EntraPassword bool + DeviceAuth bool + EntraAuth bool } // defaultFlowsConfig returns the default flows configuration (all modes enabled). func defaultFlowsConfig() flowsConfig { return flowsConfig{ - DeviceAuth: true, - EntraPassword: true, + DeviceAuth: true, + EntraAuth: true, } } @@ -498,18 +498,18 @@ func parseFlowsConfig(section *ini.Section) (flowsConfig, error) { } } - if section.HasKey(flowsEntraPasswordKey) { - val, err := section.Key(flowsEntraPasswordKey).Bool() + if section.HasKey(flowsEntraAuthKey) { + val, err := section.Key(flowsEntraAuthKey).Bool() if err != nil { - log.Warningf(context.Background(), "invalid value for %q in [%s] section, using default (true)", flowsEntraPasswordKey, flowsSection) + log.Warningf(context.Background(), "invalid value for %q in [%s] section, using default (true)", flowsEntraAuthKey, flowsSection) } else { - fc.EntraPassword = val + fc.EntraAuth = val } } - if !fc.DeviceAuth && !fc.EntraPassword { + if !fc.DeviceAuth && !fc.EntraAuth { return flowsConfig{}, fmt.Errorf("invalid [%s] configuration: all authentication flows are disabled; at least one of the %q or %q flows must be enabled", - flowsSection, flowsDeviceAuthKey, flowsEntraPasswordKey) + flowsSection, flowsDeviceAuthKey, flowsEntraAuthKey) } return fc, nil diff --git a/authd-oidc-brokers/internal/broker/config_test.go b/authd-oidc-brokers/internal/broker/config_test.go index 31fea25ae6..e65964ea9f 100644 --- a/authd-oidc-brokers/internal/broker/config_test.go +++ b/authd-oidc-brokers/internal/broker/config_test.go @@ -69,7 +69,7 @@ client_id = client_id [flows] device_code = false -entra_password = false +entra_auth = false `, "valid+one_flow_disabled": ` @@ -79,7 +79,7 @@ client_id = client_id [flows] device_code = false -entra_password = true +entra_auth = true `, "invalid_device_code_value": ` @@ -91,13 +91,13 @@ client_id = client_id device_code = not-a-bool `, - "invalid_entra_password_value": ` + "invalid_entra_auth_value": ` [oidc] issuer = https://issuer.url.com client_id = client_id [flows] -entra_password = not-a-bool +entra_auth = not-a-bool `, "invalid_register_device_value": ` @@ -128,9 +128,9 @@ client_id = lower_precedence_client_id issuer = https://higher-precedence-issuer.url.com `, - "overwrite_enable_entra_password": ` + "overwrite_enable_entra_auth": ` [flows] -entra_password = true +entra_auth = true `, } @@ -146,13 +146,13 @@ func TestParseConfig(t *testing.T) { wantErr bool wantErrContainsDropInConfigPath bool }{ - "Successfully_parse_config_file": {}, - "Successfully_parse_config_file_with_optional_values": {configType: "valid+optional"}, - "Successfully_parse_config_file_with_register_device": {configType: "valid+register_device"}, - "Successfully_parse_config_file_with_flow_values": {configType: "valid+one_flow_disabled"}, - "Warns_and_uses_default_for_invalid_device_code_value": {configType: "invalid_device_code_value"}, - "Warns_and_uses_default_for_invalid_entra_password_flow_value": {configType: "invalid_entra_password_value"}, - "Successfully_parse_config_with_drop_in_files": {dropInType: "valid"}, + "Successfully_parse_config_file": {}, + "Successfully_parse_config_file_with_optional_values": {configType: "valid+optional"}, + "Successfully_parse_config_file_with_register_device": {configType: "valid+register_device"}, + "Successfully_parse_config_file_with_flow_values": {configType: "valid+one_flow_disabled"}, + "Warns_and_uses_default_for_invalid_device_code_value": {configType: "invalid_device_code_value"}, + "Warns_and_uses_default_for_invalid_entra_auth_flow_value": {configType: "invalid_entra_auth_value"}, + "Successfully_parse_config_with_drop_in_files": {dropInType: "valid"}, "Successfully_parse_config_with_flow_drop_in_files": { configType: "valid+flows_disabled", dropInType: "flows", @@ -216,7 +216,7 @@ func TestParseConfig(t *testing.T) { err = os.WriteFile(confPath, []byte(configTypes["valid+optional"]), 0600) require.NoError(t, err, "Setup: Failed to write config file") case "flows": - err = os.WriteFile(filepath.Join(dropInDir, "00-drop-in.conf"), []byte(configTypes["overwrite_enable_entra_password"]), 0600) + err = os.WriteFile(filepath.Join(dropInDir, "00-drop-in.conf"), []byte(configTypes["overwrite_enable_entra_auth"]), 0600) require.NoError(t, err, "Setup: Failed to write drop-in file") case "unreadable-dir": err = os.Chmod(dropInDir, 0000) diff --git a/authd-oidc-brokers/internal/broker/export_test.go b/authd-oidc-brokers/internal/broker/export_test.go index 008407cd8b..403fdff0e6 100644 --- a/authd-oidc-brokers/internal/broker/export_test.go +++ b/authd-oidc-brokers/internal/broker/export_test.go @@ -2,6 +2,7 @@ package broker import ( "sync" + "time" "github.com/canonical/authd/authd-oidc-brokers/internal/providers/msentraid/himmelblau" ) @@ -12,6 +13,15 @@ var ( IsPromptMethod = isPromptMethod ) +// SetFIDODeviceWaitTimeout overrides how long entraAuthFidoAuth waits for a +// security key before falling back to the device code flow, so tests need not +// wait the production timeout. It returns a func that restores the default. +func SetFIDODeviceWaitTimeout(d time.Duration) (restore func()) { + prev := fidoDeviceWaitTimeout + fidoDeviceWaitTimeout = d + return func() { fidoDeviceWaitTimeout = prev } +} + func (cfg *Config) Init() { cfg.ownerMutex = &sync.RWMutex{} cfg.flows = defaultFlowsConfig() @@ -82,10 +92,10 @@ func (cfg *Config) SetAllowedSSHSuffixes(allowedSSHSuffixes []string) { cfg.allowedSSHSuffixes = allowedSSHSuffixes } -func (cfg *Config) SetFlows(deviceAuth, entraPassword bool) { +func (cfg *Config) SetFlows(deviceAuth, entraAuth bool) { cfg.flows = defaultFlowsConfig() cfg.flows.DeviceAuth = deviceAuth - cfg.flows.EntraPassword = entraPassword + cfg.flows.EntraAuth = entraAuth } func (cfg *Config) SetProvider(provider provider) { @@ -246,7 +256,7 @@ const MaxAuthAttempts = maxAuthAttempts const CachedPasswordMessage = cachedPasswordMessage // SetSessionMFAFlowActive lets tests set mfaFlowActive on a session without -// going through entraPasswordAuth. The challenge info is left nil so that +// going through entraAuth. The challenge info is left nil so that // tests can exercise the "flow active but no challenge metadata" guard in // entraMFAWaitAuth. func (b *Broker) SetSessionMFAFlowActive(sessionID string, flow *himmelblau.MFAFlowState) error { diff --git a/authd-oidc-brokers/internal/broker/fido_msentraid.go b/authd-oidc-brokers/internal/broker/fido_msentraid.go new file mode 100644 index 0000000000..379f586efb --- /dev/null +++ b/authd-oidc-brokers/internal/broker/fido_msentraid.go @@ -0,0 +1,11 @@ +//go:build withmsentraid + +package broker + +import "github.com/canonical/authd/authd-oidc-brokers/internal/fido" + +// defaultFIDOAuthenticator returns the libfido2-backed authenticator used to +// perform WebAuthn assertions with a locally connected security key. +func defaultFIDOAuthenticator() fidoAuthenticator { + return fido.Authenticator{} +} diff --git a/authd-oidc-brokers/internal/broker/fido_stub.go b/authd-oidc-brokers/internal/broker/fido_stub.go new file mode 100644 index 0000000000..4c72b3f3f4 --- /dev/null +++ b/authd-oidc-brokers/internal/broker/fido_stub.go @@ -0,0 +1,10 @@ +//go:build !withmsentraid + +package broker + +// defaultFIDOAuthenticator returns nil in builds without msentraid support: +// libfido2 is only linked behind the withmsentraid tag, and a nil +// authenticator disables the FIDO auth modes. +func defaultFIDOAuthenticator() fidoAuthenticator { + return nil +} diff --git a/authd-oidc-brokers/internal/broker/helper_test.go b/authd-oidc-brokers/internal/broker/helper_test.go index 83c77af034..8aa619bbec 100644 --- a/authd-oidc-brokers/internal/broker/helper_test.go +++ b/authd-oidc-brokers/internal/broker/helper_test.go @@ -30,7 +30,7 @@ type brokerForTestConfig struct { forceAccessCheckWithProvider bool registerDevice bool deviceAuthFlowDisabled bool - entraPasswordFlowDisabled bool + entraAuthFlowDisabled bool allowedUsers map[string]struct{} allUsersAllowed bool ownerAllowed bool @@ -41,6 +41,7 @@ type brokerForTestConfig struct { homeBaseDir string allowedSSHSuffixes []string provider providers.Provider + fidoAuthenticator broker.FIDOAuthenticator apiVersion uint getGroupsFails bool @@ -98,8 +99,8 @@ func newBrokerForTests(t *testing.T, cfg *brokerForTestConfig) (b *broker.Broker if cfg.registerDevice { cfg.SetRegisterDevice(cfg.registerDevice) } - if cfg.deviceAuthFlowDisabled || cfg.entraPasswordFlowDisabled { - cfg.SetFlows(!cfg.deviceAuthFlowDisabled, !cfg.entraPasswordFlowDisabled) + if cfg.deviceAuthFlowDisabled || cfg.entraAuthFlowDisabled { + cfg.SetFlows(!cfg.deviceAuthFlowDisabled, !cfg.entraAuthFlowDisabled) } if cfg.homeBaseDir != "" { cfg.SetHomeBaseDir(cfg.homeBaseDir) @@ -147,11 +148,11 @@ func newBrokerForTests(t *testing.T, cfg *brokerForTestConfig) (b *broker.Broker if cfg.ClientID() == "" { cfg.SetClientID("test-client-id") } - if !cfg.entraPasswordFlowDisabled && cfg.clientSecret == "" && !cfg.registerDevice { - if _, ok := providers.ProviderAs[himmelblau.EntraPasswordProvider](provider); ok { - // Most Entra password broker tests are not exercising startup validation; + if !cfg.entraAuthFlowDisabled && cfg.clientSecret == "" && !cfg.registerDevice { + if _, ok := providers.ProviderAs[himmelblau.EntraAuthProvider](provider); ok { + // Most Entra auth broker tests are not exercising startup validation; // give them a minimal Graph group source so they keep building a valid - // broker after New() started rejecting unusable entra_password configs. + // broker after New() started rejecting unusable entra_auth configs. cfg.SetClientSecret("test-client-secret") } } @@ -175,7 +176,17 @@ func newBrokerForTests(t *testing.T, cfg *brokerForTestConfig) (b *broker.Broker apiVersion = cfg.apiVersion } - b, err := broker.New(cfg.Config, apiVersion, broker.WithCustomProvider(provider)) + opts := []broker.Option{broker.WithCustomProvider(provider)} + // Override the FIDO authenticator when a mock is provided: the real one + // enumerates USB devices, which would make tests depend on the hardware + // plugged into the machine running them. When no mock is provided, leave + // the default (nil in non-withmsentraid builds) so fidoAvailable() + // reports false. + if cfg.fidoAuthenticator != nil { + opts = append(opts, broker.WithCustomFIDOAuthenticator(cfg.fidoAuthenticator)) + } + + b, err := broker.New(cfg.Config, apiVersion, opts...) require.NoError(t, err, "Setup: New should not have returned an error") return b } @@ -248,19 +259,19 @@ type tokenOptions struct { gecos string groups []info.Group - expired bool - noRefreshToken bool - refreshTokenExpired bool - refreshTokenInactiveExpired bool - refreshTokenStale bool - noIDToken bool - invalid bool - invalidClaims bool - noUserInfo bool - isForDeviceRegistration bool - deviceIsDisabled bool - userIsDisabled bool - obtainedViaEntraPasswordAuth bool + expired bool + noRefreshToken bool + refreshTokenExpired bool + refreshTokenInactiveExpired bool + refreshTokenStale bool + noIDToken bool + invalid bool + invalidClaims bool + noUserInfo bool + isForDeviceRegistration bool + deviceIsDisabled bool + userIsDisabled bool + obtainedViaEntraAuth bool } func generateCachedInfo(t *testing.T, options tokenOptions) *token.AuthCachedInfo { @@ -296,9 +307,9 @@ func generateCachedInfo(t *testing.T, options tokenOptions) *token.AuthCachedInf RefreshToken: "refreshtoken", Expiry: time.Now().Add(1000 * time.Hour), }, - DeviceIsDisabled: options.deviceIsDisabled, - UserIsDisabled: options.userIsDisabled, - ObtainedViaEntraPasswordAuth: options.obtainedViaEntraPasswordAuth, + DeviceIsDisabled: options.deviceIsDisabled, + UserIsDisabled: options.userIsDisabled, + ObtainedViaEntraAuth: options.obtainedViaEntraAuth, } if options.expired { diff --git a/authd-oidc-brokers/internal/broker/options_test.go b/authd-oidc-brokers/internal/broker/options_test.go index e0cc0e8de2..09ffc6e69c 100644 --- a/authd-oidc-brokers/internal/broker/options_test.go +++ b/authd-oidc-brokers/internal/broker/options_test.go @@ -8,3 +8,15 @@ func WithCustomProvider(p providers.Provider) Option { o.provider = p } } + +// FIDOAuthenticator re-exports the unexported fidoAuthenticator interface so +// that external test packages can provide mocks. +type FIDOAuthenticator = fidoAuthenticator + +// WithCustomFIDOAuthenticator returns an option that sets a custom FIDO +// authenticator for the broker. +func WithCustomFIDOAuthenticator(a FIDOAuthenticator) Option { + return func(o *option) { + o.fido = a + } +} diff --git a/authd-oidc-brokers/internal/broker/testdata/golden/TestParseConfig/Warns_and_uses_default_for_invalid_entra_password_flow_value/config.txt b/authd-oidc-brokers/internal/broker/testdata/golden/TestParseConfig/Warns_and_uses_default_for_invalid_entra_auth_flow_value/config.txt similarity index 100% rename from authd-oidc-brokers/internal/broker/testdata/golden/TestParseConfig/Warns_and_uses_default_for_invalid_entra_password_flow_value/config.txt rename to authd-oidc-brokers/internal/broker/testdata/golden/TestParseConfig/Warns_and_uses_default_for_invalid_entra_auth_flow_value/config.txt diff --git a/authd-oidc-brokers/internal/dbusservice/methods_test.go b/authd-oidc-brokers/internal/dbusservice/methods_test.go index cd98f19e54..c27c70ba46 100644 --- a/authd-oidc-brokers/internal/dbusservice/methods_test.go +++ b/authd-oidc-brokers/internal/dbusservice/methods_test.go @@ -26,7 +26,7 @@ issuer = `+defaultIssuerURL+` client_id = test-client-id [flows] -entra_password = false +entra_auth = false `), 0600), "Setup: writing broker config should not fail") cfg := broker.Config{ConfigFile: confPath, DataDir: t.TempDir()} diff --git a/authd-oidc-brokers/internal/fido/assertion.go b/authd-oidc-brokers/internal/fido/assertion.go new file mode 100644 index 0000000000..855bbc339d --- /dev/null +++ b/authd-oidc-brokers/internal/fido/assertion.go @@ -0,0 +1,186 @@ +// Package fido performs WebAuthn assertions against locally connected FIDO2 +// security keys, producing the assertion format that Entra ID's MFA flow (via +// libhimmelblau) expects back as auth_data. +// +// The wire format is load-bearing: libhimmelblau posts buildAssertionJSON's +// output verbatim to Entra's ests-fido endpoint, so it is defined by that live +// server, not the W3C WebAuthn spec. ests-fido is Microsoft's own token-service +// endpoint, so its payload differs from the browser-to-relying-party +// PublicKeyCredential JSON: the assertion is a FLAT object (not the nested §6.3 +// shape) and the challenge is base64url-ENCODED into clientDataJSON (the C +// accessor returns it unencoded). These are not workarounds for a bug here — +// himmelblau-idm's own PAM builds the byte-identical shape for the same +// endpoint (himmelblau_unix_common::auth::fido_auth), which is the reference +// this was matched against. "Correcting" either detail toward the W3C spec +// breaks login, so verify against a real Entra login before changing them. +package fido + +import ( + "encoding/base64" + "encoding/binary" + "encoding/json" + "errors" + "fmt" +) + +// relyingPartyOrigin is the WebAuthn origin of Entra ID's FIDO login page. +// Assertions signed for any other origin are rejected. +const relyingPartyOrigin = "https://login.microsoft.com" + +// Sentinel errors returned by Authenticator implementations so the broker can +// route the failure (collect a PIN, let the user retry, or deny) without +// depending on libfido2 specifics. +var ( + // ErrNoDevice means no FIDO2 device is connected. + ErrNoDevice = errors.New("no FIDO2 device found") + // ErrNoCredentials means the connected device has no credential accepted + // by the MFA challenge. + ErrNoCredentials = errors.New("security key has no matching credential") + // ErrPINRequired means the device requires a client PIN for this + // assertion and none was provided. + ErrPINRequired = errors.New("security key requires a PIN") + // ErrPINInvalid means the provided PIN was wrong; the user can retry + // with a new PIN (devices hard-block after repeated failures). + ErrPINInvalid = errors.New("security key PIN is incorrect") + // ErrPINBlocked means the device refuses PIN authentication until it is + // reinserted or reset; the assertion cannot proceed. + ErrPINBlocked = errors.New("security key PIN is blocked") + // ErrTimeout means the user did not touch the device in time. + ErrTimeout = errors.New("security key was not touched in time") + // ErrCanceled means the assertion was canceled (e.g. the session ended). + ErrCanceled = errors.New("FIDO assertion canceled") +) + +// clientDataJSON builds the WebAuthn client data for a Get ceremony. The raw +// challenge is base64url-encoded (load-bearing; see the package doc). +func clientDataJSON(challenge string) []byte { + data := struct { + Type string `json:"type"` + Challenge string `json:"challenge"` + Origin string `json:"origin"` + }{ + Type: "webauthn.get", + Challenge: base64.RawURLEncoding.EncodeToString([]byte(challenge)), + Origin: relyingPartyOrigin, + } + // Marshaling a struct of strings cannot fail. + out, _ := json.Marshal(data) + return out +} + +// buildAssertionJSON assembles the auth_data for a FidoKey method as a FLAT +// object (load-bearing; see the package doc). Fields are base64url-encoded +// without padding; an absent user handle serializes as an empty string. +func buildAssertionJSON(credentialID, clientData, authData, signature, userHandle []byte) (string, error) { + b64url := base64.RawURLEncoding.EncodeToString + response := struct { + ID string `json:"id"` + ClientDataJSON string `json:"clientDataJSON"` + AuthenticatorData string `json:"authenticatorData"` + Signature string `json:"signature"` + UserHandle string `json:"userHandle"` + }{ + ID: b64url(credentialID), + AuthenticatorData: b64url(authData), + ClientDataJSON: b64url(clientData), + Signature: b64url(signature), + UserHandle: b64url(userHandle), + } + out, err := json.Marshal(response) + if err != nil { + return "", fmt.Errorf("failed to marshal assertion response: %v", err) + } + return string(out), nil +} + +// decodeAllowList decodes the credential IDs from the MFA flow's FIDO allow +// list. Entra ID sends bare standard-base64 credential IDs (which is what +// upstream himmelblau decodes), but some responses carry WebAuthn +// PublicKeyCredentialDescriptor JSON objects with a base64url id instead, so +// both forms are accepted. Undecodable entries are skipped; it is an error +// when a non-empty list yields no usable credential ID. +func decodeAllowList(entries []string) ([][]byte, error) { + var credentialIDs [][]byte + for _, entry := range entries { + id, err := decodeAllowListEntry(entry) + if err != nil { + continue + } + credentialIDs = append(credentialIDs, id) + } + if len(entries) > 0 && len(credentialIDs) == 0 { + return nil, fmt.Errorf("no credential ID in the FIDO allow list could be decoded") + } + return credentialIDs, nil +} + +func decodeAllowListEntry(entry string) ([]byte, error) { + var descriptor struct { + ID string `json:"id"` + } + if err := json.Unmarshal([]byte(entry), &descriptor); err == nil && descriptor.ID != "" { + entry = descriptor.ID + } + + for _, encoding := range []*base64.Encoding{ + base64.StdEncoding, + base64.RawStdEncoding, + base64.URLEncoding, + base64.RawURLEncoding, + } { + // Reject a decoded empty credential ID (which every encoding produces + // for an empty string without an error) rather than passing a + // zero-length credential ID to the authenticator. + if id, err := encoding.DecodeString(entry); err == nil && len(id) > 0 { + return id, nil + } + } + return nil, fmt.Errorf("credential ID is not valid base64: %q", entry) +} + +// rawAuthData unwraps the CBOR byte string that libfido2 returns as the +// authenticator data (fido_assert_authdata_ptr is CBOR-wrapped). Entra ID +// expects the raw authenticator data bytes in the assertion response. +func rawAuthData(cborAuthData []byte) ([]byte, error) { + if len(cborAuthData) == 0 { + return nil, fmt.Errorf("empty authenticator data") + } + + const majorTypeByteString = 2 + if cborAuthData[0]>>5 != majorTypeByteString { + return nil, fmt.Errorf("authenticator data is not a CBOR byte string (leading byte %#x)", cborAuthData[0]) + } + + var length, offset int + switch info := int(cborAuthData[0] & 0x1f); { + case info < 24: + length, offset = info, 1 + case info == 24: // one-byte length + offset = 2 + if len(cborAuthData) < offset { + return nil, fmt.Errorf("truncated CBOR length") + } + length = int(cborAuthData[1]) + case info == 25: // two-byte length + offset = 3 + if len(cborAuthData) < offset { + return nil, fmt.Errorf("truncated CBOR length") + } + length = int(binary.BigEndian.Uint16(cborAuthData[1:3])) + case info == 26: // four-byte length + offset = 5 + if len(cborAuthData) < offset { + return nil, fmt.Errorf("truncated CBOR length") + } + length = int(binary.BigEndian.Uint32(cborAuthData[1:5])) + default: + // 27 (eight-byte) exceeds any plausible authenticator data size and + // 28-31 (reserved/indefinite) are not produced by libfido2. + return nil, fmt.Errorf("unsupported CBOR length encoding %#x", cborAuthData[0]) + } + + if len(cborAuthData) < offset+length { + return nil, fmt.Errorf("truncated authenticator data: want %d bytes, have %d", length, len(cborAuthData)-offset) + } + return cborAuthData[offset : offset+length], nil +} diff --git a/authd-oidc-brokers/internal/fido/assertion_test.go b/authd-oidc-brokers/internal/fido/assertion_test.go new file mode 100644 index 0000000000..ba7f38b843 --- /dev/null +++ b/authd-oidc-brokers/internal/fido/assertion_test.go @@ -0,0 +1,194 @@ +package fido + +import ( + "encoding/base64" + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestClientDataJSON(t *testing.T) { + t.Parallel() + + got := clientDataJSON("test-challenge") + + var parsed struct { + Type string `json:"type"` + Challenge string `json:"challenge"` + Origin string `json:"origin"` + } + require.NoError(t, json.Unmarshal(got, &parsed), "clientDataJSON must be valid JSON") + require.Equal(t, "webauthn.get", parsed.Type) + require.Equal(t, "dGVzdC1jaGFsbGVuZ2U", parsed.Challenge, + "challenge must be base64url(raw challenge) without padding") + require.Equal(t, "https://login.microsoft.com", parsed.Origin) +} + +func TestBuildAssertionJSON(t *testing.T) { + t.Parallel() + + credentialID := []byte{0x01, 0x02, 0xfb, 0xff} + clientData := clientDataJSON("test-challenge") + authData := []byte{0xaa, 0xbb, 0xcc} + signature := []byte{0x30, 0x45, 0x02, 0x20} + userHandle := []byte("user-id") + + got, err := buildAssertionJSON(credentialID, clientData, authData, signature, userHandle) + require.NoError(t, err) + + var parsed struct { + ID string `json:"id"` + ClientDataJSON string `json:"clientDataJSON"` + AuthenticatorData string `json:"authenticatorData"` + Signature string `json:"signature"` + UserHandle string `json:"userHandle"` + } + require.NoError(t, json.Unmarshal([]byte(got), &parsed), "assertion must be valid JSON") + + b64url := base64.RawURLEncoding.EncodeToString + require.Equal(t, b64url(credentialID), parsed.ID) + require.Equal(t, b64url(clientData), parsed.ClientDataJSON) + require.Equal(t, b64url(authData), parsed.AuthenticatorData) + require.Equal(t, b64url(signature), parsed.Signature) + require.Equal(t, b64url(userHandle), parsed.UserHandle) +} + +func TestBuildAssertionJSONEmptyUserHandle(t *testing.T) { + t.Parallel() + + got, err := buildAssertionJSON([]byte{0x01}, []byte("{}"), []byte{0x02}, []byte{0x03}, nil) + require.NoError(t, err) + + var parsed map[string]string + require.NoError(t, json.Unmarshal([]byte(got), &parsed)) + require.Equal(t, "", parsed["userHandle"], + "an absent user handle must serialize as an empty string, like upstream himmelblau") +} + +func TestDecodeAllowList(t *testing.T) { + t.Parallel() + + credID := []byte{0xfb, 0xef, 0xff, 0x01, 0x02} + stdB64 := base64.StdEncoding.EncodeToString(credID) // contains + / = + rawURLB64 := base64.RawURLEncoding.EncodeToString(credID) // contains - _ + + tests := map[string]struct { + entries []string + want [][]byte + wantErr bool + }{ + "standard base64 entries, like upstream himmelblau": { + entries: []string{stdB64}, + want: [][]byte{credID}, + }, + "base64url entries": { + entries: []string{rawURLB64}, + want: [][]byte{credID}, + }, + "webauthn JSON descriptor entries": { + entries: []string{`{"type":"public-key","id":"` + rawURLB64 + `"}`}, + want: [][]byte{credID}, + }, + "undecodable entries are skipped": { + entries: []string{"!!!not-base64!!!", stdB64}, + want: [][]byte{credID}, + }, + "empty entries are skipped, not decoded as a zero-length credential ID": { + entries: []string{"", stdB64}, + want: [][]byte{credID}, + }, + "empty list": { + entries: nil, + want: nil, + }, + "error when nothing is decodable": { + entries: []string{"!!!not-base64!!!"}, + wantErr: true, + }, + "error when the only entry is empty": { + entries: []string{""}, + wantErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + got, err := decodeAllowList(tc.entries) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +func TestRawAuthData(t *testing.T) { + t.Parallel() + + payload := make([]byte, 37) // typical authenticator data length + for i := range payload { + payload[i] = byte(i) + } + + tests := map[string]struct { + input []byte + want []byte + wantErr bool + }{ + "short form length": { + input: append([]byte{0x40 | 5}, payload[:5]...), + want: payload[:5], + }, + "one-byte length (0x58)": { + input: append([]byte{0x58, 37}, payload...), + want: payload, + }, + "two-byte length (0x59)": { + input: append([]byte{0x59, 0x00, 37}, payload...), + want: payload, + }, + "four-byte length (0x5a)": { + input: append([]byte{0x5a, 0x00, 0x00, 0x00, 37}, payload...), + want: payload, + }, + "empty input": { + input: nil, + wantErr: true, + }, + "not a byte string (CBOR array)": { + input: []byte{0x81, 0x01}, + wantErr: true, + }, + "indefinite length is unsupported": { + input: []byte{0x5f, 0x41, 0x01, 0xff}, + wantErr: true, + }, + "truncated payload": { + input: []byte{0x58, 37, 0x01, 0x02}, + wantErr: true, + }, + "truncated length": { + input: []byte{0x59, 0x00}, + wantErr: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + got, err := rawAuthData(tc.input) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} diff --git a/authd-oidc-brokers/internal/fido/authenticator.go b/authd-oidc-brokers/internal/fido/authenticator.go new file mode 100644 index 0000000000..65d8a31840 --- /dev/null +++ b/authd-oidc-brokers/internal/fido/authenticator.go @@ -0,0 +1,228 @@ +//go:build withmsentraid + +package fido + +import ( + "context" + "crypto/sha256" + "errors" + "fmt" + + "github.com/canonical/authd/log" + libfido2 "github.com/keys-pub/go-libfido2" +) + +const ( + // fidoErrUVBlocked is CTAP2's FIDO_ERR_UV_BLOCKED (0x3c): the + // authenticator's built-in user verification (e.g. fingerprint) is + // temporarily blocked, most commonly after too many unmatched touches on a + // biometric device. This vendored go-libfido2 predates CTAP2.1 and has no + // named error for it, so it surfaces only as the generic + // libfido2.Error{Code: 0x3c}. + fidoErrUVBlocked = 0x3c + // fidoErrUVInvalid is CTAP2's FIDO_ERR_UV_INVALID (0x3f): built-in user + // verification failed. Prompting for the PIN lets the same device satisfy + // user verification without unplugging it. + fidoErrUVInvalid = 0x3f +) + +// relyingPartyID is the WebAuthn relying party ID that Entra ID registers +// credentials under. +const relyingPartyID = "login.microsoft.com" + +// Authenticator performs WebAuthn Get ceremonies with the first connected +// FIDO2 device via libfido2. The zero value is ready to use. +type Authenticator struct{} + +// DevicePresent reports whether at least one FIDO device is connected. It is +// used to gate the FIDO auth modes: a session that cannot reach a device +// (e.g. SSH into a headless server) must fall back to another flow. +func (Authenticator) DevicePresent() bool { + locations, err := libfido2.DeviceLocations() + if err != nil { + return false + } + return len(locations) > 0 +} + +// DeviceRequiresPIN reports whether the connected device needs a client PIN +// for user verification. Devices with built-in user verification (e.g. a +// fingerprint reader) or without a configured PIN do not. +func (Authenticator) DeviceRequiresPIN() (bool, error) { + device, err := firstDevice() + if err != nil { + return false, err + } + + info, err := device.Info() + if errors.Is(err, libfido2.ErrNotFIDO2) { + // U2F-only devices have no PIN concept. + return false, nil + } + if err != nil { + return false, fmt.Errorf("failed to query FIDO device info: %v", err) + } + + var pinSet, builtinUV bool + for _, option := range info.Options { + switch option.Name { + case "clientPin": + pinSet = option.Value == libfido2.True + case "uv": + builtinUV = option.Value == libfido2.True + } + } + requiresPIN := pinSet && !builtinUV + // A wrong decision here is the usual cause of a failed ceremony, so record + // what the device actually reported. + log.Debugf(context.Background(), "FIDO device capabilities: clientPin=%v uv=%v -> requiresPIN=%v", pinSet, builtinUV, requiresPIN) + return requiresPIN, nil +} + +// Assert performs the WebAuthn Get ceremony for the given MFA-flow challenge +// and allow list, blocking until the user touches the device, the device +// times out, or ctx is canceled. It returns the assertion JSON that +// libhimmelblau's acquire_token_by_mfa_flow expects as auth_data. +// +// pin may be empty when the device does not require one (see +// DeviceRequiresPIN). Failures that the broker can act on are reported as the +// package's sentinel errors (ErrPINRequired, ErrPINInvalid, ...). +func (Authenticator) Assert(ctx context.Context, challenge string, allowList []string, pin string) (string, error) { + device, err := firstDevice() + if err != nil { + return "", err + } + + credentialIDs, err := decodeAllowList(allowList) + if err != nil { + return "", err + } + + clientData := clientDataJSON(challenge) + clientDataHash := sha256.Sum256(clientData) + + opts := &libfido2.AssertionOpts{UP: libfido2.True} + if pin == "" { + // Ask for built-in user verification (e.g. fingerprint) when no PIN + // is used; with a PIN, user verification is provided by the PIN + // protocol and requesting UV as well fails on PIN-only devices. + if builtinUV, err := hasBuiltinUV(device); err == nil && builtinUV { + opts.UV = libfido2.True + } + } + + type assertResult struct { + assertion *libfido2.Assertion + err error + } + resultCh := make(chan assertResult, 1) + go func() { + assertion, err := device.Assertion(relyingPartyID, clientDataHash[:], credentialIDs, pin, opts) + resultCh <- assertResult{assertion, err} + }() + + var result assertResult + select { + case result = <-resultCh: + case <-ctx.Done(): + // Interrupt the ceremony so the device stops blinking, then wait for + // the in-flight call to return: the Device must not be garbage + // collected while libfido2 still uses it. + if err := device.Cancel(); err != nil { + result = <-resultCh + return "", errors.Join(ErrCanceled, fmt.Errorf("failed to cancel FIDO assertion: %v", err)) + } + <-resultCh + return "", ErrCanceled + } + if result.err != nil { + // The raw error carries the CTAP code that mapAssertionError collapses + // into a sentinel; log it with the verification we asked for. + log.Debugf(ctx, "FIDO assertion ceremony failed (pinProvided=%v, uvRequested=%v): %v", pin != "", opts.UV == libfido2.True, result.err) + return "", mapAssertionError(result.err) + } + + authData, err := rawAuthData(result.assertion.AuthDataCBOR) + if err != nil { + return "", err + } + + return buildAssertionJSON( + result.assertion.CredentialID, + clientData, + authData, + result.assertion.Sig, + result.assertion.User.ID, + ) +} + +// firstDevice returns the first connected FIDO device. Sessions with several +// connected devices are not supported: the ceremony runs on the first one. +func firstDevice() (*libfido2.Device, error) { + locations, err := libfido2.DeviceLocations() + if err != nil { + return nil, fmt.Errorf("failed to enumerate FIDO devices: %v", err) + } + if len(locations) == 0 { + return nil, ErrNoDevice + } + // The ceremony always uses locations[0]; flag when several are connected so + // a wrong-device pick is visible. + if len(locations) > 1 { + log.Debugf(context.Background(), "%d FIDO devices connected; using the first (%q)", len(locations), locations[0].Path) + } + device, err := libfido2.NewDevice(locations[0].Path) + if err != nil { + return nil, fmt.Errorf("failed to open FIDO device %q: %v", locations[0].Path, err) + } + return device, nil +} + +// hasBuiltinUV reports whether the device performs user verification on its +// own (e.g. a fingerprint reader). +func hasBuiltinUV(device *libfido2.Device) (bool, error) { + info, err := device.Info() + if err != nil { + return false, err + } + for _, option := range info.Options { + if option.Name == "uv" { + return option.Value == libfido2.True, nil + } + } + return false, nil +} + +// mapAssertionError translates libfido2 errors to the package's sentinel +// errors where the broker can act on them, keeping the original error text +// for the logs. +func mapAssertionError(err error) error { + switch { + case errors.Is(err, libfido2.ErrPinRequired): + return ErrPINRequired + case errors.Is(err, libfido2.ErrPinNotSet): + // Prompting for a PIN cannot help: the key has none configured. + return fmt.Errorf("the security key requires a PIN but none is configured on it: %v", err) + case errors.Is(err, libfido2.ErrPinInvalid): + return ErrPINInvalid + case errors.Is(err, libfido2.ErrPinAuthBlocked), errors.Is(err, libfido2.ErrPinPolicyViolation): + return ErrPINBlocked + case errors.Is(err, libfido2.ErrActionTimeout): + return ErrTimeout + case errors.Is(err, libfido2.ErrKeepaliveCancel): + return ErrCanceled + case errors.Is(err, libfido2.ErrNoCredentials): + return ErrNoCredentials + default: + var ferr libfido2.Error + if errors.As(err, &ferr) && (ferr.Code == fidoErrUVBlocked || ferr.Code == fidoErrUVInvalid) { + // The device supports both a PIN and built-in UV, so Assert + // requested fingerprint verification (see the pin == "" branch + // there); with fingerprint verification failed or blocked, the + // PIN is the next recovery path. CTAP2 accepts it as an equally + // valid verification method regardless of what blocked UV. + return ErrPINRequired + } + return fmt.Errorf("FIDO assertion failed: %v", err) + } +} diff --git a/authd-oidc-brokers/internal/fido/authenticator_test.go b/authd-oidc-brokers/internal/fido/authenticator_test.go new file mode 100644 index 0000000000..07fb17b22e --- /dev/null +++ b/authd-oidc-brokers/internal/fido/authenticator_test.go @@ -0,0 +1,39 @@ +//go:build withmsentraid + +package fido + +import ( + "testing" + + libfido2 "github.com/keys-pub/go-libfido2" + "github.com/stretchr/testify/require" +) + +func TestMapAssertionError(t *testing.T) { + t.Parallel() + + require.ErrorIs(t, mapAssertionError(libfido2.ErrPinRequired), ErrPINRequired) + require.ErrorIs(t, mapAssertionError(libfido2.ErrPinInvalid), ErrPINInvalid) + require.ErrorIs(t, mapAssertionError(libfido2.ErrPinAuthBlocked), ErrPINBlocked) + require.ErrorIs(t, mapAssertionError(libfido2.ErrPinPolicyViolation), ErrPINBlocked) + require.ErrorIs(t, mapAssertionError(libfido2.ErrActionTimeout), ErrTimeout) + require.ErrorIs(t, mapAssertionError(libfido2.ErrKeepaliveCancel), ErrCanceled) + + // A key without a configured PIN must NOT ask the user for a PIN. + require.NotErrorIs(t, mapAssertionError(libfido2.ErrPinNotSet), ErrPINRequired) + + require.ErrorIs(t, mapAssertionError(libfido2.ErrNoCredentials), ErrNoCredentials) + + // FIDO_ERR_UV_BLOCKED (0x3c) and FIDO_ERR_UV_INVALID (0x3f): a Bio-series + // key's fingerprint verification is blocked or failed. This vendored + // go-libfido2 has no named errors for these CTAP2.1 codes, so they must be + // recognized by code and routed to the PIN fallback rather than failing + // generically; unplugging the key is not required. + require.ErrorIs(t, mapAssertionError(libfido2.Error{Code: fidoErrUVBlocked}), ErrPINRequired) + require.ErrorIs(t, mapAssertionError(libfido2.Error{Code: fidoErrUVInvalid}), ErrPINRequired) + + // Unclassified errors stay generic but keep their message. + err := mapAssertionError(libfido2.ErrTX) + require.NotErrorIs(t, err, ErrPINRequired) + require.ErrorContains(t, err, "tx") +} diff --git a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/entrapwd.go b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/entrapwd.go index 687eed2b45..7d74b5a6bf 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/entrapwd.go +++ b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/entrapwd.go @@ -9,10 +9,10 @@ import ( "golang.org/x/oauth2" ) -// EntraPasswordProvider is an optional interface that providers can implement +// EntraAuthProvider is an optional interface that providers can implement // to support the Entra ID password + MFA authentication flow. -type EntraPasswordProvider interface { - // InitiateEntraPasswordAuth starts the Entra password + MFA flow. +type EntraAuthProvider interface { + // InitiateEntraAuth starts the Entra password/passwordless + MFA flow. // It submits credentials and returns an MFA challenge state. // clientID is the OIDC application client ID (on_behalf_of_client_id); // it is used to build the OIDC app inside the Rust layer so that the @@ -20,13 +20,16 @@ type EntraPasswordProvider interface { // When withDeviceScope is true, the MFA flow adds Intune enrollment // resources to the token request (needed for PRT-based token exchange). // When false, it uses only MS Graph scopes. - InitiateEntraPasswordAuth( + // authOpts toggles optional flow behaviors (e.g. AuthOptionFido to let + // Entra ID negotiate a FIDO/security-key challenge). + InitiateEntraAuth( ctx context.Context, clientID string, issuerURL string, username, password string, deviceRegistrationData []byte, withDeviceScope bool, + authOpts ...AuthOption, ) (*MFAFlowState, *MFAChallengeInfo, error) // AcquireTokenByMFAFlow completes the MFA challenge. @@ -45,7 +48,7 @@ type EntraPasswordProvider interface { deviceRegistrationData []byte, ) (*oauth2.Token, error) - // RefreshEntraPasswordToken refreshes a cached Entra password + MFA refresh + // RefreshEntraToken refreshes a cached Entra password/passwordless + MFA refresh // token to re-verify the account against Entra ID on a returning login, the // same way the device-auth flow's token refresh does. It is a plain OAuth2 // refresh as a public client (no client_secret) for basic scopes only — never @@ -56,7 +59,7 @@ type EntraPasswordProvider interface { // persisted). On an Entra rejection it returns an *oauth2.RetrieveError so the // broker can classify it with the same checks it uses for device-auth // (IsUserDisabledError → AADSTS50057, IsTokenExpiredError → AADSTS50173, etc.). - RefreshEntraPasswordToken( + RefreshEntraToken( ctx context.Context, issuerURL string, refreshToken string, @@ -111,12 +114,43 @@ func FreeMFAFlowState(flow *MFAFlowState) { flow.opaque = nil } +// AuthOption toggles optional behaviors of the MFA flow initiation, mirroring +// libhimmelblau's AuthOption without exposing its C enum values. +type AuthOption int + +const ( + // AuthOptionNoDAGFallback suppresses the silent Device Authorization Grant + // fallback in libhimmelblau. The broker surfaces MFA challenges through + // dedicated auth modes and never wants the DAG fallback, so this is always + // passed by InitiateMFAFlow. + AuthOptionNoDAGFallback AuthOption = iota + + // AuthOptionFido advertises that the caller can perform a FIDO/WebAuthn + // assertion. Without it, Entra ID may still select a FIDO method for the + // user, but libhimmelblau does not fetch the WebAuthn challenge, so the + // flow cannot be completed locally. + AuthOptionFido + + // AuthOptionPasswordless asks libhimmelblau to attempt passwordless factors + // (Authenticator number-matching, TAP, FIDO/security key, ...) as primary + // authentication. It is the intent switch, orthogonal to the password + // argument: InitiateMFAFlow sets it whenever no password is supplied. + AuthOptionPasswordless +) + // MFAChallengeInfo describes the MFA challenge that must be presented to the user. type MFAChallengeInfo struct { Message string Method string PollingIntervalMs int MaxPollAttempts int + + // FidoChallenge is the WebAuthn challenge to sign when a FIDO method was + // negotiated. Empty for non-FIDO challenges. + FidoChallenge string + // FidoAllowList contains the credential IDs (base64-encoded) that Entra ID + // accepts for the FIDO assertion. Empty for non-FIDO challenges. + FidoAllowList []string } // MFAErrorCategory classifies an MFA error so the broker can route @@ -139,6 +173,10 @@ const ( // re-enter the code without restarting the flow. See newMFAError for how // this is detected. MFAErrorRetryableCode + // MFAErrorPasswordRequired means a passwordless flow found no usable + // passwordless method for the account, so authentication needs the + // password flow instead. + MFAErrorPasswordRequired ) // MFAError represents an error from initiating or continuing an MFA flow. @@ -182,3 +220,10 @@ func (e *MFAError) IsMFARequired() bool { func (e *MFAError) IsMFARetryableCode() bool { return e.Category == MFAErrorRetryableCode } + +// IsMFAPasswordRequired returns true if the error indicates that a +// passwordless flow found no usable passwordless method for the account, so +// authentication needs the password flow instead. +func (e *MFAError) IsMFAPasswordRequired() bool { + return e.Category == MFAErrorPasswordRequired +} diff --git a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go index 1f946eee30..b2dcdb4664 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go +++ b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go @@ -338,48 +338,90 @@ func AcquireAccessTokenForGraphAPI( return accessToken, nil } -// InitiateMFAFlowWithPassword starts the password+MFA flow for a user. +// InitiateMFAFlow starts the password/passwordless + MFA flow for a user. // It submits the user's credentials to Entra ID and returns an MFAFlowState // that can be used to complete the MFA challenge. // When withDeviceScope is true, the MFA flow requests scopes required for device // enrollment. When false, it uses standard scopes without enrollment resources. -func InitiateMFAFlowWithPassword(ctx context.Context, clientID, tenantID string, data *DeviceRegistrationData, username, password string, withDeviceScope bool) (*MFAFlowState, *MFAChallengeInfo, error) { +// authOpts toggles optional flow behaviors (e.g. AuthOptionFido to let Entra ID +// negotiate a FIDO/security-key challenge). +// +// An empty password selects passwordless authentication: libhimmelblau then +// negotiates a passwordless method (Authenticator number-matching, TAP, +// security key, ...) from the user's credential type. Passwordless +// authentication cannot enroll a device (there is no password to derive the +// enrollment from), so callers must pass withDeviceScope=false in that case. +func InitiateMFAFlow(ctx context.Context, clientID, tenantID string, data *DeviceRegistrationData, username, password string, withDeviceScope bool, authOpts ...AuthOption) (*MFAFlowState, *MFAChallengeInfo, error) { + if password == "" && withDeviceScope { + return nil, nil, fmt.Errorf("passwordless authentication cannot be used for device enrollment") + } brokerClientApp, err := brokerClientAppFor(clientID, tenantID, data) if err != nil { return nil, nil, fmt.Errorf("failed to initialize broker client application: %v", err) } log.Debugf(ctx, "Initiating MFA flow for user %q (withDeviceScope=%v)", username, withDeviceScope) + // Always request NoDAGFallback: the broker surfaces MFA challenges through + // dedicated auth modes and never wants the silent DAG fallback. + opts := append([]AuthOption{AuthOptionNoDAGFallback}, authOpts...) + // An empty password means there is no secret to validate, so this is a + // passwordless login: ask libhimmelblau to attempt passwordless factors. + // The option is the intent switch; the NULL password alone does not select + // passwordless. + if password == "" { + opts = append(opts, AuthOptionPasswordless) + } var flow *MFAFlowState if withDeviceScope { - flow, err = initiateMFAFlowForEnrollment(brokerClientApp, username, password) + flow, err = initiateMFAFlowForEnrollment(brokerClientApp, username, password, opts) } else { - flow, err = initiateMFAFlow(brokerClientApp, username, password) + flow, err = initiateMFAFlow(brokerClientApp, username, password, opts) } if err != nil { return nil, nil, err } - msg, err := mfaFlowMessage(flow) + challengeInfo, err := mfaChallengeInfoFromFlow(flow) if err != nil { FreeMFAFlowState(flow) return nil, nil, err } + return flow, challengeInfo, nil +} + +// mfaChallengeInfoFromFlow reads the challenge metadata from the native flow +// state. On error the caller still owns the flow and must release it. +func mfaChallengeInfoFromFlow(flow *MFAFlowState) (*MFAChallengeInfo, error) { + msg, err := mfaFlowMessage(flow) + if err != nil { + return nil, err + } + method, err := mfaFlowMethod(flow) if err != nil { - FreeMFAFlowState(flow) - return nil, nil, err + return nil, err + } + + fidoChallenge, err := mfaFlowFidoChallenge(flow) + if err != nil { + return nil, err } - challengeInfo := &MFAChallengeInfo{ + fidoAllowList, err := mfaFlowFidoAllowList(flow) + if err != nil { + return nil, err + } + + return &MFAChallengeInfo{ Message: msg, Method: method, PollingIntervalMs: mfaFlowPollingInterval(flow), MaxPollAttempts: mfaFlowMaxPollAttempts(flow), - } - return flow, challengeInfo, nil + FidoChallenge: fidoChallenge, + FidoAllowList: fidoAllowList, + }, nil } // AcquireTokenByMFAFlow completes the MFA challenge (poll or code submission). diff --git a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go index 65a2ad8422..566790328c 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go +++ b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go @@ -55,6 +55,7 @@ var ( codeAuthorizationDenied = uint32(C.AUTHORIZATION_DENIED) codeMFAInvalidCode = uint32(C.MFA_INVALID_CODE) codeMFADAGFallbackDisab = uint32(C.MFA_DAG_FALLBACK_DISABLED) + codePasswordRequired = uint32(C.PASSWORD_REQUIRED) ) // mfaErrorCategory maps a libhimmelblau MSAL error code into an @@ -72,6 +73,8 @@ func mfaErrorCategory(code uint32) MFAErrorCategory { return MFAErrorRetryableCode case codeMFADAGFallbackDisab: return MFAErrorRequired + case codePasswordRequired: + return MFAErrorPasswordRequired } return MFAErrorOther } @@ -434,24 +437,58 @@ func refreshTokenFromUserToken(userToken *C.UserToken) (refreshToken string, err return C.GoString(cRefreshToken), nil } -func initiateMFAFlow(broker *brokerClientApplication, username, password string) (*MFAFlowState, error) { +// cgo maps the C typedef-enum `AuthOption` to Go's plain uint32 (not C.uint or +// C.AuthOption), so the initiate functions' parameter `const enum AuthOption *` +// becomes *uint32 in the generated binding. These constants convert the cgo +// enum values to the matching Go type; they are variables only so that tests +// can reference them without importing "C". +var ( + cAuthOptionNoDAGFallback = uint32(C.NoDAGFallback) + cAuthOptionFido = uint32(C.Fido) + cAuthOptionPasswordless = uint32(C.Passwordless) +) + +// cAuthOptions translates portable AuthOption values to the C AuthOption enum. +// Unknown options are ignored. +func cAuthOptions(authOpts []AuthOption) []uint32 { + var options []uint32 + for _, opt := range authOpts { + switch opt { + case AuthOptionNoDAGFallback: + options = append(options, cAuthOptionNoDAGFallback) + case AuthOptionFido: + options = append(options, cAuthOptionFido) + case AuthOptionPasswordless: + options = append(options, cAuthOptionPasswordless) + } + } + return options +} + +func initiateMFAFlow(broker *brokerClientApplication, username, password string, authOpts []AuthOption) (*MFAFlowState, error) { cUsername := C.CString(username) defer C.free(unsafe.Pointer(cUsername)) - cPassword := C.CString(password) - defer C.free(unsafe.Pointer(cPassword)) + // An empty password is passed as a NULL pointer: there is simply no secret + // to submit. Passwordless method negotiation is driven by + // AuthOptionPasswordless in authOpts (added by InitiateMFAFlow), not by the + // NULL password itself. + var cPassword *C.char + if password != "" { + cPassword = C.CString(password) + defer C.free(unsafe.Pointer(cPassword)) + } - // cgo maps the C typedef-enum `AuthOption` to Go's plain uint32 (not C.uint or - // C.AuthOption), so the function parameter `const enum AuthOption *` becomes - // *uint32 in the generated binding. uint32(C.NoDAGFallback) converts the cgo - // constant to the matching Go type. - options := [1]uint32{uint32(C.NoDAGFallback)} + options := cAuthOptions(authOpts) var flow *C.MFAAuthContinue msalErr := C.broker_initiate_acquire_token_by_mfa_flow( (*C.BrokerClientApplication)(unsafe.Pointer(broker)), cUsername, cPassword, - &options[0], + // Not &options[0]: cAuthOptions ignores unknown values, so the slice + // can be empty and indexing it would panic. SliceData returns nil for + // a nil slice, and the C API accepts NULL with length 0. + unsafe.SliceData(options), C.uintptr_t(len(options)), &flow, ) @@ -500,20 +537,21 @@ func newMFAError(msalErr *C.MSAL_ERROR) *MFAError { } } -func initiateMFAFlowForEnrollment(broker *brokerClientApplication, username, password string) (*MFAFlowState, error) { +func initiateMFAFlowForEnrollment(broker *brokerClientApplication, username, password string, authOpts []AuthOption) (*MFAFlowState, error) { cUsername := C.CString(username) defer C.free(unsafe.Pointer(cUsername)) cPassword := C.CString(password) defer C.free(unsafe.Pointer(cPassword)) - options := [1]uint32{uint32(C.NoDAGFallback)} + options := cAuthOptions(authOpts) var flow *C.MFAAuthContinue msalErr := C.broker_initiate_acquire_token_by_mfa_flow_for_device_enrollment( (*C.BrokerClientApplication)(unsafe.Pointer(broker)), cUsername, cPassword, - &options[0], + // See initiateMFAFlow: the slice can be empty, so use SliceData. + unsafe.SliceData(options), C.uintptr_t(len(options)), &flow, ) @@ -631,3 +669,59 @@ func mfaFlowMaxPollAttempts(flow *MFAFlowState) int { } return int(C.mfa_auth_continue_max_poll_attempts(c)) } + +// mfaFlowFidoChallenge returns the WebAuthn challenge negotiated for a FIDO +// method, or "" when the flow is not a FIDO flow (the C accessor reports the +// absence as a NULL string without an error). +func mfaFlowFidoChallenge(flow *MFAFlowState) (string, error) { + if flow == nil { + return "", fmt.Errorf("missing MFA flow state") + } + flow.mu.Lock() + defer flow.mu.Unlock() + c := cFlow(flow) + if c == nil { + return "", fmt.Errorf("missing MFA flow state") + } + var cChallenge *C.char + msalErr := C.mfa_auth_continue_fido_challenge(c, &cChallenge) + if msalErr != nil { + return "", fmt.Errorf("failed to get FIDO challenge: %v", msalErrorMsg(msalErr)) + } + if cChallenge == nil { + return "", nil + } + defer C.free(unsafe.Pointer(cChallenge)) + return C.GoString(cChallenge), nil +} + +// mfaFlowFidoAllowList returns the credential IDs Entra ID accepts for the +// FIDO assertion, or nil when the flow is not a FIDO flow. +func mfaFlowFidoAllowList(flow *MFAFlowState) ([]string, error) { + if flow == nil { + return nil, fmt.Errorf("missing MFA flow state") + } + flow.mu.Lock() + defer flow.mu.Unlock() + c := cFlow(flow) + if c == nil { + return nil, fmt.Errorf("missing MFA flow state") + } + var cList **C.char + var cCount C.int + msalErr := C.mfa_auth_continue_fido_allow_list(c, &cList, &cCount) + if msalErr != nil { + return nil, fmt.Errorf("failed to get FIDO allow list: %v", msalErrorMsg(msalErr)) + } + if cList == nil || cCount <= 0 { + return nil, nil + } + defer C.mfa_auth_continue_free_fido_allow_list(cList, cCount) + + entries := unsafe.Slice(cList, int(cCount)) + allowList := make([]string, 0, len(entries)) + for _, entry := range entries { + allowList = append(allowList, C.GoString(entry)) + } + return allowList, nil +} diff --git a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c_test.go b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c_test.go index eb5b3a132c..f78d0baf3f 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c_test.go +++ b/authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c_test.go @@ -40,6 +40,8 @@ func TestMFAErrorCategoryMapping(t *testing.T) { "MFA_INVALID_CODE must map to MFAErrorRetryableCode") require.Equal(t, MFAErrorRequired, mfaErrorCategory(codeMFADAGFallbackDisab), "MFA_DAG_FALLBACK_DISABLED must map to MFAErrorRequired") + require.Equal(t, MFAErrorPasswordRequired, mfaErrorCategory(codePasswordRequired), + "PASSWORD_REQUIRED must map to MFAErrorPasswordRequired") // The original bug hardcoded mfaRequiredCode=24, which is actually // AUTH_CODE_RECEIVED once the changepassword feature shifts the enum. That @@ -62,3 +64,24 @@ func TestMFAErrorCategoryMapping(t *testing.T) { require.Equal(t, uint32(27), codeMFAInvalidCode, "MFA_INVALID_CODE is expected to be 27") require.Equal(t, uint32(28), codeMFADAGFallbackDisab, "MFA_DAG_FALLBACK_DISABLED is expected to be 28") } + +// TestCAuthOptionsMapping pins the translation from portable AuthOption values +// to the C AuthOption enum. +func TestCAuthOptionsMapping(t *testing.T) { + t.Parallel() + + require.Empty(t, cAuthOptions(nil), + "no options must produce an empty slice") + require.Equal(t, []uint32{cAuthOptionNoDAGFallback}, + cAuthOptions([]AuthOption{AuthOptionNoDAGFallback}), + "AuthOptionNoDAGFallback must map to the C NoDAGFallback option") + require.Equal(t, []uint32{cAuthOptionNoDAGFallback, cAuthOptionFido}, + cAuthOptions([]AuthOption{AuthOptionNoDAGFallback, AuthOptionFido}), + "AuthOptionFido must map to the C Fido option") + require.Equal(t, []uint32{cAuthOptionPasswordless}, + cAuthOptions([]AuthOption{AuthOptionPasswordless}), + "AuthOptionPasswordless must map to the C Passwordless option") + require.Equal(t, []uint32{cAuthOptionNoDAGFallback, cAuthOptionFido}, + cAuthOptions([]AuthOption{AuthOptionNoDAGFallback, AuthOptionFido, AuthOption(-1)}), + "unknown options must be ignored") +} diff --git a/authd-oidc-brokers/internal/providers/msentraid/msentraid.go b/authd-oidc-brokers/internal/providers/msentraid/msentraid.go index 2c986dc6d9..c8b1bffb02 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/msentraid.go +++ b/authd-oidc-brokers/internal/providers/msentraid/msentraid.go @@ -191,7 +191,7 @@ func (p *Provider) UserInfoFromAccessToken(accessToken string) (info.User, error // 1. Client credentials (app-only): used when a [oidc] client_secret is // configured and the current token does not already carry the // GroupMember.Read.All scope. This is the path that makes the -// entra_password + MFA flow work *without* device registration: the +// entra_auth flow work *without* device registration: the // delegated token issued by the Microsoft Broker App during native MFA // cannot be exchanged for a Graph-scoped delegated token for our OIDC app // (the FOCI scope wall — see below), so we fall back to an @@ -250,8 +250,8 @@ func (p *Provider) GetGroups( // (needsAccessTokenForGraphAPI): those have a PRT that can be exchanged for a // Graph-scoped token (strategy 2), which preserves the user's delegated // session semantics. This keeps register_device=true logins — both - // device-code and entra_password — on the PRT path even when a client_secret - // is configured, so only entra_password-without-device-registration tokens + // device-code and entra_auth — on the PRT path even when a client_secret + // is configured, so only entra_auth-without-device-registration tokens // take the app-only path. if p.graphClientSecret != "" && !accessTokenHasGraphScope && !needsAccessTokenForGraphAPI { if parseErr != nil { @@ -740,7 +740,7 @@ func (p *Provider) NormalizeUsername(username string) string { // SupportedOnlineAuthModes returns the authentication modes supported by the // provider that require a connection to Entra ID. func (p *Provider) SupportedOnlineAuthModes() []string { - return []string{authmodes.EntraPassword, authmodes.Device, authmodes.DeviceQr} + return []string{authmodes.EntraAuth, authmodes.Device, authmodes.DeviceQr} } // unmarshalOptionalDeviceRegistrationData decodes JSON device-registration data @@ -756,14 +756,15 @@ func unmarshalOptionalDeviceRegistrationData(raw []byte) (*himmelblau.DeviceRegi return data, nil } -// InitiateEntraPasswordAuth starts the Entra password + MFA flow. -func (p *Provider) InitiateEntraPasswordAuth( +// InitiateEntraAuth starts the Entra password/passwordless + MFA flow. +func (p *Provider) InitiateEntraAuth( ctx context.Context, clientID string, issuerURL string, username, password string, deviceRegistrationData []byte, withDeviceScope bool, + authOpts ...himmelblau.AuthOption, ) (*himmelblau.MFAFlowState, *himmelblau.MFAChallengeInfo, error) { tid := tenantID(issuerURL) @@ -772,7 +773,7 @@ func (p *Provider) InitiateEntraPasswordAuth( return nil, nil, err } - return himmelblau.InitiateMFAFlowWithPassword(ctx, clientID, tid, data, username, password, withDeviceScope) + return himmelblau.InitiateMFAFlow(ctx, clientID, tid, data, username, password, withDeviceScope, authOpts...) } // AcquireTokenByMFAFlow completes the MFA challenge. @@ -796,18 +797,19 @@ func (p *Provider) AcquireTokenByMFAFlow( return himmelblau.AcquireTokenByMFAFlow(ctx, clientID, tid, data, username, flow, authData, pollAttempt) } -// RefreshEntraPasswordToken refreshes the cached Entra password + MFA refresh token -// as the Microsoft Broker app (a public client, no client_secret) for basic scopes -// only, to re-verify the account on a returning login. The Broker app is the client -// that issued the family refresh token during the MFA flow; the configured OIDC app -// cannot redeem it. Basic scopes (never Microsoft Graph) avoid the Broker-app↔Graph -// preauthorization wall (AADSTS65002), so this works for any register_device setting. -// A failure is returned as the underlying *oauth2.RetrieveError so the broker can -// classify it exactly like the device-auth refresh. -func (p *Provider) RefreshEntraPasswordToken(ctx context.Context, issuerURL, refreshToken string) (*oauth2.Token, error) { +// RefreshEntraToken refreshes the cached Entra auth-flow refresh +// token as the Microsoft Broker app (a public client, no client_secret) for basic +// scopes only, to re-verify the account on a returning login. The Broker app is the +// client that issued the family refresh token during the MFA flow; the configured +// OIDC app cannot redeem it. Basic scopes (never Microsoft Graph) avoid the +// Broker-app↔Graph preauthorization wall (AADSTS65002), so this works for any +// register_device setting. A failure is returned as the underlying +// *oauth2.RetrieveError so the broker can classify it exactly like the device-auth +// refresh. +func (p *Provider) RefreshEntraToken(ctx context.Context, issuerURL, refreshToken string) (*oauth2.Token, error) { tokenURL, err := clientCredentialsTokenURL(issuerURL) if err != nil { - return nil, fmt.Errorf("could not build token URL for Entra password refresh: %w", err) + return nil, fmt.Errorf("could not build token URL for Entra auth refresh: %w", err) } cfg := oauth2.Config{ @@ -882,7 +884,7 @@ func (p *Provider) VerifyUsername(requestedUsername, authenticatedUsername strin } // IsTokenForDeviceRegistration reports whether the cached token carries -// device-registration data. The entra_password MFA flow issues tokens under the +// device-registration data. The entra_auth flow issues tokens under the // Microsoft Broker App ID too, so the App ID alone cannot distinguish a // device-registration token; the presence of device-registration data can. func (p *Provider) IsTokenForDeviceRegistration(authInfo *token.AuthCachedInfo) bool { diff --git a/authd-oidc-brokers/internal/providers/msentraid/msentraid_test.go b/authd-oidc-brokers/internal/providers/msentraid/msentraid_test.go index 0818ffbf3a..0a9763cbcc 100644 --- a/authd-oidc-brokers/internal/providers/msentraid/msentraid_test.go +++ b/authd-oidc-brokers/internal/providers/msentraid/msentraid_test.go @@ -151,7 +151,7 @@ func TestUserInfoFromAccessToken(t *testing.T) { require.Equal(t, info.NewUser("test-user@email.com", "", "saved-user-id", "", "test-user", nil), got) } -func TestRefreshEntraPasswordToken(t *testing.T) { +func TestRefreshEntraToken(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -173,17 +173,17 @@ func TestRefreshEntraPasswordToken(t *testing.T) { mockServer, cleanup := startMockMSServer(t, &mockMSServerConfig{RefreshHandler: tc.refreshHandler}) t.Cleanup(cleanup) - got, err := msentraid.New().RefreshEntraPasswordToken( + got, err := msentraid.New().RefreshEntraToken( context.Background(), mockServer.URL+"/tenant-id/v2.0", "refreshtoken", ) if tc.wantErr { - require.Error(t, err, "RefreshEntraPasswordToken should fail") + require.Error(t, err, "RefreshEntraToken should fail") require.Contains(t, err.Error(), tc.wantErrSubstr, "unexpected error from refresh") return } - require.NoError(t, err, "RefreshEntraPasswordToken should succeed for an active user") + require.NoError(t, err, "RefreshEntraToken should succeed for an active user") require.NotEmpty(t, got.AccessToken, "expected a rotated token on success") require.Nil(t, got.Extra("preferred_username"), "refresh should not add redundant preferred_username extras") require.Nil(t, got.Extra("sub"), "refresh should not add redundant sub extras") diff --git a/authd-oidc-brokers/internal/providers/providers.go b/authd-oidc-brokers/internal/providers/providers.go index c4c903870a..c5ab8f3bb9 100644 --- a/authd-oidc-brokers/internal/providers/providers.go +++ b/authd-oidc-brokers/internal/providers/providers.go @@ -22,7 +22,7 @@ type Provider interface { // SupportedOnlineAuthModes returns the authentication modes that require a // working connection to the identity provider (in contrast to the local // password mode, which the broker prepends). These are not necessarily OIDC - // flows: entra_password issues OAuth 2.0 tokens without an OIDC id_token. + // flows: entra_auth issues OAuth 2.0 tokens without an OIDC id_token. SupportedOnlineAuthModes() []string VerifyUsername(requestedUsername, authenticatedUsername string) error } @@ -51,7 +51,7 @@ type DeviceRegisterer interface { // IsTokenForDeviceRegistration reports whether the cached token carries // device-registration data (i.e. the device was registered). This is the // authoritative signal: tokens issued by the Microsoft Broker App (e.g. the - // entra_password MFA flow) are not device-registration tokens unless a device + // entra_auth flow) are not device-registration tokens unless a device // was actually registered. IsTokenForDeviceRegistration(authInfo *token.AuthCachedInfo) bool MaybeRegisterDevice( diff --git a/authd-oidc-brokers/internal/token/token.go b/authd-oidc-brokers/internal/token/token.go index ea060d3fd5..95caf3afff 100644 --- a/authd-oidc-brokers/internal/token/token.go +++ b/authd-oidc-brokers/internal/token/token.go @@ -21,12 +21,12 @@ type AuthCachedInfo struct { DeviceRegistrationData []byte DeviceIsDisabled bool UserIsDisabled bool - // ObtainedViaEntraPasswordAuth is set when the token was obtained through the - // entra_password MFA flow. On a returning login it selects the refresh path: + // ObtainedViaEntraAuth is set when the token was obtained through the + // entra_auth flow. On a returning login it selects the refresh path: // these tokens are refreshed as the Microsoft Broker App (public client, no // client_secret) for the liveness/revocation check, rather than via the OIDC // app refresh used by device-auth tokens. - ObtainedViaEntraPasswordAuth bool + ObtainedViaEntraAuth bool } // NewAuthCachedInfo creates a new AuthCachedInfo. It sets the provided token and rawIDToken and the provider-specific diff --git a/e2e-tests/e2e-tests-msentraid.env.template b/e2e-tests/e2e-tests-msentraid.env.template index 5828ec039e..3312093cbb 100644 --- a/e2e-tests/e2e-tests-msentraid.env.template +++ b/e2e-tests/e2e-tests-msentraid.env.template @@ -9,7 +9,10 @@ BROKER=authd-msentraid -# Test user credentials +# Test user credentials (password + TOTP MFA). +# Also used by login_entra_passwordless.robot for passwordless TAP login; +# see resources/EntraTAP.py for the required tenant policy and Graph +# permissions. E2E_USER= E2E_PASSWORD= TOTP_SECRET= diff --git a/e2e-tests/resources/EntraTAP.py b/e2e-tests/resources/EntraTAP.py new file mode 100644 index 0000000000..349a1adc45 --- /dev/null +++ b/e2e-tests/resources/EntraTAP.py @@ -0,0 +1,273 @@ +"""Robot Framework library to manage Temporary Access Passes (TAP) for e2e tests. + +Mints a one-time TAP for a user via the Microsoft Graph API so a passwordless +Entra ID login can be tested without a dedicated account: the broker's +empty-password probe gets a code-entry MFA challenge, and the TAP code is +entered at that prompt. + +Requires ``UserAuthenticationMethod.ReadWrite.All`` (or the least-privilege +``UserAuthMethod-TAP.ReadWrite.All``) admin-consented as an Application +permission on the app registration, and the TAP method enabled for the test +user in the tenant's Authentication methods policy. +""" + +import json +import os +import time +import urllib.error +import urllib.parse +import urllib.request +from datetime import datetime, timedelta, timezone + +from robot.api.deco import keyword, library + + +@library +class EntraTAP: + """Manages Entra ID Temporary Access Passes for e2e tests.""" + + def _tenant_id_from_issuer(self, issuer_url: str) -> str: + """Extract the tenant UUID from an Entra issuer URL. + + Targets the Entra v2.0 issuer form + ``https://login.microsoftonline.com//v2.0`` (the value the broker + is configured with for these tests) and bare tenant GUIDs. Other issuer + hosts (e.g. ``sts.windows.net``) are not handled, which is deliberate: + the token endpoint below is a ``login.microsoftonline.com`` URL anyway. + """ + stripped = issuer_url.rstrip("/") + # Remove the scheme and split by "/" + path = stripped.split("://", 1)[-1] + segments = [s for s in path.split("/") if s] + # The tenant ID immediately follows the host segment. + for i, seg in enumerate(segments): + if "microsoftonline" in seg: + if i + 1 < len(segments): + return segments[i + 1] + # Fallback: return the whole string if it looks like a bare GUID. + if len(stripped) == 36 and stripped.count("-") == 4: + return stripped + raise ValueError( + f"Could not extract tenant ID from issuer URL: {issuer_url!r}" + ) + + def _acquire_token(self, tenant_id: str, client_id: str, client_secret: str) -> str: + """Acquire an app-only token for Microsoft Graph.""" + token_url = ( + f"https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token" + ) + payload = urllib.parse.urlencode( + { + "grant_type": "client_credentials", + "client_id": client_id, + "client_secret": client_secret, + "scope": "https://graph.microsoft.com/.default", + } + ).encode() + req = urllib.request.Request(token_url, data=payload, method="POST") + req.add_header("Content-Type", "application/x-www-form-urlencoded") + try: + with urllib.request.urlopen(req) as resp: + result = json.load(resp) + except urllib.error.HTTPError as exc: + body = exc.read().decode(errors="replace") + raise RuntimeError( + f"Token request failed ({exc.code}): {body}" + ) from exc + + if "error" in result: + raise RuntimeError( + f"Token error {result['error']!r}: {result.get('error_description')}" + ) + return result["access_token"] + + def _graph(self, token: str, method: str, path: str, body=None): + """Perform a Microsoft Graph v1.0 request. + + Returns the parsed JSON body on success, or ``None`` for 204 No Content. + Raises ``RuntimeError`` on HTTP errors. + """ + url = f"https://graph.microsoft.com/v1.0{path}" + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request(url, data=data, method=method) + req.add_header("Authorization", f"Bearer {token}") + if data is not None: + req.add_header("Content-Type", "application/json") + try: + with urllib.request.urlopen(req) as resp: + if resp.status == 204: + return None + return json.load(resp) + except urllib.error.HTTPError as exc: + body = exc.read().decode(errors="replace") + raise RuntimeError( + f"Graph {method} {path} failed ({exc.code}): {body}" + ) from exc + + @keyword + def create_tap_for_user( + self, + user_upn: str, + lifetime_in_minutes: int = 60, + is_usable_once: bool = True, + stale_after_minutes: int = 10, + ) -> tuple: + """Create a Temporary Access Pass for *user_upn* and return its passcode and id. + + Entra allows only one TAP per user. An existing one younger than + ``stale_after_minutes`` is left alone and raises instead of being + deleted, since it likely belongs to a passwordless test still running + in another concurrent release channel against the same account; wrap + this keyword in ``Wait Until Keyword Succeeds`` to retry until that + test's teardown frees it up. An existing TAP older than that is + treated as abandoned and removed before creating a new one. The + default of 10 minutes is deliberately above the two 120s prompt waits + already in the passwordless login flow itself (MFA code entry, new + password entry), so a legitimately still-running test isn't mistaken + for stale. + ``lifetime_in_minutes`` defaults to 60 to satisfy tenants that enforce + that as their policy minimum. Requires ``AUTHD_MSENTRAID_ISSUER_ID``, + ``AUTHD_MSENTRAID_CLIENT_ID``, and ``AUTHD_MSENTRAID_CLIENT_SECRET`` + to be set. + """ + issuer = os.environ["AUTHD_MSENTRAID_ISSUER_ID"] + client_id = os.environ["AUTHD_MSENTRAID_CLIENT_ID"] + client_secret = os.environ["AUTHD_MSENTRAID_CLIENT_SECRET"] + tenant_id = self._tenant_id_from_issuer(issuer) + + token = self._acquire_token(tenant_id, client_id, client_secret) + + tap_path = f"/users/{user_upn}/authentication/temporaryAccessPassMethods" + + # Remove a stale, abandoned TAP. A fresh one is left alone rather than + # deleted out from under a test that's still using it elsewhere. + existing = self._graph(token, "GET", tap_path) + for method in (existing or {}).get("value", []): + tap_id = method.get("id") + if not tap_id: + continue + if not self._older_than(method, stale_after_minutes): + raise RuntimeError( + f"An active TAP already exists for {user_upn!r}; it may " + "belong to a test running in another release channel." + ) + self._graph(token, "DELETE", f"{tap_path}/{tap_id}") + + result = self._graph( + token, + "POST", + tap_path, + { + "lifetimeInMinutes": int(lifetime_in_minutes), + "isUsableOnce": bool(is_usable_once), + }, + ) + tap = (result or {}).get("temporaryAccessPass") + tap_id = (result or {}).get("id") + if not tap or not tap_id: + raise RuntimeError( + f"TAP creation returned no passcode. Full response: {result!r}" + ) + + # A freshly minted TAP isn't always usable immediately; poll until + # Graph confirms it so the caller doesn't hand out a code that falls + # through to the Entra password prompt. If the poll itself fails, + # delete the TAP we just created rather than stranding it: the caller + # never gets tap_id back to clean it up later, and it would otherwise + # sit there for up to stale_after_minutes blocking any retry. + try: + self._wait_until_tap_usable(token, tap_path, tap_id) + except Exception: + try: + self._graph(token, "DELETE", f"{tap_path}/{tap_id}") + except Exception: + pass + raise + + return tap, tap_id + + def _wait_until_tap_usable( + self, token: str, tap_path: str, tap_id: str, timeout_s: int = 30 + ) -> None: + """Poll Graph until the TAP reports ``isUsable``, up to ``timeout_s`` seconds. + + Some tenants omit ``isUsable`` from the response; treated as usable + rather than blocked on, since this is a best-effort wait, not a gate. + """ + deadline = time.monotonic() + timeout_s + while True: + method = self._graph(token, "GET", f"{tap_path}/{tap_id}") + if method is None or "isUsable" not in method or method.get("isUsable"): + return + if time.monotonic() >= deadline: + return + time.sleep(2) + + @keyword + def delete_tap_for_user(self, user_upn: str, min_age_minutes: int = 0) -> None: + """Delete a Temporary Access Pass for *user_upn*, if one exists. + + No-op if the user has none. If ``min_age_minutes`` is set, a TAP + younger than that is left alone instead of deleted: the msentraid + e2e suite runs three release channels concurrently against the same + account, so a very fresh TAP may belong to a passwordless test that's + still running in another channel rather than being a stray one. Prefer + ``delete_tap_by_id`` for a test's own teardown, where the exact id is + known; this keyword is for clearing a stray TAP whose id isn't known. + Requires ``AUTHD_MSENTRAID_ISSUER_ID``, ``AUTHD_MSENTRAID_CLIENT_ID``, + and ``AUTHD_MSENTRAID_CLIENT_SECRET`` to be set. + """ + issuer = os.environ["AUTHD_MSENTRAID_ISSUER_ID"] + client_id = os.environ["AUTHD_MSENTRAID_CLIENT_ID"] + client_secret = os.environ["AUTHD_MSENTRAID_CLIENT_SECRET"] + tenant_id = self._tenant_id_from_issuer(issuer) + + token = self._acquire_token(tenant_id, client_id, client_secret) + + tap_path = f"/users/{user_upn}/authentication/temporaryAccessPassMethods" + existing = self._graph(token, "GET", tap_path) + for method in (existing or {}).get("value", []): + tap_id = method.get("id") + if not tap_id: + continue + if min_age_minutes and not self._older_than(method, min_age_minutes): + continue + self._graph(token, "DELETE", f"{tap_path}/{tap_id}") + + @keyword + def delete_tap_by_id(self, user_upn: str, tap_id: str) -> None: + """Delete a specific Temporary Access Pass of *user_upn* by its Graph id. + + Unlike ``delete_tap_for_user``, this doesn't guess by age: use it in a + test's own teardown with the id returned by ``create_tap_for_user``, + so cleanup can't accidentally remove a different, concurrently + running test's TAP. Requires ``AUTHD_MSENTRAID_ISSUER_ID``, + ``AUTHD_MSENTRAID_CLIENT_ID``, and ``AUTHD_MSENTRAID_CLIENT_SECRET`` + to be set. + """ + if not tap_id: + raise ValueError("tap_id must not be empty") + + issuer = os.environ["AUTHD_MSENTRAID_ISSUER_ID"] + client_id = os.environ["AUTHD_MSENTRAID_CLIENT_ID"] + client_secret = os.environ["AUTHD_MSENTRAID_CLIENT_SECRET"] + tenant_id = self._tenant_id_from_issuer(issuer) + + token = self._acquire_token(tenant_id, client_id, client_secret) + tap_path = f"/users/{user_upn}/authentication/temporaryAccessPassMethods" + self._graph(token, "DELETE", f"{tap_path}/{tap_id}") + + def _older_than(self, method: dict, min_age_minutes: int) -> bool: + """Return whether *method*'s ``createdDateTime`` is at least *min_age_minutes* old. + + Treated as old enough if the timestamp is missing or unparseable, + since that shouldn't get in the way of clearing an otherwise-stale TAP. + """ + created = method.get("createdDateTime") + if not created: + return True + try: + created_at = datetime.fromisoformat(created.replace("Z", "+00:00")) + except ValueError: + return True + return datetime.now(timezone.utc) - created_at >= timedelta(minutes=min_age_minutes) diff --git a/e2e-tests/resources/broker.resource b/e2e-tests/resources/broker.resource index c87796977c..f44a9b7f0f 100644 --- a/e2e-tests/resources/broker.resource +++ b/e2e-tests/resources/broker.resource @@ -10,6 +10,7 @@ Resource resources/utils.resource Resource resources/authd.resource Library ./Browser.py AS Browser Library ./TOTP.py AS TOTP +Library ./EntraTAP.py AS EntraTAP Library Hid.py AS Hid Library OperatingSystem Library String @@ -21,17 +22,17 @@ Enable Edge Broker SSH.Execute snap refresh ${BROKER_SNAP_NAME} --edge -Disable Entra Password Via Drop In - [Documentation] Writes a drop-in that sets entra_password = false. +Disable Entra Auth Via Drop In + [Documentation] Writes a drop-in that sets entra_auth = false. ... ... More robust than Change Broker Configuration for this key, ... because stable snapshots provisioned before the [flows] - ... section was added may not contain an entra_password line + ... section was added may not contain an entra_auth line ... at all, causing a silent sed no-op. # TODO(stable-release): Can be removed once the stable broker has been # updated to include the [flows] section. SSH.Execute mkdir -p ${BROKER_CFG_DIR} - SSH.Execute printf '[flows]\nentra_password = false\n' > ${BROKER_CFG_DIR}/99-disable-entra-password.conf + SSH.Execute printf '[flows]\nentra_auth = false\n' > ${BROKER_CFG_DIR}/99-disable-entra-auth.conf SSH.Execute snap restart ${BROKER_SNAP_NAME} @@ -72,7 +73,21 @@ Regenerate QR Code Match Text ${DEVICE_URL} 15 +Ensure No Stray Entra TAP + [Documentation] Best-effort: deletes a Temporary Access Pass left active on + ... %{E2E_USER} so Entra can't intercept this sign-in with a TAP challenge + ... instead of the expected password/MFA prompt. A stray TAP can linger from + ... an unrelated run; see resources/EntraTAP.py. Only clears TAPs older than + ... 10 minutes, matching create_tap_for_user's stale_after_minutes, since the + ... release channels run concurrently against the same account and a fresh + ... TAP may still be in legitimate use by the passwordless test in another + ... channel. No-op for brokers other than msentraid. + IF '%{BROKER}' != 'authd-msentraid' RETURN + Run Keyword And Warn On Failure EntraTAP.Delete TAP For User %{E2E_USER} min_age_minutes=10 + + Continue Log In With Remote User: Authenticate In External Browser + Ensure No Stray Entra TAP # Wait until the verification URL and login code are displayed Match Text ${DEVICE_URL} # Read the user code. @@ -443,8 +458,9 @@ Check That Device Was Registered ... msg=token.json has no DeviceRegistrationData; the device was not registered. -Log In With Remote User Through CLI: Entra Password +Log In With Remote User Through CLI: Entra Auth [Arguments] ${username} + Ensure No Stray Entra TAP Try machinectl login Prompt Hid.Type String ${username} Hid.Keys Combo Return @@ -467,3 +483,23 @@ Log In With Remote User Through CLI: Entra Password # Wait for the authenticated shell prompt. Match Text ${username}@ubuntu:~$ 120 + + +Log In With Remote User Through CLI: Entra Passwordless TAP + [Arguments] ${username} ${local_password} ${tap_code} + Try machinectl login Prompt + Hid.Type String ${username} + Hid.Keys Combo Return + + Match Text Select your provider: 15 + Match Text 2. ${PROVIDER_DISPLAY_NAME} + Hid.Type String 2 + + # Passwordless probe: entra_auth auto-selects and Entra returns a TAP + # code-entry challenge instead of a password prompt. + Match Text Enter your MFA code: 120 similarity=88 + Hid.Type String ${tap_code} + Hid.Keys Combo Return + + # No cached Entra password: chain into newpassword to set a local one. + Continue Log In With Remote User Through CLI: Define Local Password ${username} ${local_password} diff --git a/e2e-tests/tests/login_entra_password.robot b/e2e-tests/tests/login_entra_auth.robot similarity index 81% rename from e2e-tests/tests/login_entra_password.robot rename to e2e-tests/tests/login_entra_auth.robot index bc98104071..7824029764 100644 --- a/e2e-tests/tests/login_entra_password.robot +++ b/e2e-tests/tests/login_entra_auth.robot @@ -13,12 +13,12 @@ Test Teardown utils.Test Teardown *** Keywords *** Test Setup utils.Test Setup snapshot=%{BROKER}-installed - # Enable the Entra ID password flow and disable device auth so only the + # Enable the Entra auth flow and disable device auth so only the # new password+MFA mode is offered, avoiding a provider-selection menu. - # entra_password requires register_device=true (or a client_secret) to fetch + # entra_auth requires register_device=true (or a client_secret) to fetch # groups from Microsoft Graph on first login. Change Broker Configuration register_device true - Change Broker Configuration entra_password true + Change Broker Configuration entra_auth true Change Broker Configuration device_code false @@ -30,12 +30,12 @@ ${local_password} %{E2E_PASSWORD} *** Test Cases *** -Test login with CLI using Entra ID password and MFA +Test login with CLI using Entra auth and MFA [Documentation] Verify that a user can authenticate via the Entra ID direct-password ... + MFA flow through the CLI (machinectl login). ... ... With the device code flow disabled the broker auto-selects the single available - ... authentication mode (entra_password), so the user goes straight to the + ... authentication mode (entra_auth), so the user goes straight to the ... password prompt after choosing the provider. After successful MFA the ... Entra password is cached locally; the provisioning checks verify that ... the cached password works for sudo. @@ -45,7 +45,7 @@ Test login with CLI using Entra ID password and MFA # First login: Entra ID password + TOTP MFA. Open Terminal - Log In With Remote User Through CLI: Entra Password ${username} + Log In With Remote User Through CLI: Entra Auth ${username} # This shared provisioning check covers NSS, group membership, and the # cached local-password path via sudo. Check If User Was Added Properly ${username} diff --git a/e2e-tests/tests/login_entra_password_client_secret.robot b/e2e-tests/tests/login_entra_auth_client_secret.robot similarity index 87% rename from e2e-tests/tests/login_entra_password_client_secret.robot rename to e2e-tests/tests/login_entra_auth_client_secret.robot index efd203d5f1..7dea60038d 100644 --- a/e2e-tests/tests/login_entra_password_client_secret.robot +++ b/e2e-tests/tests/login_entra_auth_client_secret.robot @@ -16,13 +16,13 @@ Test Setup # Inject the OIDC client secret into broker.conf at runtime. The base # snapshot ships with the secret commented out (so public-client flows are # not broken by AADSTS700025); this test is the only one that needs it, - # because entra_password must stay available with register_device=false + # because entra_auth must stay available with register_device=false # by falling back to the app-only Graph token (client credentials). ${secret}= Get Environment Variable AUTHD_MSENTRAID_CLIENT_SECRET Should Not Be Empty ${secret} AUTHD_MSENTRAID_CLIENT_SECRET must be set to run this test Change Broker Configuration client_secret ${secret} Change Broker Configuration register_device false - Change Broker Configuration entra_password true + Change Broker Configuration entra_auth true Change Broker Configuration device_code false @@ -34,21 +34,21 @@ ${local_password} %{E2E_PASSWORD} *** Test Cases *** -Test login with CLI using Entra ID password and MFA with client secret +Test login with CLI using Entra auth and MFA with client secret [Documentation] Verify that the Entra ID direct-password + MFA flow works ... through the CLI when device registration is disabled and the broker is ... provisioned with a client secret. ... ... The client secret is injected into broker.conf at setup (not baked into ... the snapshot), so the base snapshot stays secret-free for public-client - ... flows. This covers the alternate configuration where entra_password stays + ... flows. This covers the alternate configuration where entra_auth stays ... available without register_device=true because Microsoft Graph access ... comes from the configured application secret instead. Log In Open Terminal - Log In With Remote User Through CLI: Entra Password ${username} + Log In With Remote User Through CLI: Entra Auth ${username} # This shared provisioning check covers NSS, group membership, and the # cached local-password path via sudo. Check If User Was Added Properly ${username} diff --git a/e2e-tests/tests/login_entra_passwordless.robot b/e2e-tests/tests/login_entra_passwordless.robot new file mode 100644 index 0000000000..f12521f62c --- /dev/null +++ b/e2e-tests/tests/login_entra_passwordless.robot @@ -0,0 +1,93 @@ +*** Settings *** +Resource resources/utils.resource +Resource resources/authd.resource +Resource resources/broker.resource + +Test Tags requires:msentraid + +Test Setup Test Setup +Test Teardown Test Teardown + + +*** Keywords *** +Test Setup + utils.Test Setup snapshot=%{BROKER}-installed + # Enable the Entra auth flow with device registration so group membership + # can be resolved from Microsoft Graph on first login. + # Disable the device code flow so only entra_auth is offered and the broker + # auto-selects it, bypassing the provider-selection menu. + Change Broker Configuration register_device true + Change Broker Configuration entra_auth true + Change Broker Configuration device_code false + +Test Teardown + # Best-effort: removes the TAP this test created, by exact id, so cleanup + # can't affect a different test's TAP if one is concurrently in flight in + # another release channel. tap_id may be unset if setup failed before + # creation ran. Retries a few times since this is a network call and a + # transient failure here would otherwise leave the TAP active for its + # full lifetime; warnings don't block the VM restore below. + ${tap_id} = Get Variable Value $tap_id ${None} + IF $tap_id is not None + Run Keyword And Warn On Failure Wait Until Keyword Succeeds 3x 2s + ... EntraTAP.Delete Tap By Id ${username} ${tap_id} + END + utils.Test Teardown + + +*** Variables *** +${username} %{E2E_USER} +# local_password is the password the user sets at the newpassword step. +# It becomes the credential for subsequent offline/local-password logins. +${local_password} qwer1234 + + +*** Test Cases *** +Test login with CLI using Entra passwordless auth and TAP + [Documentation] Verify that an Entra ID user can log in passwordlessly via + ... a Temporary Access Pass (TAP) through the CLI (machinectl login). + ... + ... Reuses ``E2E_USER`` instead of a dedicated passwordless account: a TAP + ... is minted just before the test and deleted again in teardown, so it + ... never lingers and gets picked up by the password-based Entra tests + ... that share the account. + ... + ... The broker's passwordless probe finds the TAP and returns a code-entry + ... MFA challenge instead of a password prompt. With no Entra password + ... submitted, the broker chains into the newpassword step to set a local + ... password for offline authentication. + ... + ... See ``resources/EntraTAP.py`` for the required tenant policy and Graph + ... permissions. + + # Mint a fresh TAP right before use; the teardown removes it again as a + # backstop. Retries if another release channel's passwordless test still + # holds the account's one allowed TAP, until that test's own teardown + # frees it up, or until create_tap_for_user's own stale_after_minutes + # (10 min) lets it reclaim an abandoned one; retry past that window so a + # merely slow sibling run doesn't cause a spurious failure here. + ${tap_code} ${tap_id} = Wait Until Keyword Succeeds 12x 60s + ... EntraTAP.Create TAP For User ${username} + + # Log in with the local desktop user to open a terminal. + Log In + + Open Terminal + Log In With Remote User Through CLI: Entra Passwordless TAP + ... ${username} ${local_password} ${tap_code} + + # Verify the user was provisioned correctly: NSS visibility, group + # membership, and that the cached local password works for sudo. + Check If User Was Added Properly ${username} + + # NSS may be briefly unavailable while authd commits the new user record. + Wait Until Keyword Succeeds 30s 3s Check Home Directory ${username} + + Log Out From Terminal Session + Close Focused Window + + # Verify that subsequent logins use the cached local password (offline path). + Open Terminal + Log In With Remote User Through CLI: Local Password ${username} ${local_password} + Log Out From su Session + Close Focused Window diff --git a/e2e-tests/tests/migration_authd_broker.robot b/e2e-tests/tests/migration_authd_broker.robot index 5231056c06..f52876fed4 100644 --- a/e2e-tests/tests/migration_authd_broker.robot +++ b/e2e-tests/tests/migration_authd_broker.robot @@ -36,10 +36,10 @@ Test login after upgrading authd and broker to edge channel Log Out From su Session Close Focused Window - # Disable entra_password before upgrading: the edge broker refuses to start + # Disable entra_auth before upgrading: the edge broker refuses to start # if this flow is enabled without register_device or a client_secret, which # are both not configured. - Disable Entra Password Via Drop In + Disable Entra Auth Via Drop In # Switch to the edge channel for the broker snap and the edge PPA for authd Enable Edge Repository For Authd diff --git a/e2e-tests/tests/migration_broker.robot b/e2e-tests/tests/migration_broker.robot index ff86ef9ba5..a5ab3bc9f3 100644 --- a/e2e-tests/tests/migration_broker.robot +++ b/e2e-tests/tests/migration_broker.robot @@ -36,10 +36,10 @@ Test login with broker on edge channel Log Out From su Session Close Focused Window - # Disable entra_password before upgrading: the edge broker refuses to start + # Disable entra_auth before upgrading: the edge broker refuses to start # if this flow is enabled without register_device or a client_secret, which # are both not configured. - Disable Entra Password Via Drop In + Disable Entra Auth Via Drop In # Switch to edge channel for the broker snap Enable Edge Broker diff --git a/e2e-tests/vm/provision-authd.sh b/e2e-tests/vm/provision-authd.sh index 0b3874a49a..1739a740e2 100755 --- a/e2e-tests/vm/provision-authd.sh +++ b/e2e-tests/vm/provision-authd.sh @@ -222,7 +222,7 @@ function install_broker() { -e "s||${issuer_id}|g" \ -e "s||${client_id}|g" \ -e "s||${client_secret}|g" \ - -e "s/^#entra_password = .*/entra_password = false/" \ + -e "s/^#entra_auth = .*/entra_auth = false/" \ /var/snap/${broker}/current/broker.conf echo 'verbosity: 2' > /var/snap/${broker}/current/${broker}.yaml systemctl restart authd.service diff --git a/snap/variants/msentraid/snapcraft.yaml b/snap/variants/msentraid/snapcraft.yaml index 37a30af314..10a8d23fb2 100644 --- a/snap/variants/msentraid/snapcraft.yaml +++ b/snap/variants/msentraid/snapcraft.yaml @@ -34,6 +34,12 @@ apps: - dbus-authd plugs: - network + # Access to FIDO2/U2F security keys for the local WebAuthn MFA step. + # Not auto-connected by default: run + # snap connect authd-msentraid:u2f-devices + # to enable it. Without the connection no key is reachable, so a FIDO + # login waits briefly for one and then falls back to the device code flow. + - u2f-devices restart-condition: never activates-on: [dbus-authd] environment: @@ -85,6 +91,12 @@ parts: - go build-packages: - retry + # Headers for the cgo bindings to libfido2 (internal/fido). + - libfido2-dev + stage-packages: + # Runtime library for the FIDO2 WebAuthn assertions; core24 does not + # provide it. + - libfido2-1 after: - libhimmelblau - version