Skip to content

e2e-tests: Add tests for the entra_password flow - #1633

Merged
nooreldeenmansour merged 5 commits into
002-entra-password-mfa-authfrom
e2e-tests-entra-password-mfa
Jul 1, 2026
Merged

e2e-tests: Add tests for the entra_password flow#1633
nooreldeenmansour merged 5 commits into
002-entra-password-mfa-authfrom
e2e-tests-entra-password-mfa

Conversation

@nooreldeenmansour

@nooreldeenmansour nooreldeenmansour commented Jun 26, 2026

Copy link
Copy Markdown
Member

Important

This is a draft PR based on #1520, it will eventually be merged into it

These are new end-to-end tests that verify the new entra_password flow. Two new tests are intended to be added:

  • A test that validates the enrollment path (register_device=true)

    • Local password caching by executing a sudo command)
  • A test that validates the non-enrollment path (register_device=false and client_secret configuration)

UDENG-10836

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.29%. Comparing base (469f980) to head (0c060a5).

Additional details and impacted files
@@                       Coverage Diff                       @@
##           002-entra-password-mfa-auth    #1633      +/-   ##
===============================================================
- Coverage                        87.54%   86.29%   -1.26%     
===============================================================
  Files                              124       25      -99     
  Lines                             8463     1612    -6851     
  Branches                           111        0     -111     
===============================================================
- Hits                              7409     1391    -6018     
+ Misses                             998      221     -777     
+ Partials                            56        0      -56     

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

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

@nooreldeenmansour nooreldeenmansour added the e2e-tests This issue is related to end-to-end tests / Run end-to-end tests on this pull request label Jun 26, 2026
@nooreldeenmansour
nooreldeenmansour force-pushed the e2e-tests-entra-password-mfa branch from dbb447d to ef3ab43 Compare June 26, 2026 14:08
@nooreldeenmansour

nooreldeenmansour commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Good news, the two new tests are passing.
Bad news, four tests failed on the three releases runs with this error from journalctl

Jun 26 14:54:47 ubuntu authd-msentraid[1714]: Failed to refresh token: oauth2: "invalid_client" "AADSTS700025: Client is public so neither 'client_assertion' nor 'client_secret' should be presented. Trace ID: 49050e15-78f3-4f02-9eb0-69385e690f00 Correlation ID: d3bb37fd-71af-4007-af45-60d4bd0c416f Timestamp: 2026-06-26 14:54:47Z"

So while this could point out to a broker/e2e-vm provisioning misconfiguration, it likely points out to a latent issue in #1520, I'll investigate this further and see if it could be reliably fixed

The failing tests are:

  • ssh login
  • password change
  • login gdm
  • authctl group set gid

Edit: Looking at libhimmelblau codebase, I see this line https://gitlab.com/samba-team/libhimmelblau/-/blob/248738549d4a26830950116f5747c7bc79c11088/src/aadsts_err_gen.rs#L188, likely the origin of the error.. but still investigating how to "prevent" this from happening

Edit: Tests are passing now, but the client_secret for Entra is currently scoped only to the test that needs it. Rather than a snapshot or provisioning.. Also, added a small commit to prevent the error from sending the client_secret for the Microsoft Broker App, which caused the earlier error. but further investigation is required into this. Will look into the test runs for this PR (because the commit only wasn't sufficient to fix this... and this error only happened in this PR, when client_secret was introduced in the e2e provisioning...) and update my comment with findings

Edit: The root cause was that cfg.clientSecret was flowing into OIDC token requests unconditionally, including for flows that authenticate via the Microsoft Broker App (always a public client). Baking it into the provisioning snapshot made every test hit AADSTS700025 on token refresh, latent in #1520, first observable here. The fix has two parts: the secret is no longer provisioned into the snapshot and is injected at test-setup time only for the test that needs it, and the broker now suppresses it for Broker App flows at initialization (the Graph API path uses a separate client-credentials endpoint and is unaffected).

@nooreldeenmansour
nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch from fb41ec0 to 552d04b Compare June 26, 2026 20:04
@nooreldeenmansour
nooreldeenmansour force-pushed the e2e-tests-entra-password-mfa branch from 375d3ff to 9bbf3ae Compare June 26, 2026 21:09
@nooreldeenmansour
nooreldeenmansour marked this pull request as ready for review June 29, 2026 05:01
@nooreldeenmansour
nooreldeenmansour force-pushed the e2e-tests-entra-password-mfa branch 5 times, most recently from b050f69 to 9b6d677 Compare June 29, 2026 07:26
Comment thread .github/workflows/e2e-tests-run.yaml Outdated
Comment thread e2e-tests/vm/config.sh.template Outdated
Comment thread e2e-tests/resources/TOTP.py Outdated
Comment thread e2e-tests/resources/broker.resource
Comment thread e2e-tests/resources/broker.resource Outdated
Comment thread e2e-tests/resources/broker.resource Outdated
Comment thread e2e-tests/resources/broker.resource Outdated

@adombeck adombeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work, just a few comments

@nooreldeenmansour
nooreldeenmansour force-pushed the 002-entra-password-mfa-auth branch from 552d04b to 469f980 Compare June 29, 2026 12:09
The new entra_password flow has no automated coverage. Add an end-to-end
test that exercises the register_device=true configuration: the broker
authenticates via the Microsoft Broker App, registers the device on first
login, and caches the password locally for subsequent offline use.

Disabling device_auth ensures the broker auto-selects the single available
mode, keeping the test focused on the password+MFA flow without an
interactive provider-selection step.
Cover the register_device=false configuration, where the broker uses a
configured client_secret to obtain an app-only Graph token for group
lookup instead of registering a device.

The secret is injected into broker.conf at test setup time rather than
baked into the provisioning snapshot, keeping the base image clean for
public-client flows. Without the secret the test fails immediately at
setup, surfacing a misconfigured CI run as a clear error instead of a
silent missing-auth-mode failure.
When client_secret is configured alongside register_device, refresh
token requests against the Microsoft Broker App fail with AADSTS700025
— Entra ID rejects secrets on public clients unconditionally. The Graph
API credential (cfg.clientSecret) goes to the client-credentials
endpoint, not the OIDC token endpoint, so it is separate and unaffected.
@adombeck
adombeck self-requested a review June 30, 2026 12:35

@adombeck adombeck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

`entra_password` is an Entra ID-specific broker option, but these tests
ran against every broker, including google, where the config keys
don't apply and the password+MFA UI never appears. CI runs against
the google broker were failing instead of skipping.
@nooreldeenmansour
nooreldeenmansour merged commit e1995bc into 002-entra-password-mfa-auth Jul 1, 2026
23 of 29 checks passed
@nooreldeenmansour
nooreldeenmansour deleted the e2e-tests-entra-password-mfa branch July 1, 2026 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants