Page MenuHomePhabricator

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

Authored by epriestley on Jul 9 2014, 2:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 17, 8:55 AM
Unknown Object (File)
Tue, Dec 17, 5:53 AM
Unknown Object (File)
Wed, Dec 11, 2:53 PM
Unknown Object (File)
Sat, Dec 7, 8:00 PM
Unknown Object (File)
Sat, Dec 7, 3:42 PM
Unknown Object (File)
Sat, Dec 7, 11:52 AM
Unknown Object (File)
Thu, Dec 5, 9:46 PM
Unknown Object (File)
Sat, Nov 30, 12:16 PM
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
Branch
legalpolicy
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1583
Build 1584: [Placeholder Plan] Wait for 30 Seconds

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).