schubergphilis / awsapilib

A python library exposing services that are not covered by the official boto3 library but are driven by undocumented APIs.
MIT License
60 stars 8 forks source link

@costastf Fixed! #35

Closed matherp-ppb closed 2 years ago

matherp-ppb commented 2 years ago

@costastf Fixed! Also regarding the issue you mentioned, I do get sometimes an error in the line:

        if not all([success,
                    response.json().get('properties').get('RedirectTo') is not None]):
            raise InvalidAuthentication(f'Unable to authenticate, response received was: {response.text} '
                                        f'with status code: {response.status_code}')

Where there is no RedirectTo information, this might happen because of the fast pace of the lib (like you mentioned, but the status_code is 200).... But I was not able to dive into it.

Originally posted by @rzamana in https://github.com/schubergphilis/awsapilib/issues/34#issuecomment-1125900859

costastf commented 2 years ago

Does that mean that this issue should be closed?

matherp-ppb commented 2 years ago

Having just installed this from GitHub after you merged this I'm getting the same issue myself but every single time. Process was...

git clone git@github.com:schubergphilis/awsapilib.git
cd awsapilib
python3 ./setup.py  sdist bdist_egg

cat ./tmp1
from awsapilib import AccountManager, PasswordManager
password_manager = PasswordManager()
account_manager = AccountManager('example@example.com', 'EXAMPLEEXAMPLEEXAMPLE', 'eu-west-1')
seed = account_manager.mfa.create_virtual_device()
print(seed)

python3 ./tmp1

Captcha: EXAMPLE
Traceback (most recent call last):
  File "./tmp1", line 4, in <module>
    seed = account_manager.mfa.create_virtual_device()
  File "/Users/matherp/Projects/src/github.com/awsapilib/awsapilib/console/console.py", line 733, in mfa
    session = self._get_iam_session(self.email, self.password, self.region, self.mfa_serial)
  File "/Users/matherp/Projects/src/github.com/awsapilib/awsapilib/console/console.py", line 554, in _get_iam_session
    redirect_url = self._get_root_console_redirect(email, password, session, mfa_serial=mfa_serial)
  File "/Users/matherp/Projects/src/github.com/awsapilib/awsapilib/console/console.py", line 541, in _get_root_console_redirect
    raise InvalidAuthentication(f'Unable to authenticate, response received was: {response.text} '
awsapilib.console.consoleexceptions.InvalidAuthentication: Unable to authenticate, response received was: {"state":"SUCCESS","properties":{"CES":"EXAMPLE","Captcha":"true","CaptchaURL":"https://opfcaptcha-prod.s3.amazonaws.com/7fbae8f364544fa9b5a09a64c67ffbc4.jpg?AWSAccessKeyId\uEXAMPLESignature\uEXAMPLE","email":"EXAMPLE@EXAMPLE.com","captchaObfuscationToken":"{\"b64KeyCipherData\":\"EXAMPLE\",\"b64CipherData\":\"EXAMPLE\"}"}} with status code: 200

Apologies if I've redacted a bit too much there but as far as I can tell it's successful yet failing because there's no redirectURL in the response?

costastf commented 2 years ago

Hi @matherp-ppb , thanks for reporting, i will have a look at this tomorrow, i hope that is ok.

costastf commented 2 years ago

Hi @matherp-ppb , can you please enable debugging logging and redact all the stuff again and provide me with a log output. I am having a little trouble understanding the usage case since the method that fails is _get_root_console_redirect which is supposed to provide with a console redirect URL so not having response.json().get('properties').get('RedirectTo') set makes it ok to fail. Not sure how we have have a successful auth process without that redirect url.

matherp-ppb commented 2 years ago

Just tried it this morning, absolutely no changes and it's worked perfectly but without presenting me a captcha this time. I have a lot of accounts available to test with so will enable debug in the meantime and keep testing/setuping out process. For time being all I can tell you is that it's something to do with the Captcha path but nothing else sorry.

rzamana commented 2 years ago

So, if we fix for optional captcha we broke for mandatory captcha!?!?

matherp-ppb commented 2 years ago

I don't think so, all seems good right now but I think there's some users who when they do get a Captcha aren't receiving a redirect URL. I worry this might be a corporate proxy issue? My testing so far has been from my laptop not from our "infrastructure" so wonder if our ForcePoint proxy is striping the redirect for inspection or something silly. Might also be some setting we have on our AWS accounts perhaps or that our corporate VPN's external IP makes a lot of requests to AWS who are then tagging us for some Captchas.

costastf commented 2 years ago

This is very interesting, please keep us updated with your findings. I will refrain for a couple of days before releasing this latest code on public pypi, ok @rzamana @matherp-ppb ?

matherp-ppb commented 2 years ago

This is fine with me, have checked the repo out and building it from latest src so no problem. Even if it's something that's intermittent on our or AWS' side then even just retrying a few times with your library to automate it is still a massive improvement from doing it all manually. 💯

matherp-ppb commented 2 years ago

I have continued testing both locally and whilst setting up Jenkins jobs in different environments and encountered no further captcha requests or other errors so would say maybe there was some sort of hiccup my end? Maybe I had the last release installed still and then updated to the source version with the fix in without realising somehow, my bad maybe?

I did find a trivial error in your documentation... https://awsapilib.readthedocs.io/en/latest/usage.html#usage-for-account-manager ...mentions the function delete_virtual_mfa which should be delete_virtual_device as per https://awsapilib.readthedocs.io/en/latest/awsapilib.console.html#awsapilib.console.console.MfaManager.delete_virtual_device Not meaning to be lazy but it seemed a bit minor to bother creating a branch and PR for :^).

costastf commented 2 years ago

Thanks for reporting and looking into this. Fixed the documentation inconsistency.