Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthError not raised on 403 errors #196

Closed
bowlofeggs opened this issue May 3, 2017 · 4 comments · Fixed by #197
Closed

AuthError not raised on 403 errors #196

bowlofeggs opened this issue May 3, 2017 · 4 comments · Fixed by #197
Assignees

Comments

@bowlofeggs
Copy link
Contributor

bowlofeggs commented May 3, 2017

I noticed today that the fedora.client.openidbaseclient.requires_login decorator does not handle 403 return codes though it looks like it's intended to. The issue is that the output variable (a requests.models.Response) does two weird things:

  1. It casts to False:
ipdb> bool(output)
False
  1. "anding" a Response with a bool does not return a bool:
ipdb> print (output and output.status_code)                                                                           
<Response [403]>

We probably want to do this instead:

elif hasattr(output, 'status_code') and output.status_code == 403:
@pypingou
Copy link
Member

pypingou commented May 3, 2017

bool(output) is I think a documented feature, everything that is not a 20* code will return False.

The anding though it a little more odd for the least

@pypingou
Copy link
Member

pypingou commented May 3, 2017

Isn't the idea of this line to protect against output being None? If so should we make it elif output is not None and output.status_code == 403 ?

@bowlofeggs
Copy link
Contributor Author

@pypingou Yeah probably, though hasattr on None will also be False. Either way should work.

@jeremycline
Copy link
Member

jeremycline commented May 3, 2017

Just a heads-up, the __bool__ method is gone from Response objects in requests-3.0 and code that wants to transition smoothly shouldn't rely on it: https://github.com/kennethreitz/requests/issues/2002

@bowlofeggs bowlofeggs self-assigned this May 3, 2017
bowlofeggs added a commit to bowlofeggs/python-fedora that referenced this issue May 3, 2017
fixes fedora-infra#196

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
bowlofeggs added a commit to bowlofeggs/python-fedora that referenced this issue May 3, 2017
fixes fedora-infra#196

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
puiterwijk pushed a commit that referenced this issue May 3, 2017
fixes #196

Signed-off-by: Randy Barlow <randy@electronsweatshop.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants