1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-16 03:42:41 +01:00
Commit graph

382 commits

Author SHA1 Message Date
epriestley
049fb2018b Add very basic "quick create" menu
Summary:
Ref T3623. This is like a pre-v0, in that it doesn't have a dropdown yet.

Clicking the button takes you to a page which can serve as a right click / mobile / edit target in the long run, but is obviously not great for desktop use. I'll add the dropdown in the next iteration.

Test Plan: {F105631}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T3623

Differential Revision: https://secure.phabricator.com/D8088
2014-01-28 20:18:01 -08:00
epriestley
e26fbbf973 Deprecate user.find
Summary: Super old method which is completely obsoleted by `user.query`

Test Plan: Poked around web UI, ran method.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8071
2014-01-25 14:23:39 -08:00
epriestley
f9ac534f25 Support CSRF for logged-out users
Summary: Fixes T4339. If you're anonymous, we use a digest of your session key to generate a CSRF token. Otherwise, everything works normally.

Test Plan: Logged out, logged in, tweaked CSRF in forms -- I'll add some inlines.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4339

Differential Revision: https://secure.phabricator.com/D8046
2014-01-23 14:03:54 -08:00
epriestley
69ddb0ced6 Issue "anonymous" sessions for logged-out users
Summary:
Ref T4339. Ref T4310. Currently, sessions look like `"afad85d675fda87a4fadd54"`, and are only issued for logged-in users. To support logged-out CSRF and (eventually) external user sessions, I made two small changes:

  - First, sessions now have a "kind", which is indicated by a prefix, like `"A/ab987asdcas7dca"`. This mostly allows us to issue session queries more efficiently: we don't have to issue a query at all for anonymous sessions, and can join the correct table for user and external sessions and save a query. Generally, this gives us more debugging information and more opportunity to recover from issues in a user-friendly way, as with the "invalid session" error in this diff.
  - Secondly, if you load a page and don't have a session, we give you an anonymous session. This is just a secret with no special significance.

This does not implement CSRF yet, but gives us a client secret we can use to implement it.

Test Plan:
  - Logged in.
  - Logged out.
  - Browsed around.
  - Logged in again.
  - Went through link/register.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4310, T4339

Differential Revision: https://secure.phabricator.com/D8043
2014-01-23 14:03:22 -08:00
epriestley
0727418023 Consolidate use of magical cookie name strings
Summary: Ref T4339. We have more magical cookie names than we should, move them all to a central location.

Test Plan: Registered, logged in, linked account, logged out. See inlines.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4339

Differential Revision: https://secure.phabricator.com/D8041
2014-01-23 14:01:35 -08:00
epriestley
01a80af976 Fix typo from initializeNewLog() refactor
Summary: I typo'd this.

See: <https://github.com/facebook/phabricator/issues/486>

Test Plan: Changed primary email address.

Auditors: btrahan
2014-01-20 10:04:22 -08:00
Chad Little
31a2bebf63 Move PhabricatorTagView to PHUITagView
Summary: For consistency and great justice.

Test Plan: tested audit, uiexamples, action headers

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7967
2014-01-14 14:09:52 -08:00
epriestley
a716fe99f3 Perform search indexing in the worker queue and respect bin/search index --background
Summary: Fixes T3857. Earlier work made this trivial and just left product questions, which I've answered by requiring the daemons to run on reasonable installs.

Test Plan: Ran `bin/search index` and `bin/search index --background`. Observed indexes write in the former case and tasks queue in the latter case. Commented with a unique string on a revision and searched for it a moment later, got exactly one result (that revision), verifying that reindexing works correctly.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3857

Differential Revision: https://secure.phabricator.com/D7966
2014-01-14 13:22:56 -08:00
epriestley
eef314b701 Separate session management from PhabricatorUser
Summary: Ref T4310. Ref T3720. Session operations are currently part of PhabricatorUser. This is more tightly coupled than needbe, and makes it difficult to establish login sessions for non-users. Move all the session management code to a `SessionEngine`.

Test Plan:
  - Viewed sessions.
  - Regenerated Conduit certificate.
  - Verified Conduit sessions were destroyed.
  - Logged out.
  - Logged in.
  - Ran conduit commands.
  - Viewed sessions again.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4310, T3720

Differential Revision: https://secure.phabricator.com/D7962
2014-01-14 13:22:27 -08:00
epriestley
220d680f37 Allow PhabricatorUserLog to store non-user PHIDs
Summary:
Ref T4310. This is a small step toward separating out the session code so we can establish sessions for `ExternalAccount` and not just `User`.

Also fix an issue with strict MySQL and un-admin / un-disable from web UI.

Test Plan: Logged in, logged out, admined/de-admin'd user, added email address, checked user log for all those events.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4310

Differential Revision: https://secure.phabricator.com/D7953
2014-01-14 11:05:26 -08:00
Chad Little
b74c7a3d37 Simplify PHUIObjectBoxViews handling of Save and Error states
Summary: This removes the bulk of the "Form Errors" text, some variations likely exists. These are a bit redundant and space consuming. I'd also like to back ErrorView more into PHUIObjectBox.

Test Plan: Test out the forms, see errors without the text.

Reviewers: epriestley, btrahan

CC: Korvin, epriestley, aran, hach-que

Differential Revision: https://secure.phabricator.com/D7924
2014-01-10 09:17:37 -08:00
Chad Little
ba7ac39130 Move people box on project pages
Summary: This removes the people box and adds a members property list item it's place. We may want some show/hide/see all if a project has more than //n// members, but these seems more reasonable than previous layout.

Test Plan: Tested a project with and without members, grepped for removed CSS, and tested mobile and desktop layouts.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7870
2013-12-31 16:28:35 -08:00
epriestley
a5dc9067af Provide convenience method addTextCrumb() to PhabricatorCrumbsView
Summary: We currently have a lot of calls to `addCrumb(id(new PhabricatorCrumbView())->...)` which can be expressed much more simply with a convenience method. Nearly all crumbs are only textual.

Test Plan:
  - This was mostly automated, then I cleaned up a few unusual sites manually.
  - Bunch of grep / randomly clicking around.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: hach-que, aran

Differential Revision: https://secure.phabricator.com/D7787
2013-12-18 17:47:34 -08:00
epriestley
5863f792a6 Remove many redundant implementations of canLoadNamedObject()
Summary:
These just got copy/pasted like crazy, the base class has the correct default implementation.

(I'm adding "H" for Herald Rules, which is why I was in this code.)

I also documented the existing prefixes at [[ Object Name Prefixes ]].

Test Plan: Verified base implementation. Typed some object names into the jump nav.

Reviewers: btrahan

Reviewed By: btrahan

CC: hach-que, aran

Differential Revision: https://secure.phabricator.com/D7785
2013-12-18 12:00:01 -08:00
epriestley
6eb02af314 Allow "bin/auth recover" to succeed before phabricator.base-uri is set
Summary:
Fixes T4132. If you run "bin/auth recover" before setting the base URI, it throws when trying to generate a production URI.

Instead, just show the path. We can't figure out the domain, and I think this is less confusing than showing "your.phabricator.example.com", etc.

Test Plan: Ran `bin/auth recover <user>` for valid and missing base-uri.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4132

Differential Revision: https://secure.phabricator.com/D7615
2013-11-20 10:36:00 -08:00
epriestley
c0e1a63a63 Implement an approval queue
Summary:
  - Add an option for the queue.
  - By default, enable it.
  - Dump new users into the queue.
  - Send admins an email to approve them.

Test Plan:
  - Registered new accounts with queue on and off.
  - As an admin, approved accounts and disabled the queue from email.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7576
2013-11-13 11:24:56 -08:00
epriestley
0fa411083f Show an "approval queue" item on the home page for admins, and sort out menu item visibility
Summary:
  - If you're an administrator and there are users waiting for approval, show a count on the home page.
  - Sort out the `isUserActivated()` access check.
  - Hide all the menu widgets except "Logout" for disabled and unapproved users.
  - Add a "Log In" item.
  - Add a bunch of unit tests.

Test Plan: Ran unit tests, clicked around as unapproved/approved/logged-in/logged-out users.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Differential Revision: https://secure.phabricator.com/D7574
2013-11-13 11:24:38 -08:00
epriestley
c8320923c4 Implement most of the administrative UI for approval queues
Summary:
Nothing fancy here, just:

  - UI to show users needing approval.
  - "Approve" and "Disable" actions.
  - Send "Approved" email on approve.
  - "Approve" edit + log operations.
  - "Wait for Approval" state for users who need approval.

There's still no natural way for users to end up not-approved -- you have to write directly to the database.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7573
2013-11-13 11:24:18 -08:00
epriestley
7f11e8d740 Improve handling of email verification and "activated" accounts
Summary:
Small step forward which improves existing stuff or lays groudwork for future stuff:

  - Currently, to check for email verification, we have to single-query the email address on every page. Instead, denoramlize it into the user object.
    - Migrate all the existing users.
    - When the user verifies an email, mark them as `isEmailVerified` if the email is their primary email.
    - Just make the checks look at the `isEmailVerified` field.
  - Add a new check, `isUserActivated()`, to cover email-verified plus disabled. Currently, a non-verified-but-not-disabled user could theoretically use Conduit over SSH, if anyone deployed it. Tighten that up.
  - Add an `isApproved` flag, which is always true for now. In a future diff, I want to add a default-on admin approval queue for new accounts, to prevent configuration mistakes. The way it will work is:
    - When the queue is enabled, registering users are created with `isApproved = false`.
    - Admins are sent an email, "[Phabricator] New User Approval (alincoln)", telling them that a new user is waiting for approval.
    - They go to the web UI and approve the user.
    - Manually-created accounts are auto-approved.
    - The email will have instructions for disabling the queue.

I think this queue will be helpful for new installs and give them peace of mind, and when you go to disable it we have a better opportunity to warn you about exactly what that means.

Generally, I want to improve the default safety of registration, since if you just blindly coast through the path of least resistance right now your install ends up pretty open, and realistically few installs are on VPNs.

Test Plan:
  - Ran migration, verified `isEmailVerified` populated correctly.
  - Created a new user, checked DB for verified (not verified).
  - Verified, checked DB (now verified).
  - Used Conduit, People, Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Differential Revision: https://secure.phabricator.com/D7572
2013-11-12 14:37:04 -08:00
Jakub Vrana
a29b5b070f Replace some hsprintf() by phutil_tag()
Test Plan: Looked at a diff with inline comment.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7549
2013-11-11 09:23:23 -08:00
epriestley
2a5c987c71 Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.

This has several parts:

  - For PolicyAware queries, provide an application class name method.
  - If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
  - For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.

Test Plan:
  - Added a unit test to verify I got all the class names right.
  - Browsed around, logged in/out as a normal user with public policies on and off.
  - Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
epriestley
d66972c9f2 Tie application event listeners to the applications they listen for
Summary:
Ref T3675. Some of these listeners shouldn't do their thing if the viewer doesn't have access to an application (for example, users without access to Differential should not be able to "Edit Tasks"). Set the stage for that:

  - Introduce `PhabricatorEventListener`, which has an application.
  - Populate this for event listeners installed by applications.
  - Rename the "PeopleMenu" listeners to "ActionMenu" listeners, which better describes their modern behavior.

This doesn't actually change any behaviors.

Test Plan: Viewed Maniphest, Differntial, People.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3675

Differential Revision: https://secure.phabricator.com/D7364
2013-10-21 17:00:21 -07:00
epriestley
c4abf160cc Fix some file policy issues and add a "Query Workspace"
Summary:
Ref T603. Several issues here:

  1. Currently, `FileQuery` does not actually respect object attachment edges when doing policy checks. Everything else works fine, but this was missing an `array_keys()`.
  2. Once that's fixed, we hit a bunch of recursion issues. For example, when loading a User we load the profile picture, and then that loads the User, and that loads the profile picture, etc.
  3. Introduce a "Query Workspace", which holds objects we know we've loaded and know we can see but haven't finished filtering and/or attaching data to. This allows subqueries to look up objects instead of querying for them.
    - We can probably generalize this a bit to make a few other queries more efficient. Pholio currently has a similar (but less general) "mock cache". However, it's keyed by ID instead of PHID so it's not easy to reuse this right now.

This is a bit complex for the problem being solved, but I think it's the cleanest approach and I believe the primitive will be useful in the future.

Test Plan: Looked at pastes, macros, mocks and projects as a logged-in and logged-out user.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7309
2013-10-14 14:36:06 -07:00
epriestley
073cb0e78c Make PhabricatorPolicyInterface require a getPHID() method
Summary:
Ref T603. This cleans up an existing callsite in the policy filter, and opens up some stuff in the future.

Some policy objects don't have real PHIDs:

  PhabricatorTokenGiven
  PhabricatorSavedQuery
  PhabricatorNamedQuery
  PhrequentUserTime
  PhabricatorFlag
  PhabricatorDaemonLog
  PhabricatorConduitMethodCallLog
  ConduitAPIMethod
  PhabricatorChatLogEvent
  PhabricatorChatLogChannel

Although it would be reasonable to add real PHIDs to some of these (like `ChatLogChannel`), it probably doesn't make much sense for others (`DaemonLog`, `MethodCallLog`). Just let them return `null`.

Also remove some duplicate `$id` and `$phid` properties. These are declared on `PhabricatorLiskDAO` and do not need to be redeclared.

Test Plan: Ran the `testEverythingImplemented` unit test, which verifies that all classes conform to the interface.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7306
2013-10-14 14:35:47 -07:00
Chad Little
97c690fc0f PHUIPropertyListView
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.

Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Differential Revision: https://secure.phabricator.com/D7283
2013-10-11 07:53:56 -07:00
epriestley
0598600476 Always pass handles to tokenizers, not <phid -> name> maps
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.

Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

Differential Revision: https://secure.phabricator.com/D7248
2013-10-07 12:51:24 -07:00
epriestley
515f9a36ab When editing objects which use files, attach the files to the objects
Summary: Ref T603. Fixes T3921. Tightens up policy controls for file/object relationships in existing applications.

Test Plan:
  - Uploaded new project image, verified it got an edge to the project.
  - Uploaded new profile image, verified it got an edge to me.
  - Uploaded new macro image, verified it got an edge to the macro.
  - Uploaded new paste via web UI and conduit, verified it got attached.
  - Replaced, added images to a mock, verified they got edges.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3921, T603

Differential Revision: https://secure.phabricator.com/D7254
2013-10-06 17:07:55 -07:00
epriestley
13dae05193 Make most file reads policy-aware
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.

Test Plan:
  - Viewed Differential changesets.
  - Used `file.info`.
  - Used `file.download`.
  - Viewed a file.
  - Deleted a file.
  - Used `/Fnnnn` to access a file.
  - Uploaded an image, verified a thumbnail generated.
  - Created and edited a macro.
  - Added a meme.
  - Did old-school attach-a-file-to-a-task.
  - Viewed a paste.
  - Viewed a mock.
  - Embedded a mock.
  - Profiled a page.
  - Parsed a commit with image files linked to a revision with image files.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7178
2013-09-30 09:38:13 -07:00
epriestley
b592630d72 Provide more structure to PHUIObjectBoxView
Summary:
Three changes here.

  - Add `setActionList()`, and use that to set the action list.
  - Add `setPropertyList()`, and use that to set the property list.

These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.

  - Replace `addContent()` with `appendChild()`.

This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.

Test Plan:
  - Viewed "All Config".
  - Viewed a countdown.
  - Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
  - Viewed Diffusion (browse, change, history, repository, lint).
  - Viewed Drydock (resource, lease).
  - Viewed Files.
  - Viewed Herald.
  - Viewed Legalpad.
  - Viewed macro (edit, edit audio, view).
  - Viewed Maniphest.
  - Viewed Applications.
  - Viewed Paste.
  - Viewed People.
  - Viewed Phulux.
  - Viewed Pholio.
  - Viewed Phame (blog, post).
  - Viewed Phortune (account, product).
  - Viewed Ponder (questions, answers, comments).
  - Viewed Releeph.
  - Viewed Projects.
  - Viewed Slowvote.

NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?

NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D7174
2013-09-30 09:36:04 -07:00
Chad Little
94d0704fdb Add objectheaders to new View
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.

Test Plan: tested each new layout.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7162
2013-09-28 15:55:38 -07:00
epriestley
2e5ac128b3 Explain policy exception rules to users
Summary:
Ref T603. Adds clarifying text which expands on policies and explains exceptions and rules. The goal is to provide an easy way for users to learn about special policy rules, like "task owners can always see a task".

This presentation might be a little aggressive. That's probably OK as we introduce policies, but something a little more tempered might be better down the road.

Test Plan: See screenshot.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7150
2013-09-27 08:43:41 -07:00
Chad Little
9be7a948f9 Move PHUIFormBoxView to PHUIObjectBoxView
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.

Test Plan: reload a few forms in maniphest, projects, differential

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7120
2013-09-25 11:23:29 -07:00
Chad Little
0d77a7f39f ObjectHeader Status icons
Summary: Adds status icons and colors to Maniphest and Differential. Also minor tweaks to them in hovercards. Probably some other stuff too.

Test Plan: Test many diff and task states.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7098
2013-09-24 08:42:04 -07:00
epriestley
3f24232d2b Allow custom fields to have validation logic
Summary:
Ref T418. This is fairly messy, but basically:

  - Add a validation phase to TransactionEditor.
  - Add a validation phase to CustomField.
  - Bring it to StandardField.
  - Add validation logic for the int field.
  - Provide support in related classes.

Test Plan: See screenshot.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D7028
2013-09-18 15:31:58 -07:00
Chad Little
e8bb24fd60 Policy, Status in PHUIHeaderView
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.

Test Plan: Implemented in Paste, Pholio. Tested various states.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7016
2013-09-17 09:12:37 -07:00
epriestley
ed7a5078f9 Add "user" and "users" standard custom fields
Summary: These end up a little weird with subclassing instead of `switch`, but some day we could alias them to one another or something I guess. If I'm feeling brave, I might get rid of the "user" variant when I migrate Maniphest custom field specs, and turn it into "users, limit = 1" or something like that.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7010
2013-09-16 16:04:46 -07:00
epriestley
6115670615 Add a "date" standard custom field
Summary: See previous revisions. As maniphest.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7009
2013-09-16 16:04:31 -07:00
epriestley
ed126cd47e Provide ApplicationSearch hooks in Maniphest
Summary: Ref T418. Adds hooks to support customized ApplicationSearch (you can't currently add indexable fields without writing custom code).

Test Plan: Wrote custom code to add an indexable field.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D7002
2013-09-16 16:03:09 -07:00
epriestley
c8574cf6fd Integrate ApplicationSearch with CustomField
Summary:
Ref T2625. Ref T3794. Ref T418. Ref T1703.

This is a more general version of D5278. It expands CustomField support to include real integration with ApplicationSearch.

Broadly, custom fields may elect to:

  - build indicies when objects are updated;
  - populate ApplicationSearch forms with new controls;
  - read inputs entered into those controls out of the request; and
  - apply constraints to search queries.

Some utility/helper stuff is provided to make this easier. This part could be cleaner, but seems reasonable for a first cut. In particular, the Query and SearchEngine must manually call all the hooks right now instead of everything happening magically. I think that's fine for the moment; they're pretty easy to get right.

Test Plan:
I added a new searchable "Company" field to People:

{F58229}

This also cleaned up the disable/reorder view a little bit:

{F58230}

As it did before, this field appears on the edit screen:

{F58231}

However, because it has `search`, it also appears on the search screen:

{F58232}

When queried, it returns the expected results:

{F58233}

And the actually good bit of all this is that the query can take advantage of indexes:

  mysql> explain SELECT * FROM `user` user JOIN `user_customfieldstringindex` `appsearch_0` ON `appsearch_0`.objectPHID = user.phid AND `appsearch_0`.indexKey = 'mk3Ndy476ge6' AND `appsearch_0`.indexValue IN ('phacility') ORDER BY user.id DESC LIMIT 101;
  +----+-------------+-------------+--------+-------------------+----------+---------+------------------------------------------+------+----------------------------------------------+
  | id | select_type | table       | type   | possible_keys     | key      | key_len | ref                                      | rows | Extra                                        |
  +----+-------------+-------------+--------+-------------------+----------+---------+------------------------------------------+------+----------------------------------------------+
  |  1 | SIMPLE      | appsearch_0 | ref    | key_join,key_find | key_find | 232     | const,const                              |    1 | Using where; Using temporary; Using filesort |
  |  1 | SIMPLE      | user        | eq_ref | phid              | phid     | 194     | phabricator2_user.appsearch_0.objectPHID |    1 |                                              |
  +----+-------------+-------------+--------+-------------------+----------+---------+------------------------------------------+------+----------------------------------------------+
  2 rows in set (0.00 sec)

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418, T1703, T2625, T3794

Differential Revision: https://secure.phabricator.com/D6992
2013-09-16 13:44:34 -07:00
Bob Trahan
07b8becfc6 Policy - introduce parentQuery and pass around policy configuration from parent to child
Summary: Ref T603. Ref D6941.

Test Plan: Clicked around all over - looked good. I plan to re-test D6941 to make sure the executeOne case works now as intended

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6944
2013-09-11 12:19:34 -07:00
Chad Little
5ba20b8924 Move PhabricatorObjectItem to PHUIObjectItem, add 'plain' setting for lists.
Summary: Adds plain support for object lists that just look like lists

Test Plan: review UIexamples and a number of other applications

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6922
2013-09-09 14:14:34 -07:00
epriestley
d1225e782b Don't try to load user profile images in PhabricatorPeopleQuery if no users have any
Summary:
Fixes T3810. In PhabricatorPeopleQuery, we issue an unnecessary query like this:

  SELECT f.* FROM file f WHERE (f.phid IN ('')) ORDER BY f.id DESC

...if we're loading a user without a profile picture. Filter the file PHIDs before loading them to prevent this.

This doesn't change anything, but saves us a spurious/silly query.

Also makes `PhabricatorPeopleProfileController` use `needProfileImage()`, moving us closer to getting rid of `loadProfileImageURI()` eventually.

Test Plan: Looked at profiles of users with and without profile pictures. Checked query log in DarkConsole.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T3810

Differential Revision: https://secure.phabricator.com/D6913
2013-09-08 09:43:27 -07:00
Gareth Evans
fcba0c74d9 Replace all "attach first..." exceptions with assertAttached()
Summary:
Ref T3599
Go through everything, grep a bit, replace some bits.

Test Plan: Navigate around a bit

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3599

Differential Revision: https://secure.phabricator.com/D6871
2013-09-03 06:02:14 -07:00
Aviv Eyal
cf13885736 User preference for time format
Summary: Also, don't try to load prefs for non-users.

Test Plan: toggle, save, look at something with a time. arc unit.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6796
2013-09-02 12:55:11 -07:00
Chad Little
bb9be01d55 Update forms to use PHUIFormBoxView
Summary: Some more callsites, let me know if you see others, I think think is 98% of them now.

Test Plan: tested each page

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6814
2013-08-26 15:45:58 -07:00
Chad Little
fe2a96e37f Update Form Layouts
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.

TODO: will take another pass as consolidating these colors and new gradients in another diff.

Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6806
2013-08-26 11:53:11 -07:00
epriestley
f034fd80db Remove getApplicationObjectTypeName from ApplicationTransactions
Summary:
We can get this out of PHIDType reasonably in all cases and simplify implementation here.

None of these translate correctly anyway so they're basically debugging/development strings.

Test Plan: `grep`, browsed some transactions

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6786
2013-08-21 12:32:06 -07:00
epriestley
751cd547c2 Remove dust from page construction
Summary:
  ^\s+(['"])dust\1\s*=>\s*true,?\s*$\n

Test Plan: Looked through the diff.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6769
2013-08-19 18:09:35 -07:00
Chad Little
d02eb46ad6 Add hovercard on/off option to PhabricatorFeedStory
Summary: Defaults hovercards off everywhere feed stories are shown. I tried to find where to put this in so /feed/ could display them, but got horribly lost and confused in SearchQueryLandView

Test Plan: turn hovercards on and off, inspect elements.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6757
2013-08-14 13:20:25 -07:00
epriestley
026137f92f Further simplify PhabricatorCustomFieldInterface
Summary:
Ref T1703. Ref T3718. This introduces `PhabricatorCustomFieldAttachment`, which is just a fancy `array()`. The goal here is to simplify `PhabricatorCustomFieldInterface` as much as possible.

In particular, it can now use common infrastructure (`assertAttached()`) and is more difficult to get wrong.

Test Plan: Edited custom fields on profile.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1703, T3718

Differential Revision: https://secure.phabricator.com/D6752
2013-08-14 12:34:09 -07:00
epriestley
938b63aaa9 Simplify and improve PhabricatorCustomField APIs
Summary:
Ref T1703. Ref T3718. The `PhabricatorCustomFieldList` seems like a pretty good idea. Move more code into it to make it harder to get wrong.

Also the sequencing on old/new values for these transactions was a bit off; fix that up.

Test Plan: Edited standard and custom profile fields.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1703, T3718

Differential Revision: https://secure.phabricator.com/D6751
2013-08-14 12:34:08 -07:00
epriestley
ca0115b361 Support configuration-driven custom fields
Summary:
Ref T1702. Ref T3718. There are a couple of things going on here:

**PhabricatorCustomFieldList**: I added `PhabricatorCustomFieldList`, which is just a convenience class for dealing with lists of fields. Often, current field code does something like this inline in a Controller:

  foreach ($fields as $field) {
    // do some junk
  }

Often, that junk has some slightly subtle implications. Move all of it to `$list->doSomeJunk()` methods (like `appendFieldsToForm()`, `loadFieldsFromStorage()`) to reduce code duplication and prevent errors. This additionally moves an existing list-convenience method there, out of `PhabricatorPropertyListView`.

**PhabricatorUserConfiguredCustomFieldStorage**: Adds `PhabricatorUserConfiguredCustomFieldStorage` for storing custom field data (like "ICQ Handle", "Phone Number", "Desk", "Favorite Flower", etc).

**Configuration-Driven Custom Fields**: Previously, I was thinking about doing these with interfaces, but as I thought about it more I started to dislike that approach. Instead, I built proxies into `PhabricatorCustomField`. Basically, this means that fields (like a custom, configuration-driven "Favorite Flower" field) can just use some other Field to actually provide their implementation (like a "standard" field which knows how to render text areas). The previous approach would have involed subclasssing the "standard" field and implementing an interface, but that would mean that every application would have at least two "base" fields and generally just seemed bleh as I worked through it.

The cost of this approach is that we need a bunch of `proxy` junk in the base class, but that's a one-time cost and I think it simplifies all the implementations and makes them a lot less magical (e.g., all of the custom fields now extend the right base field classes).

**Fixed Some Bugs**: Some of this code hadn't really been run yet and had minor bugs.

Test Plan:
{F54240}
{F54241}
{F54242}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1702, T1703, T3718

Differential Revision: https://secure.phabricator.com/D6749
2013-08-14 12:33:53 -07:00
epriestley
a530004ac7 Raise an error if a user tries to register with an excessively long username
Summary: Fixes T2348. We should probably do some of this more broadly, but can tackle them one at a time as they arise, since many fields have no effective length limit.

Test Plan: {F54126}

Reviewers: btrahan, asherkin

Reviewed By: asherkin

CC: aran

Maniphest Tasks: T2348

Differential Revision: https://secure.phabricator.com/D6744
2013-08-13 14:37:23 -07:00
epriestley
7298589c86 Proof of concept mitigation of BREACH
Summary: Ref T3684 for discussion. This could be cleaned up a bit (it would be nice to draw entropy once per request, for instance, and maybe respect CSRF_TOKEN_LENGTH more closely) but should effectively mitigate BREACH.

Test Plan: Submitted forms; submitted forms after mucking with CSRF and observed CSRF error. Verified that source now has "B@..." tokens.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3684

Differential Revision: https://secure.phabricator.com/D6686
2013-08-07 16:09:05 -07:00
epriestley
fd2593e8ab Restore setting "disabled" on user handles of disabled users
Summary:
Fixes T3666. D6585 updated the User handles, but accidentally dropped this unusual property.

We should get rid of this -- it doesn't really make any sense on Handles -- but restore the previous beahvior to fix T3666 until we can nuke it.

Test Plan: Clicked some pages? (Actually testing this properly is a bit of a pain and I am super lazy.)

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3666

Differential Revision: https://secure.phabricator.com/D6644
2013-08-01 14:50:45 -07:00
Bob Trahan
1cb0db8755 Move PhabricatorUser to new phid stuff
Summary: Ref T2715. Had to start loading status information in the query class. Debated trying to clean up some of the attach / load stuff but decided to just add status under the new paradigm for now.

Test Plan: phid.query  also made a status and checked that out. also played in conpherence.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6585
2013-07-26 14:05:19 -07:00
epriestley
db3a0c90bb Use Application PHIDs for XUSR
Summary: Ref T2715. XUSR -> apps

Test Plan: `phid.query`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6558
2013-07-24 14:12:39 -07:00
epriestley
ad38252c68 Fix fatal on setting default profile picture
Summary: I thought I fixed this but must have not saved in my editor or something. Fixes T3552.

Test Plan: Set profile image to default.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3552

Differential Revision: https://secure.phabricator.com/D6477
2013-07-16 13:54:14 -07:00
Bob Trahan
9838251515 Make PhabricatorActionListView logged-out user savvy
Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.

Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.

Test Plan: played around with a mock all logged out (Ref T2652) and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2691

Differential Revision: https://secure.phabricator.com/D6416
2013-07-12 11:39:47 -07:00
epriestley
d1e25e3d11 Fix UI for choosing a profile picture when user has no picture
Summary: Currently, we always show "current picture" even if you don't have one. In this case, the first available picture becomes the "current picture". Instead, show "current picture" only if there's actually a current picture to show.

Test Plan: {F49875}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6432
2013-07-12 11:20:18 -07:00
epriestley
cff8c50903 Modernize email verification page
Summary: Fixes T3517. Moves the email verification page out of People and into Auth. Makes it look less awful.

Test Plan: {F49636} {F49637}

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3517

Differential Revision: https://secure.phabricator.com/D6425
2013-07-10 18:53:09 -07:00
epriestley
a55089e628 Move roles and status into properties on profile view
Summary: See discussion in D6403.

Test Plan: {F49488}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6409
2013-07-10 12:34:09 -07:00
epriestley
6a40df529d Use ActionListView for all profile actions
Summary:
This leaves the space between the properties and the blurb looking a bit empty, but there will be more stuff there soon (status, VCS names, email, phone/fax numbers, etc., and custom user fields).

I removed "view lint messages" since I'm pretty sure no one has ever clicked it. I think providing better search (e.g, T2625) to that UI in Diffusion is a preferable approach.

Test Plan: {F49423}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6403
2013-07-10 05:11:08 -07:00
epriestley
0125984226 Use tags to show user status
Summary: I'll probably move "away" to a custom field, but this is technically broken right now.

Test Plan: {F49416}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6402
2013-07-10 05:10:54 -07:00
epriestley
b92fe7dbda Merge profile "About" into main profile and simplify some custom field stuff
Summary:
Ref T1703. Drive "user since" with a custom field and make the other fields render into a property list.

Users can make their profiles a little more personal/obnoxious now.

Also delete a bunch of code.

Test Plan: {F49415}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6401
2013-07-10 05:09:59 -07:00
epriestley
37b13ef2c9 Improve UI for selecting profile pictures
Summary:
Ref T1703. Move profile pictures to a separate, dedicated interface. Instead of the 35 controls we currently provide, just show all the possible images we can find and then let the user upload an additional one if they want.

Possible improvements to this interface:

  - Write an edge so we can show old profile pictures too.
  - The cropping/scaling got a bit buggy at some point, fix that.
  - Refresh OAuth sources which we're capable of refreshing before showing images (more work than I really want to deal with).
  - We could show little inset icons for the image source ("f" for Facebook, etc.) instead of just the tooltips.

Test Plan:
Chose images, uploaded new images, hit various error cases.

{F49344}

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2919, T1703

Differential Revision: https://secure.phabricator.com/D6398
2013-07-09 16:23:54 -07:00
epriestley
62ab1dcc62 Move profile-related "settings" and "account" panel information to "edit profile"
Summary: Ref T1703. Put this stuff on Profile -> Edit Profile instead of spread across Settings -> Account and Settings -> Profile. Makes it custom-field driven.

Test Plan: {F49333} {F49334}

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6397
2013-07-09 16:23:45 -07:00
epriestley
c5f2e617d8 Simplify and generalize remarkup engine construction
Summary:
Depends on D6395.

  - Now that inline rules have explicit priorities, they can just go in applications in all cases.
  - We don't need the inline rule conditionals anymore after D6395.

Test Plan: Wrote remarkup with mentions, phriction links, countdowns, etc.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6396
2013-07-09 16:23:33 -07:00
epriestley
f548dc0067 Remove PhabricatorProfileHeaderView in favor of PhabricatorHeaderView
Summary:
We have this old view which is only used in two places and looks the same but has totally different markup. Get rid of it.

@chad, I'm generally going to move the user/project profiles a step toward looking like other object detail view with the custom field stuff. Not sure if you have any grand vision here; we can easily do something else later since this is like 80% "delete weird epriestley one-offs that don't look quite right in favor of standard elements".

Test Plan: {F49324} {F49325} {F49326}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6394
2013-07-09 16:23:22 -07:00
epriestley
e5f200c654 Allow custom fields to be reordered and disabled from Config
Summary: Ref T1703. Put a more reasonable UI than "blob of JSON" on top of this.

Test Plan:
Reordered, enabled and disabled user profile fields.

{F49317}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6393
2013-07-09 08:27:19 -07:00
epriestley
23e18b1ca5 Provide PhabricatorSavedQuery to renderResultsList()
Summary: This allows the SavedQuery to modify what the result list looks like (e.g., include display flags and similar).

Test Plan: Looked at some ApplicationSearch apps.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6346
2013-07-03 05:46:04 -07:00
epriestley
1834584e98 Provide contextual help on auth provider configuration
Summary:
Ref T1536.

  - Move all the provider-specific help into contextual help in Auth.
  - This provides help much more contextually, and we can just tell the user the right values to use to configure things.
  - Rewrite account/registration help to reflect the newer state of the word.
  - Also clean up a few other loose ends.

Test Plan: {F46937}

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6247
2013-06-20 11:18:48 -07:00
epriestley
3b9ccf11f2 Drive auth config with the database
Summary: Ref T1536. This is the last major migration. Moves us over to the DB and drops all the config stuff.

Test Plan:
  - Ran the migration.
  - Saw all my old config brought forward and respected, with accurate settings.
  - Ran LDAP import.
  - Grepped for all removed config options.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran, wez

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6243
2013-06-20 11:18:11 -07:00
epriestley
a905e8ae5a Disambiguate "id" column in People query which joins Email
Summary: We end up with both "user.id" and "email.id". Disambiguate for ORDER.

Test Plan: Ran Conduit user.query query with "email".

Reviewers: wez, chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6234
2013-06-19 11:18:40 -07:00
epriestley
73c2c1d2e6 Send old login code to the bottom of the sea
Summary:
Ref T1536. This is extremely reachable and changes the login code to the new stuff.

Notes:

  - I've hard-disabled password registration since I want installs to explicitly flip it on via config if they want it. New installs will get it by default in the future, but old installs shouldn't have their auth options change.
  - Google doesn't let us change the redirect URI, so keep the old one working.
  - We need to keep a bit of LDAP around for now for LDAP import.
  - **Facebook:** This causes substantive changes in what login code is executed.

Test Plan:
  - Logged in / logged out / registered, hit new flows.
  - Logged in with google.
  - Verified no password registration by default.

Reviewers: btrahan, chad

Reviewed By: chad

CC: wez, nh, aran, mbishopim3

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6222
2013-06-19 01:33:27 -07:00
epriestley
30237aaa47 Clean up image loading for ExternalAccounts
Summary: Ref T1536. This gets the single queries out of the View and builds a propery Query class for ExternalAccount.

Test Plan: Linked/unlinked accounts, logged out, logged in.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6212
2013-06-17 12:14:00 -07:00
epriestley
b040f889de Move all account link / unlink to new registration flow
Summary:
Ref T1536. Currently, we have separate panels for each link/unlink and separate controllers for OAuth vs LDAP.

Instead, provide a single "External Accounts" panel which shows all linked accounts and allows you to link/unlink more easily.

Move link/unlink over to a full externalaccount-based workflow.

Test Plan:
  - Linked and unlinked OAuth accounts.
  - Linked and unlinked LDAP accounts.
  - Registered new accounts.
  - Exercised most/all of the error cases.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran, mbishopim3

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6189
2013-06-17 06:12:45 -07:00
epriestley
c108ada7e4 Provide start screen and full registration flow on the new auth stuff
Summary:
Ref T1536. Code is intentionally made unreachable (see PhabricatorAuthProviderOAuthFacebook->isEnabled()).

This adds:

  - A provider-driven "start" screen (this has the list of ways you can login/register).
  - Registration actually works.
  - Facebook OAuth works.

@chad, do you have any design ideas on the start screen? I think we poked at it before, but the big issue was that there were a limitless number of providers. Today, we have:

  - Password
  - LDAP
  - Facebook
  - GitHub
  - Phabricator
  - Disqus
  - Google

We plan to add:

  - Asana
  - An arbitrary number of additional instances of Phabricator

Users want to add:

  - OpenID
  - Custom providers

And I'd like to have these at some point:

  - Stripe
  - WePay
  - Amazon
  - Bitbucket

So basically any UI for this has to accommodate 300 zillion auth options. I don't think we need to solve any UX problems here (realistically, installs enable 1-2 auth options and users don't actually face an overwhelming number of choices) but making the login forms less ugly would be nice. No combination of prebuilt elements seems to look very good for this use case.

Test Plan: Registered a new acount with Facebook.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6161
2013-06-16 10:15:16 -07:00
epriestley
c05ee9ed68 Generalize login flows for new registration
Summary:
Ref T1536. None of this code is reachable.

`PhabricatorAuthLoginController` provides a completely generic login/link flow, similar to how D6155 provides a generic registration flow.

`PhabricatorAuthProvider` wraps a `PhutilAuthAdapter` and glues the generic top-level flow to a concrete authentication provider.

Test Plan: Static only, code isn't meaningfully reachable.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6159
2013-06-16 10:14:07 -07:00
epriestley
db1cf41ec4 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
epriestley
8744cdb699 Migrate PhabricatorUserLDAPInfo to PhabricatorExternalAccount
Summary: Ref T1536. This is similar to D6172 but much simpler: we don't need to retain external interfaces here and can do a straight migration.

Test Plan: TBA

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6173
2013-06-16 09:55:55 -07:00
epriestley
8111dc74bf Migrate the OAuthInfo table to the ExternalAccount table
Summary: Ref T1536. Migrates the OAuthInfo table to ExternalAccount, and makes `PhabricatorUserOAuthInfo` a wrapper for an ExternalAccount.

Test Plan: Logged in with OAuth, registered with OAuth, linked/unlinked OAuth accounts, checked OAuth status screen, deleted an account with related OAuth.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6172
2013-06-14 07:04:41 -07:00
epriestley
bce4b7addf Internalize storage access for PhabricatorUserOAuthInfo
Summary:
Ref T1536. Move all access to the underlying storage to inside the class. My plan is:

  - Migrate the table to ExternalAccount.
  - Nuke the table.
  - Make this class read from and write to ExternalAccount instead.

We can't get rid of OAuthInfo completely because Facebook still depends on it for now, via registration hooks.

Test Plan: Logged in and registered with OAuth.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6171
2013-06-14 07:00:29 -07:00
epriestley
3005811b9e Remove OAuth token/expiry interfaces
Summary:
Ref T1536. Currently, we store OAuth tokens along with their expiry times and status. However, all we use this for is refreshing profile pictures and showing a silly (and probably somewhat confusing) interface about token status.

I want to move this storage over to `PhabricatorExternalAccount` to make the cutover easier. Drop it for now, including all the profile image stuff (I plan to rebuild that in a more sensible way anyway).

Test Plan: Viewed screen; linked/unlinked accounts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6170
2013-06-14 06:59:23 -07:00
epriestley
8886416e30 Expand the "PhabricatorExternalAccount" table for new registration
Summary:
Ref T1536. This is the schema code for `PhabricatorExternalAccount` which was previously in D4647. I'm splitting it out so I can put it earlier in the sequence and because it's simple and standalone.

Expands `PhabricatorExternalAccount` to have everything we need for the rest of registration.

Test Plan: Implemented the remainder of new registration on top of this.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6169
2013-06-14 06:55:18 -07:00
Chad Little
fed0a2a496 Fix search and core icons in top nav
Summary: Fixes T3330

Test Plan: Test desktop and mobile menus in chrome and ios.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T3330

Differential Revision: https://secure.phabricator.com/D6157
2013-06-09 09:11:32 -07:00
epriestley
059183f6b5 Allow configuration to have custom UI types
Summary:
Ref T1703. This sets the stage for (but does not yet implement) custom UI types for config. In particular, a draggable list for custom fields.

I might make all the builtin types go through this at some point too, but don't really want to bother for the moment. It would be very slightly cleaner but woudn't get us much of anything.

Test Plan:
UI now renders via custom code, although that code does nothing (produces an unadorned text field):

{F45693}

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6154
2013-06-07 12:36:18 -07:00
epriestley
77c03a8a42 Add "Title" and "Blurb" to new profile editor
Summary:
Ref T1703.

  - Adds "Title".
  - Adds "Blurb".
  - Adds `user.fields` config for selecting and reordering. This will get UI in the next patch.

Test Plan:
{F45689}

{F45690}

Edited the fields, too.

Reviewers: chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6153
2013-06-07 10:22:45 -07:00
epriestley
6ffbee115b Add ApplicationTransactions/CustomField based user profile editor
Summary:
Adds a profile edit controller (with just one field and on links to it) that uses ApplicationTransactions and CustomField.

{F45617}

My plan is to move the other profile fields to this interface and get rid of Settings -> Profile. Basically, these will be "settings":

  - Sex
  - Language
  - Timezone

These will be "profile":

  - Real Name
  - Title
  - Blurb
  - Profile Image (but I'm going to put this on a separate UI)
  - Other custom fields

Test Plan: Edited my realname using the new interface.

Reviewers: chad, seporaitis

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6152
2013-06-07 09:55:55 -07:00
epriestley
524c2acb3d Flesh out ApplicationTransactions/CustomField integration
Summary:
None of this code is reachable yet. See discussion in D6147. Ref T1703.

Provide tighter integration between ApplicationTransactions and CustomField. Basically, I'm just trying to get all the shared stuff into the base implementation.

Test Plan: Code not reachable.

Reviewers: chad, seporaitis

Reviewed By: chad

CC: aran

Maniphest Tasks: T1703

Differential Revision: https://secure.phabricator.com/D6149
2013-06-06 14:53:07 -07:00
epriestley
7615e59cf9 Default application search to the topmost filter
Summary: Allow users to set a default by dragging it to the top. When they land on a page without a saved query, choose their default.

Test Plan: Hit `/paste/`, got my default results, etc.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6140
2013-06-05 18:58:50 -07:00
Chad Little
f1bf27959f PHUIList, PHUIDocument updates
Summary:
This diff covers a bit of ground.

- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView

Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.

Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6130
2013-06-05 08:41:43 -07:00
epriestley
59cea9bfc3 Implement ApplicationSearch in People
Summary:
Ref T2625. Fixes T2812. Implement ApplicationSearch in People.

{F44788}

Test Plan: Made People queries. Used Conduit. Used `@mentions`.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T2625, T2812

Differential Revision: https://secure.phabricator.com/D6092
2013-05-31 10:51:20 -07:00
Jakub Vrana
32f91557f8 Store hash of session key
Summary:
This prevents security by obscurity.
If I have read-only access to the database then I can pretend to be any logged-in user.

I've used `PhabricatorHash::digest()` (even though we don't need salt as the hashed string is random) to be compatible with user log.

Test Plan:
Applied patch.
Verified I'm still logged in.
Logged out.
Logged in.

  $ arc tasks

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D6080
2013-05-30 17:30:06 -07:00
epriestley
27a0265367 Fix a small bug with empty user queries
Summary: This does the wrong thing (fatals) if there are no passed PHIDs.

Test Plan: No more fatal.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D5940
2013-05-17 03:55:45 -07:00
Chad Little
a420f3cbbc Update Forms on People.
Summary: Makes all forms on People app consistent with rest of site.

Test Plan: Click each page

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D5865
2013-05-08 10:42:20 -07:00
Bryan Cuccioli
c5692bb9bf No longer send welcome mail from a real user.
Summary: Remove lines that set from field.

Test Plan: Check that emails are sent without from field.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan

Differential Revision: https://secure.phabricator.com/D5845
2013-05-06 14:00:40 -07:00
Afaque Hussain
f76ecd9e76 Adding Handle Suport
Summary: Added PHID_TYPE_XUSR to loadObjectsofType() of PhabricatorObjectHandle data. Was trying to solve the setActor() issue. Created a getPhabricator method in PhabricatorExternalAccount. When I try to set authorPHID using setAuthorPHID() in PhabricatorExternalAccount it's saying bad setter call, if I don't, then it's saying authorPHID can't be null. Sending you diff for comments on this.

Test Plan:
{F42523}

{F42525}

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan

Differential Revision: https://secure.phabricator.com/D5839
2013-05-06 11:34:48 -07:00
Zedstar
f53cde8f92 Using PhabricatorExternalAccount
Summary: Using PhabricatorExternalAccount in place maniphest.default-public-author.

Test Plan:
Using receivemail to see if the a new entry is made in the 'phabircator_user.user_externalaccount' table. Few things, I noticed that phabricator creates table 'user_externalaccout'. And now it throws up error 'Unknown column 'dateCreated' in 'field list''. Awaiting your comments.
{F41370}

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5747
2013-04-28 13:22:33 -07:00
deedydas
1e2f01efc2 Fixed Task Generation
Summary: Ref T2903

Test Plan: Successfully generated tasks with authors and owners.

Reviewers: epriestley, AnhNhan

Reviewed By: epriestley

CC: aran, Korvin, AnhNhan, chad

Maniphest Tasks: T2903

Differential Revision: https://secure.phabricator.com/D5704
2013-04-24 18:17:31 -07:00