Ref T7153. I am not sure if this is 100% correct because sometimes you have to POST vs GET and I don't know if the redirect response will / can do the right thing? I think options to fix this would be to 1) restrict this functionality to JUST the Phabricator OAuth provider type or 2) something really fancy with an HTTP(S) future. The other rub right now is when you logout you get half auto-logged in again... Thoughts on that?
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T7153: Improve login workflow for Phacility instances
- Commits
- Restricted Diffusion Commit
rP472f316bbd90: Auth - allow for "auto login" providers
setup my local instance to JUST have phabricator oauth available to login. was presented with the dialog automagically...!
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
To fix the "user clicks logout and then is almost logged in again" problem we could
- do nothing
- set a cookie on logout that we detect inside the auto_login code... if its set, delete it and don't auto login.
- ?
I am pretty sure AphrontRedirectResponse will indeed do the right thing for me.
Comment Actions
- ?
How about we send you to a URL like /auth/goodbye/ (or something less goofy) that points to the same AuthStart controller but disables auto-login?
This otherwise looks correct to me.
I'm not sure if other providers need POST or not. If we have to POST, I think we need to write a <form> out and then form.submit() it in JS, which is a big pain. Maybe only put this on Phabricator OAuth for now rather than bothering to figure out what the answer is, unless you have other stuff configured already. Users have never asked for this in the general case so it seems reasonable to me to start narrow and maybe broaden the scope later if someone is like "hey, that's cool, can we do that too?".
| src/applications/auth/controller/PhabricatorAuthStartController.php | ||
|---|---|---|
| 99 | ...so this part would be like: if (!$request->getURIData('goodbye') && count($providers) == 1) { ... } | |
Comment Actions
- scope this new power to just PhabricatorPhabricatorAuthProvider
- add /auth/loggedout/ to handle the log out flow and not auto-log you back in...