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

9 commits

Author SHA1 Message Date
epriestley
889440ead0 Allow structured destruction of Differential Revisions
Summary:
Ref T4749. Ref T3265. Ref T4909.

  - Remove old "destroy revision" script.
  - Move to structured `bin/remove` destruction.
  - Fix some edge issues.
  - Add transaction destruction support.

Test Plan:
  - Destroyed a bunch of revisions.
  - Saw diffs, changesets, hunks, transactions, edges, and inlines also get wiped out.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4749, T4909, T3265

Differential Revision: https://secure.phabricator.com/D8943
2014-05-01 18:25:30 -07:00
epriestley
2022a70e16 Implement bin/remove, for structured destruction of objects
Summary:
Ref T4749. Ref T3265. Ref T4909. Several goals here:

  - Move user destruction to the CLI to limit the power of rogue admins.
  - Start consolidating all "destroy named object" scripts into a single UI, to make it easier to know how to destroy things.
  - Structure object destruction so we can do a better and more automatic job of cleaning up transactions, edges, search indexes, etc.
  - Log when we destroy objects so there's a record if data goes missing.

Test Plan: Used `bin/remove destroy` to destroy several users.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3265, T4749, T4909

Differential Revision: https://secure.phabricator.com/D8940
2014-05-01 18:23:31 -07:00
epriestley
23e654ec2b Rate limit multi-factor actions
Summary: Ref T4398. Prevent users from brute forcing multi-factor auth by rate limiting attempts. This slightly refines the rate limiting to allow callers to check for a rate limit without adding points, and gives users credit for successfully completing an auth workflow.

Test Plan: Tried to enter hisec with bad credentials 11 times in a row, got rate limited.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4398

Differential Revision: https://secure.phabricator.com/D8911
2014-04-30 14:30:31 -07:00
epriestley
d9cdbdb9fa When we fail to process mail, tell the user about it
Summary:
Ref T4371. Ref T4699. Fixes T3994.

Currently, we're very conservative about sending errors back to users. A concern I had about this was that mistakes could lead to email loops, massive amounts of email spam, etc. Because of this, I was pretty hesitant about replying to email with more email when I wrote this stuff.

However, this was a long time ago. We now have Message-ID deduplication, "X-Phabricator-Sent-This-Mail", generally better mail infrastructure, and rate limiting. Together, these mechanisms should reasonably prevent anything crazy (primarily, infinite email loops) from happening.

Thus:

  - When we hit any processing error after receiving a mail, try to send the author a reply with details about what went wrong. These are limited to 6 per hour per address.
  - Rewrite most of the errors to be more detailed and informative.
  - Rewrite most of the errors in a user-facing voice ("You sent this mail..." instead of "This mail was sent..").
  - Remove the redundant, less sophisticated code which does something similar in Differential.

Test Plan:
  - Using `scripts/mail/mail_receiver.php`, artificially received a pile of mail.
  - Hit a bunch of different errors.
  - Saw reasonable error mail get sent to me.
  - Saw other reasonable error mail get rate limited.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3994, T4371, T4699

Differential Revision: https://secure.phabricator.com/D8692
2014-04-03 18:43:18 -07:00
epriestley
c9311a9eae Make errors in dialogs look reasonable instead of hideous
Summary: I accidentally made these exceptionally ugly recently.

Test Plan: {F137411}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley, chad

Differential Revision: https://secure.phabricator.com/D8684
2014-04-03 11:23:03 -07:00
epriestley
847b7977c1 Add semi-generic rate limiting infrastructure
Summary:
This adds a system which basically keeps a record of recent actions, who took them, and how many "points" they were worth, like:

  epriestley email.add 1 1233989813
  epriestley email.add 1 1234298239
  epriestley email.add 1 1238293981

We can use this to rate-limit actions by examining how many actions the user has taken in the past hour (i.e., their total score) and comparing that to an allowed limit.

One major thing I want to use this for is to limit the amount of error email we'll send to an email address. A big concern I have with sending more error email is that we'll end up in loops. We have some protections against this in headers already, but hard-limiting the system so it won't send more than a few errors to a particular address per hour should provide a reasonable secondary layer of protection.

This use case (where the "actor" needs to be an email address) is why the table uses strings + hashes instead of PHIDs. For external users, it might be appropriate to rate limit by cookies or IPs, too.

To prove it works, I rate limited adding email addresses. This is a very, very low-risk security thing where a user with an account can enumerate addresses (by checking if they get an error) and sort of spam/annoy people (by adding their address over and over again). Limiting them to 6 actions / hour should satisfy all real users while preventing these behaviors.

Test Plan:
This dialog is uggos but I'll fix that in a sec:

{F137406}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8683
2014-04-03 11:22:38 -07:00
epriestley
838f781285 Add a robots.txt file to disallow /diffusion/
Summary:
Fixes T4610. Open to suggestions, etc., if there's anything I'm missing.

Also:

  - Moves these "system" endpoints into a real application.
  - Makes `isUnlisted()` work a little more consistently.

Test Plan: Accessed `/robots.txt`, `/status/` and `/debug/`.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: aran, epriestley

Maniphest Tasks: T4610

Differential Revision: https://secure.phabricator.com/D8532
2014-03-14 11:53:17 -07: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
14569ae491 Add a user-accessible hook for dumping debug code into an install
Summary:
Currently, there's no easy way for me to tell a user "run this code from the webserver and tell me what it says". Sometimes installs can add new .php files to, e.g., `webroot/rsrc/`, but this is setup-dependent and not universal. Generally I resort to saying "put this into index.php", but that's error prone and not acceptable on active installs.

Add a "debug" controller so I can instead say "put this into support/debug.php, then visit /debug/".

Test Plan: Visited /debug/ with and without support/debug.php files. Visited /staus/.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5212
2013-03-04 13:45:51 -08:00