Skip to content

Commit 0e958af

Browse files
committed
Update authcode.py
1 parent 4319ebf commit 0e958af

File tree

1 file changed

+24
-5
lines changed

1 file changed

+24
-5
lines changed

msal/oauth2cli/authcode.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,32 @@ class _AuthCodeHandler(BaseHTTPRequestHandler):
111111
def do_GET(self):
112112
# For flexibility, we choose to not check self.path matching redirect_uri
113113
#assert self.path.startswith('/THE_PATH_REGISTERED_BY_THE_APP')
114-
qs = parse_qs(urlparse(self.path).query)
115-
if qs.get('code') or qs.get("error"): # Auth response via query string is not allowed
116-
logger.error("Received auth response via query string (GET request). "
114+
parsed_url = urlparse(self.path)
115+
qs = parse_qs(parsed_url.query)
116+
117+
if qs.get('code'): # Auth code via GET is a security risk - reject it
118+
logger.error("Received auth code via query string (GET request). "
117119
"This is a security risk. Only form_post (POST) is supported.")
118120
self._send_full_response(
119121
"Authentication method not supported. "
120122
"The application requires response_mode=form_post for security. "
121123
"Please ensure your application registration uses form_post response mode.",
122124
is_ok=False)
125+
elif qs.get("error"): # Errors can come via GET - process them
126+
auth_response = _qs2kv(qs)
127+
self._process_auth_response(auth_response)
128+
elif not qs and parsed_url.path == '/':
129+
# GET request to root with no query params - this is likely from clicking OK on eSTS error
130+
# Treat it as an error since success would come via POST
131+
logger.warning("Received GET request to root path with no query parameters - treating as authentication error")
132+
auth_response = {
133+
"error": "authentication_failed",
134+
"error_description": "Please close this window and try again."
135+
}
136+
# Include the original state so state validation doesn't fail
137+
if self.server.auth_state:
138+
auth_response["state"] = self.server.auth_state
139+
self._process_auth_response(auth_response)
123140
else:
124141
self._send_full_response(self.server.welcome_page)
125142
# NOTE: Don't do self.server.shutdown() here. It'll halt the server.
@@ -147,6 +164,8 @@ def _process_auth_response(self, auth_response):
147164
# OAuth2 successful and error responses contain state when it was used
148165
# https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2.1
149166
self._send_full_response("State mismatch") # Possibly an attack
167+
# Still set auth_response so the server doesn't hang forever
168+
self.server.auth_response = auth_response
150169
else:
151170
template = (self.server.success_template
152171
if "code" in auth_response else self.server.error_template)
@@ -347,10 +366,10 @@ def _get_auth_response(self, result, auth_uri=None, timeout=None, state=None,
347366
auth_uri_callback(_uri)
348367

349368
self._server.success_template = Template(success_template or
350-
"Authentication complete. You can return to the application. Please close this browser tab. "
369+
"Authentication complete. You can return to the application. Please close this browser tab.\n\n"
351370
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
352371
self._server.error_template = Template(error_template or
353-
"Authentication failed. $error: $error_description. ($error_uri) "
372+
"Authentication failed. $error: $error_description.\n\n"
354373
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
355374

356375
self._server.timeout = timeout # Otherwise its handle_timeout() won't work

0 commit comments

Comments
 (0)