Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 2FA test and 2FA support for integration tests #51

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FranciscoPombal
Copy link

  • Credentials for non 2FA/MFA-enabled accounts as well as 2FA/MFA-enabled accounts are supplied from the environment.

  • For tests that can use both types of credentials, credentials for non 2FA/MFA-enabled accounts are preferred, if available.

  • Tests that require one or the other type of credentials/accounts are skipped if the required type of credentials is not available.

  • A new test-only dependency is introduced for generating 2FA/MFA codes from their secrets: github.com/pquerna/[email protected].

  • golang.org/x/crypto has been bumped to the latest version.


This is not ready because I cannot get the tests to pass properly.
With a 2FA/MFA-enabled account, I can get all tests to pass except one if I:

  • Run them one at a time.
  • Wait a bit between running each test (a few tens of seconds).
  • Clear the login sessions created by the tests in the MEGA web app's "Session history" interface between each test.

When these workarounds are not all performed, the tests will fail with the following error: The upload target URL you are trying to access has expired. Please request a fresh one.
Additionally, TestEventNotify is not passing no matter what, with the same error.

I have not tested with a non 2FA/MFA-enabled account.


Currently my knowledge of MEGA's API and SDK internals is limited, so any help fixing these issues is appreciated.
I would expect we need to at least clean up sessions between tests (perform a logout).


Test setup to replicate the above results (for a 2FA/MFA-enabled account:

  1. Export these variables to the environment:

    [email protected]
    MEGA_PASSWD_MFA=foobar123
    MEGA_SECRET_MFA=SECRETTOKEN2FAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA====
    
  2. Run tests.

    Note that you may need to increase go test's -timeout if the MEGA account user for testing has many files, because for such accounts it may take a long time to login.
    This also explains the decision to pass the 2FA/MFA secret and calculate the codes just-in-time for each test instead of passing in a code for all tests.
    With a large account, some tests can take more than 30s to run, causing the remaining tests to be skipped due to any single code becoming invalid in the meantime.

- Credentials for non 2FA/MFA-enabled accounts as well as
 2FA/MFA-enabled accounts are supplied from the environment.

- For tests that can use both types of credentials, credentials for non
 2FA/MFA-enabled accounts are preferred, if available.

- Tests that require one or the other type of credentials/accounts are
 skipped if the required type of credentials is not available.

- A new test-only dependency is introduced for generating 2FA/MFA codes
 from their secrets: `github.com/pquerna/[email protected]`.

- `golang.org/x/crypto` has been bumped to the latest version.
@ncw
Copy link
Collaborator

ncw commented Aug 12, 2024

I would expect we need to at least clean up sessions between tests (perform a logout).

We really should be doing this anyway as the sessions just pile up. Rclone now has a Shutdown method which could be used for this.

Currently my knowledge of MEGA's API and SDK internals is limited, so any help fixing these issues is appreciated.
I would expect we need to at least clean up sessions between tests (perform a logout).

My knowledge is limited too! I took on a maintainer role for this repo so I could keep it working with rclone, but I don't have time to dig into the SDK to find out how stuff works. I think you know more about it than me now!

These tests look safe to merge as they won't run unless we have the correct env var.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants