awslabs / awsprocesscreds

Process credential providers for AWS SDKs and Tools
Apache License 2.0
132 stars 40 forks source link

replace-ascii-parsing-with-utf-8 #30

Open ghost opened 5 years ago

ghost commented 5 years ago

Issue: When attempting login and return token for aws with adfs the program produces this error:

File "/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-packages/awsprocesscreds/saml.py", line 423, in _parse_roles root = ET.fromstring(base64.b64decode(assertion).decode('ascii')) UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 10504: ordinal not in range(128)

When passed credentials containing only ascii characters. Further inspection revealed if a user has 17 roles the login fails with above error. Normal login works as expected.

Description of changes:

Changed decoding from ascii to utf-8. Issue was resolved.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-io commented 5 years ago

Codecov Report

Merging #30 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #30   +/-   ##
======================================
  Coverage    98.1%   98.1%           
======================================
  Files           5       5           
  Lines         317     317           
  Branches       42      42           
======================================
  Hits          311     311           
  Misses          3       3           
  Partials        3       3
Impacted Files Coverage Δ
awsprocesscreds/saml.py 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a216e70...6ed7fc6. Read the comment docs.

ericdbarry commented 5 years ago

Hey, I am not a maintainer - just someone trying to make this work, but I noticed there were other instances - all tests - that use this as well and this patch should probably fix them too:

https://github.com/awslabs/awsprocesscreds/search?q=ascii&unscoped_q=ascii