Page MenuHomePhabricator

Use shouldAllowPublic(), not shouldRequireLogin(), for Legalpad
ClosedPublic

Authored by epriestley on Jul 9 2014, 2:56 PM.
Tags
None
Referenced Files
F13084175: D9857.diff
Wed, Apr 24, 10:49 PM
Unknown Object (File)
Thu, Apr 11, 10:03 AM
Unknown Object (File)
Sat, Apr 6, 3:55 PM
Unknown Object (File)
Sat, Mar 30, 2:54 PM
Unknown Object (File)
Wed, Mar 27, 9:39 PM
Unknown Object (File)
Feb 29 2024, 8:58 PM
Unknown Object (File)
Feb 29 2024, 8:58 PM
Unknown Object (File)
Feb 11 2024, 6:01 AM
Subscribers

Details

Summary

This got written a while ago and is using slightly incorrect gating on logged-out users. The names of these methods should probably be more clear too, but basically "shouldAllowPublic()" is for "this page may be usable to logged-out users, if policies allow it", while "shouldRequireLogin()" is for "this page should skip various credential checks". One of the skipped checks is email verification. This method should maybe be something like "isAuthenticationRelatedOrNoncredentialPage()" but I don't have a good name for that.

Test Plan

Unverified users are now prompted to verify email when viewing a legalpad document, instead of allowed to sign it.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Use shouldAllowPublic(), not shouldRequireLogin(), for Legalpad.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added reviewers: chasemp, chad.
chad edited edge metadata.
This revision is now accepted and ready to land.Jul 9 2014, 2:57 PM
epriestley updated this revision to Diff 23655.

Closed by commit rPfe29db6b92de (authored by @epriestley).