Summary:
Ref T6755. This is a partial fix, but:
- Allow netblocks to be blacklisted instead of making the feature all-or-nothing.
- Default to disallow requests to all reserved private/local/special IP blocks. This should generally be a "safe" setting.
- Explain the risks better.
- Improve the errors rasied by Macro when failing.
- Removed `security.allow-outbound-http`, as it is superseded by this setting and is somewhat misleading.
- We still make outbound HTTP requests to OAuth.
- We still make outbound HTTP requests for repositories.
From a technical perspective:
- Separate URIs that are safe to link to or redirect to (basically, not "javascript://") from URIs that are safe to fetch (nothing in a private block).
- Add the default blacklist.
- Be more careful with response data in Macro fetching, and don't let the user see it if it isn't ultimately valid.
Additionally:
- I want to do this check before pulling repositories, but that's enough of a mess that it should go in a separate diff.
- The future implementation of T4190 needs to perform the fetch check.
Test Plan:
- Fetched a valid macro.
- Fetched a non-image, verified it didn't result in a viewable file.
- Fetched a private-ip-space image, got an error.
- Fetched a 404, got a useful-enough error without additional revealing response content (which is usually HTML anyway and not useful).
- Fetched a bad protocol, got an error.
- Linked to a local resource, a phriction page, a valid remote site, all worked.
- Linked to private IP space, which worked fine (we want to let you link and redierect to other private services, just not fetch them).
- Added and executed unit tests.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6755
Differential Revision: https://secure.phabricator.com/D12136
Summary: Changes the text to just "Stay", which is still obvious what it means, with less copy. Fixes T7027
Test Plan: Now works on mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7027
Differential Revision: https://secure.phabricator.com/D12075
Summary: Fixes T7496, T7511. Sets text for registration is not enabled, sets can_manage on add_provider button.
Test Plan: Test with a logged in admin and logged in normal joe user.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7496, T7511
Differential Revision: https://secure.phabricator.com/D12014
Summary: Renames the method in PHUIObjectBoxView to match the new PHUIInfoView class.
Test Plan: grepped codebase. Went to Calendar and tried a new status.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D12005
Summary: Since this element isn't strictly about errors, re-label as info view instead.
Test Plan: Grepped for all callsites, tested UIExamples and a few other random pages.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11867
Summary: This wasn't actually being skipped for invites; really skip it.
Test Plan:
- Registered without invite, captcha.
- Registered with invite, no captcha.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D11877
Summary: Fixes T7159.
Test Plan:
Created a legalpad document that needed a signature and I was required to sign it no matter what page I hit. Signed it and things worked! Added a new legalpad document and I had to sign again!
Ran unit tests and they passed!
Logged out as a user who was roadblocked into signing a bunch of stuff and it worked!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7159
Differential Revision: https://secure.phabricator.com/D11759
Summary:
Ref T7152. This substantially completes the upstream login flow. Basically, we just cookie you and push you through normal registration, with slight changes:
- All providers allow registration if you have an invite.
- Most providers get minor text changes to say "Register" instead of "Login" or "Login or Register".
- The Username/Password provider changes to just a "choose a username" form.
- We show the user that they're accepting an invite, and who invited them.
Then on actual registration:
- Accepting an invite auto-verifies the address.
- Accepting an invite auto-approves the account.
- Your email is set to the invite email and locked.
- Invites get to reassign nonprimary, unverified addresses from other accounts.
But 98% of the code is the same.
Test Plan:
- Accepted an invite.
- Verified a new address on an existing account via invite.
- Followed a bad invite link.
- Tried to accept a verified invite.
- Reassigned an email by accepting an unverified, nonprimary invite on a new account.
- Verified that reassigns appear in the activity log.
{F291493}
{F291494}
{F291495}
{F291496}
{F291497}
{F291498}
{F291499}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11737
Summary:
Ref T7152. This builds the core of email invites and implements all the hard logic for them, covering it with a pile of tests.
There's no UI to create these yet, so users can't actually get invites (and administrators can't send them).
This stuff is a complicated mess because there are so many interactions between accounts, email addresses, email verification, email primary-ness, and user verification. However, I think I got it right and got test coverage everwhere.
The degree to which this is exception-driven is a little icky, but I think it's a reasonable way to get the testability we want while still making it hard for callers to get the flow wrong. In particular, I expect there to be at least two callers (one invite flow in the upstream, and one derived invite flow in Instances) so I believe there is merit in burying as much of this logic inside the Engine as is reasonably possible.
Test Plan: Unit tests only.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T7152
Differential Revision: https://secure.phabricator.com/D11723
Summary:
Ref T7208. Now that we have approvals (new installs are safe by default), take those into account when generating this warning.
Try to soften the warning to cover the case discussed in T7208, hopefully without requiring additional measures.
Test Plan:
{F286014}
{F286015}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T7208
Differential Revision: https://secure.phabricator.com/D11708
Summary: Uses more standard boxes for display, and icons!
Test Plan:
Test with all enabled, all disabled, and a mix.
{F285945}
{F285946}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11707
Summary: 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?
Test Plan: setup my local instance to JUST have phabricator oauth available to login. was presented with the dialog automagically...!
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7153
Differential Revision: https://secure.phabricator.com/D11701
Summary: Clean up the error view styling.
Test Plan:
Tested as many as I could find, built additional tests in UIExamples
{F280452}
{F280453}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: hach-que, Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11605
Summary: Ref T3404. The only mildly sketchy bit is these codepaths all load the application email directly, by-passing privacy. I think this is necessary because not getting to see an application doesn't mean you should be able to break the application by registering a colliding email address.
Test Plan:
Tried to add a registered application email to a user account via the web ui and got a pretty error.
Ran unit tests.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T3404
Differential Revision: https://secure.phabricator.com/D11565
Summary: Add a setBorder call to CrumbsView to be more deliberate when a border is drawn. Could not find any CSS hacks to set it conditionally CSS.
Test Plan: Browsed every application that called crumbs and make a design decision. Also fixed a few bad layouts.
Reviewers: btrahan, epriestley
Reviewed By: btrahan
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D11533
Summary: Ref T6822. This method needs to be `public` because it is called from `PhabricatorApplicationSearchController::buildApplicationMenu()`.
Test Plan: I wouldn't expect //increasing// method visibility to break anything.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11416
Summary: Ref T6971. This fixes the error the user reported. Not sure what's up with the root cause of their issue.
Test Plan: Went to `/auth/config/new/asdfqwer/` and got a 404 instead of an exception.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T6971
Differential Revision: https://secure.phabricator.com/D11388
Summary: Ref T6947.
Test Plan: toggled setting in application settings and changes stuck. set policy to admin user a only and could not add a provider as a admin user b.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6947
Differential Revision: https://secure.phabricator.com/D11356
Summary:
Ref T6870. Since it does not make sense to redirect the user to the login form after they log in, we try not to set the login form as the `next` cookie.
However, the current check is hard-coded to `/auth/start/`, and the form can also be served at `/login/`. This has no real effect on normal users, but did make debugging T6870 confusing.
Instead of using a hard-coded path check, test if the controller was delegated to. If it was, store the URI. If it's handling the request without delegation, don't.
Test Plan:
- Visited login form at `/login/` and `/auth/start/`, saw it not set a next URI.
- Visited login form at `/settings/` (while logged out), saw it set a next URI.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, lpriestley
Maniphest Tasks: T6870
Differential Revision: https://secure.phabricator.com/D11292
Summary: So meta it hurts. Fixes T887.
Test Plan: created a second instance of phabricator locally. made an account on oauth server phabricator. set up my normal dev phabricator to use this new oauth phabricator. noted the form worked. created an account via the oauth method and it worked.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T887
Differential Revision: https://secure.phabricator.com/D11287
Summary: Ref T6822.
Test Plan: Visual inspection. These methods are only called from within `PhabricatorController` subclasses.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6822
Differential Revision: https://secure.phabricator.com/D11241
Summary:
Ref T4712. Thus far, it seems that most "non-standard" things can be done pretty easily in the controller. Aside from deploying, this diff had to fix a few bugs / missing implementations of stuff.
(Notably, PhabricatorAuthProviderConfig, HeraldRule, PhabricatorSlowvotePoll, and AlmanacNetwork needed to implement PhabricatorApplicationTransactionInterface, PhabricatorAuthAuthProviderPHIDType had to be added, and a rendering bug in transactions of type PhabricatorOAuth2AuthProvider had to be fixed.)
Test Plan: Almanac - looked at binding, device, network, and service view controllers and verified timeline displayed properly. Herald - looked at a rule and verified timeline. Slowvote - looked at a vote and verified timeline. Auth - looked at an auth provider (Facebook) and verified proper display of transactions within timeline.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T4712
Differential Revision: https://secure.phabricator.com/D10921
Summary:
Ref T6240. Some discussion in that task. In instance/cluster environments, daemons need to make Conduit calls that bypass policy checks.
We can't just let anyone add SSH keys with this capability to the web directly, because then an adminstrator could just add a key they own and start signing requests with it, bypassing policy checks.
Add a `bin/almanac trust-key --id <x>` workflow for trusting keys. Only trusted keys can sign requests.
Test Plan:
- Generated a user key.
- Generated a device key.
- Trusted a device key.
- Untrusted a device key.
- Hit the various errors on trust/untrust.
- Tried to edit a trusted key.
{F236010}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T6240
Differential Revision: https://secure.phabricator.com/D10878
Summary:
Ref T5833. I want to add SSH keys to Almanac devices, but the edit workflows for them are currently bound tightly to users.
Instead, decouple key management from users and the settings panel.
Test Plan:
- Uploaded, generated, edited and deleted SSH keys.
- Hit missing name, missing key, bad key format, duplicate key errors.
- Edited/generated/deleted/etc keys for a bot user as an administrator.
- Got HiSec'd on everything.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5833
Differential Revision: https://secure.phabricator.com/D10824
Summary: Missed this in previous pass. Send these as links in HTML emails.
Test Plan: Register a new user that nees approval.
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D10815
Summary:
Fixes T5900. We have some very old code here which does not let you update your password if the `account.editable` flag is set.
This was approximately introduced in D890, and I think it was mostly copy/pasted at that point. I'm not sure this ever really made sense. The option is not documented as affecting this, for example. In the modern environment of auth providers, it definitely does not make sense.
Instead, always allow users to change passwords if the install has a password provider configured.
Test Plan:
- Set `account.editable` to false.
- Used a password reset link.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5900
Differential Revision: https://secure.phabricator.com/D10331
Summary: Ref T5861. Adds an option to opt out of all notification email. We'll still send you password resets, email verifications, etc.
Test Plan:
{F189484}
- Added unit tests.
- With preference set to different things, tried to send myself mail. Mail respected preferences.
- Sent password reset email, which got through the preference.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: rush898, epriestley
Maniphest Tasks: T5861
Differential Revision: https://secure.phabricator.com/D10237
Summary:
Via HackerOne. If a user adds an email address and typos it, entering `alinculne@gmailo.com`, and it happens to be a valid address which an evil user controls, the evil user can request a password reset and compromise the account.
This strains the imagination, but we can implement a better behavior cheaply.
- If an account has any verified addresses, only send to verified addresses.
- If an account has no verified addresses (e.g., is a new account), send to any address.
We've also received several reports about reset links not being destroyed as aggressively as researchers expect. While there's no specific scenario where this does any harm, revoke all outstanding reset tokens when a reset link is used to improve the signal/noise ratio of the reporting channel.
Test Plan:
- Tried to send a reset link to an unverified address on an account with a verified address (got new error).
- Tried to send a reset link to a verified adddress on an account with a verified address (got email).
- Tried to send a reset link to an invalid address (got old error).
- Tried to send a reset link to an unverified address on an account with only unverified addresses -- a new user (got email).
- Requested several reset links, used one, verified all the others were revoked.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D10206
Summary:
Fixes T5509. Currently, existing sessions live on even if you change your password.
Over the course of the program, we've recieved a lot of HackerOne reports that sessions do not terminate when users change their passwords. I hold that this isn't a security vulnerability: users can explicitly manage sessions, and this is more general and more powerful than tying session termination to password resets. In particular, many installs do not use a password provider at all (and no researcher has reported this in a general, application-aware way that discusses multiple authentication providers).
That said, dealing with these false positives is vaguely time consuming, and the "expected" behavior isn't bad for users, so just align behavior with researcher expectations: when passwords are changed, providers are removed, or multi-factor authentication is added to an account, terminate all other active login sessions.
Test Plan:
- Using two browsers, established multiple login sessions.
- In one browser, changed account password. Saw session terminate and logout in the second browser.
- In one browser, removed an authentication provider. Saw session terminate and logout in the second browser.
- In one browser, added MFA. Saw session terminate and logout in the second browser.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5509
Differential Revision: https://secure.phabricator.com/D10135
Summary:
Fixes T5506. Depends on D10133. When users remove an email address or change their primary email address, invalidate any outstanding password reset links.
This is a very small security risk, but the current behavior is somewhat surprising, and an attacker could sit on a reset link for up to 24 hours and then use it to re-compromise an account.
Test Plan:
- Changed primary address and removed addreses.
- Verified these actions invalidated outstanding one-time login temporary tokens.
- Tried to use revoked reset links.
- Revoked normally from new UI panel.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5506
Differential Revision: https://secure.phabricator.com/D10134
Summary:
Ref T5506. This makes it easier to understand and manage temporary tokens.
Eventually this could be more user-friendly, since it's relatively difficult to understand what this screen means. My short-term goal is just to make the next change easier to implement and test.
The next diff will close a small security weakness: if you change your email address, password reset links which were sent to the old address are still valid. Although an attacker would need substantial access to exploit this (essentially, it would just make it easier for them to re-compromise an already compromised account), it's a bit surprising. In the next diff, email address changes will invalidate outstanding password reset links.
Test Plan:
- Viewed outstanding tokens.
- Added tokens to the list by making "Forgot your password?" requests.
- Revoked tokens individually.
- Revoked all tokens.
- Tried to use a revoked token.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5506
Differential Revision: https://secure.phabricator.com/D10133
Summary: Ref T4896. Instead of using custom stuff, use standard stuff.
Test Plan: Viewed a bunch of feed stories and published some over the Asana bridge.
Reviewers: joshuaspence, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4896
Differential Revision: https://secure.phabricator.com/D10114
Summary:
Fixes T3732. Ref T1205. Ref T3116.
External accounts (like emails used as identities, Facebook accounts, LDAP accounts, etc.) are stored in "ExternalAccount" objects.
Currently, we have a very restrictive `CAN_VIEW` policy for ExternalAccounts, to add an extra layer of protection to make sure users can't use them in unintended ways. For example, it would be bad if a user could link their Phabricator account to a Facebook account without proper authentication. All of the controllers which do sensitive things have checks anyway, but a restrictive CAN_VIEW provided an extra layer of protection. Se T3116 for some discussion.
However, this means that when grey/external users take actions (via email, or via applications like Legalpad) other users can't load the account handles and can't see anything about the actor (they just see "Restricted External Account" or similar).
Balancing these concerns is mostly about not making a huge mess while doing it. This seems like a reasonable approach:
- Add `CAN_EDIT` on these objects.
- Make that very restricted, but open up `CAN_VIEW`.
- Require `CAN_EDIT` any time we're going to do something authentication/identity related.
This is slightly easier to get wrong (forget CAN_EDIT) than other approaches, but pretty simple, and we always have extra checks in place anyway -- this is just a safety net.
I'm not quite sure how we should identify external accounts, so for now we're just rendering "Email User" or similar -- clearly not a bug, but not identifying. We can figure out what to render in the long term elsewhere.
Test Plan:
- Viewed external accounts.
- Linked an external account.
- Refreshed an external account.
- Edited profile picture.
- Viewed sessions panel.
- Published a bunch of stuff to Asana/JIRA.
- Legalpad signature page now shows external accounts.
{F171595}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T3732, T1205, T3116
Differential Revision: https://secure.phabricator.com/D9767
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.
Test Plan: Eyeballed it.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9431
Summary: Ref T5089. Adds a `security.require-multi-factor-auth` which forces all users to enroll in MFA before they can use their accounts.
Test Plan:
Config:
{F159750}
Roadblock:
{F159748}
After configuration:
{F159749}
- Required MFA, got roadblocked, added MFA, got unblocked.
- Removed MFA, got blocked again.
- Used `bin/auth strip` to strip MFA, got blocked.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5089
Differential Revision: https://secure.phabricator.com/D9285
Summary: Both email verify and welcome links now verify email, centralize them and record them in the user activity log.
Test Plan:
- Followed a "verify email" link and got verified.
- Followed a "welcome" (verifying) link.
- Followed a "reset" (non-verifying) link.
- Looked in the activity log for the verifications.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9284
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
Summary: This is useful when you're trying to onboard an entire office and you end up using the Google OAuth anyway.
Test Plan: tested locally. Maybe I should write some tests?
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9150
Summary: Did a more exhaustive grep on setIcon and found 99.9% of the icons.
Test Plan: I verified icon names on UIExamples, but unable to test some of the more complex flows visually. Mostly a read and replace.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9088
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.
Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin, hach-que
Differential Revision: https://secure.phabricator.com/D9052
Summary: Fixes T4728, first pass, Make real name optional on user accounts
Test Plan: Default real name config should be false (not required). Create new user, real name should not be required. Toggle config, real name should be required. Users with no real name should be always listed by their usernames.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4728
Differential Revision: https://secure.phabricator.com/D9027
Summary:
Ref T4398. This prompts users for multi-factor auth on login.
Roughly, this introduces the idea of "partial" sessions, which we haven't finished constructing yet. In practice, this means the session has made it through primary auth but not through multi-factor auth. Add a workflow for bringing a partial session up to a full one.
Test Plan:
- Used Conduit.
- Logged in as multi-factor user.
- Logged in as no-factor user.
- Tried to do non-login-things with a partial session.
- Reviewed account activity logs.
{F149295}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4398
Differential Revision: https://secure.phabricator.com/D8922
