Maniphest T11982

Preventing users from seeing your custom logo prevents them from seeing the whole site
Closed, ResolvedPublic
Actions

Assigned To

None

Authored By

	jonathon
	Dec 9 2016, 11:49 AM

Description

Outline:

If the cache is purged after a user has initiated text entry in a text box, other users cannot access the site until the user has navigated away from the page.

Steps to replicate:

Log in
Click in a text entry box (e.g. new comment)
Stop services (Apache, PHP-FPM, PHD)
Purge cache with $SRVROOT/phabricator/bin/cache purge --purge-all
Start services
Attempt to log in with a new browser profile (e.g. incognito/private browsing)
Site is inaccessible with "Shall Not Pass" exception

Versions:

arcanist: fad85844314b151994769a461825c90f7400c145
libphutil: 5ac2ca1214890d865bc57fab2715a322fdf02ab6
phabricator: 58ea40ad64536ca73b2b9f0f4889ce83dd3c76d0

Revisions and Commits

rP Phabricator
	D17015	rP9c72c1b1daa9 When rendering the "you were invited" header, query the inviting user with the…
	D17011	rPcde37acb4bd4 When a custom logo is configured but has a restrictive visibility policy, fall…

Related Objects

Mentioned Here: rARCade25facfdf2: Fix a bad interaction between "arc diff --reviewers" and "the first line of a…
rPHU9d85dfab0f53: Where available, use async_signals in libphutil
rP2d4eb460abb2: Fix MenuItem names not getting attached
D17011: When a custom logo is configured but has a restrictive visibility policy, fall back to the default logo
rARCfad85844314b: Make "aliases" show up in "arc get-config" help
rPHU5ac2ca121489: Don't let stemming reduce a word beneath 3 characters
rP58ea40ad6453: Hash Diffusion README cachekey components

Event Timeline

jonathon created this task.Dec 9 2016, 11:49 AM

I'm going to attempt to reproduce this as written, but I will be shocked if these steps work to reproduce the issue you describe.

I can't reproduce this. Here's what I did:

Typed some text in a text entry box.

Screen Shot 2016-12-09 at 4.01.06 AM.png (1×2 px, 284 KB)

Purged the cache:

epriestley@orbital ~/dev/phabricator $ ./bin/cache purge --purge-all
Purging remarkup cache...Done.
Purging changeset cache...Done.
Purging general cache...Done.
Purging user cache...Done.

Logged in in a new incognito window:

Screen Shot 2016-12-09 at 4.02.23 AM.png (1×2 px, 520 KB)

Everything worked as expected.

I think the stack trace in Q528 points at the actual issue, though.

Ick. I could replicate it three times with those steps (without typing into the comment box - only a click; I need to check what happens with typing). I wonder if it needs a service restart somewhere in the mix (my update script is in Q528). Let me do some more testing.

Here are more reliable reproduction steps:

Edit ui.logo in Config to specify a custom logo.
Find that logo in Files, and change the permission to be restrictive (for example, set it to only yourself, so other users could not see your custom logo).
Purge the general cache.
Restart apache or PHP-FPM to clear the immutable cache in APC/APCu.
Log in as a different user.

Step (2) is the most relevant one.

I'll make us bypass policies on this query. Empowering this query violates the letter of the policy system, but not the spirit.

(Actually, I'm just going to respect the file policy but ignore policy failures.)

epriestley renamed this task from Site unavailable after cache purge to Preventing users from seeing your custom logo prevents them from seeing the whole site.Dec 9 2016, 12:18 PM

epriestley added a project: Config.

Um. That's a different bug to the one I'm reporting. The custom logo I have set is visible to Public.

Let me update the replication steps:

Log in
Click in a text entry box (e.g. new comment). Do not type anything.
Stop services (Apache, PHP-FPM, PHD)
Purge cache with $SRVROOT/phabricator/bin/cache purge --purge-all
Start services
Attempt to log in with a new browser profile (e.g. incognito/private browsing)
Fail hard

jonathon updated the task description. (Show Details)Dec 9 2016, 12:24 PM

epriestley added a revision: D17011: When a custom logo is configured but has a restrictive visibility policy, fall back to the default logo.Dec 9 2016, 12:24 PM

When you say "fail hard", you specifically mean that you get an exception message beginning with this text?

(PhabricatorPolicyException) [You Shall Not Pass: Restricted File] (Can View)

...with a stack trace in your error log that includes this stack frame?

[Wed Dec 07 09:11:48.792456 2016] [:error] [pid 5389] [client 144.32.*:44110]   #5 <#2> PhabricatorMainMenuView::renderPhabricatorLogo() called at [<phabricator>/src/view/page/menu/PhabricatorMainMenuView.php:162]

What makes you believe that this is not a file policy issue related to logo image policies, given that the exception complains about a "restricted file" and occurs while loading the logo?

Well, the fact that the logo isn't restricted:

And yeah, I should have been explicit in the steps. The failure is the exception message.

If you do this:

As a user who can log in, go to the Config application.
Go to the ui.logo setting.
Copy the "PHID".
Paste it into the global search box.
Press return to search for the PHID.

...does it take you to F2, or to a different file?

Specifically, this is the PHID you're looking for in the UI:

Same file.

Let me try replicating without a custom logo at all.

OK, after removing the custom logo (even though it's Public) I can't replicate any more.

Weird, and apologies.

No problem -- we gave you a false start in Q528 and this behavior is pretty hard to guess at if you don't know the internals.

I think the reason that the comment stuff seemed to work was not related to comments -- instead, it was the navigation away. When a user who can see the logo loads a page, that generates a public cache of the logo. That is, this is a "fix":

Reproduce the issue.
Have any user who can see the logo load any page at all.
Issue will vanish mysteriously for everyone.

I think your reproduction steps did that, it just looked like a more complex dance with comment drafts.

As for the "Public" setting, does your install actually have policy.allow-public set?

Oh. No - no it doesn't.

With this setting disabled, the 'Public' policy is not available, and the most open policy is 'All Users' (which means users must have accounts and be logged in to view things).

Bingo.

Thanks for the fix! :)

The "Public" vs "All Users" stuff is sort of a confusing behavior on our part (the workflow is what set "Public", which doesn't really make sense given the rest of your config) but I think it's largely mooted by the fix in D17011, and to be more clear about it we'd have to make this one particular file magical somehow (we can't choose either "users" or "public" as the policy unambiguously, because the install may change the value of policy.allow-public later without also updating the logo).

The patch should land upstream later today and be available on stable within about 24 hours.

Getting another instance of what looks to be a similar issue but for a different function, specifically

[Fri Dec 09 13:58:01.821123 2016] [:error] [pid 23441] [client 144.32.*:49793]   #5 <#2> PhabricatorAuthController::renderInviteHeader(PhabricatorAuthInvite) called at [<phabricator>/src/applications/auth/controller/PhabricatorAuthStartController.php:194]

Full trace:

[Fri Dec 09 13:58:01.820117 2016] [:error] [pid 23441] [client 144.32.*:49793] [2016-12-09 13:58:01] EXCEPTION: (PhabricatorPolicyException) [You Shall Not Pass: Restricted User] (Can View) You do not have permission to view this object. // Logged in users can take this action. at [<phabricator>/src/applications/policy/filter/PhabricatorPolicyFilter.php:604]
[Fri Dec 09 13:58:01.821065 2016] [:error] [pid 23441] [client 144.32.*:49793] arcanist(head=master, ref.master=fad85844314b), phabricator(head=master, ref.master=ffdc082852fe), phutil(head=master, ref.master=5ac2ca121489)
[Fri Dec 09 13:58:01.821089 2016] [:error] [pid 23441] [client 144.32.*:49793]   #0 <#2> PhabricatorPolicyFilter::rejectObject(PhabricatorUser, string, string) called at [<phabricator>/src/applications/policy/filter/PhabricatorPolicyFilter.php:496]
[Fri Dec 09 13:58:01.821097 2016] [:error] [pid 23441] [client 144.32.*:49793]   #1 <#2> PhabricatorPolicyFilter::checkCapability(PhabricatorUser, string) called at [<phabricator>/src/applications/policy/filter/PhabricatorPolicyFilter.php:214]
[Fri Dec 09 13:58:01.821104 2016] [:error] [pid 23441] [client 144.32.*:49793]   #2 <#2> PhabricatorPolicyFilter::apply(array) called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:258]
[Fri Dec 09 13:58:01.821110 2016] [:error] [pid 23441] [client 144.32.*:49793]   #3 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:168]
[Fri Dec 09 13:58:01.821117 2016] [:error] [pid 23441] [client 144.32.*:49793]   #4 <#2> PhabricatorPolicyAwareQuery::executeOne() called at [<phabricator>/src/applications/auth/controller/PhabricatorAuthController.php:266]
[Fri Dec 09 13:58:01.821123 2016] [:error] [pid 23441] [client 144.32.*:49793]   #5 <#2> PhabricatorAuthController::renderInviteHeader(PhabricatorAuthInvite) called at [<phabricator>/src/applications/auth/controller/PhabricatorAuthStartController.php:194]
[Fri Dec 09 13:58:01.821129 2016] [:error] [pid 23441] [client 144.32.*:49793]   #6 <#2> PhabricatorAuthStartController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/handler/PhabricatorPolicyRequestExceptionHandler.php:46]
[Fri Dec 09 13:58:01.821135 2016] [:error] [pid 23441] [client 144.32.*:49793]   #7 <#2> PhabricatorPolicyRequestExceptionHandler::handleRequestException(AphrontRequest, PhabricatorPolicyException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:689]
[Fri Dec 09 13:58:01.821142 2016] [:error] [pid 23441] [client 144.32.*:49793]   #8 <#2> AphrontApplicationConfiguration::handleException(PhabricatorPolicyException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:274]
[Fri Dec 09 13:58:01.821148 2016] [:error] [pid 23441] [client 144.32.*:49793]   #9 <#2> AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:181]
[Fri Dec 09 13:58:01.821155 2016] [:error] [pid 23441] [client 144.32.*:49793]   #10 <#2> AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
[Fri Dec 09 13:58:01.821161 2016] [:error] [pid 23441] [client 144.32.*:49793]   #11 phlog(PhabricatorPolicyException) called at [<phabricator>/src/aphront/response/AphrontUnhandledExceptionResponse.php:20]
[Fri Dec 09 13:58:01.821167 2016] [:error] [pid 23441] [client 144.32.*:49793]   #12 AphrontUnhandledExceptionResponse::setException(PhabricatorPolicyException) called at [<phabricator>/webroot/index.php:21]

I can add a separate report for this if that's better? Edit: actually this is probably the exact same issue. I'll manually patch and check again. Edit2: after manually patching it just throws an error; I'm probably doing something wrong so will revert to the standard logo until the patch lands. Edit3: Reverting to the standard logo also 'fixes' this error.

epriestley closed this task as Resolved by committing rPcde37acb4bd4: When a custom logo is configured but has a restrictive visibility policy, fall….Dec 9 2016, 3:51 PM

epriestley added a commit: rPcde37acb4bd4: When a custom logo is configured but has a restrictive visibility policy, fall….

That's similar but not directly related.

epriestley added a revision: D17015: When rendering the "you were invited" header, query the inviting user with the omnipotent viewer.Dec 9 2016, 4:22 PM

epriestley closed this task as Resolved by committing rP9c72c1b1daa9: When rendering the "you were invited" header, query the inviting user with the….Dec 9 2016, 4:35 PM

epriestley added a commit: rP9c72c1b1daa9: When rendering the "you were invited" header, query the inviting user with the….

jonathon updated the task description. (Show Details)Jan 18 2017, 8:46 AM

arcanist: ade25facfdf22aed1c1e20fed3e58e60c0be3c2b
libphutil: 9d85dfab0f532d50c2343719e92d574a4827341b
phabricator: 2d4eb460abb2ba34a944962c7cfea5741e099ff9

I've just (this morning) started getting a similar error. The error log/trace is different though:

[Wed Jan 18 08:46:35.632792 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #0 <#2> PhabricatorPolicyFilter::rejectObject(PhabricatorFavoritesApplication, string, string) called at [<phabricator>/src/applications/policy/filter/PhabricatorPolicyFilter.php:502]
[Wed Jan 18 08:46:35.632803 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #1 <#2> PhabricatorPolicyFilter::checkCapability(PhabricatorFavoritesApplication, string) called at [<phabricator>/src/applications/policy/filter/PhabricatorPolicyFilter.php:220]
[Wed Jan 18 08:46:35.632807 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #2 <#2> PhabricatorPolicyFilter::apply(array) called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:258]
[Wed Jan 18 08:46:35.632809 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #3 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:168]
[Wed Jan 18 08:46:35.632812 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #4 <#2> PhabricatorPolicyAwareQuery::executeOne() called at [<phabricator>/src/applications/favorites/application/PhabricatorFavoritesApplication.php:56]
[Wed Jan 18 08:46:35.632815 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #5 <#2> PhabricatorFavoritesApplication::renderFavoritesDropdown(PhabricatorUser) called at [<phabricator>/src/applications/favorites/application/PhabricatorFavoritesApplication.php:46]
[Wed Jan 18 08:46:35.632817 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #6 <#2> PhabricatorFavoritesApplication::buildMainMenuExtraNodes(PhabricatorUser, PhabricatorAuthStartController) called at [<phabricator>/src/view/page/menu/PhabricatorMainMenuView.php:93]
[Wed Jan 18 08:46:35.632820 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #7 <#2> PhabricatorMainMenuView::render() called at [<phabricator>/src/view/page/PhabricatorStandardPageView.php:381]
[Wed Jan 18 08:46:35.632822 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #8 <#2> PhabricatorStandardPageView::willRenderPage() called at [<phabricator>/src/view/page/AphrontPageView.php:46]
[Wed Jan 18 08:46:35.632825 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #9 <#2> AphrontPageView::render() called at [<phabricator>/src/view/page/PhabricatorStandardPageView.php:884]
[Wed Jan 18 08:46:35.632827 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #10 <#2> PhabricatorStandardPageView::produceAphrontResponse() called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:649]
[Wed Jan 18 08:46:35.632830 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #11 <#2> AphrontApplicationConfiguration::produceResponse(AphrontRequest, PhabricatorStandardPageView) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:278]
[Wed Jan 18 08:46:35.632833 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #12 <#2> AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:181]
[Wed Jan 18 08:46:35.632835 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #13 <#2> AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
[Wed Jan 18 08:46:35.632838 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #14 phlog(PhabricatorPolicyException) called at [<phabricator>/src/aphront/response/AphrontUnhandledExceptionResponse.php:20]
[Wed Jan 18 08:46:35.632840 2017] [:error] [pid 23630] [client x.x.x.x:33714]   #15 AphrontUnhandledExceptionResponse::setException(PhabricatorPolicyException) called at [<phabricator>/webroot/index.php:21]

Looking at recent commit logs there has been a lot of menu/header work done so it's possibly a regression?

If you'd like us to look at it, please file a new issue following Contributing Bug Reports.

In general, it is much easier for us to merge duplicate issues (we click one button) than separate conflated issues, so you should err on the side of filing new issues.

Righty; I didn't want to create any extra noise. New ticket, ho.

	F2096610: Screen Shot 2016-12-09 at 4.01.06 AM.png
	Dec 9 2016, 12:03 PM

	F2096617: Screen Shot 2016-12-09 at 4.02.23 AM.png
	Dec 9 2016, 12:03 PM

	F2096705: logo.png
	Dec 9 2016, 12:33 PM

	F2096698: logo.png
	Dec 9 2016, 12:31 PM

Preventing users from seeing your custom logo prevents them from seeing the whole siteClosed, ResolvedPublicActions