Re-enable Native Auth SSPR tests with Mail.tm, Fixes AB#3729032 - #2553
Re-enable Native Auth SSPR tests with Mail.tm, Fixes AB#3729032#2553Disha Janardhan (djanardhan98) wants to merge 21 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a dedicated PR-branch test invocation with the Native Auth and Mail.tm inputs required by SSPRTest. Use a deterministic too-short password for the invalid-format case and update Common for Mail.tm HTTP/1.1 compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update Common to the latest mail-tm-native-auth-e2e commit and broaden the dedicated PR test filter to the complete Native Auth E2E package, including SSPR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Work item link check complete. Description contains link AB#3720943 to an Azure Boards work item. |
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Co-authored-by: djanardhan98 <307255345+djanardhan98@users.noreply.github.com>
Point the Common submodule at the latest commit from AzureAD/microsoft-authentication-library-common-for-android#3219. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83947e30-5bc9-4ec7-a8ed-c03d4415da92
There was a problem hiding this comment.
Pull request overview
Migrates Native Auth E2E tests to Mail.tm and re-enables SSPR coverage. The current Common submodule API is incompatible with the shared test helper, causing compilation failure.
Changes:
- Adds registered inboxes and OTP checkpoints.
- Runs Native Auth E2E tests in PR validation.
- Updates Common and AB ID extraction.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/validate-pr-ab-id.yml |
Extracts explicit AB IDs. |
azure-pipelines/pull-request-validation/pr-msal.yml |
Adds Native Auth E2E validation. |
common |
Updates the Common submodule. |
NativeAuthPublicClientApplicationAbstractTest.kt |
Centralizes email service and retries. |
SSPRTest.kt |
Re-enables Mail.tm-backed SSPR tests. |
SignInEmailOTPTest.kt |
Adds OTP checkpoints. |
SignInJITTest.kt |
Creates authenticated inboxes. |
SignInMFATest.kt |
Adds MFA OTP checkpoints. |
SignUpEmailPasswordTest.kt |
Migrates signup inbox handling. |
SignUpEmailPasswordAttributesTest.kt |
Migrates attribute-flow inboxes. |
SignUpEmailOTPTest.kt |
Migrates OTP signup inboxes. |
SignUpEmailOTPAttributesTest.kt |
Migrates attributed OTP signup inboxes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 83947e30-5bc9-4ec7-a8ed-c03d4415da92
Restore validate-pr-ab-id.yml to the dev branch version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f268d2a-b8c4-4a22-826e-de2d9b9f59a6
…3219) ## Summary - Replace the Native Auth 1secmail inbox integration with authenticated Mail.tm mailboxes. - Pass the Mail.tm password through the existing Gradle `BuildConfig`/`BuildValues` pattern without committing credentials. - Add checkpoint-aware OTP polling, resend handling, MIME-aware OTP extraction, and bounded retries. - Force HTTP/1.1 for compatibility with the existing OkHttp client and Mail.tm. - Remove obsolete 1secmail response models and add regression coverage for the Mail.tm transport and inbox behavior. ## Motivation Native Auth end-to-end tests need both dynamically created signup inboxes and an authenticated persistent inbox for SSPR. Mail.tm supports both workflows, while the previous unauthenticated 1secmail integration cannot support the persistent SSPR mailbox used by the re-enabled tests. ## Validation - `.\gradlew.bat :testutils:clean :testutils:assembleDebug :testutils:testDebugUnitTest --no-daemon`: passed locally. - All MSAL tests passed after applying the corresponding MSAL changes against Common commit `ddb306a3884f9621e53b6eb3256fc14ea1497c7c`. - The corresponding MSAL changes will be submitted in a separate PR. ## Pipeline configuration - The MSAL consumer-validation task passes the secured Azure DevOps variable `EMAIL_PROVIDER_PASSWORD` from the pipeline's linked variable groups to Common as `-PemailProviderPassword=$(EMAIL_PROVIDER_PASSWORD)`; Common exposes it through generated `BuildConfig` and `BuildValues` without committing the secret. ## Related work - Fixes [AB#3720943](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3720943) - AzureAD/microsoft-authentication-library-for-android#2553 ## Merge sequencing This PR is the expand step of a cross-repository migration: it temporarily retains `generateRandomEmailAddressLocally()` so both `dev` branches remain buildable. After AzureAD/microsoft-authentication-library-for-android#2553 migrates callers to `createRandomEmailAddress()` or `generateRandomUnregisteredEmailAddress()` and merges, the compatibility method will be removed in a Common follow-up. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Copilot-Session: 83947e30-5bc9-4ec7-a8ed-c03d4415da92
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the Common submodule pointer in favor of the current Common dev commit containing the merged Mail.tm integration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| - task: Gradle@2 | ||
| displayName: Run Native Auth E2E Tests | ||
| inputs: | ||
| tasks: msal:testLocalDebugUnitTest -Plabtest -ProbolectricSdkVersion=${{variables.robolectricSdkVersion}} -PnativeAuthConfigString=$(NATIVE_AUTH_CONFIG_STRING) --tests com.microsoft.identity.client.e2e.tests.network.nativeauth.* |
There was a problem hiding this comment.
Are we gating every PR on these now? They hit mail.tm + a live tenant, and these are the same tests that were iGnOrE (@ignore)'d for OTP flakiness. I feel like unrelated PRs will start failing validation whenever mail.tm has a hiccup or the tenant throttles OTPs. Could these run on a nightly/scheduled pipeline instead of the per-PR gate?
| testResultsFiles: '**/build/test-results/testLocalDebugUnitTest/TEST-*.xml' | ||
| testRunTitle: Native Auth E2E Tests (PR Branch) | ||
| env: | ||
| EMAIL_PROVIDER_PASSWORD: $(EMAIL_PROVIDER_PASSWORD) |
There was a problem hiding this comment.
This drops EMAIL_PROVIDER_PASSWORD into the env of a task that runs the PR branch's own code (checkout: self), and these tests already make outbound http calls. So anyone who can push to a PR branch could read it with a getenv and ship it out to themselves. It's a throwaway mail.tm password so low stakes, but on a public repo are we sure fork/external PRs don't get secrets here?
| retryCount++ | ||
| return | ||
| } catch (e: AssertionError) { | ||
| if (!e.message.orEmpty().contains("AADSTS701014")) { |
There was a problem hiding this comment.
What's AADSTS701014 here? We only retry AssertionErrors whose message contains it and rethrow everything else. Is it a transient eSTS thing that's actually worth riding out with the backoff? Matching the code as a substring of the assert message also feels a bit brittle since that text isn't really a stable contract.
| } | ||
|
|
||
| // Avoid repeatedly requesting OTPs while the Native Auth test tenant is throttling them. | ||
| Thread.sleep(5_000L * (1L shl retryCount)) |
There was a problem hiding this comment.
maxRetries=5 with this backoff means a persistently throttled test sleeps 5+10+20+40+80 = ~155s before it finally fails, and every retry re-runs the whole flow (new inbox + signup + OTP request). Across the suite on a throttling day that adds up fast. Do we really need 5 retries here?
Summary
Motivation
Native Auth end-to-end tests need both dynamically created signup inboxes and an authenticated persistent inbox for SSPR. The Mail.tm integration in Common supports both workflows and allows the previously disabled SSPR coverage to run in CI.
Validation
ddb306a3884f9621e53b6eb3256fc14ea1497c7c.Pipeline configuration
EMAIL_PROVIDER_PASSWORDvariable to the Native Auth end-to-end test task.Dependency and merge sequencing
Merge the Common PR first, then update this PR's Common submodule reference to the merged commit before merging.
Fixes AB#3729032