Differential D20578

Don't require users be logged in to access the Logout controller, so users with no Spaces can log out
ClosedPublic
Actions

Authored by epriestley on Jun 13 2019, 11:59 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13310: Users with no access to Spaces can not log out

Commits

rP4af73a625fe1: Don't require users be logged in to access the Logout controller, so users with…

Summary

Fixes T13310. Use cases in the form "users with no access to any spaces can not <do things>" are generally unsupported (that is, we consider this to mean that the install is misconfigured), but "log out" is a somewhat more reasonable sort of thing to do and easy to support.

Drop the requirement that users be logged in to access the Logout controller. This skips the check for access to any Spaces and allows users with no Spaces to log out.

For users who are already logged out, this just redirects home with no effect.

Test Plan

As a user with access to no Spaces, logged out. (Before: error; after: worked).
As a logged-out user, logged out (was redirected).
As a normal user, logged out (normal logout).

Diff Detail

Repository

rP Phabricator

Branch

logout1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22971
Build 31527: Run Core Tests
Build 31526: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	1 ms	MetaMTAEmailTransactionCommandTestCase::testGetAllTypes EXCEPTION (Exception): msort() was passed a method ("getRevisionActionOrderVector") which returns "PhutilSortVector" objects. Use "msortv()", not "msort()", to sort a list which produces vectors. #0 /core/data/drydock/workingcopy-86/repo/phabricator/src/applications/differential/command/DifferentialActionEmailCommand.php(59): msort(Array, 'getRevisionActi...') #1 [internal function]: DifferentialActionEmailCommand->getCommandObjects()
	1 ms	AlmanacNamesTestCase::testServiceOrDeviceNames 30 assertions passed.
	0 ms	AlmanacServiceTypeTestCase::testGetAllServiceTypes 1 assertion passed.
	0 ms	AphrontHTTPSinkTestCase::testHTTPHeaderNames 2 assertions passed.
	0 ms	AphrontHTTPSinkTestCase::testHTTPSinkBasics 3 assertions passed.
		View Full Test Results (1 Failed · 369 Passed)

Event Timeline

epriestley created this revision.Jun 13 2019, 11:59 PM

Harbormaster completed remote builds in B22957: Diff 49082.Jun 14 2019, 12:00 AM

epriestley requested review of this revision.Jun 14 2019, 12:00 AM

amckinley accepted this revision.Jun 17 2019, 8:22 PM

amckinley added inline comments.

src/applications/auth/controller/PhabricatorLogoutController.php
7–26

This revision is now accepted and ready to land.Jun 17 2019, 8:22 PM

epriestley added inline comments.Jun 17 2019, 8:31 PM

src/applications/auth/controller/PhabricatorLogoutController.php
7–26	Haha, in retrospect this is fairly confusing -- let me take another shot at it.

Explain the behavior more clearly -- there are three classes of users:

Logged in, normal users.
Logged in users with no access to Spaces.
Logged out users.

If we require login, users in bucket (1) can log out but users in bucket (2) get a "No Access To Spaces" error.

If we don't require login, users in buckets (1) and (2) can log out, since the "No Access To Spaces" error only cares about login-required pages.

Users in bucket (3) also get to log out, which is silly/pointless (they are already logged out), but not harmful.

Harbormaster failed remote builds in B22971: Diff 49096!Jun 17 2019, 8:38 PM

Rebase to pick up the unit test fix.

Harbormaster completed remote builds in B22972: Diff 49097.Jun 17 2019, 8:41 PM

Closed by commit rP4af73a625fe1: Don't require users be logged in to access the Logout controller, so users with… (authored by epriestley). · Explain WhyJun 17 2019, 8:44 PM

This revision was automatically updated to reflect the committed changes.

epriestley added a commit: rP4af73a625fe1: Don't require users be logged in to access the Logout controller, so users with….

Revision Contents
Changeset List

Path

Size

src/

applications/

auth/

controller/

PhabricatorLogoutController.php

20 lines

Diff 49096

View Options

src/applications/auth/controller/PhabricatorLogoutController.php

Don't require users be logged in to access the Logout controller, so users with no Spaces can log outClosedPublicActions