Remove GET method for OAuth2 access token retrieval (#1472)

* Remove GET method for OAuth2 access token retrieval

* Update OAuth2 Admin form

misago/conf/admin/forms/oauth2.py

@@ -22,7 +22,6 @@ class ChangeOAuth2SettingsForm(ChangeSettingsForm):
         "oauth2_scopes",
         "oauth2_login_url",
         "oauth2_token_url",
-        "oauth2_token_method",
         "oauth2_token_extra_headers",
         "oauth2_json_token_path",
         "oauth2_user_url",
@@ -90,14 +89,6 @@ class ChangeOAuth2SettingsForm(ChangeSettingsForm):
         max_length=500,
         required=False,
     )
-    oauth2_token_method = forms.ChoiceField(
-        label=_("Request method"),
-        choices=[
-            ("POST", "POST"),
-            ("GET", "GET"),
-        ],
-        widget=forms.RadioSelect(),
-    )
     oauth2_token_extra_headers = forms.CharField(
         label=_("Extra HTTP headers in token request"),
         help_text=_(

misago/conf/admin/tests/test_change_oauth2_settings.py

@@ -14,7 +14,6 @@ def test_oauth2_can_be_enabled(admin_client):
             "oauth2_scopes": "some scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -41,7 +40,6 @@ def test_oauth2_can_be_enabled(admin_client):
     assert settings["oauth2_scopes"] == "some scope"
     assert settings["oauth2_login_url"] == "https://example.com/login/"
     assert settings["oauth2_token_url"] == "https://example.com/token/"
-    assert settings["oauth2_token_method"] == "POST"
     assert settings["oauth2_token_extra_headers"] == ""
     assert settings["oauth2_json_token_path"] == "access_token"
     assert settings["oauth2_user_url"] == "https://example.com/user/"
@@ -67,7 +65,6 @@ def test_oauth2_can_be_enabled_without_avatar(admin_client):
             "oauth2_scopes": "some scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -94,7 +91,6 @@ def test_oauth2_can_be_enabled_without_avatar(admin_client):
     assert settings["oauth2_scopes"] == "some scope"
     assert settings["oauth2_login_url"] == "https://example.com/login/"
     assert settings["oauth2_token_url"] == "https://example.com/token/"
-    assert settings["oauth2_token_method"] == "POST"
     assert settings["oauth2_token_extra_headers"] == ""
     assert settings["oauth2_json_token_path"] == "access_token"
     assert settings["oauth2_user_url"] == "https://example.com/user/"
@@ -118,7 +114,6 @@ def test_oauth2_cant_be_enabled_with_some_value_missing(admin_client):
         "oauth2_scopes": "some scope",
         "oauth2_login_url": "https://example.com/login/",
         "oauth2_token_url": "https://example.com/token/",
-        "oauth2_token_method": "POST",
         "oauth2_token_extra_headers": "",
         "oauth2_json_token_path": "access_token",
         "oauth2_user_url": "https://example.com/user/",
@@ -136,7 +131,6 @@ def test_oauth2_cant_be_enabled_with_some_value_missing(admin_client):
     skip_settings = (
         "enable_oauth2_client",
         "oauth2_json_avatar_path",
-        "oauth2_token_method",
         "oauth2_token_extra_headers",
         "oauth2_user_method",
         "oauth2_user_token_location",
@@ -207,7 +201,6 @@ def test_oauth2_scopes_are_normalized(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -238,7 +231,6 @@ def test_oauth2_extra_token_headers_are_normalized(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": ("Lorem:   ipsum\n   Dolor: Met-elit"),
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -270,7 +262,6 @@ def test_oauth2_extra_token_headers_are_validated(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": (
                 "Lorem:   ipsum\n   Dolor-amet\n Dolor: Met-elit"
             ),
@@ -301,7 +292,6 @@ def test_oauth2_extra_user_headers_are_normalized(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -333,7 +323,6 @@ def test_oauth2_extra_user_headers_are_validated(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -362,7 +351,6 @@ def test_oauth2_extra_headers_are_validated_to_have_colons(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -391,7 +379,6 @@ def test_oauth2_extra_headers_are_validated_to_have_names(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -423,7 +410,6 @@ def test_oauth2_extra_headers_are_validated_to_have_values(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",
@@ -455,7 +441,6 @@ def test_oauth2_extra_headers_are_validated_to_be_unique(admin_client):
             "oauth2_scopes": "some some    scope",
             "oauth2_login_url": "https://example.com/login/",
             "oauth2_token_url": "https://example.com/token/",
-            "oauth2_token_method": "POST",
             "oauth2_token_extra_headers": "",
             "oauth2_json_token_path": "access_token",
             "oauth2_user_url": "https://example.com/user/",

misago/conf/migrations/0009_delete_oauth2_access_token_method.py

@@ -0,0 +1,15 @@
+from django.db import migrations
+
+
+def delete_oauth2_token_method_setting(apps, _):
+    Setting = apps.get_model("misago_conf", "Setting")
+    Setting.objects.filter(setting="oauth2_token_method").delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("misago_conf", "0008_delete_sso_settings"),
+    ]
+
+    operations = [migrations.RunPython(delete_oauth2_token_method_setting)]

misago/oauth2/client.py

@@ -61,21 +61,12 @@ def get_access_token(request, code_grant):
     headers = get_headers_dict(request.settings.oauth2_token_extra_headers)
 
     try:
-        if request.settings.oauth2_token_method == "GET":
-            token_url += "&" if "?" in token_url else "?"
-            token_url += urlencode(data)
-            r = requests.get(
-                token_url,
-                headers=headers,
-                timeout=REQUESTS_TIMEOUT,
-            )
-        else:
-            r = requests.post(
-                token_url,
-                data=data,
-                headers=headers,
-                timeout=REQUESTS_TIMEOUT,
-            )
+        r = requests.post(
+            token_url,
+            data=data,
+            headers=headers,
+            timeout=REQUESTS_TIMEOUT,
+        )
     except RequestException:
         raise exceptions.OAuth2AccessTokenRequestError()
 

misago/oauth2/tests/test_get_access_token.py

@@ -23,79 +23,6 @@ def mock_request(dynamic_settings):
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="GET",
-)
-def test_access_token_is_retrieved_using_get_request(mock_request):
-    get_mock = Mock(
-        return_value=Mock(
-            status_code=200,
-            json=Mock(
-                return_value={
-                    "access_token": ACCESS_TOKEN,
-                },
-            ),
-        ),
-    )
-
-    with patch("requests.get", get_mock):
-        assert get_access_token(mock_request, CODE_GRANT) == ACCESS_TOKEN
-
-        get_mock.assert_called_once_with(
-            (
-                "https://example.com/oauth2/token"
-                "?grant_type=authorization_code"
-                "&client_id=clientid123"
-                "&client_secret=secr3t"
-                "&redirect_uri=http%3A%2F%2Fmysite.com%2Foauth2%2Fcomplete%2F"
-                "&code=valid-code"
-            ),
-            headers={},
-            timeout=REQUESTS_TIMEOUT,
-        )
-
-
-@override_dynamic_settings(
-    oauth2_client_id="clientid123",
-    oauth2_client_secret="secr3t",
-    oauth2_token_url="https://example.com/oauth2/token?exchange=1",
-    oauth2_token_method="GET",
-)
-def test_access_token_get_request_url_respects_existing_querystring(
-    mock_request,
-):
-    get_mock = Mock(
-        return_value=Mock(
-            status_code=200,
-            json=Mock(
-                return_value={
-                    "access_token": ACCESS_TOKEN,
-                },
-            ),
-        ),
-    )
-
-    with patch("requests.get", get_mock):
-        assert get_access_token(mock_request, CODE_GRANT) == ACCESS_TOKEN
-
-        get_mock.assert_called_once_with(
-            (
-                "https://example.com/oauth2/token?exchange=1"
-                "&grant_type=authorization_code"
-                "&client_id=clientid123"
-                "&client_secret=secr3t"
-                "&redirect_uri=http%3A%2F%2Fmysite.com%2Foauth2%2Fcomplete%2F"
-                "&code=valid-code"
-            ),
-            headers={},
-            timeout=REQUESTS_TIMEOUT,
-        )
-
-
-@override_dynamic_settings(
-    oauth2_client_id="clientid123",
-    oauth2_client_secret="secr3t",
-    oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_access_token_is_retrieved_using_post_request(mock_request):
     post_mock = Mock(
@@ -130,7 +57,6 @@ def test_access_token_is_retrieved_using_post_request(mock_request):
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
     oauth2_token_extra_headers="Accept: application/json\nApi-Ver:1234",
 )
 def test_access_token_is_retrieved_using_extra_headers(mock_request):
@@ -169,7 +95,6 @@ def test_access_token_is_retrieved_using_extra_headers(mock_request):
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
     oauth2_json_token_path="data.auth.token",
 )
 def test_access_token_is_extracted_from_json(mock_request):
@@ -198,7 +123,6 @@ def test_access_token_is_extracted_from_json(mock_request):
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_exception_is_raised_if_access_token_request_times_out(mock_request):
     post_mock = Mock(side_effect=Timeout())
@@ -214,7 +138,6 @@ def test_exception_is_raised_if_access_token_request_times_out(mock_request):
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_exception_is_raised_if_access_token_request_response_is_not_200(mock_request):
     post_mock = Mock(
@@ -234,7 +157,6 @@ def test_exception_is_raised_if_access_token_request_response_is_not_200(mock_re
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_exception_is_raised_if_access_token_request_response_is_not_json(mock_request):
     post_mock = Mock(
@@ -257,7 +179,6 @@ def test_exception_is_raised_if_access_token_request_response_is_not_json(mock_r
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_exception_is_raised_if_access_token_request_response_json_is_not_object(
     mock_request,
@@ -282,7 +203,6 @@ def test_exception_is_raised_if_access_token_request_response_json_is_not_object
     oauth2_client_id="clientid123",
     oauth2_client_secret="secr3t",
     oauth2_token_url="https://example.com/oauth2/token",
-    oauth2_token_method="POST",
 )
 def test_exception_is_raised_if_access_token_request_response_json_misses_token(
     mock_request,

misago/oauth2/tests/test_oauth2_complete_view.py

@@ -133,7 +133,6 @@ TEST_SETTINGS = {
     "oauth2_client_secret": "oauth2_client_secret",
     "oauth2_login_url": "https://example.com/oauth2/login",
     "oauth2_token_url": "https://example.com/oauth2/token",
-    "oauth2_token_method": "POST",
     "oauth2_json_token_path": "token.bearer",
     "oauth2_user_url": "https://example.com/oauth2/user",
     "oauth2_user_method": "POST",

misago/templates/misago/admin/conf/oauth2_settings.html

@@ -28,7 +28,6 @@
     <legend>{% trans "Retrieving access token" %}</legend>
 
     {% form_row form.oauth2_token_url %}
-    {% form_row form.oauth2_token_method %}
     {% form_row form.oauth2_token_extra_headers %}
     {% form_row form.oauth2_json_token_path %}