python / cpython

The Python programming language
https://www.python.org
Other
63.55k stars 30.45k forks source link

urllib/urllib2 AbstractDigestAuthHandler locked to retried count of 5 #72478

Open abd105c6-c408-46b8-ab62-241dacbd46e8 opened 8 years ago

abd105c6-c408-46b8-ab62-241dacbd46e8 commented 8 years ago
BPO 28291
Nosy @orsenthil, @bitdancer, @secynic, @matorban
Files
  • issue28291.patch: First pass at patch, added retry_count arg
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.7', 'type-feature', 'library'] title = 'urllib/urllib2 AbstractDigestAuthHandler locked to retried count of 5' updated_at = user = 'https://github.com/secynic' ``` bugs.python.org fields: ```python activity = actor = 'ned.deily' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'secynic' dependencies = [] files = ['44850'] hgrepos = [] issue_num = 28291 keywords = ['patch'] message_count = 5.0 messages = ['277549', '277556', '277558', '278553', '278571'] nosy_count = 4.0 nosy_names = ['orsenthil', 'r.david.murray', 'secynic', 'matorban'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue28291' versions = ['Python 3.7'] ```

    abd105c6-c408-46b8-ab62-241dacbd46e8 commented 8 years ago

    urllib/urllib2 AbstractDigestAuthHandler is hardcoded to 5 retries (self.retried). Normally this wouldn't be an issue.

    Certain products link basic HTTP auth to Active Directory (yes, this shouldn't be a thing). When you have a failed login attempt lockout set on AD, this will lockout accounts on the very first failed Python basic auth attempt, if the AD lockout is set to 5 or less.

    In my specific use case, I was able to override request.HTTPBasicAuthHandler.__init__() and request.HTTPBasicAuthHandler.reset_retry_count() by setting self.retried=5. One way to fix this would be to add a new retry_count argument to AbstractDigestAuthHandler.

    I am a bit busy at the moment, but will submit a patch as soon as I get time.

    bitdancer commented 8 years ago

    For (convoluted) background, see bpo-8797.

    A solution that uses a new would be a new feature and could only go in 3.7, but perhaps the solutions in the above issue will point to a backward compatible solution.

    abd105c6-c408-46b8-ab62-241dacbd46e8 commented 8 years ago

    It is a very limited use case; I won't gripe about 3.7+ only support. At the end of the day, even shown in the comments of that class, it shouldn't be set to a static 5 count.

    At least this is a very easy patch that won't affect existing code.

    e6b699f0-d455-45ae-afae-df6e87a1a648 commented 8 years ago

    Just checked (for my first contribution at cpython) this patch. For me this patch could be merge.

    bitdancer commented 8 years ago

    Well, it's missing doc changes and tests, so even if it is still applicable it isn't ready for merge yet.