Skip to content

Commit a2ac365

Browse files
committed
Deprecate response_mode parameter, better messages and more robust test
1 parent 908c84b commit a2ac365

File tree

4 files changed

+112
-90
lines changed

4 files changed

+112
-90
lines changed

msal/application.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ def __init__(
258258
client_credential=None, authority=None, validate_authority=True,
259259
token_cache=None,
260260
http_client=None,
261-
verify=True, proxies=None, timeout=None,
261+
verify=False, proxies=None, timeout=None,
262262
client_claims=None, app_name=None, app_version=None,
263263
client_capabilities=None,
264264
azure_region=None, # Note: We choose to add this param in this base class,
@@ -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,18 +963,10 @@ 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 is 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>`
966+
.. deprecated::
967+
This parameter is deprecated and will be removed in a future version.
968+
The response_mode is always set to ``form_post`` for security reasons,
969+
regardless of the value provided. Do not use this parameter.
975970
976971
:return:
977972
The auth code flow. It is a dict in this form::
@@ -991,6 +986,13 @@ def initiate_auth_code_flow(
991986
3. and then relay this dict and subsequent auth response to
992987
:func:`~acquire_token_by_auth_code_flow()`.
993988
"""
989+
if response_mode is not None:
990+
import warnings
991+
warnings.warn(
992+
"The 'response_mode' parameter is deprecated and will be removed in a future version. "
993+
"Response mode is always 'form_post' for security reasons.",
994+
DeprecationWarning,
995+
stacklevel=2)
994996
client = _ClientWithCcsRoutingInfo(
995997
{"authorization_endpoint": self.authority.authorization_endpoint},
996998
self.client_id,

msal/oauth2cli/authcode.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,11 @@ def _get_auth_response(self, result, auth_uri=None, timeout=None, state=None,
347347
auth_uri_callback(_uri)
348348

349349
self._server.success_template = Template(success_template or
350-
"Authentication completed. You can close this window now.")
350+
"Authentication complete. You can return to the application. Please close this browser tab. "
351+
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
351352
self._server.error_template = Template(error_template or
352-
"Authentication failed. $error: $error_description. ($error_uri)")
353+
"Authentication failed. $error: $error_description. ($error_uri) "
354+
"For your security: Do not share the contents of this page, the address bar, or take screenshots.")
353355

354356
self._server.timeout = timeout # Otherwise its handle_timeout() won't work
355357
self._server.auth_response = {} # Shared with _AuthCodeHandler

msal/oauth2cli/oauth2.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,7 @@ def _build_auth_request_params(self, response_type, **kwargs):
181181
if 'response_mode' in kwargs and kwargs['response_mode'] != 'form_post':
182182
import warnings
183183
warnings.warn(
184-
"response_mode='{}' is not supported for security reasons. "
185-
"Using form_post instead. Query string transmission of authorization "
186-
"codes is insecure and has been disabled.".format(kwargs['response_mode']),
184+
"The 'response_mode' parameter will be overridden to 'form_post' for better security.",
187185
UserWarning)
188186
params.update({k: v for k, v in kwargs.items() if k != 'response_mode'}) # Exclude response_mode from kwargs
189187
params = {k: v for k, v in params.items() if v is not None} # clean up
@@ -476,6 +474,13 @@ def initiate_auth_code_flow(
476474
3. and then relay this dict and subsequent auth response to
477475
:func:`~obtain_token_by_auth_code_flow()`.
478476
"""
477+
if "response_mode" in kwargs:
478+
import warnings
479+
warnings.warn(
480+
"The 'response_mode' parameter is deprecated and will be removed in a future version. "
481+
"Response mode is always 'form_post' for security reasons.",
482+
DeprecationWarning,
483+
stacklevel=2)
479484
response_type = kwargs.pop("response_type", "code") # Auth Code flow
480485
# Must be "code" when you are using Authorization Code Grant.
481486
# The "token" for Implicit Grant is not applicable thus not allowed.

tests/test_response_mode.py

Lines changed: 85 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Tests for form_post response mode in authorization code flow"""
1+
"""Integration tests for form_post response mode in authorization code flow"""
22
import unittest
33
import warnings
44
try:
@@ -7,10 +7,12 @@
77
from urlparse import urlparse, parse_qs
88

99
from msal.oauth2cli import Client
10+
from msal.oauth2cli.authcode import AuthCodeReceiver
11+
import requests
1012

1113

12-
class TestResponseMode(unittest.TestCase):
13-
"""Test response_mode parameter in authorization code flow"""
14+
class TestResponseModeIntegration(unittest.TestCase):
15+
"""Integration test for response_mode with end-to-end authentication flow"""
1416

1517
def setUp(self):
1618
self.client = Client(
@@ -20,92 +22,103 @@ def setUp(self):
2022
},
2123
"test_client_id"
2224
)
23-
24-
def test_default_response_mode_is_form_post(self):
25-
"""Test that response_mode defaults to form_post for security"""
26-
flow = self.client.initiate_auth_code_flow(
27-
scope=["openid", "profile"],
28-
redirect_uri="http://localhost:8080"
29-
)
30-
31-
# Parse the auth_uri to check query parameters
32-
parsed = urlparse(flow["auth_uri"])
33-
params = parse_qs(parsed.query)
25+
26+
def _send_post_auth_response(self, port, **params):
27+
"""Helper to send POST auth response to the receiver"""
28+
try:
29+
from urllib.parse import urlencode
30+
except ImportError:
31+
from urllib import urlencode
3432

35-
# Verify response_mode is set to form_post
36-
self.assertIn("response_mode", params)
37-
self.assertEqual(params["response_mode"][0], "form_post")
33+
response = requests.post(
34+
"http://localhost:{}".format(port),
35+
data=urlencode(params),
36+
headers={"Content-Type": "application/x-www-form-urlencoded"}
37+
)
38+
return response
3839

39-
def test_explicit_query_mode_shows_security_warning(self):
40-
"""Test that attempting to use query mode raises a security warning"""
40+
def test_query_mode_override_with_successful_post_authentication(self):
41+
"""
42+
Comprehensive integration test: When response_mode='query' is set,
43+
verify deprecation warning, override to form_post, and successful POST authentication.
44+
"""
45+
# Part 1: Verify deprecation and override warnings
4146
with warnings.catch_warnings(record=True) as w:
4247
warnings.simplefilter("always")
4348
flow = self.client.initiate_auth_code_flow(
4449
scope=["openid", "profile"],
4550
redirect_uri="http://localhost:8080",
46-
response_mode="query"
51+
response_mode="query" # Explicitly request query mode (should be overridden)
4752
)
4853

49-
# Verify a warning was raised
50-
self.assertEqual(len(w), 1)
51-
self.assertTrue(issubclass(w[0].category, Warning))
52-
self.assertIn("security", str(w[0].message).lower())
53-
54-
# Verify form_post is still enforced despite explicit query request
55-
parsed = urlparse(flow["auth_uri"])
56-
params = parse_qs(parsed.query)
57-
self.assertEqual(params["response_mode"][0], "form_post")
58-
59-
def test_explicit_fragment_mode_shows_security_warning(self):
60-
"""Test that attempting to use fragment mode raises a security warning"""
61-
with warnings.catch_warnings(record=True) as w:
62-
warnings.simplefilter("always")
63-
flow = self.client.initiate_auth_code_flow(
64-
scope=["openid", "profile"],
65-
redirect_uri="http://localhost:8080",
66-
response_mode="fragment"
67-
)
54+
# Verify deprecation warning
55+
deprecation_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)]
56+
self.assertEqual(len(deprecation_warnings), 1, "Should have exactly one deprecation warning")
57+
self.assertIn("deprecated", str(deprecation_warnings[0].message).lower())
6858

69-
# Verify a warning was raised
70-
self.assertEqual(len(w), 1)
71-
self.assertIn("fragment", str(w[0].message))
59+
# Verify override warning
60+
user_warnings = [x for x in w if issubclass(x.category, UserWarning)]
61+
self.assertEqual(len(user_warnings), 1, "Should have exactly one override warning")
62+
self.assertIn("overridden", str(user_warnings[0].message).lower())
7263

73-
# Verify form_post is still enforced
64+
# Part 2: Verify form_post is enforced in auth_uri
7465
parsed = urlparse(flow["auth_uri"])
7566
params = parse_qs(parsed.query)
76-
self.assertEqual(params["response_mode"][0], "form_post")
77-
78-
def test_build_auth_request_params_enforces_form_post(self):
79-
"""Test that _build_auth_request_params enforces form_post"""
80-
params = self.client._build_auth_request_params(
81-
response_type="code",
82-
redirect_uri="http://localhost:8080",
83-
scope=["openid", "profile"],
84-
state="test_state"
85-
)
86-
87-
# Verify response_mode is form_post
8867
self.assertIn("response_mode", params)
89-
self.assertEqual(params["response_mode"], "form_post")
68+
self.assertEqual(params["response_mode"][0], "form_post",
69+
"response_mode should be overridden to form_post")
70+
71+
# Part 3: Verify actual POST authentication works (form_post flow)
72+
with AuthCodeReceiver() as receiver:
73+
test_code = "test_auth_code_xyz"
74+
test_state = "test_state_abc"
75+
76+
receiver._scheduled_actions = [(
77+
1,
78+
lambda: self._send_post_auth_response(
79+
receiver.get_port(),
80+
code=test_code,
81+
state=test_state
82+
)
83+
)]
84+
85+
result = receiver.get_auth_response(
86+
timeout=3,
87+
state=test_state,
88+
success_template="Success: Got code $code",
89+
)
90+
91+
# Verify the POST request succeeded
92+
self.assertIsNotNone(result, "POST authentication should succeed")
93+
self.assertEqual(result.get("code"), test_code,
94+
"Should receive auth code via POST (form_post)")
95+
self.assertEqual(result.get("state"), test_state,
96+
"Should receive correct state via POST")
9097

91-
def test_build_auth_request_params_ignores_explicit_query_mode(self):
92-
"""Test that _build_auth_request_params ignores explicit query mode"""
93-
with warnings.catch_warnings(record=True) as w:
94-
warnings.simplefilter("always")
95-
params = self.client._build_auth_request_params(
96-
response_type="code",
97-
redirect_uri="http://localhost:8080",
98-
scope=["openid", "profile"],
99-
state="test_state",
100-
response_mode="query"
98+
def test_query_mode_get_request_rejected(self):
99+
"""
100+
Verify that GET requests with auth code are rejected when form_post is enforced.
101+
"""
102+
with AuthCodeReceiver() as receiver:
103+
test_code = "test_auth_code_via_get"
104+
105+
# Try to send auth code via GET (query string)
106+
try:
107+
from urllib.parse import urlencode
108+
except ImportError:
109+
from urllib import urlencode
110+
111+
response = requests.get(
112+
"http://localhost:{}?{}".format(
113+
receiver.get_port(),
114+
urlencode({"code": test_code, "state": "test"})
115+
)
101116
)
102117

103-
# Verify warning was raised
104-
self.assertGreater(len(w), 0)
105-
106-
# Verify form_post is enforced despite explicit request
107-
self.assertIn("response_mode", params)
108-
self.assertEqual(params["response_mode"], "form_post")
118+
# Verify the GET request is rejected
119+
self.assertEqual(response.status_code, 400, "GET with auth code should be rejected")
120+
self.assertIn("not supported", response.text.lower(),
121+
"Error message should indicate method not supported")
109122

110123

111124
if __name__ == '__main__':

0 commit comments

Comments
 (0)