Summary:
Ref T4398. We have several auth-related systems which require (or are improved by) the ability to hand out one-time codes which expire after a short period of time.
In particular, these are:
- SMS multi-factor: we need to be able to hand out one-time codes for this in order to prove the user has the phone.
- Password reset emails: we use a time-based rotating token right now, but we could improve this with a one-time token, so once you reset your password the link is dead.
- TOTP auth: we don't need to verify/invalidate keys, but can improve security by doing so.
This adds a generic one-time code storage table, and strengthens the TOTP enrollment process by using it. Specifically, you can no longer edit the enrollment form (the one with a QR code) to force your own key as the TOTP key: only keys Phabricator generated are accepted. This has no practical security impact, but generally helps raise the barrier potential attackers face.
Followup changes will use this for reset emails, then implement SMS multi-factor.
Test Plan:
- Enrolled in TOTP multi-factor auth.
- Submitted an error in the form, saw the same key presented.
- Edited the form with web tools to provide a different key, saw it reject and the server generate an alternate.
- Change the expiration to 5 seconds instead of 1 hour, submitted the form over and over again, saw it cycle the key after 5 seconds.
- Looked at the database and saw the tokens I expected.
- Ran the GC and saw all the 5-second expiry tokens get cleaned up.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4398
Differential Revision: https://secure.phabricator.com/D9217
Summary:
Ref T4673.
IMPORTANT: I had to break one thing (see TODO) to get this working. Not sure how you want to deal with that. I might be able to put the element //inside// the workboard, or I could write some JS. But I figured I'd get feedback first.
General areas for improvement:
- It would be nice to give you some feedback that you have a filter applied.
- It would be nice to let you save and quickly select common filters.
- These would probably both be covered by a dropdown menu instead of a button, but that's more JS than I want to sign up for right now.
- Managing custom filters is also a significant amount of extra UI to build.
- Also, maybe these filters should be sticky per-board? Or across all boards? Or have a "make this my default view"? I tend to dislike implicit stickiness.
Test Plan:
Before:
{F157543}
Apply Filter:
{F157544}
Filtered:
{F157545}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: qgil, swisspol, epriestley
Maniphest Tasks: T4673
Differential Revision: https://secure.phabricator.com/D9211
Summary: Fixes T4983, Panel prefix 'W' should be recognized as a shortcut to a dashboard panel
Test Plan: Open any comment input, type '{W1}', or other existing panel, preview should embed that panel.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T4983
Differential Revision: https://secure.phabricator.com/D9215
Summary: Adds basic borders, spacing to tab panels and text boxes.
Test Plan:
Added some of each to my Dashboard.
{F157622}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5078
Differential Revision: https://secure.phabricator.com/D9216
Summary: Make `->withPHIDs(array())` throw on this query instead of selecting everything.
Test Plan: Poked around.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9210
Summary: A basic rendering of feed in dashboards
Test Plan:
built a feed in dashboards, viewed it as my homepage.
{F157119}
Reviewers: btrahan, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9209
Summary:
See title. Adds PhabricatorDashboardInstall data object which scopes installs to objectPHID + applicationClass. This is because we already have a collision for user home pages and user profiles. Assume only one dashboard per objectPHID + applicationClass though at the database level.
Fixes T5076.
Test Plan: From dashboard view, installed a dashboard - success! Went back to dashboard view and uninstalled it!
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5076
Differential Revision: https://secure.phabricator.com/D9206
Summary: To get there, upgrade "headerless" to "headerMode". Add a new removepanel controller. Fixes T5084.
Test Plan: removed some panels to much success
Reviewers: chad, epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5078, T5084
Differential Revision: https://secure.phabricator.com/D9156
Summary: Ref T4029. When checking the view policy of a document, require the viewer to also be able to see all of the ancestors.
Test Plan:
- Hard-coded `/x/y/` to "no one".
- Checked that `/x/y/` is not visible.
- Checked that `/x/y/z/` is not visible.
- Checked that `/x/`, `/x/q/`, etc., are still visible.
- Tested project pages and sub-pages for project visibility.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D9199
Summary: Ref T4029. We use a lot of very outdated content loading in Phriction, which blocks T4029.
Test Plan:
- Called phriction.info
- Called phriction.history
- Called phriction.edit
- Viewed document list.
- Deleted a document.
- Viewed history.
- Viewed a diff.
- Created a document.
- Edited a document.
- Moved a document.
- Tried to overwrite a document with "new".
- Tried to overwrite a document with "move".
- Viewed a moved document note.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: shadowhand, epriestley
Maniphest Tasks: T4029
Differential Revision: https://secure.phabricator.com/D9194
Summary:
Ref T4967. Adds a "Watch" relationship to projects, which is stronger than member/subscribed.
Specifically, when a task is tagged with a project, we'll include all project watchers in the email/notifications. Normally we don't include projects unless they're explicitly CC'd, or have some other active role in the object (like being a reviewer or auditor).
This allows you to closely follow a project without needing to write a Herald rule for every project you care about.
Test Plan:
- Watched/unwatched a project.
- Tested the watch/subscribe/member relationships:
- Watching implies subscribe.
- Joining implies subscribe.
- Leaving implies unsubscribe + unwatch.
- You can't unsubscribe until you unwatch (slightly better would be unsubscribe implies unwatch, but this is a bit tricky).
- Watched a project, then recevied email about a tagged task without otherwise being involved.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4967
Differential Revision: https://secure.phabricator.com/D9185
Summary: Fixes T5104. It's still OK to reveal public keys of locked credentials -- the controller has the right logic, this UI just isn't in sync.
Test Plan: Viewed passphrase; saw enabled menu item on locked credential.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5104
Differential Revision: https://secure.phabricator.com/D9186
Summary:
Fixes T5108. If we render a property list with no properties, it doesn't render anything. This hides any attached action list.
Instead, insert an empty property if we have an action list but no properties.
(This could use some cleanup eventually, but resolve the issue for now.)
Test Plan: Viewed a property list with actions but no properties; saw actions.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5108
Differential Revision: https://secure.phabricator.com/D9201
Summary: Fixes T4930. We currently show the edit/quote menu if a transaction group has //inline// comments, but this doesn't make sense and doesn't work properly. Only show this menu if the group has a normal comment.
Test Plan:
Viewed these groups:
- Normal comment (edits fine).
- Just inlines (no more edit menu).
- Inline + comment (edits fine, affects the normal comment properly).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: bitglue, epriestley
Maniphest Tasks: T4930
Differential Revision: https://secure.phabricator.com/D9180
Summary:
Fixes T5041. Pretty sure this is the issue: if a diff contains a large number of identical lines longer than 30 characters, we end up paying O(N^2) for each set.
Instead, when N > 16, opt to pay 0.
Test Plan: Added a test which dropped from ~100s to ~0 after changes (this diff includes a reduced-strenght version of the test, since parsing a 4,000 line diff is a little bit pricey).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5041
Differential Revision: https://secure.phabricator.com/D9178
Summary: Ref T4830. Also deletes some very obsolete code.
Test Plan: Looked at Facts as logged out user.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4830
Differential Revision: https://secure.phabricator.com/D9177
Summary:
Ref T4968. If you add a comment to revision you aren't currently subscribed to, the email currently chooses "[Changed Subscribers]" as the action title. This is less interesting than "[Commented]", provided the affected subscriber is you (adding other people //is// usually interesting).
In this case, reduce the strength of this action below the strength of "comment".
Test Plan: Made several comments in conjunction with implicit and explicit subscriptions. Saw "[Commented]" for stuff affecting me, and "[Changed Subscribers]" for stuff affecting others.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4968
Differential Revision: https://secure.phabricator.com/D9168
Summary:
Ref T5008. Three notes:
- I'm not hiding these even if the status change is open -> open or closed -> closed. I think these are OK, but might be a little spammy.
- These show in feed, but shouldn't, since they're very redundant with stories which will almost always appear adjacently. Probably a bit spammy, see TODO. We can't hide them from feed without also squelching the notifications right now, which I //don't// want to do.
- You get a notification even if you're on the original task which changed status. This is definitely spammy, see other TODO.
Test Plan: {F156217}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5008
Differential Revision: https://secure.phabricator.com/D9166
Summary: Fixes T4299, Add status dropdown to mock edit view
Test Plan: Edit mock, close mock, thumbnail title should read (Disabled). Default mocks list should show only open mocks.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: chad, epriestley, Korvin
Maniphest Tasks: T4299
Differential Revision: https://secure.phabricator.com/D9145
Summary: Object tags in Chrome/Blink on Android wrap randomly and horribly as if it's their job in life. Let's put an end to that, probably. Fixes T5103
Test Plan: Chrome 34 / Android. Tested Chrome Regular Flavor desktop and mobile.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5103
Differential Revision: https://secure.phabricator.com/D9191
Summary: Apply the `ArcanistChmodLinter` from D9187.
Test Plan: Removed all other linters from the `.arclint` file and executed `arc lint --everything`. Confirmed that there were no issues raised.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9188
Summary:
Updates policy, headers, typeaheads to FA over policy icons
Need advice - can't seem to place where icons come from on Typeahead? Wrong icons and wrong colors.... it is late
Test Plan:
- grepped for SPRITE_STATUS
- grepped for sprite-status
- grepped for setStatus for headers
- grepped individual icons names
Browsed numerous places, checked new dropdowns, see pudgy people.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4739
Differential Revision: https://secure.phabricator.com/D9179
Summary: Fixes T4859. See that for details.
Test Plan:
- Verified things still work on my local (domain root) install.
- Added some unit tests.
- Did not verify a non-root install since I don't have one handy, hopefully @salehe can help.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: salehe, epriestley
Maniphest Tasks: T4859
Differential Revision: https://secure.phabricator.com/D8836
Summary:
Fixes T4898. After we increased the strictness of the `%s` conversion, most `serialize()` output is rejected from the cache.
Drop the cache, change the column type to latin1_bin, and then use `%B` to mark the data as binary during query construction.
Test Plan: Viewed Differential, saw cache fills.
Reviewers: btrahan, spicyj
Reviewed By: spicyj
Subscribers: epriestley
Maniphest Tasks: T4898
Differential Revision: https://secure.phabricator.com/D9171
Summary:
Ref T4994. This stuff works:
- You can dump a blob of coverage information into `diffusion.updatecoverage`. This wipes existing coverage information and replaces it.
- It shows up when viewing files.
- It shows up when viewing commits.
This stuff does not work:
- When viewing files, the Javascript hover interaction isn't tied in yet.
- We always show this information, even if you're behind the commit where it was generated.
- You can't do incremental updates.
- There's no aggregation at the file (this file has 90% coverage), diff (the changes in this commit are 90% covered), or directory (the code in this directory has 90% coverage) levels yet.
- This is probably not the final form of the UI, storage, or API, so you should expect occasional changes over time. I've marked the method as "Unstable" for now.
Test Plan:
- Ran `save_lint.php` to check for collateral damage; it worked fine.
- Ran `save_lint.php` on a new branch to check creation.
- Published some fake coverage information.
- Viewed an affected commit.
- Viewed an affected file.
{F151915}
{F151916}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: jhurwitz, epriestley, zeeg
Maniphest Tasks: T5044, T4994
Differential Revision: https://secure.phabricator.com/D9022
Summary: Fixes T3044. We currently don't add these to the index.
Test Plan: Made a unique inline comment on a commit, then searched for it.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T3044
Differential Revision: https://secure.phabricator.com/D9170
Summary:
Fixes T3854. Subversion allows commits with no message, and in other cases we might not have imported the message yet. In these cases, we may not render any text inside the link.
When we hit these cases, render appropriate replacement text.
Test Plan: {F156229}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T3854
Differential Revision: https://secure.phabricator.com/D9169
Summary: Fixes T5093. Ref T4830.
Test Plan:
- As a logged out user, viewed a public countdown detail page.
- (Tried to view a nonpublic one, got asked to login.)
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4830, T5093
Differential Revision: https://secure.phabricator.com/D9162
Summary: In general these are fairly readable, but if not it cleans up on hover (and hover card).
Test Plan: tested a closed task in my sandbox
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9158
Summary: Changes to using FontAwesome
Test Plan:
Testing UIExamples and each of the pages (except releelph)
{F155942}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Differential Revision: https://secure.phabricator.com/D9157
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: This doesn't have any impact on anything right now (we don't persist the query) but could in the future, so I just left it as-is but fixed the typo.
Test Plan: looked at it carefully
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: WikiChad, epriestley
Differential Revision: https://secure.phabricator.com/D9155
Summary:
D9153 fixed half of this, but exposed another issue, which is that we don't actually serve ".eot" and ".ttf" through Celerity right now.
Make sure we include them in the routes.
Test Plan:
- Downloaded CSS, JS, TTF, EOT, WOFF, JPG, etc., through Celerity.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9154
Summary:
See <6a45b7e670>
These URIs have "?hack=iefix#ieieielol" on them, which the parser doesn't recognize as a known resource, so it errs on the side of caution by not rewriting.
Instead, strip this bit off, attempt to rewrite, then put it back on.
Test Plan: Loaded `font-awesome.css` locally and saw properly rewritten URIs.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D9153
Summary:
I have added a dialog box which pops up when a user starts or stops tracking time on an issue with Phrequent. These dialogs allow the user to modify the time if it so happens that they forgot to either clock in or out.
I have also added a Note field in the dialog when a user stops tracking time. This allows them to enter a note about the time, and is entered into the database, but is currently (as far as I know) not visible anywhere in Phabricator.
I have made these changes according to the suggestions found in T3568
Also, upon clocking in or out, if the time entered is a future time, an error is returned and the user is asked to enter a valid time.
Test Plan:
Start tracking time and edit the start date/time, then end the time and edit that timestamp as well.
Also, try entering future dates/times and ensure that the dialog reports an error and asks for the time again.
Ensure that these edited times are recorded properly.
Reviewers: epriestley, #blessed_reviewers
Subscribers: epriestley, Korvin
Maniphest Tasks: T3568
Differential Revision: https://secure.phabricator.com/D9147
Summary:
Fixes T5081. This block enforces policies and prevents you from seeing groups associated with projects you can't see. However, it incorrectly removes tasks which end up with no group key. This can happen in two cases:
- The task isn't in a project.
- The task is in a project, but the query includes an "In All Projects: <that project>" constraint. In that case, we don't show the group becuase it would always contain every task.
Test Plan:
Replicated the setup in T5081, saw an "Ungrouped" group with "Task A":
{F155766}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5081
Differential Revision: https://secure.phabricator.com/D9152
Summary:
Fixes T5069. T2222 mostly-intentionally stopped emitting these.
My sense is that users generally find event listeners (or, really, writing PHP at all) much less preferable to things like Herald rules or HTTP hooks. This is generally good, since those things are way easier to maintain, so I plan to continue moving away from events in cases where we have reasonable alternatives.
We also generally have more and better alternatives now than when these were written.
Test Plan: `grep`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5069
Differential Revision: https://secure.phabricator.com/D9151
Summary: we should do a redirect if its not an ajax request, which solves this problem since folks with no javascript aren't sending ajax requests. Fixes T5049.
Test Plan: inverted the predicate, tried ajax requests, and got redirected
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T5049
Differential Revision: https://secure.phabricator.com/D9149
Summary:
Ref T4986. This mostly just makes tab panels a little nicer.
Maybe this will be modal (header = "none", "edit", "view") in a few diffs but we can clean it up then if so.
Test Plan: {F155491}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4986
Differential Revision: https://secure.phabricator.com/D9143
Summary:
Ref T4986. Ref T4983. Panels will soon be able to contain other panels, either via Remarkup (`{W1}`) or maybe through new types of meta-panels.
Allow panels to detect that they are being rendered very deeply and/or within themselves.
Test Plan: Faked some errors, got failed panel renders. Since panels can't //really// contain other panels yet, this doesn't really have an impact.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4983, T4986
Differential Revision: https://secure.phabricator.com/D9140
