Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new guard clause in editor_compare_hashes helper function #940

Merged
merged 2 commits into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 16 additions & 9 deletions TWLight/users/helpers/editor_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ class BlockHashChangedEmail(template_mail.TemplateMail):


def editor_compare_hashes(
previous_block_hash: str, new_block_string: str, wp_username: str
previous_block_hash: str,
new_block_string: str,
wp_username: str,
ignore_wp_blocks: bool,
):
"""
Compares two block hashes. If they are different, it means an editor's block status
Expand All @@ -265,6 +268,8 @@ def editor_compare_hashes(
The recently generated block dictionary without hashing.
wp_username: str
The Wikipedia username of the editor we are comparing block hashes from
ignore_wp_blocks: bool
Flag that indicates whether a block is overruled for access to the library

Returns
-------
Expand All @@ -274,14 +279,16 @@ def editor_compare_hashes(
"""
if previous_block_hash != "":
if not check_password(new_block_string, previous_block_hash):
# Send email to TWL team
email = BlockHashChangedEmail()
email.send(
os.environ.get(
"TWLIGHT_ERROR_MAILTO", "wikipedialibrary@wikimedia.org"
),
{"wp_username": wp_username},
)
# Hash does not match, checking if a user has the block override
if ignore_wp_blocks:
# Send email to TWL team
email = BlockHashChangedEmail()
email.send(
os.environ.get(
"TWLIGHT_ERROR_MAILTO", "wikipedialibrary@wikimedia.org"
),
{"wp_username": wp_username},
)

return make_password(new_block_string)
else:
Expand Down
5 changes: 4 additions & 1 deletion TWLight/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,10 @@ def update_from_wikipedia(
previous_block_hash = self.wp_block_hash
blocked_dict = editor_make_block_dict(global_userinfo["merged"])
self.wp_block_hash = editor_compare_hashes(
previous_block_hash, blocked_dict, self.wp_username
previous_block_hash,
blocked_dict,
self.wp_username,
self.ignore_wp_blocks,
)

# if the account is already old enough, we shouldn't run this check everytime
Expand Down
74 changes: 71 additions & 3 deletions TWLight/users/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
editor_reg_date,
editor_bundle_eligible,
editor_make_block_dict,
editor_compare_hashes,
)

FAKE_IDENTITY_DATA = {"query": {"userinfo": {"options": {"disablemail": 0}}}}
Expand Down Expand Up @@ -1382,7 +1381,11 @@ def test_update_from_wikipedia(self):
new_identity, lang, new_global_userinfo
) # This call also saves the editor

def test_block_hash_changed(self):
def test_block_hash_changed_block_override(self):
"""
Tests that an email is sent when an editor's block status changes and
the block override is on
"""
identity = {}
identity["username"] = "evil_dr_porkchop"
# Users' unique WP IDs should not change across API calls, but are
Expand Down Expand Up @@ -1410,7 +1413,9 @@ def test_block_hash_changed(self):

# Don't change self.editor, or other tests will fail! Make a new one
# to test instead.
new_editor = EditorFactory(wp_registered=None, wp_block_hash="")
new_editor = EditorFactory(
wp_registered=None, wp_block_hash="", ignore_wp_blocks=True
)
new_identity = dict(identity)
new_global_userinfo = dict(global_userinfo)
new_identity["sub"] = new_editor.wp_sub
Expand Down Expand Up @@ -1441,6 +1446,69 @@ def test_block_hash_changed(self):
self.assertFalse(check_password(blocked_dict, new_editor.wp_block_hash))
self.assertEqual(len(mail.outbox), 1)

def test_block_hash_changed_no_block_override(self):
"""
Tests that an email is not sent when an editor's block status changes and
the block override is off
"""
identity = {}
identity["username"] = "evil_dr_porkchop"
# Users' unique WP IDs should not change across API calls, but are
# needed by update_from_wikipedia.
identity["sub"] = self.editor.wp_sub
identity["rights"] = ["deletion", "spaceflight"]
identity["groups"] = ["charismatic megafauna"]
# We should now be ignoring the oauth editcount
identity["editcount"] = 42
identity["email"] = "porkchop@example.com"
identity["iss"] = "zh-classical.wikipedia.org"
identity["registered"] = "20130205230142"
# validity
identity["blocked"] = False

global_userinfo = {}
global_userinfo["home"] = "zh_classicalwiki"
global_userinfo["id"] = identity["sub"]
global_userinfo["registration"] = "2013-02-05T23:01:42Z"
global_userinfo["name"] = identity["username"]
# We should now be using the global_userinfo editcount
global_userinfo["editcount"] = 960

global_userinfo["merged"] = copy.copy(FAKE_MERGED_ACCOUNTS)

# Don't change self.editor, or other tests will fail! Make a new one
# to test instead.
new_editor = EditorFactory(wp_registered=None, wp_block_hash="")
new_identity = dict(identity)
new_global_userinfo = dict(global_userinfo)
new_identity["sub"] = new_editor.wp_sub
new_global_userinfo["id"] = new_identity["sub"]

lang = get_language()
new_editor.update_from_wikipedia(
new_identity, lang, new_global_userinfo
) # This call also saves the editor

blocked_dict = editor_make_block_dict(new_global_userinfo["merged"])

self.assertTrue(check_password(blocked_dict, new_editor.wp_block_hash))
# No emails should be sent since the wp_block_hash was blank
self.assertEqual(len(mail.outbox), 0)

# Add a new block from the user
copied_merged_blocked_array = copy.copy(FAKE_MERGED_ACCOUNTS_BLOCKED)
new_global_userinfo["merged"] = copied_merged_blocked_array

new_editor.update_from_wikipedia(
new_identity, lang, new_global_userinfo
) # This call also saves the editor

new_blocked_dict = editor_make_block_dict(new_global_userinfo["merged"])

self.assertTrue(check_password(new_blocked_dict, new_editor.wp_block_hash))
self.assertFalse(check_password(blocked_dict, new_editor.wp_block_hash))
self.assertEqual(len(mail.outbox), 0)


class OAuthTestCase(TestCase):
@classmethod
Expand Down