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

Other types of auth errors #128

Open
maxlinc opened this issue Jan 28, 2015 · 1 comment
Open

Other types of auth errors #128

maxlinc opened this issue Jan 28, 2015 · 1 comment

Comments

@maxlinc
Copy link
Contributor

maxlinc commented Jan 28, 2015

WinRMAuthorizationError is a subclass of WinRMError and so it doesn't store any info about the HTTP response. I think it should be a subclass of WinRMHTTPTransportError so that the response code, and possibly other response details, are saved in the exception?

Why? There could arguably be more than one response code that should raise a WinRMAuthorizationError. A case could be made that [403 Forbidden](https://www.danielirvine.com/blog/2011/07/18/understanding-403-forbidden/) should also raise the error, though they might be different enough to warrant different exception types (a 401, for example, might mean that the session expired but that re-authenticating and retrying the request with the same credentials may work; a 403 means re-authenticating with the same credentials won't help and that you either need to switch users or elevate the existing users permissions).

Even if the exception is only raised for 401, there are multiple "subcodes" of 401 that Microsoft uses. These are sent in the HTTP response Reason-Phrase, which is "intended to give a short textual description of the Status-Code. The Status-Code is intended for use by automata and the Reason-Phrase is intended for the human user."

Summary/recommendations:

  • Make sure that WinRMHTTPTransportError includes the HTTP Response Reason-Phrase, either as part of the message or as a attribute like status_code.
  • WinRMAuthorizationError should probably be a subclass of WinRMHTTPTransportError, not WinError.
  • Something should be done to handle 403 responses. I recommend the following in order to avoid breaking backwards compatibility while also allowing applications to distinguish between the two errors in future releases:
    • Make two subclasses of WinRMAuthorizationError: WinRMUnauthorizedError and WinRMForbiddenError
      • HTTP 401 should raise WinRMUnauthorizedError
      • HTTP 403 should raise WinRMForbiddenError
@sneal
Copy link
Member

sneal commented Jan 28, 2015

Odd. I was thinking the same thing this morning about WinRMAuthorizationError inheritance, but more from a backwards compatibility standpoint.

👍 Seems like a good proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants