Skip to content

Commit

Permalink
Revert oauthlib behaviour
Browse files Browse the repository at this point in the history
Previously, a 401 was returned when invalid credentials were used with the Resource Owner Password Credentials Grant flow.
See oauthlib/oauthlib#264 and oauthlib/oauthlib#619
Versions 3+ return 400 making it tougher to distinguish between incorrect passwords and other errors.
This patch reverts the status code back to 401, but only in the case where credentials were incorrect.
  • Loading branch information
ushkarev committed Nov 11, 2021
1 parent 53bab93 commit 89a3079
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 0 deletions.
36 changes: 36 additions & 0 deletions mtp_api/apps/mtp_auth/patches.py
@@ -0,0 +1,36 @@
import json

from oauth2_provider.views import TokenView


class ModifiedTokenView(TokenView):
"""
Patches responses to be like those returned by `oauthlib` prior to version 3.
Previously, a 401 was returned when invalid credentials were used with
the Resource Owner Password Credentials Grant flow.
See https://github.com/oauthlib/oauthlib/issues/264 and https://github.com/oauthlib/oauthlib/issues/619
Versions 3+ return 400 making it tougher to distinguish between incorrect passwords and other errors.
This patch reverts the status code back to 401, but only in the case where credentials were incorrect.
"""
# Sample of a true 400 response:
# {"error": "unsupported_grant_type"}
# Sample of a true 403 response (e.g. when request and credentials are valid, but client does not match):
# {"error": "restricted_client"}
# Sample of a response that will be reverted to 401
# {"error": "invalid_grant", "error_description": "Invalid credentials given."}
def post(self, request, *args, **kwargs):
response = super().post(request, *args, **kwargs)
if response.status_code == 400:
response_content = json.loads(response.content)
if (
response_content.get('error') == 'invalid_grant' and
'Invalid credentials given' in response_content.get('error_description')
):
response.status_code = 401
return response


def patch_oauth2_provider_token_view():
import oauth2_provider.views

oauth2_provider.views.TokenView = ModifiedTokenView
55 changes: 55 additions & 0 deletions mtp_api/apps/mtp_auth/tests/test_patches.py
@@ -0,0 +1,55 @@
import logging
from unittest import mock

from django.urls import reverse
from mtp_common.test_utils import silence_logger
from oauth2_provider.models import Application
from oauthlib.oauth2 import InvalidRequestError, UnsupportedGrantTypeError

from core.tests.utils import make_test_users
from mtp_auth.constants import CASHBOOK_OAUTH_CLIENT_ID
from mtp_auth.tests.test_views import AuthBaseTestCase


class OauthTokenRequestPatchTestCase(AuthBaseTestCase):
def setUp(self):
super().setUp()
self.user = make_test_users(clerks_per_prison=1, num_security_fiu_users=0)['prison_clerks'][0]
self.cashbook_client = Application.objects.get(client_id=CASHBOOK_OAUTH_CLIENT_ID)

def try_login(self):
with silence_logger('django.request', level=logging.ERROR):
return self.client.post(
reverse('oauth2_provider:token'),
{
# this would be successful unless mocked
'grant_type': 'password',
'username': self.user.username,
'password': self.user.username,
'client_id': self.cashbook_client.client_id,
'client_secret': self.cashbook_client.client_secret,
}
)

@mock.patch('mtp_auth.validators.ApplicationRequestValidator.validate_user')
def test_invalid_grant_with_invalid_credentials_401(self, mocked_validate_user):
# pretends that username/password were wrong: status code SHOULD be modified back to 401

mocked_validate_user.return_value = False
response = self.try_login()
self.assertEqual(response.status_code, 401)
response_data = response.json()
self.assertEqual(response_data['error'], 'invalid_grant')
self.assertIn('Invalid credentials', response_data['error_description'])

@mock.patch('oauthlib.oauth2.ResourceOwnerPasswordCredentialsGrant.validate_token_request')
def test_invalid_grant_with_other_problem_still_400(self, mocked_token_request):
# pretends that request was malformed: status code should NOT be modified remaining 400

mocked_token_request.side_effect = UnsupportedGrantTypeError()
response = self.try_login()
self.assertEqual(response.status_code, 400)

mocked_token_request.side_effect = InvalidRequestError('Request is missing client_secret parameter.')
response = self.try_login()
self.assertEqual(response.status_code, 400)
3 changes: 3 additions & 0 deletions mtp_api/urls.py
Expand Up @@ -7,8 +7,11 @@
from moj_irat.views import HealthcheckView, PingJsonView
from mtp_common.metrics.views import metrics_view

from mtp_auth.patches import patch_oauth2_provider_token_view
from .views import schema_view

patch_oauth2_provider_token_view()


urlpatterns = [
url(r'^', include('prison.urls')),
Expand Down

0 comments on commit 89a3079

Please sign in to comment.