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

[BUG] earthaccess.login tries other strategies when it should not #945

Open
1 task done
chuckwondo opened this issue Feb 10, 2025 · 8 comments
Open
1 task done

Comments

@chuckwondo
Copy link
Collaborator

Is this issue already tracked somewhere, or is this a new report?

  • I've reviewed existing issues and couldn't find a duplicate for this problem.

Current Behavior

When calling earthaccess.login with a strategy other than "all", it might call one or more other strategies.

Expected Behavior

When calling earthaccess.login with a strategy other than "all", it should attempt only the specified strategy.

Steps To Reproduce

The following should ignore the env vars as well as netrc, and only prompt for creds, but does not.

It uses the env vars to login, even though the strategy is "interactive", not "all" (of course, replace username and password with valid Earthdata Login creds):

EARTHDATA_USERNAME=username EARTHDATA_PASSWORD=password python -c 'import earthaccess; earthaccess.login("interactive")'

If the creds are valid, you are not prompted for creds, as the env vars are used to successfully login, and thus the "interactive" strategy is skipped.

Conversely, if you use invalid creds for the env vars, login will still attempt to use them, but since login will fail with them, the "netrc" strategy will be tried. If you have a valid netrc entry, login will succeed without prompting you for creds.

Only when neither valid env vars nor a valid netrc entry exists will you will be prompted to enter creds. However, neither of the other strategies should be attempted at all, not even if a user enters invalid creds interactively.

Environment

- OS: macOS 15.3
- Python: 3.10.15

Additional Context

No response

@mfisher87
Copy link
Collaborator

mfisher87 commented Feb 10, 2025

There's the problem:

for strategy in ["environment", "netrc"]:

This is pretty magical! There are two places we are trying multiple strategies: one in api.py and again in __init__.py. Seems like we should only have one place that does this.

@mfisher87
Copy link
Collaborator

Perhaps the "all strategy" logic should be moved into the Auth class and out of both __init__.py and api.py.

@chuckwondo
Copy link
Collaborator Author

There's the problem:

earthaccess/earthaccess/init.py

Line 89 in 3f8b03a

for strategy in ["environment", "netrc"]:
This is pretty magical! There are two places we are trying multiple strategies: one in api.py and again in __init__.py. Seems like we should only have one place that does this.

Yep, we should not be doing that.

@mfisher87
Copy link
Collaborator

Yeah, to me it makes the most sense for this to all be encapsulated in the Auth class. We've got "spooky action" as is.

@eigenbeam
Copy link

eigenbeam commented Feb 25, 2025

I was just going to report this bug. Calling earthaccess.login(strategy="environment") with invalid credentials in the environment results in three attempts at authentication.

It looks like a call to earthaccess.login hits this line:
https://github.com/nsidc/earthaccess/blob/main/earthaccess/api.py#L218

which pops into the module-level __getattr__ because of the earthaccess.__auth__ access.
https://github.com/nsidc/earthaccess/blob/main/earthaccess/__init__.py#L76

This function then calls Auth().login twice, once for each of the two strategies "environment" and "netrc".

After those both fail, the original call to earthaccess.login continues along by calling login on the return value of the module-level __getattr__ which is the Auth object constructed in the module __init__.

I'm not sure what the origin of the module-level __getattr__ is, or the use-case it is needed for, but that appears to be the cause of the 3x authentication attempts when calling earthaccess.login with invalid credentials in the environment.

Debugger session to show the three authn attempts:

>>> earthaccess.login(strategy="environment", system=earthaccess.UAT)
Authentication with Earthdata Login failed with:
{"error":"invalid_credentials","error_description":"Invalid user credentials"}
Authentication with Earthdata Login failed with:
{"error":"invalid_credentials","error_description":"Invalid user credentials"}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/kbeam/Library/Caches/pypoetry/virtualenvs/nsidc-metgenc-Ry6uIukv-py3.12/lib/python3.12/site-packages/earthaccess/api.py", line 218, in login
    earthaccess.__auth__.login(
  File "/Users/kbeam/Library/Caches/pypoetry/virtualenvs/nsidc-metgenc-Ry6uIukv-py3.12/lib/python3.12/site-packages/earthaccess/auth.py", line 136, in login
    self._environment()
  File "/Users/kbeam/Library/Caches/pypoetry/virtualenvs/nsidc-metgenc-Ry6uIukv-py3.12/lib/python3.12/site-packages/earthaccess/auth.py", line 303, in _environment
    return self._get_credentials(username, password)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/kbeam/Library/Caches/pypoetry/virtualenvs/nsidc-metgenc-Ry6uIukv-py3.12/lib/python3.12/site-packages/earthaccess/auth.py", line 316, in _get_credentials
    raise LoginAttemptFailure(msg)
earthaccess.exceptions.LoginAttemptFailure: Authentication with Earthdata Login failed with:
{"error":"invalid_credentials","error_description":"Invalid user credentials"}

@mfisher87
Copy link
Collaborator

👋 Hi, Kevin, good to see you! Want work on this with us at a hackathon sometime soon? 😁 I think we need a test that when the user specifies a specific strategy, earthaccess only uses that strategy.

@eigenbeam
Copy link

I would like to, I'll see what I can do. Do you have an idea for what the fix would look like?

@mfisher87
Copy link
Collaborator

I think I need to do some debugging to feel more comfortable about the solution. What I know is that we have two places where we're programmatically trying multiple strategies: One in the Auth class and again in __init__.py. I expected that with v0.14.0, failed login attempts would raise an exception instead of trying more strategies. But I didn't take into account the behavior in __init__.py. Separately, we need to write a very simple story for authentication strategy behaviors and write some tests that check the library is obeying this contract, and then fix any bugs. For example:

If no strategy is specified, the following strategies will be tried in order: ... If no credentials are found for a strategy, the next strategy will be attempted. If a strategy is specified with earthaccess.login(strategy=...) (link to API docs), only the specified strategy will be attempted, and no others. If at any point Earthdata Login rejects a login attempt, earthaccess will raise a SomethingException and abort. If you receive this error, please fix your credentials before re-runing your code; if Earthdata Login receives too many failed login attempts, your account may be locked.

@chuckwondo @betolink @asteiker @jhkennedy @itcarroll what do you think of this behavior contract? 👆 We should be super explicit in the docstring.

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

No branches or pull requests

3 participants