Canonical SSO provider

Merge lp:~maxiberta/canonical-identity-provider/drop_honeypot_from_delete_account into lp:canonical-identity-provider/release

  1. drop_honeypot_from_delete_account
  2. Merge into trunk
Proposed by Maximiliano Bertacchini
Status: Merged
Approved by: Maximiliano Bertacchini
Approved revision: no longer in the source branch.
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~maxiberta/canonical-identity-provider/drop_honeypot_from_delete_account
Merge into: lp:canonical-identity-provider/release
Diff against target: 56 lines (+19/-3)
2 files modified
src/identityprovider/middleware/honeypot.py (+1/-1)
src/identityprovider/tests/test_middleware.py (+18/-2)
To merge this branch: bzr merge lp:~maxiberta/canonical-identity-provider/drop_honeypot_from_delete_account
Reviewer Review Type Date Requested Status
Daniel Manrique (community) Approve
Review via email: mp+369777@code.launchpad.net

Commit message

Exclude the delete account view from honeypot checks.

Description of the change

This fixes account deletion on browsers with overeager/buggy password managers (such as Chrome/Chromium). The web form still includes the honeypot field ("openid.usernamesecret") to help reproduce the issue, but it's ignored server-side so that the processing can proceed regardless.

The affected URL: /+delete.

Additionally, fixed `HoneypotWhitelistedTestCase`'s setup to actually use the honeypot middleware, otherwise tests would pass unconditionally.

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

LGTM, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/identityprovider/middleware/honeypot.py'
--- src/identityprovider/middleware/honeypot.py 2018-08-02 15:51:07 +0000
+++ src/identityprovider/middleware/honeypot.py 2019-07-05 15:47:40 +0000
@@ -15,7 +15,7 @@
15)15)
1616
17HONEYPOT_EXEMPT_URLS = re.compile(17HONEYPOT_EXEMPT_URLS = re.compile(
18 '^/api/|^/[A-Za-z_]+/\+openid|^/\+openid|^/\+saml|^/saml')18 '^/api/|^/[A-Za-z_]+/\+openid|^/\+openid|^/\+saml|^/saml|^/\+delete$')
1919
2020
21class HoneypotMiddleware(upstream_honeypot_middleware):21class HoneypotMiddleware(upstream_honeypot_middleware):
2222
=== modified file 'src/identityprovider/tests/test_middleware.py'
--- src/identityprovider/tests/test_middleware.py 2018-08-02 16:26:42 +0000
+++ src/identityprovider/tests/test_middleware.py 2019-07-05 15:47:40 +0000
@@ -20,7 +20,7 @@
20from identityprovider.middleware.useraccount import (20from identityprovider.middleware.useraccount import (
21 UserAccountConversionMiddleware)21 UserAccountConversionMiddleware)
22from identityprovider.models import Account, OpenIDRPConfig22from identityprovider.models import Account, OpenIDRPConfig
23from identityprovider.models.const import TokenScope23from identityprovider.models.const import AccountStatus, TokenScope
24from identityprovider.oops_config import ErrorReporter24from identityprovider.oops_config import ErrorReporter
25from identityprovider.tests import DEFAULT_USER_PASSWORD25from identityprovider.tests import DEFAULT_USER_PASSWORD
26from identityprovider.tests.utils import SSOBaseTestCase26from identityprovider.tests.utils import SSOBaseTestCase
@@ -519,7 +519,10 @@
519 self.assertContains(response, "<!DOCTYPE html>")519 self.assertContains(response, "<!DOCTYPE html>")
520520
521521
522class HoneypotWhitelistedTestCase(TestCase):522@modify_settings(MIDDLEWARE_CLASSES={
523 'append': 'identityprovider.middleware.honeypot.HoneypotMiddleware'
524})
525class HoneypotWhitelistedTestCase(SSOBaseTestCase):
523526
524 def assert_honeypot_not_required(self, url, **kwargs):527 def assert_honeypot_not_required(self, url, **kwargs):
525 response = self.client.post(url, **kwargs)528 response = self.client.post(url, **kwargs)
@@ -555,6 +558,19 @@
555 self.assert_honeypot_not_required(558 self.assert_honeypot_not_required(
556 url, data={'SAMLRequest': 'foo', 'RelayState': 'bar'})559 url, data={'SAMLRequest': 'foo', 'RelayState': 'bar'})
557560
561 def test_post_to_delete_account_not_required_to_have_honeypot(self):
562 account = self.factory.make_account()
563 login_successful = self.client.login(
564 username=account.user.email, password=DEFAULT_USER_PASSWORD)
565 assert login_successful
566
567 response = self.client.post(
568 '/+delete', {'password': DEFAULT_USER_PASSWORD})
569
570 self.assertNotContains(response, 'Bad bot')
571 account.refresh_from_db()
572 self.assertEqual(account.status, AccountStatus.DELETED)
573
558574
559class SetRemoteAddrFromForwardedForMiddlewareTestCase(SSOBaseTestCase):575class SetRemoteAddrFromForwardedForMiddlewareTestCase(SSOBaseTestCase):
560576