kapadia / usgs

Client library for interfacing with USGS datasets
http://kapadia.github.io/usgs/
ISC License
110 stars 38 forks source link

`api.logout` and `api.login` fail if key expires #62

Closed aazuspan closed 2 years ago

aazuspan commented 2 years ago

If you run api.logout after your key has expired, it throws the following exception.

---------------------------------------------------------------------------
USGSError                                 Traceback (most recent call last)
<ipython-input-17-43027361ce26> in <module>
----> 1 api.logout()

~\anaconda3\envs\geostats\lib\site-packages\usgs\api.py in logout()
    145     response = r.json()
    146 
--> 147     _check_for_usgs_error(response)
    148 
    149     if os.path.exists(TMPFILE):

~\anaconda3\envs\geostats\lib\site-packages\usgs\api.py in _check_for_usgs_error(data)
     33     error = data['errorMessage']
     34 
---> 35     raise USGSError('%s: %s' % (error_code, error))
     36 
     37 def _create_session(api_key):

USGSError: AUTH_EXPIRED: API key has expired due to inactivity

This happens before it removes the old TMPFILE, so the expired key is kept and subsequent calls to api.login will also fail.

---------------------------------------------------------------------------
USGSError                                 Traceback (most recent call last)
<ipython-input-16-8b0deb086896> in <module>
---> 13 key = api.login(username, password)["data"]

~\anaconda3\envs\geostats\lib\site-packages\usgs\api.py in login(username, password, save)
    121     r = session.post(url, payload)
    122     if r.status_code != 200:
--> 123         raise USGSError(r.text)
    124 
    125     response = r.json()

USGSError: {"errorCode":"AUTH_EXPIRED","errorMessage":"API key has expired due to inactivity","data":null,"requestId":0,"sessionId":null,"version":null}

I think this is a great tool and I'd be happy to contribute a PR if you're open to it, so I took a look at the source.

It seems like the underlying issues are: 1) api.logout fails on AUTH_EXPIRED errors, so you can't delete your old key. 2) api.login calls api._create_session without a key which causes api._get_api_key to load the expired key, so you can't get a new key.

I think those could be solved by: 1) Ignore AUTH_EXPIRED error codes for api.logout. The point of logging out is to deactivate and delete the key, so if it is already expired then the key just needs to be deleted. Maybe a warning could be raised, though? 2) Modify api._create_session to skip the api._get_api_key call and just create a session without a key if none is provided. This would allow api.login to run and overwrite the expired key if api.logout was never called. All other calls to api._create_session explicity pass a key, so they would be unaffected. At this point, api.logout should be modified to accept an optional api_key kwarg and explicitly pass a key (e.g. same pattern as api.scene_metadata).

Let me know if you're open to PRs and if so, what you think of the plan.

kapadia commented 2 years ago

@aazuspan Thanks for the details and flagging the issue. Option 1 makes most sense, remove the file regardless of the returned authentication error.

Feel free to drop a PR and I'll gladly approve it. Glad you're finding this library useful :)

In case others run into this, a temporary workaround would be to remove the credential file manually.

rm ~/.usgs
aazuspan commented 2 years ago

Thanks for the quick response @kapadia! I'll open a PR with option 1 shortly.