Skip to content

Commit 90a7a71

Browse files
committed
Revert "Refactor to specify response_mode=form_post in a better spot"
This reverts commit 3eca00f.
1 parent 3eca00f commit 90a7a71

6 files changed

Lines changed: 333 additions & 207 deletions

File tree

msal/application.py

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,9 @@ def initiate_auth_code_flow(
920920
):
921921
"""Initiate an auth code flow.
922922
923+
.. deprecated::
924+
The response_mode parameter is deprecated and will be removed in a future version.
925+
923926
Later when the response reaches your redirect_uri,
924927
you can use :func:`~acquire_token_by_auth_code_flow()`
925928
to complete the authentication/authorization.
@@ -960,21 +963,14 @@ def initiate_auth_code_flow(
960963
New in version 1.15.
961964
962965
:param str response_mode:
963-
OPTIONAL. Specifies the method with which response parameters should be returned.
964-
The default value is equivalent to ``query``, which was still secure enough in MSAL Python
965-
(because MSAL Python does not transfer tokens via query parameter in the first place).
966-
For even better security, we recommend using the value ``form_post``.
967-
In "form_post" mode, response parameters
968-
will be encoded as HTML form values that are transmitted via the HTTP POST method and
969-
encoded in the body using the application/x-www-form-urlencoded format.
970-
Valid values can be either "form_post" for HTTP POST to callback URI or
971-
"query" (the default) for HTTP GET with parameters encoded in query string.
972-
More information on possible values
973-
`here <https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes>`
974-
and `here <https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html#FormPostResponseMode>`
975-
976-
.. note::
977-
You should configure your web framework to accept form_post responses instead of query responses.
966+
.. deprecated::
967+
This parameter is deprecated and will be removed in a future version.
968+
969+
**For PublicClientApplication**: response_mode is automatically set to
970+
``form_post`` for security reasons. This parameter is ignored.
971+
972+
**For ConfidentialClientApplication**: You should configure your web
973+
framework to accept form_post responses instead of query responses.
978974
While this parameter still works, it will be removed in a future version.
979975
Using query-based response modes is less secure and should be avoided.
980976
@@ -996,9 +992,15 @@ def initiate_auth_code_flow(
996992
3. and then relay this dict and subsequent auth response to
997993
:func:`~acquire_token_by_auth_code_flow()`.
998994
"""
999-
# Note to maintainers: Do not emit warning for the use of response_mode here,
1000-
# because response_mode=form_post is still the recommended usage for MSAL Python 1.x.
1001-
# App developers making the right call shall not be disturbed by unactionable warnings.
995+
if response_mode is not None:
996+
import warnings
997+
warnings.warn(
998+
"The 'response_mode' parameter is deprecated and will be removed in a future version. "
999+
"For public clients, response_mode is automatically set to 'form_post'. "
1000+
"For confidential clients, configure your web framework to use 'form_post'. "
1001+
"Query-based response modes are less secure.",
1002+
DeprecationWarning,
1003+
stacklevel=2)
10021004
client = _ClientWithCcsRoutingInfo(
10031005
{"authorization_endpoint": self.authority.authorization_endpoint},
10041006
self.client_id,

msal/oauth2cli/authcode.py

Lines changed: 54 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
It optionally opens a browser window to guide a human user to manually login.
66
After obtaining an auth code, the web server will automatically shut down.
77
"""
8-
from collections import defaultdict
98
import logging
109
import os
1110
import socket
@@ -110,47 +109,70 @@ def _printify(text):
110109

111110
class _AuthCodeHandler(BaseHTTPRequestHandler):
112111
def do_GET(self):
112+
# For flexibility, we choose to not check self.path matching redirect_uri
113+
#assert self.path.startswith('/THE_PATH_REGISTERED_BY_THE_APP')
114+
113115
qs = parse_qs(urlparse(self.path).query)
114-
if qs:
116+
if qs.get('code') or qs.get('error'):
115117
# GET request with auth code or error - reject for security (form_post only)
116118
self._send_full_response(
117-
"response_mode=query is not supported for authentication responses. "
118-
"This application operates in response_mode=form_post mode only.",
119+
"GET method is not supported for authentication responses. "
120+
"This application requires form_post response mode.",
119121
is_ok=False)
122+
elif not qs:
123+
# Blank redirect from eSTS error - show generic error and mark done
124+
self._send_full_response(
125+
"Authentication could not be completed. "
126+
"You can close this window and return to the application.")
127+
self.server.done = True
120128
else:
121129
# Other GET requests - show welcome page
122130
self._send_full_response(self.server.welcome_page)
123131
# NOTE: Don't do self.server.shutdown() here. It'll halt the server.
124132

125-
def do_POST(self): # Handle form_post response where auth code is in body
126-
# For flexibility, we choose to not check self.path matching redirect_uri
127-
#assert self.path.startswith('/THE_PATH_REGISTERED_BY_THE_APP')
133+
def do_POST(self):
134+
# Handle form_post response mode where auth code is sent via POST body
128135
content_length = int(self.headers.get('Content-Length', 0))
129136
post_data = self.rfile.read(content_length).decode('utf-8')
137+
130138
qs = parse_qs(post_data)
131139
if qs.get('code') or qs.get('error'): # So, it is an auth response
132-
self._process_auth_response(_qs2kv(qs))
140+
auth_response = _qs2kv(qs)
141+
logger.debug("Got auth response via POST: %s", auth_response)
142+
self._process_auth_response(auth_response)
133143
else:
134144
self._send_full_response("Invalid POST request", is_ok=False)
135145
# NOTE: Don't do self.server.shutdown() here. It'll halt the server.
136146

137147
def _process_auth_response(self, auth_response):
138148
"""Process the auth response from either GET or POST request."""
139-
logger.debug("Got auth response: %s", auth_response)
140149
if self.server.auth_state and self.server.auth_state != auth_response.get("state"):
141150
# OAuth2 successful and error responses contain state when it was used
142151
# https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2.1
143-
self._send_full_response( # Possibly an attack
144-
"State mismatch. Waiting for next response... or you may abort.", is_ok=False)
152+
self._send_full_response("State mismatch") # Possibly an attack
153+
# Don't set auth_response for security, but mark as done to avoid hanging
154+
self.server.done = True
145155
else:
146156
template = (self.server.success_template
147157
if "code" in auth_response else self.server.error_template)
148158
if _is_html(template.template):
149159
safe_data = _escape(auth_response) # Foiling an XSS attack
150160
else:
151-
safe_data = auth_response
152-
filled_data = defaultdict(str, safe_data) # So that missing keys will be empty string
153-
self._send_full_response(template.safe_substitute(**filled_data))
161+
safe_data = dict(auth_response) # Make a copy to avoid mutating original
162+
# Provide default values for common OAuth2 response fields
163+
# to avoid showing literal placeholder text like "$error_description"
164+
safe_data.setdefault("error", "")
165+
safe_data.setdefault("error_description", "")
166+
# Format error message nicely: include ": description." only if description exists
167+
if "code" not in auth_response: # This is an error response
168+
error_desc = auth_response.get("error_description", "").strip()
169+
if error_desc:
170+
safe_data["error_message"] = f"{safe_data['error']}: {error_desc}."
171+
else:
172+
safe_data["error_message"] = safe_data["error"]
173+
else:
174+
safe_data["error_message"] = ""
175+
self._send_full_response(template.safe_substitute(**safe_data))
154176
self.server.auth_response = auth_response # Set it now, after the response is likely sent
155177

156178
def _send_full_response(self, body, is_ok=True):
@@ -236,7 +258,6 @@ def get_auth_response(self, timeout=None, **kwargs):
236258
237259
:param str auth_uri:
238260
If provided, this function will try to open a local browser.
239-
Starting from 2026, the built-in http server will require response_mode=form_post.
240261
:param int timeout: In seconds. None means wait indefinitely.
241262
:param str state:
242263
You may provide the state you used in auth_uri,
@@ -309,20 +330,17 @@ def _get_auth_response(self, result, auth_uri=None, timeout=None, state=None,
309330
welcome_uri = "http://localhost:{p}".format(p=self.get_port())
310331
abort_uri = "{loc}?error=abort".format(loc=welcome_uri)
311332
logger.debug("Abort by visit %s", abort_uri)
312-
333+
334+
# Enforce response_mode=form_post for security
313335
if auth_uri:
314-
# Note to maintainers:
315-
# Do not enforce response_mode=form_post by secretly hardcoding it here.
316-
# Just validate it here, so we won't surprise caller by changing their auth_uri behind the scene.
317-
params = parse_qs(urlparse(auth_uri).query)
318-
assert params.get('response_mode', [None])[0] == 'form_post', (
319-
"The built-in http server supports HTTP POST only. "
320-
"The auth_uri must be built with response_mode=form_post")
321-
322-
self._server.welcome_page = Template(
323-
welcome_template or
324-
"<a href='$auth_uri'>Sign In</a>, or <a href='$abort_uri'>Abort</a>"
325-
).safe_substitute(auth_uri=auth_uri, abort_uri=abort_uri)
336+
parsed = urlparse(auth_uri)
337+
params = parse_qs(parsed.query)
338+
params['response_mode'] = ['form_post'] # Enforce form_post
339+
new_query = urlencode(params, doseq=True)
340+
auth_uri = parsed._replace(query=new_query).geturl()
341+
342+
self._server.welcome_page = Template(welcome_template or "").safe_substitute(
343+
auth_uri=auth_uri, abort_uri=abort_uri)
326344
if auth_uri: # Now attempt to open a local browser to visit it
327345
_uri = welcome_uri if welcome_template else auth_uri
328346
logger.info("Open a browser on this device to visit: %s" % _uri)
@@ -351,22 +369,22 @@ def _get_auth_response(self, result, auth_uri=None, timeout=None, state=None,
351369
auth_uri_callback(_uri)
352370

353371
self._server.success_template = Template(success_template or
354-
"Authentication complete. You can return to the application. Please close this browser tab.")
372+
"Authentication complete. You can return to the application. Please close this browser tab.\n\n"
373+
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
355374
self._server.error_template = Template(error_template or
356-
# Do NOT invent new placeholders in this template. Just use standard keys defined in OAuth2 RFC.
357-
# Otherwise there is no obvious canonical way for caller to know what placeholders are supported.
358-
# Besides, we have been using these standard keys for years. Changing now would break backward compatibility.
359-
"Authentication failed. $error: $error_description. ($error_uri)")
375+
"Authentication failed. $error_message\n\n"
376+
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
360377

361378
self._server.timeout = timeout # Otherwise its handle_timeout() won't work
362379
self._server.auth_response = {} # Shared with _AuthCodeHandler
363380
self._server.auth_state = state # So handler will check it before sending response
381+
self._server.done = False # Flag to indicate completion without setting auth_response
364382
while not self._closing: # Otherwise, the handle_request() attempt
365383
# would yield noisy ValueError trace
366384
# Derived from
367385
# https://docs.python.org/2/library/basehttpserver.html#more-examples
368386
self._server.handle_request()
369-
if self._server.auth_response:
387+
if self._server.auth_response or self._server.done:
370388
break
371389
result.update(self._server.auth_response) # Return via writable result param
372390

@@ -407,6 +425,8 @@ def __exit__(self, exc_type, exc_val, exc_tb):
407425
)
408426
print(json.dumps(receiver.get_auth_response(
409427
auth_uri=flow["auth_uri"],
428+
welcome_template=
429+
"<a href='$auth_uri'>Sign In</a>, or <a href='$abort_uri'>Abort</a",
410430
error_template="<html>Oh no. $error</html>",
411431
success_template="Oh yeah. Got $code",
412432
timeout=args.timeout,

msal/oauth2cli/oauth2.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,22 @@ def _build_auth_request_params(self, response_type, **kwargs):
176176
response_type = self._stringify(response_type)
177177

178178
params = {'client_id': self.client_id, 'response_type': response_type}
179-
params.update(kwargs) # Note: None values will override params
179+
params.update(kwargs)
180+
if 'response_mode' in params:
181+
import warnings
182+
warnings.warn(
183+
"The 'response_mode' parameter is deprecated and will be removed in a future version. "
184+
"For public clients, response_mode is automatically set to 'form_post'. "
185+
"For confidential clients, configure your web framework to use 'form_post'. "
186+
"Query-based response modes are less secure.",
187+
DeprecationWarning,
188+
stacklevel=3)
189+
if params['response_mode'] != 'form_post':
190+
warnings.warn(
191+
"response_mode='form_post' is recommended for better security. "
192+
"See https://tools.ietf.org/html/rfc6749#section-4.1.2",
193+
UserWarning,
194+
stacklevel=3)
180195
params = {k: v for k, v in params.items() if v is not None} # clean up
181196
if params.get('scope'):
182197
params['scope'] = self._stringify(params['scope'])
@@ -396,21 +411,12 @@ def obtain_token_by_device_flow(self,
396411

397412
def _build_auth_request_uri(
398413
self,
399-
response_type,
400-
*,
401-
redirect_uri=None, scope=None, state=None, response_mode=None,
402-
**kwargs):
414+
response_type, redirect_uri=None, scope=None, state=None, **kwargs):
403415
if "authorization_endpoint" not in self.configuration:
404416
raise ValueError("authorization_endpoint not found in configuration")
405417
authorization_endpoint = self.configuration["authorization_endpoint"]
406-
if response_mode != 'form_post':
407-
warnings.warn(
408-
"response_mode='form_post' is recommended for better security. "
409-
"See https://www.rfc-editor.org/rfc/rfc9700.html#section-4.3.1"
410-
)
411418
params = self._build_auth_request_params(
412419
response_type, redirect_uri=redirect_uri, scope=scope, state=state,
413-
response_mode=response_mode,
414420
**kwargs)
415421
sep = '&' if '?' in authorization_endpoint else '?'
416422
return "%s%s%s" % (authorization_endpoint, sep, urlencode(params))
@@ -678,7 +684,6 @@ def _obtain_token_by_browser(
678684
flow = self.initiate_auth_code_flow(
679685
redirect_uri=redirect_uri,
680686
scope=_scope_set(scope) | _scope_set(extra_scope_to_consent),
681-
response_mode='form_post', # The auth_code_receiver has been changed to require it
682687
**(auth_params or {}))
683688
auth_response = auth_code_receiver.get_auth_response(
684689
auth_uri=flow["auth_uri"],

0 commit comments

Comments
 (0)