scrapinghub / shub

Scrapinghub Command Line Client
https://shub.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
125 stars 79 forks source link

Hide apikey in Target or apikey repr #264

Closed chekunkov closed 5 years ago

chekunkov commented 7 years ago

I accidentally posted my apikey to GH with debug output, like this

(Pdb) conf.get_target_conf(108106)
Target(project_id=108106, endpoint='https://app.scrapinghub.com/api/', apikey='<WHOOPS MY APIKEY HERE>', stack=None, image='images.scrapinghub.com/project/108106', requirements_file=None, version='f092796-fix-docker-disconnects', eggs=[])

apikey is easy to change, but I realized that I'm doing something wrong only at the last moment, I easily could have missed that. maybe for better security we can customize representation of the Target or apikey? str() will return apikey string, repr() will return xxxxxxxxxxxxxxxxxxxxxxxxx.

vshlapakov commented 5 years ago

Target here is a named tuple: we should rather wrap it with a custom class (class Target(namedtuple('Target', [..])) and redefine its __repr__ logic to exclude/mask API key field, or write a custom class for API key string specifically - what you prefer. When it works as expected (excludes/masks the field value in Target representation), the tests should be updated appropriately to reflect it.

mabelvj commented 5 years ago

Thanks @vshlapakov! I noticed too and implemented it on a small class for the apikey restricted to the get_target_conf area.Sure, I will update the tests. One question, should we display a constant string as a mask or should it match the length of the original key?

vshlapakov commented 5 years ago

should we display a constant string as a mask or should it match the length of the original key?

Our API keys have a fixed length, so there's no problem with showing */x instead of each char from security pov, and using a constant string would work too, I don't have a strong preference here.