Skip to content

Fix admin username underscore#92

Open
Ishta-P-Jain wants to merge 6 commits into
wikisource:masterfrom
Ishta-P-Jain:fix-admin-username-underscore
Open

Fix admin username underscore#92
Ishta-P-Jain wants to merge 6 commits into
wikisource:masterfrom
Ishta-P-Jain:fix-admin-username-underscore

Conversation

@Ishta-P-Jain

Copy link
Copy Markdown

Summary

MediaWiki treats underscores and spaces in usernames as
interchangeable, but wscontest was comparing them as plain strings
in several places:

  • ContestsController::edit() and ::delete() compared admin names
    with strict ===.
  • ContestRepository::hasAdmin() (which controls whether the Edit
    button is shown at all) used an exact LIKE match in SQL.
  • ContestsController::save() stored whatever raw string was typed
    into the admins textarea, underscore and all.

This PR normalizes underscores to spaces on both sides of every
admin-username comparison, and normalizes admin names before they're
saved, so the two forms of a username are always treated as
equivalent.

Testing

Manually verified locally that:

  • Adding an admin with an underscored username and saving no longer
    breaks their ability to edit the contest.
  • Existing admins added before this fix (with underscores already in
    the database) can now also edit, thanks to the hasAdmin() fix.

Bug: T336157

Usernames with underscores (e.g. Bennylin_(WMID)) were not being
recognized as matching their space-separated form when checking
contest admin permissions, so affected admins couldn't edit or
delete their own contests. MediaWiki treats underscores and spaces
in usernames as interchangeable, so this normalizes both sides of
the comparison, and normalizes admin names on save.

Bug: T336157
   Companion fix to 0f3e7ed. hasAdmin() used a strict LIKE match
   between the stored admin name and the logged-in username, so
   admins saved with an underscore-separated name (before the
   previous fix) still couldn't edit their contests.

   Bug: T336157

@samwilson samwilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A few suggestions. Also, there are some CI issues.

Comment thread src/Controller/ContestsController.php
Comment thread src/Controller/ContestsController.php Outdated
Comment thread src/Controller/ContestsController.php Outdated
Comment thread src/Controller/ContestsController.php Outdated
@Ishta-P-Jain

Copy link
Copy Markdown
Author

Thanks for the review! I've addressed the suggested changes. I noticed the remaining CI failure is in IndexPageRepositoryTest with a 403 Forbidden response from the Wikidata Query Service. Is this an existing CI issue, or would you like me to investigate it further?
let me know if there's anything else that should be updated

@samwilson

Copy link
Copy Markdown
Member

Thanks! Looks good.

I'm attempting to fix the CI failure: wikisource/api#19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants