Skip to content

Conversation

@Yuki-YuXin
Copy link
Contributor

@Yuki-YuXin Yuki-YuXin commented Jan 13, 2025

The third party of 1secmail used for E2E test OTP code fetching is down.

Common PR: AzureAD/microsoft-authentication-library-common-for-android#2570

Fixes AB#3125466

@github-actions github-actions bot added the msal label Jan 13, 2025
@github-actions
Copy link

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

@Yuki-YuXin Yuki-YuXin added the No-Changelog This change does not update the changelog. label Jan 13, 2025
@github-actions github-actions bot changed the title 1secmail service hot fix 1secmail service hot fix, Fixes AB#3125466 Jan 13, 2025
@Yuki-YuXin Yuki-YuXin marked this pull request as ready for review January 13, 2025 12:41
@Yuki-YuXin Yuki-YuXin requested a review from a team as a code owner January 13, 2025 12:41
@Yuki-YuXin Yuki-YuXin changed the title 1secmail service hot fix, Fixes AB#3125466 Native Auth E2E fix, Fixes AB#3125466 Jan 13, 2025
@Yuki-YuXin Yuki-YuXin changed the title Native Auth E2E fix, Fixes AB#3125466 Native Auth E2E test fix, Fixes AB#3125466 Jan 13, 2025
* Use valid email address, but invalid OTP to receive "invalid code" error.
* (use case 2.2.7)
*/
@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this test we don't retrieve an OTP code from the email inbox, so it should not be ignored

Copy link
Contributor Author

@Yuki-YuXin Yuki-YuXin Jan 13, 2025

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This account has been locked.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use the correct explanation why we're skipping this test? We're skipping it because the user is blocked not for 1secmail service. Please, update all the other tests that apply

* Sign up with email + OTP. Server requires password authentication, which is not supported by the developer (aka redirect flow).
* (use case 2.1.10)
*/
@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this test we don't retrieve an OTP code from the email inbox, so it should not be ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @nilo-ms , for the following 3 test cases, please refer to this build https://identitydivision.visualstudio.com/Engineering/_build/results?buildId=1413056&view=logs&j=6f26e4aa-3eaf-5d24-2c84-463275687676&t=733235bb-4511-5023-8af6-9a666fd33e21

Although the test case itself has no relation with the OTP code.

    com.microsoft.identity.internal.test.labapi.ApiException: 
        at com.microsoft.identity.internal.test.labapi.ApiClient.handleResponse(ApiClient.java:911)
        at com.microsoft.identity.internal.test.labapi.ApiClient.execute(ApiClient.java:827)
        at com.microsoft.identity.internal.testutils.nativeauth.api.TemporaryEmailService$TemporaryEmailApi.generateRandomEmailAddress(TemporaryEmailService.kt:142)

It used genRandomMailbox from the service.

Copy link
Contributor

@nilo-ms nilo-ms Jan 14, 2025

Choose a reason for hiding this comment

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

Thanks Yuki, on iOS we generate the random email address locally with the following code:

func generateRandomEmailAddress() -> String {
        let randomId = UUID().uuidString.prefix(8)
        return "native-auth-signup-\(randomId)@1secmail.org"
}

Can you please do the same on Android? So, we don't need to use 1secmail for this or similar test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestions. Done.

There would be a company PR in common #2244

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to merge the common PR first then update the submodule then run the pipeline here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Yuki, I think the right link to the common PR is this one:
AzureAD/microsoft-authentication-library-common-for-android#2570
I approved that PR

private val defaultChallengeTypes = listOf("password", "oob")


@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this test we don't retrieve an OTP code from the email inbox, so it should not be ignored

* Sign up with email + password. Developer makes a request with password that does not match password complexity requirements set on portal.
* (use case 1.1.13)
*/
@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

For this test we don't retrieve an OTP code from the email inbox, so it should not be ignored

* Sign up with email + OTP. Resend email OTP.
* (hero scenario 1, use case 2.1.5)
*/
@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to ignore this test after you made the change around random email generation?

* Sign up with email + OTP. Verify email address using email OTP and sign up.
* (hero scenario 1, use case 2.1.1)
*/
@Ignore("1secmail service is down. Ignoring test for now.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@Yuki-YuXin Yuki-YuXin merged commit d90d49a into dev Jan 22, 2025
13 of 14 checks passed
@nilo-ms nilo-ms deleted the yuki/skip-1secmail branch January 23, 2025 15:42
Yuki-YuXin added a commit to AzureAD/microsoft-authentication-library-common-for-android that referenced this pull request Feb 14, 2025
Make changes inside TemporaryEmailService.kt to generate random email
account locally.

Please refer to the MSAL PR:
AzureAD/microsoft-authentication-library-for-android#2244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

msal No-Changelog This change does not update the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants