1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-22 03:29:11 +01:00
phorge-phorge/src/applications/auth/application/PhabricatorAuthApplication.php

137 lines
4.1 KiB
PHP
Raw Normal View History

<?php
final class PhabricatorAuthApplication extends PhabricatorApplication {
public function canUninstall() {
return false;
}
public function getBaseURI() {
return '/auth/';
}
public function getIconName() {
return 'authentication';
}
public function isPinnedByDefault(PhabricatorUser $viewer) {
return $viewer->getIsAdmin();
}
public function getName() {
return pht('Auth');
}
public function getShortDescription() {
return pht('Login/Registration');
}
public function getHelpURI() {
// NOTE: Although reasonable help exists for this in "Configuring Accounts
// and Registration", specifying a help URI here means we get the menu
// item in all the login/link interfaces, which is confusing and not
// helpful.
// TODO: Special case this, or split the auth and auth administration
// applications?
return null;
}
public function buildMainMenuItems(
PhabricatorUser $user,
PhabricatorController $controller = null) {
$items = array();
if ($user->isLoggedIn()) {
$item = id(new PHUIListItemView())
->addClass('core-menu-item')
->setName(pht('Log Out'))
->setIcon('logout-sm')
->setWorkflow(true)
->setHref('/logout/')
->setSelected(($controller instanceof PhabricatorLogoutController))
->setAural(pht('Log Out'))
->setOrder(900);
$items[] = $item;
} else {
if ($controller instanceof PhabricatorAuthController) {
// Don't show the "Login" item on auth controllers, since they're
// generally all related to logging in anyway.
} else {
$item = id(new PHUIListItemView())
->addClass('core-menu-item')
->setName(pht('Log In'))
// TODO: Login icon?
->setIcon('power')
->setHref('/auth/start/')
->setAural(pht('Log In'))
->setOrder(900);
$items[] = $item;
}
}
return $items;
}
public function getApplicationGroup() {
return self::GROUP_ADMIN;
}
New Registration Workflow Summary: Currently, registration and authentication are pretty messy. Two concrete problems: - The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad. - We can't practically implement OpenID because we can't reissue the authentication request. Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess. Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps: - User authenticates with remote service. - Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`. - Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc. This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this: $profile = $event->getValue('profile') $username = $profile->getDefaultUsername(); $is_employee = is_this_a_facebook_employee($username); if (!$is_employee) { throw new Exception("You are not employed at Facebook."); } $fbid = get_fbid_for_facebook_username($username); $profile->setDefaultEmail($fbid); $profile->setCanEditUsername(false); $profile->setCanEditEmail(false); $profile->setCanEditRealName(false); $profile->setShouldVerifyEmail(true); Seem reasonable? Test Plan: N/A yet, probably fatals. Reviewers: vrana, btrahan, codeblock, chad Reviewed By: btrahan CC: aran, asherkin, nh, wez Maniphest Tasks: T1536, T2394 Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 10:13:49 -07:00
public function getRoutes() {
return array(
'/auth/' => array(
'' => 'PhabricatorAuthListController',
'config/' => array(
'new/' => 'PhabricatorAuthNewController',
'new/(?P<className>[^/]+)/' => 'PhabricatorAuthEditController',
'edit/(?P<id>\d+)/' => 'PhabricatorAuthEditController',
'(?P<action>enable|disable)/(?P<id>\d+)/'
=> 'PhabricatorAuthDisableController',
),
'login/(?P<pkey>[^/]+)/(?:(?P<extra>[^/]+)/)?'
=> 'PhabricatorAuthLoginController',
Add password authentication and registration to new registration Summary: Ref T1536. Ref T1930. Code is not reachable. This provides password authentication and registration on the new provider/adapter framework. I sort of cheated a little bit and don't really route any password logic through the adapter (instead, this provider uses an empty adapter and just sets the type/domain on it). I think the right way to do this //conceptually// is to treat username/passwords as an external black box which the adapter communicates with. However, this creates a lot of practical implementation and UX problems: - There would basically be two steps -- in the first one, you interact with the "password black box", which behaves like an OAuth provider. This produces some ExternalAccount associated with the username/password pair, then we go into normal registration. - In normal registration, we'd proceed normally. This means: - The registration flow would be split into two parts, one where you select a username/password (interacting with the black box) and one where you actually register (interacting with the generic flow). This is unusual and probably confusing for users. - We would need to do a lot of re-hashing of passwords, since passwords currently depend on the username and user PHID, which won't exist yet during registration or the "black box" phase. This is a big mess I don't want to deal with. - We hit a weird condition where two users complete step 1 with the same username but don't complete step 2 yet. The box knows about two different copies of the username, with two different passwords. When we arrive at step 2 the second time we have a lot of bad choices about how to reoslve it, most of which create security problems. The most stragihtforward and "pure" way to resolve the issues is to put password-auth usernames in a separate space, but this would be incredibly confusuing to users (your login name might not be the same as your username, which is bizarre). - If we change this, we need to update all the other password-related code, which I don't want to bother with (at least for now). Instead, let registration know about a "default" registration controller (which is always password, if enabled), and let it require a password. This gives us a much simpler (albeit slightly less pure) implementation: - All the fields are on one form. - Password adapter is just a shell. - Password provider does the heavy lifting. We might make this more pure at some point, but I'm generally pretty satisfied with this. This doesn't implement the brute-force CAPTCHA protection, that will be coming soon. Test Plan: Registered with password only and logged in with a password. Hit various error conditions. Reviewers: btrahan Reviewed By: btrahan CC: aran, chad Maniphest Tasks: T1536, T1930 Differential Revision: https://secure.phabricator.com/D6164
2013-06-16 10:15:49 -07:00
'register/(?:(?P<akey>[^/]+)/)?' => 'PhabricatorAuthRegisterController',
'start/' => 'PhabricatorAuthStartController',
'validate/' => 'PhabricatorAuthValidateController',
'finish/' => 'PhabricatorAuthFinishController',
'unlink/(?P<pkey>[^/]+)/' => 'PhabricatorAuthUnlinkController',
'(?P<action>link|refresh)/(?P<pkey>[^/]+)/'
=> 'PhabricatorAuthLinkController',
'confirmlink/(?P<akey>[^/]+)/'
=> 'PhabricatorAuthConfirmLinkController',
'session/terminate/(?P<id>[^/]+)/'
=> 'PhabricatorAuthTerminateSessionController',
'token/revoke/(?P<id>[^/]+)/'
=> 'PhabricatorAuthRevokeTokenController',
'session/downgrade/'
=> 'PhabricatorAuthDowngradeSessionController',
'multifactor/'
=> 'PhabricatorAuthNeedsMultiFactorController',
New Registration Workflow Summary: Currently, registration and authentication are pretty messy. Two concrete problems: - The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad. - We can't practically implement OpenID because we can't reissue the authentication request. Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess. Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps: - User authenticates with remote service. - Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`. - Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc. This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this: $profile = $event->getValue('profile') $username = $profile->getDefaultUsername(); $is_employee = is_this_a_facebook_employee($username); if (!$is_employee) { throw new Exception("You are not employed at Facebook."); } $fbid = get_fbid_for_facebook_username($username); $profile->setDefaultEmail($fbid); $profile->setCanEditUsername(false); $profile->setCanEditEmail(false); $profile->setCanEditRealName(false); $profile->setShouldVerifyEmail(true); Seem reasonable? Test Plan: N/A yet, probably fatals. Reviewers: vrana, btrahan, codeblock, chad Reviewed By: btrahan CC: aran, asherkin, nh, wez Maniphest Tasks: T1536, T2394 Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 10:13:49 -07:00
),
'/oauth/(?P<provider>\w+)/login/'
=> 'PhabricatorAuthOldOAuthRedirectController',
'/login/' => array(
'' => 'PhabricatorAuthStartController',
'email/' => 'PhabricatorEmailLoginController',
Make password reset emails use one-time tokens Summary: Ref T4398. This code hadn't been touched in a while and had a few crufty bits. **One Time Resets**: Currently, password reset (and similar links) are valid for about 48 hours, but we always use one token to generate them (it's bound to the account). This isn't horrible, but it could be better, and it produces a lot of false positives on HackerOne. Instead, use TemporaryTokens to make each link one-time only and good for no more than 24 hours. **Coupling of Email Verification and One-Time Login**: Currently, one-time login links ("password reset links") are tightly bound to an email address, and using a link verifies that email address. This is convenient for "Welcome" emails, so the user doesn't need to go through two rounds of checking email in order to login, then very their email, then actually get access to Phabricator. However, for other types of these links (like those generated by `bin/auth recover`) there's no need to do any email verification. Instead, make the email verification part optional, and use it on welcome links but not other types of links. **Message Customization**: These links can come out of several workflows: welcome, password reset, username change, or `bin/auth recover`. Add a hint to the URI so the text on the page can be customized a bit to help users through the workflow. **Reset Emails Going to Main Account Email**: Previously, we would send password reset email to the user's primary account email. However, since we verify email coming from reset links this isn't correct and could allow a user to verify an email without actually controlling it. Since the user needs a real account in the first place this does not seem useful on its own, but might be a component in some other attack. The user might also no longer have access to their primary account, in which case this wouldn't be wrong, but would not be very useful. Mitigate this in two ways: - First, send to the actual email address the user entered, not the primary account email address. - Second, don't let these links verify emails: they're just login links. This primarily makes it more difficult for an attacker to add someone else's email to their account, send them a reset link, get them to login and implicitly verify the email by not reading very carefully, and then figure out something interesting to do (there's currently no followup attack here, but allowing this does seem undesirable). **Password Reset Without Old Password**: After a user logs in via email, we send them to the password settings panel (if passwords are enabled) with a code that lets them set a new password without knowing the old one. Previously, this code was static and based on the email address. Instead, issue a one-time code. **Jump Into Hisec**: Normally, when a user who has multi-factor auth on their account logs in, we prompt them for factors but don't put them in high security. You usually don't want to go do high-security stuff immediately after login, and it would be confusing and annoying if normal logins gave you a "YOU ARE IN HIGH SECURITY" alert bubble. However, if we're taking you to the password reset screen, we //do// want to put the user in high security, since that screen requires high security. If we don't do this, the user gets two factor prompts in a row. To accomplish this, we set a cookie when we know we're sending the user into a high security workflow. This cookie makes login finalization upgrade all the way from "partial" to "high security", instead of stopping halfway at "normal". This is safe because the user has just passed a factor check; the only reason we don't normally do this is to reduce annoyance. **Some UI Cleanup**: Some of this was using really old UI. Modernize it a bit. Test Plan: - **One Time Resets** - Used a reset link. - Tried to reuse a reset link, got denied. - Verified each link is different. - **Coupling of Email Verification and One-Time Login** - Verified that `bin/auth`, password reset, and username change links do not have an email verifying URI component. - Tried to tack one on, got denied. - Used the welcome email link to login + verify. - Tried to mutate the URI to not verify, or verify something else: got denied. - **Message Customization** - Viewed messages on the different workflows. They seemed OK. - **Reset Emails Going to Main Account Email** - Sent password reset email to non-primary email. - Received email at specified address. - Verified it does not verify the address. - **Password Reset Without Old Password** - Reset password without knowledge of old one after email reset. - Tried to do that without a key, got denied. - Tried to reuse a key, got denied. - **Jump Into Hisec** - Logged in with MFA user, got factor'd, jumped directly into hisec. - Logged in with non-MFA user, no factors, normal password reset. - **Some UI Cleanup** - Viewed new UI. - **Misc** - Created accounts, logged in with welcome link, got verified. - Changed a username, used link to log back in. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4398 Differential Revision: https://secure.phabricator.com/D9252
2014-05-22 10:41:00 -07:00
'once/'.
'(?P<type>[^/]+)/'.
'(?P<id>\d+)/'.
'(?P<key>[^/]+)/'.
'(?:(?P<emailID>\d+)/)?' => 'PhabricatorAuthOneTimeLoginController',
'refresh/' => 'PhabricatorRefreshCSRFController',
'mustverify/' => 'PhabricatorMustVerifyEmailController',
),
'/emailverify/(?P<code>[^/]+)/'
=> 'PhabricatorEmailVerificationController',
'/logout/' => 'PhabricatorLogoutController',
New Registration Workflow Summary: Currently, registration and authentication are pretty messy. Two concrete problems: - The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad. - We can't practically implement OpenID because we can't reissue the authentication request. Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess. Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps: - User authenticates with remote service. - Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`. - Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc. This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this: $profile = $event->getValue('profile') $username = $profile->getDefaultUsername(); $is_employee = is_this_a_facebook_employee($username); if (!$is_employee) { throw new Exception("You are not employed at Facebook."); } $fbid = get_fbid_for_facebook_username($username); $profile->setDefaultEmail($fbid); $profile->setCanEditUsername(false); $profile->setCanEditEmail(false); $profile->setCanEditRealName(false); $profile->setShouldVerifyEmail(true); Seem reasonable? Test Plan: N/A yet, probably fatals. Reviewers: vrana, btrahan, codeblock, chad Reviewed By: btrahan CC: aran, asherkin, nh, wez Maniphest Tasks: T1536, T2394 Differential Revision: https://secure.phabricator.com/D4647
2013-06-16 10:13:49 -07:00
);
}
}