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.

Tags

None

Referenced Files

	F14094667: D20578.id49098.diff
	Mon, Nov 25, 4:45 PM

	Unknown Object (File)
	Sun, Nov 24, 1:31 AM

	Unknown Object (File)
	Tue, Nov 19, 1:46 AM

	Unknown Object (File)
	Thu, Nov 14, 3:40 PM

	Unknown Object (File)
	Tue, Nov 12, 3:48 AM

	Unknown Object (File)
	Tue, Nov 5, 7:29 PM

	Unknown Object (File)
	Mon, Nov 4, 10:44 AM

	Unknown Object (File)
	Mon, Nov 4, 10:44 AM

Subscribers

None

Details

Reviewers

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

Branch

logout1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22972
Build 31529: Run Core Tests
Build 31528: arc lint + arc unit

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 49097

src/applications/auth/controller/PhabricatorLogoutController.php

Loading...