Merge ~suligap/canonical-identity-provider:new-style-middleware into canonical-identity-provider:master

Proposed by Przemysław Suliga
Status: Merged
Approved by: Przemysław Suliga
Approved revision: 0036cd5864416b032f552063818e5e0d6fe2f82d
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~suligap/canonical-identity-provider:new-style-middleware
Merge into: canonical-identity-provider:master
Diff against target: 153 lines (+18/-11)
9 files modified
django_project/settings_base.py (+1/-1)
django_project/settings_test.py (+1/-1)
requirements.txt (+1/-1)
src/api/v20/utils.py (+2/-1)
src/identityprovider/middleware/http.py (+2/-1)
src/identityprovider/middleware/oopsplease.py (+3/-1)
src/identityprovider/middleware/readonly.py (+4/-2)
src/identityprovider/middleware/useraccount.py (+2/-1)
src/identityprovider/tests/test_middleware.py (+2/-2)
Reviewer Review Type Date Requested Status
Hasan Ammar (community) Approve
Review via email: mp+384550@code.launchpad.net

Commit message

Switch to Django 1.10 style middleware

Description of the change

https://docs.djangoproject.com/en/1.11/topics/http/middleware/#upgrading-middleware

In most cases this is adding the MiddlewareMixin to our custom
middleware classes. Plus a version bump for django-honeypot that brings
the new style middleware compatibility.

Other middleware used (django and other 3rd party middleware) was already
compatible.

Requires
https://code.launchpad.net/~suligap/canonical-identity-provider/dependencies-django-honeypot-0.7.0/+merge/384546

To post a comment you must log in.
Revision history for this message
Hasan Ammar (hasanammar) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/django_project/settings_base.py b/django_project/settings_base.py
index 512b4d6..d131316 100644
--- a/django_project/settings_base.py
+++ b/django_project/settings_base.py
@@ -384,7 +384,7 @@ MAX_PASSWORD_RESET_TOKENS = 5
384MEDIA_ROOT = ''384MEDIA_ROOT = ''
385MEDIA_URL = ''385MEDIA_URL = ''
386MESSAGE_STORAGE = 'django.contrib.messages.storage.fallback.FallbackStorage'386MESSAGE_STORAGE = 'django.contrib.messages.storage.fallback.FallbackStorage'
387MIDDLEWARE_CLASSES = [387MIDDLEWARE = [
388 'django.middleware.security.SecurityMiddleware',388 'django.middleware.security.SecurityMiddleware',
389 ('raven.contrib.django.raven_compat.middleware'389 ('raven.contrib.django.raven_compat.middleware'
390 '.SentryResponseErrorIdMiddleware'),390 '.SentryResponseErrorIdMiddleware'),
diff --git a/django_project/settings_test.py b/django_project/settings_test.py
index b27eed7..2c57225 100644
--- a/django_project/settings_test.py
+++ b/django_project/settings_test.py
@@ -5,5 +5,5 @@ from settings_devel import * # noqa
5# https://github.com/sunlightlabs/django-honeypot/issues/125# https://github.com/sunlightlabs/django-honeypot/issues/12
6# until django-honeypot provides a way of using the middleware in tests,6# until django-honeypot provides a way of using the middleware in tests,
7# we'll remove the middleware7# we'll remove the middleware
8MIDDLEWARE_CLASSES.remove( # noqa8MIDDLEWARE.remove( # noqa
9 'identityprovider.middleware.honeypot.HoneypotMiddleware')9 'identityprovider.middleware.honeypot.HoneypotMiddleware')
diff --git a/requirements.txt b/requirements.txt
index 8c192ec..60cc1e6 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -5,7 +5,7 @@ convoy==0.4.1
5cryptography==1.3.25cryptography==1.3.2
6django==1.11.276django==1.11.27
7django-adminaudit==0.57django-adminaudit==0.5
8django-honeypot==0.6.08django-honeypot==0.7.0
9django-jsonfield==1.0.19django-jsonfield==1.0.1
10django-modeldict-yplan==1.5.210django-modeldict-yplan==1.5.2
11django-openid-auth==0.1411django-openid-auth==0.14
diff --git a/src/api/v20/utils.py b/src/api/v20/utils.py
index a90f767..1b31bf8 100644
--- a/src/api/v20/utils.py
+++ b/src/api/v20/utils.py
@@ -6,6 +6,7 @@ from operator import attrgetter
66
7from django.http import HttpResponse7from django.http import HttpResponse
8from django.urls import reverse8from django.urls import reverse
9from django.utils.deprecation import MiddlewareMixin
9from django.utils.translation import ugettext_lazy as _10from django.utils.translation import ugettext_lazy as _
1011
11from identityprovider.models.account import Account12from identityprovider.models.account import Account
@@ -15,7 +16,7 @@ from identityprovider.models.const import (
15)16)
1617
1718
18class EnsureJSONResponseOnAPIErrorMiddleware(object):19class EnsureJSONResponseOnAPIErrorMiddleware(MiddlewareMixin):
1920
20 def process_exception(self, request, exception):21 def process_exception(self, request, exception):
21 response = None22 response = None
diff --git a/src/identityprovider/middleware/http.py b/src/identityprovider/middleware/http.py
index 60df00b..0f7fcb8 100644
--- a/src/identityprovider/middleware/http.py
+++ b/src/identityprovider/middleware/http.py
@@ -3,9 +3,10 @@
33
4from django.core.exceptions import ValidationError4from django.core.exceptions import ValidationError
5from django.core.validators import validate_ipv46_address5from django.core.validators import validate_ipv46_address
6from django.utils.deprecation import MiddlewareMixin
67
78
8class SetRemoteAddrFromForwardedFor(object):9class SetRemoteAddrFromForwardedFor(MiddlewareMixin):
9 """Set REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, if the latter is present.10 """Set REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, if the latter is present.
1011
11 This is useful if you're sitting behind a reverse proxy that causes each12 This is useful if you're sitting behind a reverse proxy that causes each
diff --git a/src/identityprovider/middleware/oopsplease.py b/src/identityprovider/middleware/oopsplease.py
index f1eb56e..e8c7a1a 100644
--- a/src/identityprovider/middleware/oopsplease.py
+++ b/src/identityprovider/middleware/oopsplease.py
@@ -2,13 +2,15 @@
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
3import sys3import sys
44
5from django.utils.deprecation import MiddlewareMixin
6
5from identityprovider.oops_config import (7from identityprovider.oops_config import (
6 ManuallyGeneratedOops,8 ManuallyGeneratedOops,
7 global_error_reporter,9 global_error_reporter,
8)10)
911
1012
11class OopsPleaseMiddleware(object):13class OopsPleaseMiddleware(MiddlewareMixin):
1214
13 QUERY_STRING_KEY = 'oopsplease'15 QUERY_STRING_KEY = 'oopsplease'
1416
diff --git a/src/identityprovider/middleware/readonly.py b/src/identityprovider/middleware/readonly.py
index 34edf99..d17712c 100644
--- a/src/identityprovider/middleware/readonly.py
+++ b/src/identityprovider/middleware/readonly.py
@@ -1,7 +1,9 @@
1from django.utils.deprecation import MiddlewareMixin
2
1from identityprovider.readonly import ReadOnlyManager3from identityprovider.readonly import ReadOnlyManager
24
35
4class ReadOnlyMiddleware(object):6class ReadOnlyMiddleware(MiddlewareMixin):
5 """Check for the existance of readonly flag on each requst"""7 """Check for the existance of readonly flag on each request"""
6 def process_request(self, request):8 def process_request(self, request):
7 ReadOnlyManager().check_readonly()9 ReadOnlyManager().check_readonly()
diff --git a/src/identityprovider/middleware/useraccount.py b/src/identityprovider/middleware/useraccount.py
index 61480b0..03c63a2 100644
--- a/src/identityprovider/middleware/useraccount.py
+++ b/src/identityprovider/middleware/useraccount.py
@@ -4,13 +4,14 @@
4from django.conf import settings4from django.conf import settings
5from django.contrib.auth import logout5from django.contrib.auth import logout
6from django.contrib.auth.models import User6from django.contrib.auth.models import User
7from django.utils.deprecation import MiddlewareMixin
7from django.utils.translation import LANGUAGE_SESSION_KEY8from django.utils.translation import LANGUAGE_SESSION_KEY
89
9from identityprovider.const import SESSION_TOKEN_KEY10from identityprovider.const import SESSION_TOKEN_KEY
10from identityprovider.models import Account11from identityprovider.models import Account
1112
1213
13class UserAccountConversionMiddleware(object):14class UserAccountConversionMiddleware(MiddlewareMixin):
14 """Try to convert request.user to the corresponding Account objects.15 """Try to convert request.user to the corresponding Account objects.
1516
16 If it's a User object, and the request is outside of the admin, and vice17 If it's a User object, and the request is outside of the admin, and vice
diff --git a/src/identityprovider/tests/test_middleware.py b/src/identityprovider/tests/test_middleware.py
index 3d3775f..6a5a6fa 100644
--- a/src/identityprovider/tests/test_middleware.py
+++ b/src/identityprovider/tests/test_middleware.py
@@ -470,7 +470,7 @@ urlpatterns = original_patterns + [
470]470]
471471
472472
473@modify_settings(MIDDLEWARE_CLASSES={473@modify_settings(MIDDLEWARE={
474 'append': 'identityprovider.middleware.honeypot.HoneypotMiddleware'474 'append': 'identityprovider.middleware.honeypot.HoneypotMiddleware'
475})475})
476@override_settings(ROOT_URLCONF='identityprovider.tests.test_middleware')476@override_settings(ROOT_URLCONF='identityprovider.tests.test_middleware')
@@ -519,7 +519,7 @@ class HoneypotEnabledTestCase(TestCase):
519 self.assertContains(response, "<!DOCTYPE html>")519 self.assertContains(response, "<!DOCTYPE html>")
520520
521521
522@modify_settings(MIDDLEWARE_CLASSES={522@modify_settings(MIDDLEWARE={
523 'append': 'identityprovider.middleware.honeypot.HoneypotMiddleware'523 'append': 'identityprovider.middleware.honeypot.HoneypotMiddleware'
524})524})
525class HoneypotWhitelistedTestCase(SSOBaseTestCase):525class HoneypotWhitelistedTestCase(SSOBaseTestCase):

Subscribers

People subscribed via source and target branches