Merge remote-tracking branch 'bmo/master'

9 files changed, 764 insertions, 784 deletions

diff --git a/extensions/PhabBugz/lib/Config.pm b/extensions/PhabBugz/lib/Config.pm
index 85ba37e74..d4b71430b 100644
--- a/extensions/PhabBugz/lib/Config.pm
+++ b/extensions/PhabBugz/lib/Config.pm
@@ -31,11 +31,6 @@ sub get_param_list {
             checker => \&check_urlbase
         },
         {
-            name    => 'phabricator_sync_groups',
-            type    => 't',
-            default => '',
-        },
-        {
             name => 'phabricator_api_key',
             type => 't',
             default => '',
diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm
index 754130f0b..1692f8fb9 100644
--- a/extensions/PhabBugz/lib/Constants.pm
+++ b/extensions/PhabBugz/lib/Constants.pm
@@ -16,12 +16,16 @@ our @EXPORT = qw(
     PHAB_AUTOMATION_USER
     PHAB_ATTACHMENT_PATTERN
     PHAB_CONTENT_TYPE
-    PHAB_POLL_SECONDS
+    PHAB_FEED_POLL_SECONDS
+    PHAB_USER_POLL_SECONDS
+    PHAB_GROUP_POLL_SECONDS
 );
 
 use constant PHAB_ATTACHMENT_PATTERN => qr/^phabricator-D(\d+)/;
 use constant PHAB_AUTOMATION_USER    => 'phab-bot@bmo.tld';
 use constant PHAB_CONTENT_TYPE       => 'text/x-phabricator-request';
-use constant PHAB_POLL_SECONDS       => 5;
+use constant PHAB_FEED_POLL_SECONDS  => $ENV{PHAB_FEED_POLL} // 5;
+use constant PHAB_USER_POLL_SECONDS  => $ENV{PHAB_USER_POLL} // 60;
+use constant PHAB_GROUP_POLL_SECONDS => $ENV{PHAB_GROUP_POLL} // 300;
 
 1;
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 074ecc0f9..c46d36c13 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -9,14 +9,21 @@ package Bugzilla::Extension::PhabBugz::Feed;
 
 use 5.10.1;
 
+use IO::Async::Timer::Periodic;
+use IO::Async::Loop;
 use List::Util qw(first);
 use List::MoreUtils qw(any);
 use Moo;
+use Scalar::Util qw(blessed);
+use Try::Tiny;
 
-use Bugzilla::Logging;
 use Bugzilla::Constants;
+use Bugzilla::Error;
+use Bugzilla::Field;
+use Bugzilla::Logging;
+use Bugzilla::Mailer;
 use Bugzilla::Search;
-use Bugzilla::Util qw(diff_arrays with_writable_database with_readonly_database);
+use Bugzilla::Util qw(diff_arrays format_time with_writable_database with_readonly_database);
 
 use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Policy;
@@ -25,13 +32,9 @@ use Bugzilla::Extension::PhabBugz::User;
 use Bugzilla::Extension::PhabBugz::Util qw(
     add_security_sync_comments
     create_revision_attachment
-    edit_revision_policy
     get_bug_role_phids
-    get_phab_bmo_ids
-    get_project_phid
     get_security_sync_groups
     is_attachment_phab_revision
-    make_revision_public
     request
     set_phab_user
 );
@@ -40,38 +43,85 @@ has 'is_daemon' => ( is => 'rw', default => 0 );
 
 sub start {
     my ($self) = @_;
-    while (1) {
-        my $ok = eval {
-            if (Bugzilla->params->{phabricator_enabled}) {
+
+    # Query for new revisions or changes
+    my $feed_timer = IO::Async::Timer::Periodic->new(
+        first_interval => 0,
+        interval       => PHAB_FEED_POLL_SECONDS,
+        reschedule     => 'drift',
+        on_tick        => sub {
+            try{
                 $self->feed_query();
-                Bugzilla->_cleanup();
             }
-            1;
-        };
-        ERROR( $@ // "unknown exception" ) unless $ok;
-        sleep(PHAB_POLL_SECONDS);
-    }
+            catch {
+                FATAL($_);
+            };
+            Bugzilla->_cleanup();
+        },
+    );
+
+    # Query for new users
+    my $user_timer = IO::Async::Timer::Periodic->new(
+        first_interval => 0,
+        interval       => PHAB_USER_POLL_SECONDS,
+        reschedule     => 'drift',
+        on_tick        => sub {
+            try{
+                $self->user_query();
+            }
+            catch {
+                FATAL($_);
+            };
+            Bugzilla->_cleanup();
+        },
+    );
+
+    # Update project membership in Phabricator based on Bugzilla groups
+    my $group_timer = IO::Async::Timer::Periodic->new(
+        first_interval => 0,
+        interval       => PHAB_GROUP_POLL_SECONDS,
+        reschedule     => 'drift',
+        on_tick        => sub {
+            try{
+                $self->group_query();
+            }
+            catch {
+                FATAL($_);
+            };
+            Bugzilla->_cleanup();
+        },
+    );
+
+    my $loop = IO::Async::Loop->new;
+    $loop->add($feed_timer);
+    $loop->add($user_timer);
+    $loop->add($group_timer);
+    $feed_timer->start;
+    $user_timer->start;
+    $group_timer->start;
+    $loop->run;
 }
 
 sub feed_query {
     my ($self) = @_;
-    my $dbh = Bugzilla->dbh;
+
+    local Bugzilla::Logging->fields->{type} = 'FEED';
 
     # Ensure Phabricator syncing is enabled
     if (!Bugzilla->params->{phabricator_enabled}) {
-        INFO("PHABRICATOR SYNC DISABLED");
+        WARN("PHABRICATOR SYNC DISABLED");
         return;
     }
 
     # PROCESS NEW FEED TRANSACTIONS
 
-    INFO("FEED: Fetching new transactions");
+    INFO("Fetching new stories");
 
     my $story_last_id = $self->get_last_id('feed');
 
     # Check for new transctions (stories)
     my $new_stories = $self->new_stories($story_last_id);
-    INFO("FEED: No new stories") unless @$new_stories;
+    INFO("No new stories") unless @$new_stories;
 
     # Process each story
     foreach my $story_data (@$new_stories) {
@@ -81,25 +131,29 @@ sub feed_query {
         my $object_phid = $story_data->{objectPHID};
         my $story_text  = $story_data->{text};
 
-        DEBUG("STORY ID: $story_id");
-        DEBUG("STORY PHID: $story_phid");
-        DEBUG("AUTHOR PHID: $author_phid");
-        DEBUG("OBJECT PHID: $object_phid");
+        TRACE("STORY ID: $story_id");
+        TRACE("STORY PHID: $story_phid");
+        TRACE("AUTHOR PHID: $author_phid");
+        TRACE("OBJECT PHID: $object_phid");
         INFO("STORY TEXT: $story_text");
 
         # Only interested in changes to revisions for now.
         if ($object_phid !~ /^PHID-DREV/) {
-            DEBUG("SKIPPING: Not a revision change");
+            INFO("SKIPPING: Not a revision change");
             $self->save_last_id($story_id, 'feed');
             next;
         }
 
         # Skip changes done by phab-bot user
-        my $phab_users = get_phab_bmo_ids({ phids => [$author_phid] });
-        if (@$phab_users) {
-            my $user = Bugzilla::User->new({ id => $phab_users->[0]->{id}, cache => 1 });
-            if ($user->login eq PHAB_AUTOMATION_USER) {
-                DEBUG("SKIPPING: Change made by phabricator user");
+        my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query(
+          {
+            phids => [ $author_phid ]
+          }
+        );
+
+        if ($phab_user && $phab_user->bugzilla_id) {
+            if ($phab_user->bugzilla_user->login eq PHAB_AUTOMATION_USER) {
+                INFO("SKIPPING: Change made by phabricator user");
                 $self->save_last_id($story_id, 'feed');
                 next;
             }
@@ -111,15 +165,69 @@ sub feed_query {
         $self->save_last_id($story_id, 'feed');
     }
 
+    # Process any build targets as well.
+    my $dbh = Bugzilla->dbh;
+
+    INFO("Checking for revisions in draft mode");
+    my $build_targets = $dbh->selectall_arrayref(
+        "SELECT name, value FROM phabbugz WHERE name LIKE 'build_target_%'",
+        { Slice => {} }
+    );
+
+    my $delete_build_target = $dbh->prepare(
+        "DELETE FROM phabbugz WHERE name = ? AND VALUE = ?"
+    );
+
+    foreach my $target (@$build_targets) {
+        my ($revision_id) = ($target->{name} =~ /^build_target_(\d+)$/);
+        my $build_target  = $target->{value};
+
+        next unless $revision_id && $build_target;
+
+        INFO("Processing revision $revision_id with build target $build_target");
+
+        my $revision =
+          Bugzilla::Extension::PhabBugz::Revision->new_from_query(
+            {
+              ids => [ int($revision_id) ]
+            }
+        );
+
+        with_writable_database {
+            $self->process_revision_change($revision, " created D" . $revision->id);
+        };
+
+        # Set the build target to a passing status to
+        # allow the revision to exit draft state
+        request( 'harbormaster.sendmessage', {
+            buildTargetPHID => $build_target,
+            type            => 'pass'
+        } );
+
+        $delete_build_target->execute($target->{name}, $target->{value});
+     }
+}
+
+sub user_query {
+    my ( $self ) = @_;
+
+    local Bugzilla::Logging->fields->{type} = 'USERS';
+
+    # Ensure Phabricator syncing is enabled
+    if (!Bugzilla->params->{phabricator_enabled}) {
+        WARN("PHABRICATOR SYNC DISABLED");
+        return;
+    }
+
     # PROCESS NEW USERS
 
-    INFO("FEED: Fetching new users");
+    INFO("Fetching new users");
 
     my $user_last_id = $self->get_last_id('user');
 
     # Check for new users
     my $new_users = $self->new_users($user_last_id);
-    INFO("FEED: No new users") unless @$new_users;
+    INFO("No new users") unless @$new_users;
 
     # Process each new user
     foreach my $user_data (@$new_users) {
@@ -128,10 +236,10 @@ sub feed_query {
         my $user_realname = $user_data->{fields}{realName};
         my $object_phid   = $user_data->{phid};
 
-        DEBUG("USER ID: $user_id");
-        DEBUG("USER LOGIN: $user_login");
-        DEBUG("USER REALNAME: $user_realname");
-        DEBUG("OBJECT PHID: $object_phid");
+        TRACE("ID: $user_id");
+        TRACE("LOGIN: $user_login");
+        TRACE("REALNAME: $user_realname");
+        TRACE("OBJECT PHID: $object_phid");
 
         with_readonly_database {
             $self->process_new_user($user_data);
@@ -140,26 +248,110 @@ sub feed_query {
     }
 }
 
+sub group_query {
+    my ($self) = @_;
+
+    local Bugzilla::Logging->fields->{type} = 'GROUPS';
+
+    # Ensure Phabricator syncing is enabled
+    if ( !Bugzilla->params->{phabricator_enabled} ) {
+        WARN("PHABRICATOR SYNC DISABLED");
+        return;
+    }
+
+    # PROCESS SECURITY GROUPS
+
+    INFO("Updating group memberships");
+
+    # Loop through each group and perform the following:
+    #
+    # 1. Load flattened list of group members
+    # 2. Check to see if Phab project exists for 'bmo-<group_name>'
+    # 3. Create if does not exist with locked down policy.
+    # 4. Set project members to exact list including phab-bot user
+    # 5. Profit
+
+    my $sync_groups = Bugzilla::Group->match( { isactive => 1, isbuggroup => 1 } );
+
+    # Load phab-bot Phabricator user to add as a member of each project group later
+    my $phab_bmo_user = Bugzilla::User->new( { name => PHAB_AUTOMATION_USER, cache => 1 } );
+    my $phab_user =
+      Bugzilla::Extension::PhabBugz::User->new_from_query(
+        {
+            ids => [ $phab_bmo_user->id ]
+        }
+    );
+
+    # secure-revision project that will be used for bmo group projects
+    my $secure_revision =
+      Bugzilla::Extension::PhabBugz::Project->new_from_query(
+        {
+          name => 'secure-revision'
+        }
+    );
+
+    foreach my $group (@$sync_groups) {
+        # Create group project if one does not yet exist
+        my $phab_project_name = 'bmo-' . $group->name;
+        my $project =
+          Bugzilla::Extension::PhabBugz::Project->new_from_query(
+            {
+              name => $phab_project_name
+            }
+        );
+
+        if ( !$project ) {
+            INFO("Project $phab_project_name not found. Creating.");
+            $project = Bugzilla::Extension::PhabBugz::Project->create(
+              {
+                name        => $phab_project_name,
+                description => 'BMO Security Group for ' . $group->name,
+                view_policy => $secure_revision->phid,
+                edit_policy => $secure_revision->phid,
+                join_policy => $secure_revision->phid
+              }
+            );
+        }
+        else {
+            # Make sure that the group project permissions are set properly
+            INFO("Updating permissions on $phab_project_name");
+            $project->set_policy( 'view', $secure_revision->phid );
+            $project->set_policy( 'edit', $secure_revision->phid );
+            $project->set_policy( 'join', $secure_revision->phid );
+        }
+
+        # Make sure phab-bot also a member of the new project group so that it can
+        # make policy changes to the private revisions
+        INFO("Setting project members for " . $project->name);
+        my $set_members = $self->get_group_members( $group );
+        push @$set_members, $phab_user unless grep $_->phid eq $phab_user->phid, @$set_members;
+        $project->set_members( $set_members );
+        $project->update();
+    }
+}
+
 sub process_revision_change {
     my ($self, $revision_phid, $story_text) = @_;
 
     # Load the revision from Phabricator
-    my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] });
+    my $revision =
+        blessed $revision_phid
+        ? $revision_phid
+        : Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] });
 
     # NO BUG ID
 
     if (!$revision->bug_id) {
         if ($story_text =~ /\s+created\s+D\d+/) {
             # If new revision and bug id was omitted, make revision public
-            DEBUG("No bug associated with new revision. Marking public.");
-            $revision->set_policy('view', 'public');
-            $revision->set_policy('edit', 'users');
+            INFO("No bug associated with new revision. Marking public.");
+            $revision->make_public();
             $revision->update();
             INFO("SUCCESS");
             return;
         }
         else {
-            DEBUG("SKIPPING: No bug associated with revision change");
+            INFO("SKIPPING: No bug associated with revision change");
             return;
         }
     }
@@ -180,11 +372,8 @@ sub process_revision_change {
 
     # If bug is public then remove privacy policy
     if (!@{ $bug->groups_in }) {
-        DEBUG('Bug is public so setting view/edit public');
-        $revision->set_policy('view', 'public');
-        $revision->set_policy('edit', 'users');
-        my $secure_project_phid = get_project_phid('secure-revision');
-        $revision->remove_project($secure_project_phid);
+        INFO('Bug is public so setting view/edit public');
+        $revision->make_public();
     }
     # else bug is private.
     else {
@@ -193,54 +382,51 @@ sub process_revision_change {
         # If bug privacy groups do not have any matching synchronized groups,
         # then leave revision private and it will have be dealt with manually.
         if (!@set_groups) {
-            DEBUG('No matching groups. Adding comments to bug and revision');
+            INFO('No matching groups. Adding comments to bug and revision');
             add_security_sync_comments([$revision], $bug);
         }
         # Otherwise, we create a new custom policy containing the project
         # groups that are mapped to bugzilla groups.
         else {
-            my @set_projects = map { "bmo-" . $_ } @set_groups;
+            my $set_project_names = [ map { "bmo-" . $_ } @set_groups ];
 
             # If current policy projects matches what we want to set, then
             # we leave the current policy alone.
             my $current_policy;
             if ($revision->view_policy =~ /^PHID-PLCY/) {
-                DEBUG("Loading current policy: " . $revision->view_policy);
+                INFO("Loading current policy: " . $revision->view_policy);
                 $current_policy
                     = Bugzilla::Extension::PhabBugz::Policy->new_from_query({ phids => [ $revision->view_policy ]});
-                my $current_projects = $current_policy->rule_projects;
-                DEBUG("Current policy projects: " . join(", ", @$current_projects));
-                my ($added, $removed) = diff_arrays($current_projects, \@set_projects);
+                my $current_project_names = [ map { $_->name } @{ $current_policy->rule_projects } ];
+                INFO("Current policy projects: " . join(", ", @$current_project_names));
+                my ($added, $removed) = diff_arrays($current_project_names, $set_project_names);
                 if (@$added || @$removed) {
-                    DEBUG('Project groups do not match. Need new custom policy');
-                    $current_policy= undef;
+                    INFO('Project groups do not match. Need new custom policy');
+                    $current_policy = undef;
                 }
                 else {
-                    DEBUG('Project groups match. Leaving current policy as-is');
+                    INFO('Project groups match. Leaving current policy as-is');
                 }
             }
 
             if (!$current_policy) {
-                DEBUG("Creating new custom policy: " . join(", ", @set_projects));
-                my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects);
-                $revision->set_policy('view', $new_policy->phid);
-                $revision->set_policy('edit', $new_policy->phid);
+                INFO("Creating new custom policy: " . join(", ", @$set_project_names));
+                $revision->make_private($set_project_names);
             }
 
-            my $secure_project_phid = get_project_phid('secure-revision');
-            $revision->add_project($secure_project_phid);
+            # Subscriber list of the private revision should always match
+            # the bug roles such as assignee, qa contact, and cc members.
+            my $subscribers = get_bug_role_phids($bug);
+            $revision->set_subscribers($subscribers);
         }
-
-        # Subscriber list of the private revision should always match
-        # the bug roles such as assignee, qa contact, and cc members.
-        my $subscribers = get_bug_role_phids($bug);
-        $revision->set_subscribers($subscribers);
     }
 
     my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
 
-    my $attachment = create_revision_attachment($bug, $revision, $timestamp);
-
+    INFO('Checking for revision attachment');
+    my $attachment = create_revision_attachment($bug, $revision, $timestamp, $revision->author->bugzilla_user);
+    INFO('Attachment ' . $attachment->id . ' created or already exists.');
+    
     # ATTACHMENT OBSOLETES
 
     # fixup attachments on current bug
@@ -252,11 +438,11 @@ sub process_revision_change {
         next if $attach_revision_id != $revision->id;
 
         my $make_obsolete = $revision->status eq 'abandoned' ? 1 : 0;
-        DEBUG('Updating obsolete status on attachmment ' . $attachment->id);
+        INFO('Updating obsolete status on attachmment ' . $attachment->id);
         $attachment->set_is_obsolete($make_obsolete);
 
         if ($revision->title ne $attachment->description) {
-            DEBUG('Updating description on attachment ' . $attachment->id);
+            INFO('Updating description on attachment ' . $attachment->id);
             $attachment->set_description($revision->title);
         }
 
@@ -272,8 +458,8 @@ sub process_revision_change {
     });
     foreach my $attachment (@$other_attachments) {
         $other_bugs{$attachment->bug_id}++;
-        DEBUG('Updating obsolete status on attachment ' .
-                             $attachment->id . " for bug " . $attachment->bug_id);
+        INFO('Updating obsolete status on attachment ' .
+             $attachment->id . " for bug " . $attachment->bug_id);
         $attachment->set_is_obsolete(1);
         $attachment->update($timestamp);
     }
@@ -281,17 +467,28 @@ sub process_revision_change {
     # REVIEWER STATUSES
 
     my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids);
-    unless ($revision->status eq 'changes-planned' || $revision->status eq 'needs-review') {
-        foreach my $reviewer (@{ $revision->reviewers }) {
-            push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted';
-            push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected';
-        }
+    foreach my $reviewer (@{ $revision->reviewers }) {
+        push(@accepted_phids, $reviewer->phid) if $reviewer->{phab_review_status} eq 'accepted';
+        push(@denied_phids, $reviewer->phid) if $reviewer->{phab_review_status} eq 'rejected';
     }
 
-    my $phab_users = get_phab_bmo_ids({ phids => \@accepted_phids });
-    @accepted_user_ids = map { $_->{id} } @$phab_users;
-    $phab_users = get_phab_bmo_ids({ phids => \@denied_phids });
-    @denied_user_ids = map { $_->{id} } @$phab_users;
+    if ( @accepted_phids ) {
+        my $phab_users = Bugzilla::Extension::PhabBugz::User->match(
+          {
+            phids => \@accepted_phids
+          }
+        );
+        @accepted_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @$phab_users;
+    }
+
+    if ( @denied_phids ) {
+        my $phab_users = Bugzilla::Extension::PhabBugz::User->match(
+          {
+            phids => \@denied_phids
+          }
+        );
+        @denied_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @$phab_users;
+    }
 
     my %reviewers_hash =  map { $_->name => 1 } @{ $revision->reviewers };
 
@@ -299,19 +496,22 @@ sub process_revision_change {
         my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
         next if $revision->id != $attach_revision_id;
 
-        # Clear old flags if no longer accepted
+        # Clear old accepted review flags if no longer accepted
         my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type);
         foreach my $flag (@{ $attachment->flags }) {
             next if $flag->type->name ne 'review';
             $flag_type = $flag->type if $flag->type->is_active;
+            next if $flag->status ne '+';
             if (any { $flag->setter->id == $_ } @denied_user_ids) {
+                INFO('Denying review flag set by ' . $flag->setter->name);
                 push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
             }
             if (any { $flag->setter->id == $_ } @accepted_user_ids) {
+                INFO('Skipping as review+ already set by ' . $flag->setter->name);
                 $accepted_done{$flag->setter->id}++;
             }
-            if ($flag->status eq '+'
-                && !any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) {
+            if (!any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) {
+                INFO('Clearing review+ flag set by ' . $flag->setter->name);
                 push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
             }
         }
@@ -322,6 +522,7 @@ sub process_revision_change {
         foreach my $user_id (@accepted_user_ids) {
             next if $accepted_done{$user_id};
             my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
+            INFO('Setting new review+ flag for ' . $user->name);
             push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' });
         }
 
@@ -344,6 +545,7 @@ sub process_revision_change {
 
         if ($comment) {
             $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision->id;
+            INFO("Flag comment: $comment");
             # Add transaction_id as anchor if one present
             # $comment .= "#" . $params->{transaction_id} if $params->{transaction_id};
             $bug->add_comment($comment, {
@@ -380,7 +582,7 @@ sub process_new_user {
     my $phab_user = Bugzilla::Extension::PhabBugz::User->new($user_data);
 
     if (!$phab_user->bugzilla_id) {
-        DEBUG("SKIPPING: No bugzilla id associated with user");
+        WARN("SKIPPING: No bugzilla id associated with user");
         return;
     }
 
@@ -389,6 +591,55 @@ sub process_new_user {
     # Pre setup before querying DB
     my $old_user = set_phab_user();
 
+    # CHECK AND WARN FOR POSSIBLE USERNAME SQUATTING
+    INFO("Checking for username squatters");
+    my $dbh     = Bugzilla->dbh;
+    my $regexp  = $dbh->quote( ":?:" . quotemeta($phab_user->name) . "[[:>:]]" );
+    my $results = $dbh->selectall_arrayref( "
+        SELECT userid, login_name, realname
+          FROM profiles
+         WHERE userid != ? AND " . $dbh->sql_regexp( 'realname', $regexp ),
+        { Slice => {} },
+        $bug_user->id );
+    if (@$results) {
+        # The email client will display the Date: header in the desired timezone,
+        # so we can always use UTC here.
+        my $timestamp = Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
+        $timestamp = format_time($timestamp, '%a, %d %b %Y %T %z', 'UTC');
+
+        foreach my $row (@$results) {
+            WARN(
+                'Possible username squatter: ',
+                'phab user login: ' . $phab_user->name,
+                ' phab user realname: ' . $phab_user->realname,
+                ' bugzilla user id: ' . $row->{userid},
+                ' bugzilla login: ' . $row->{login_name},
+                ' bugzilla realname: ' . $row->{realname}
+            );
+
+            my $vars = {
+                date               => $timestamp,
+                phab_user_login    => $phab_user->name,
+                phab_user_realname => $phab_user->realname,
+                bugzilla_userid    => $bug_user->id,
+                bugzilla_login     => $bug_user->login,
+                bugzilla_realname  => $bug_user->name,
+                squat_userid       => $row->{userid},
+                squat_login        => $row->{login_name},
+                squat_realname     => $row->{realname}
+            };
+
+            my $message;
+            my $template = Bugzilla->template;
+            $template->process("admin/email/squatter-alert.txt.tmpl", $vars, \$message)
+                || ThrowTemplateError($template->error());
+
+            MessageToMTA($message);
+        }
+    }
+
+    # ADD SUBSCRIBERS TO REVSISIONS FOR CURRENT PRIVATE BUGS
+
     my $params = {
         f3  => 'OP',
         j3  => 'OR',
@@ -432,8 +683,10 @@ sub process_new_user {
     # the first value of each row should be the bug id
     my @bug_ids = map { shift @$_ } @$data;
 
+    INFO("Updating subscriber values for old private bugs");
+
     foreach my $bug_id (@bug_ids) {
-        DEBUG("Processing bug $bug_id");
+        INFO("Processing bug $bug_id");
 
         my $bug = Bugzilla::Bug->new({ id => $bug_id, cache => 1 });
 
@@ -442,7 +695,8 @@ sub process_new_user {
 
         foreach my $attachment (@attachments) {
             my ($revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
-            DEBUG("Processing revision D$revision_id");
+
+            INFO("Processing revision D$revision_id");
 
             my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query(
                 { ids => [ int($revision_id) ] });
@@ -450,7 +704,7 @@ sub process_new_user {
             $revision->add_subscriber($phab_user->phid);
             $revision->update();
 
-            DEBUG("Revision $revision_id updated");
+            INFO("Revision $revision_id updated");
         }
     }
 
@@ -467,7 +721,9 @@ sub new_stories {
     my ( $self, $after ) = @_;
     my $data = { view => 'text' };
     $data->{after} = $after if $after;
+
     my $result = request( 'feed.query_id', $data );
+
     unless ( ref $result->{result}{data} eq 'ARRAY'
         && @{ $result->{result}{data} } )
     {
@@ -487,7 +743,9 @@ sub new_users {
         }
     };
     $data->{before} = $after if $after;
+
     my $result = request( 'user.search', $data );
+
     unless ( ref $result->{result}{data} eq 'ARRAY'
         && @{ $result->{result}{data} } )
     {
@@ -504,7 +762,7 @@ sub get_last_id {
     my $last_id   = Bugzilla->dbh->selectrow_array( "
         SELECT value FROM phabbugz WHERE name = ?", undef, $type_full );
     $last_id ||= 0;
-    DEBUG( "QUERY " . uc($type_full) . ": $last_id" );
+    TRACE(uc($type_full) . ": $last_id" );
     return $last_id;
 }
 
@@ -513,9 +771,34 @@ sub save_last_id {
 
     # Store the largest last key so we can start from there in the next session
     my $type_full = $type . "_last_id";
-    DEBUG( "UPDATING " . uc($type_full) . ": $last_id" );
+    TRACE("UPDATING " . uc($type_full) . ": $last_id" );
     Bugzilla->dbh->do( "REPLACE INTO phabbugz (name, value) VALUES (?, ?)",
         undef, $type_full, $last_id );
 }
 
+sub get_group_members {
+    my ( $self, $group ) = @_;
+
+    my $group_obj =
+      ref $group ? $group : Bugzilla::Group->check( { name => $group, cache => 1 } );
+
+    my $flat_list = join(',',
+      @{ Bugzilla::Group->flatten_group_membership( $group_obj->id ) } );
+
+    my $user_query = "
+      SELECT DISTINCT profiles.userid
+        FROM profiles, user_group_map AS ugm
+       WHERE ugm.user_id = profiles.userid
+             AND ugm.isbless = 0
+             AND ugm.group_id IN($flat_list)";
+    my $user_ids = Bugzilla->dbh->selectcol_arrayref($user_query);
+
+    # Return matching users in Phabricator
+    return Bugzilla::Extension::PhabBugz::User->match(
+      {
+        ids => $user_ids
+      }
+    );
+}
+
 1;
diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm
index 8162ac52c..a86c83036 100644
--- a/extensions/PhabBugz/lib/Policy.pm
+++ b/extensions/PhabBugz/lib/Policy.pm
@@ -41,7 +41,7 @@ has 'rules' => (
 
 has 'rule_projects' => (
     is => 'lazy',
-    isa => ArrayRef[Str],
+    isa => ArrayRef[Object],
 );
 
 # {
@@ -88,7 +88,7 @@ sub new_from_query {
 }
 
 sub create {
-    my ($class, $project_names) = @_;
+    my ($class, $projects) = @_;
 
     my $data = {
         objectType => 'DREV',
@@ -97,23 +97,19 @@ sub create {
             {
                 action => 'allow',
                 rule   => 'PhabricatorSubscriptionsSubscribersPolicyRule',
+            },
+            {
+                action => 'allow',
+                rule   => 'PhabricatorDifferentialReviewersPolicyRule'
             }
         ]
     };
 
-    if (@$project_names) {
-        my $project_phids = [];
-        foreach my $project_name (@$project_names) {
-            my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ name => $project_name });
-            push @$project_phids, $project->phid if $project;
-        }
-
-        ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids;
-
+    if (@$projects) {
         push @{ $data->{policy} }, {
             action => 'allow',
-            rule   => 'PhabricatorProjectsPolicyRule',
-            value  => $project_phids,
+            rule   => 'PhabricatorProjectsAllPolicyRule',
+            value  => [ map { $_->phid } @$projects ],
         };
     }
     else {
@@ -131,11 +127,9 @@ sub _build_rule_projects {
     my ($self) = @_;
 
     return [] unless $self->rules;
-    my $rule = first { $_->{rule} eq 'PhabricatorProjectsPolicyRule'} @{ $self->rules };
+    my $rule = first { $_->{rule} =~ /PhabricatorProjects(?:All)?PolicyRule/ } @{ $self->rules };
     return [] unless $rule;
     return [
-        map  { $_->name }
-        grep { $_ }
         map  { Bugzilla::Extension::PhabBugz::Project->new_from_query( { phids => [$_] } ) }
         @{ $rule->{value} }
     ];
diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm
index b0babc58b..c52e1a661 100644
--- a/extensions/PhabBugz/lib/Project.pm
+++ b/extensions/PhabBugz/lib/Project.pm
@@ -9,15 +9,14 @@ package Bugzilla::Extension::PhabBugz::Project;
 
 use 5.10.1;
 use Moo;
+use Scalar::Util qw(blessed);
 use Types::Standard -all;
 use Type::Utils;
 
 use Bugzilla::Error;
 use Bugzilla::Util qw(trim);
-use Bugzilla::Extension::PhabBugz::Util qw(
-  request
-  get_phab_bmo_ids
-);
+use Bugzilla::Extension::PhabBugz::User;
+use Bugzilla::Extension::PhabBugz::Util qw(request);
 
 #########################
 #    Initialization     #
@@ -47,7 +46,18 @@ sub new_from_query {
 
     my $result = request( 'project.search', $data );
     if ( exists $result->{result}{data} && @{ $result->{result}{data} } ) {
-        return $class->new( $result->{result}{data}[0] );
+        # If name is used as a query param, we need to loop through and look
+        # for exact match as Conduit will tokenize the name instead of doing
+        # exact string match :( If name is not used, then return first one.
+        if ( exists $params->{name} ) {
+            foreach my $item ( @{ $result->{result}{data} } ) {
+                next if $item->{fields}{name} ne $params->{name};
+                return $class->new($item);
+            }
+        }
+        else {
+            return $class->new( $result->{result}{data}[0] );
+        }
     }
 }
 
@@ -157,7 +167,7 @@ sub create {
     my $result = request( 'project.edit', $data );
 
     return $class->new_from_query(
-        { phids => $result->{result}{object}{phid} } );
+        { phids => [ $result->{result}{object}{phid} ] } );
 }
 
 sub update {
@@ -270,20 +280,20 @@ sub set_description {
 sub add_member {
     my ( $self, $member ) = @_;
     $self->{add_members} ||= [];
-    my $member_phid = blessed $member ? $member->phab_phid : $member;
+    my $member_phid = blessed $member ? $member->phid : $member;
     push( @{ $self->{add_members} }, $member_phid );
 }
 
 sub remove_member {
     my ( $self, $member ) = @_;
     $self->{remove_members} ||= [];
-    my $member_phid = blessed $member ? $member->phab_phid : $member;
+    my $member_phid = blessed $member ? $member->phid : $member;
     push( @{ $self->{remove_members} }, $member_phid );
 }
 
 sub set_members {
     my ( $self, $members ) = @_;
-    $self->{set_members} = [ map { $_->phab_phid } @$members ];
+    $self->{set_members} = [ map { blessed $_ ? $_->phid : $_ } @$members ];
 }
 
 sub set_policy {
@@ -307,16 +317,13 @@ sub _build_members {
 
     return [] if !@phids;
 
-    my $users = get_phab_bmo_ids( { phids => \@phids } );
-
-    my @members;
-    foreach my $user (@$users) {
-        my $member = Bugzilla::User->new( { id => $user->{id}, cache => 1 } );
-        $member->{phab_phid} = $user->{phid};
-        push( @members, $member );
-    }
+    my $users = Bugzilla::Extension::PhabBugz::User->match(
+      {
+        phids => \@phids
+      }
+    );
 
-    return \@members;
+    return [ map { $_->bugzilla_user } @$users ];
 }
 
 1;
diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm
index 98c3196c2..900454220 100644
--- a/extensions/PhabBugz/lib/Revision.pm
+++ b/extensions/PhabBugz/lib/Revision.pm
@@ -17,10 +17,9 @@ use Type::Utils;
 use Bugzilla::Bug;
 use Bugzilla::Error;
 use Bugzilla::Util qw(trim);
-use Bugzilla::Extension::PhabBugz::Util qw(
-  get_phab_bmo_ids
-  request
-);
+use Bugzilla::Extension::PhabBugz::Project;
+use Bugzilla::Extension::PhabBugz::User;
+use Bugzilla::Extension::PhabBugz::Util qw(request);
 
 #########################
 #    Initialization     #
@@ -37,12 +36,12 @@ has author_phid      => ( is => 'ro',   isa => Str );
 has bug_id           => ( is => 'ro',   isa => Str );
 has view_policy      => ( is => 'ro',   isa => Str );
 has edit_policy      => ( is => 'ro',   isa => Str );
-has projects_raw     => ( is => 'ro',   isa => ArrayRef [Str] );
 has subscriber_count => ( is => 'ro',   isa => Int );
 has bug              => ( is => 'lazy', isa => Object );
 has author           => ( is => 'lazy', isa => Object );
 has reviewers        => ( is => 'lazy', isa => ArrayRef [Object] );
 has subscribers      => ( is => 'lazy', isa => ArrayRef [Object] );
+has projects         => ( is => 'lazy', isa => ArrayRef [Object] );
 has reviewers_raw => (
     is  => 'ro',
     isa => ArrayRef [
@@ -62,6 +61,12 @@ has subscribers_raw => (
         viewerIsSubscribed => Bool,
     ]
 );
+has projects_raw => (
+    is => 'ro',
+    isa => Dict [
+        projectPHIDs => ArrayRef [Str]
+    ]
+);
 
 sub new_from_query {
     my ( $class, $params ) = @_;
@@ -93,18 +98,18 @@ sub new_from_query {
 sub BUILDARGS {
     my ( $class, $params ) = @_;
 
-    $params->{title}           = $params->{fields}->{title};
-    $params->{summary}         = $params->{fields}->{summary};
-    $params->{status}          = $params->{fields}->{status}->{value};
-    $params->{creation_ts}     = $params->{fields}->{dateCreated};
-    $params->{modification_ts} = $params->{fields}->{dateModified};
-    $params->{author_phid}     = $params->{fields}->{authorPHID};
-    $params->{bug_id}          = $params->{fields}->{'bugzilla.bug-id'};
-    $params->{view_policy}     = $params->{fields}->{policy}->{view};
-    $params->{edit_policy}     = $params->{fields}->{policy}->{edit};
-    $params->{reviewers_raw} = $params->{attachments}->{reviewers}->{reviewers};
-    $params->{subscribers_raw} = $params->{attachments}->{subscribers};
-    $params->{projects}        = $params->{attachments}->{projects};
+    $params->{title}            = $params->{fields}->{title};
+    $params->{summary}          = $params->{fields}->{summary};
+    $params->{status}           = $params->{fields}->{status}->{value};
+    $params->{creation_ts}      = $params->{fields}->{dateCreated};
+    $params->{modification_ts}  = $params->{fields}->{dateModified};
+    $params->{author_phid}      = $params->{fields}->{authorPHID};
+    $params->{bug_id}           = $params->{fields}->{'bugzilla.bug-id'};
+    $params->{view_policy}      = $params->{fields}->{policy}->{view};
+    $params->{edit_policy}      = $params->{fields}->{policy}->{edit};
+    $params->{reviewers_raw}    = $params->{attachments}->{reviewers}->{reviewers};
+    $params->{subscribers_raw}  = $params->{attachments}->{subscribers};
+    $params->{projects_raw}     = $params->{attachments}->{projects};
     $params->{subscriber_count} =
       $params->{attachments}->{subscribers}->{subscriberCount};
 
@@ -284,12 +289,13 @@ sub _build_bug {
 sub _build_author {
     my ($self) = @_;
     return $self->{author} if $self->{author};
-    my $users = get_phab_bmo_ids( { phids => [ $self->author_phid ] } );
-    if (@$users) {
-        $self->{author} =
-          new Bugzilla::User( { id => $users->[0]->{id}, cache => 1 } );
-        $self->{author}->{phab_phid} = $self->author_phid;
-        return $self->{author};
+    my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query(
+      {
+        phids => [ $self->author_phid ]
+      }
+    );
+    if ($phab_user) {
+        return $self->{author} = $phab_user;
     }
 }
 
@@ -306,22 +312,22 @@ sub _build_reviewers {
 
     return [] unless @phids;
 
-    my $users = get_phab_bmo_ids( { phids => \@phids } );
+    my $users = Bugzilla::Extension::PhabBugz::User->match(
+      {
+        phids => \@phids
+      }
+    );
 
-    my @reviewers;
     foreach my $user (@$users) {
-        my $reviewer = Bugzilla::User->new( { id => $user->{id}, cache => 1 } );
-        $reviewer->{phab_phid} = $user->{phid};
         foreach my $reviewer_data ( @{ $self->reviewers_raw } ) {
-            if ( $reviewer_data->{reviewerPHID} eq $user->{phid} ) {
-                $reviewer->{phab_review_status} = $reviewer_data->{status};
+            if ( $reviewer_data->{reviewerPHID} eq $user->phid ) {
+                $user->{phab_review_status} = $reviewer_data->{status};
                 last;
             }
         }
-        push @reviewers, $reviewer;
     }
 
-    return \@reviewers;
+    return $self->{reviewers} = $users;
 }
 
 sub _build_subscribers {
@@ -335,19 +341,31 @@ sub _build_subscribers {
         push @phids, $phid;
     }
 
-    my $users = get_phab_bmo_ids( { phids => \@phids } );
+    my $users = Bugzilla::Extension::PhabBugz::User->match(
+      {
+        phids => \@phids
+      }
+    );
 
-    return [] unless @phids;
+    return $self->{subscribers} = $users;
+}
 
-    my @subscribers;
-    foreach my $user (@$users) {
-        my $subscriber =
-          Bugzilla::User->new( { id => $user->{id}, cache => 1 } );
-        $subscriber->{phab_phid} = $user->{phid};
-        push @subscribers, $subscriber;
+sub _build_projects {
+    my ($self) = @_;
+
+    return $self->{projects} if $self->{projects};
+    return [] unless $self->projects_raw->{projectPHIDs};
+
+    my @projects;
+    foreach my $phid ( @{ $self->projects_raw->{projectPHIDs} } ) {
+        push @projects, Bugzilla::Extension::PhabBugz::Project->new_from_query(
+          {
+            phids => [ $phid ]
+          }
+        );
     }
 
-    return \@subscribers;
+    return $self->{projects} = \@projects;
 }
 
 #########################
@@ -364,27 +382,27 @@ sub add_comment {
 sub add_reviewer {
     my ( $self, $reviewer ) = @_;
     $self->{add_reviewers} ||= [];
-    my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer;
+    my $reviewer_phid = blessed $reviewer ? $reviewer->phid : $reviewer;
     push @{ $self->{add_reviewers} }, $reviewer_phid;
 }
 
 sub remove_reviewer {
     my ( $self, $reviewer ) = @_;
     $self->{remove_reviewers} ||= [];
-    my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer;
+    my $reviewer_phid = blessed $reviewer ? $reviewer->phid : $reviewer;
     push @{ $self->{remove_reviewers} }, $reviewer_phid;
 }
 
 sub set_reviewers {
     my ( $self, $reviewers ) = @_;
-    $self->{set_reviewers} = [ map { $_->phab_phid } @$reviewers ];
+    $self->{set_reviewers} = [ map { $_->phid } @$reviewers ];
 }
 
 sub add_subscriber {
     my ( $self, $subscriber ) = @_;
     $self->{add_subscribers} ||= [];
     my $subscriber_phid =
-      blessed $subscriber ? $subscriber->phab_phid : $subscriber;
+      blessed $subscriber ? $subscriber->phid : $subscriber;
     push @{ $self->{add_subscribers} }, $subscriber_phid;
 }
 
@@ -392,7 +410,7 @@ sub remove_subscriber {
     my ( $self, $subscriber ) = @_;
     $self->{remove_subscribers} ||= [];
     my $subscriber_phid =
-      blessed $subscriber ? $subscriber->phab_phid : $subscriber;
+      blessed $subscriber ? $subscriber->phid : $subscriber;
     push @{ $self->{remove_subscribers} }, $subscriber_phid;
 }
 
@@ -423,4 +441,50 @@ sub remove_project {
     push @{ $self->{remove_projects} }, $project_phid;
 }
 
+sub make_private {
+    my ( $self, $project_names ) = @_;
+
+    my $secure_revision_project =
+      Bugzilla::Extension::PhabBugz::Project->new_from_query(
+        {
+          name => 'secure-revision'
+        }
+      );
+
+    my @set_projects;
+    foreach my $name (@$project_names) {
+        my $set_project =
+          Bugzilla::Extension::PhabBugz::Project->new_from_query(
+            {
+                name => $name
+            }
+            );
+        push @set_projects, $set_project;
+    }
+
+    my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects);
+    $self->set_policy('view', $new_policy->phid);
+    $self->set_policy('edit', $new_policy->phid);
+
+    foreach my $project ($secure_revision_project, @set_projects) {
+        $self->add_project($project->phid);
+    }
+
+    return $self;
+}
+
+sub make_public {
+    my ( $self ) = @_;
+
+    $self->set_policy('view', 'public');
+    $self->set_policy('edit', 'users');
+
+    my @current_group_projects = grep { $_->name =~ /^(bmo-.*|secure-revision)$/ } @{ $self->projects };
+    foreach my $project (@current_group_projects) {
+        $self->remove_project($project->phid);
+    }
+
+    return $self;
+}
+
 1;
diff --git a/extensions/PhabBugz/lib/User.pm b/extensions/PhabBugz/lib/User.pm
index 3b2d87e60..9d4e9eef4 100644
--- a/extensions/PhabBugz/lib/User.pm
+++ b/extensions/PhabBugz/lib/User.pm
@@ -116,14 +116,14 @@ sub match {
     my ( $class, $params ) = @_;
 
     # BMO id search takes precedence if bugzilla_ids is used.
-    my $bugzilla_ids = delete $params->{bugzilla_ids};
+    my $bugzilla_ids = delete $params->{ids};
     if ($bugzilla_ids) {
         my $bugzilla_data =
           $class->get_phab_bugzilla_ids( { ids => $bugzilla_ids } );
         $params->{phids} = [ map { $_->{phid} } @$bugzilla_data ];
     }
 
-    return [] if !$params->{phids};
+    return [] if !@{ $params->{phids} };
 
     # Look for BMO external user id in external-accounts attachment
     my $data = {
@@ -177,6 +177,7 @@ sub get_phab_bugzilla_ids {
 
         # Store new values in memcache for later retrieval
         foreach my $user ( @{ $result->{result} } ) {
+            next if !$user->{phid};
             $memcache->set(
                 {
                     key   => "phab_user_bugzilla_id_" . $user->{id},
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 844d8c0b5..d25f62f68 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -21,63 +21,26 @@ use Bugzilla::Extension::PhabBugz::Constants;
 use JSON::XS qw(encode_json decode_json);
 use List::Util qw(first);
 use LWP::UserAgent;
+use Taint::Util qw(untaint);
+use Try::Tiny;
 
 use base qw(Exporter);
 
 our @EXPORT = qw(
-    add_comment_to_revision
     add_security_sync_comments
     create_revision_attachment
-    create_private_revision_policy
-    create_project
-    edit_revision_policy
     get_attachment_revisions
     get_bug_role_phids
-    get_members_by_bmo_id
-    get_members_by_phid
-    get_phab_bmo_ids
-    get_project_phid
-    get_revisions_by_ids
-    get_revisions_by_phids
+    get_needs_review
     get_security_sync_groups
     intersect
     is_attachment_phab_revision
-    make_revision_private
-    make_revision_public
     request
     set_phab_user
-    set_project_members
-    set_revision_subscribers
 );
 
-sub get_revisions_by_ids {
-    my ($ids) = @_;
-    return _get_revisions({ ids => $ids });
-}
-
-sub get_revisions_by_phids {
-    my ($phids) = @_;
-    return _get_revisions({ phids => $phids });
-}
-
-sub _get_revisions {
-    my ($constraints) = @_;
-
-    my $data = {
-        queryKey    => 'all',
-        constraints => $constraints
-    };
-
-    my $result = request('differential.revision.search', $data);
-
-    ThrowUserError('invalid_phabricator_revision_id')
-        unless (exists $result->{result}{data} && @{ $result->{result}{data} });
-
-    return $result->{result}{data};
-}
-
 sub create_revision_attachment {
-    my ( $bug, $revision, $timestamp ) = @_;
+    my ( $bug, $revision, $timestamp, $submitter ) = @_;
 
     my $phab_base_uri = Bugzilla->params->{phabricator_base_uri};
     ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri;
@@ -97,22 +60,38 @@ sub create_revision_attachment {
         ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
     }
 
-    my $attachment = Bugzilla::Attachment->create(
-        {
-            bug         => $bug,
-            creation_ts => $timestamp,
-            data        => $revision_uri,
-            description => $revision->title,
-            filename    => 'phabricator-D' . $revision->id . '-url.txt',
-            ispatch     => 0,
-            isprivate   => 0,
-            mimetype    => PHAB_CONTENT_TYPE,
+    # If submitter, then switch to that user when creating attachment
+    my ($old_user, $attachment);
+    try {
+        if ($submitter) {
+            $old_user = Bugzilla->user;
+            $submitter->{groups} = [ Bugzilla::Group->get_all ]; # We need to always be able to add attachment
+            Bugzilla->set_user($submitter);
         }
-    );
 
-    # Insert a comment about the new attachment into the database.
-    $bug->add_comment($revision->summary, { type       => CMT_ATTACHMENT_CREATED,
-                                            extra_data => $attachment->id });
+        $attachment = Bugzilla::Attachment->create(
+            {
+                bug         => $bug,
+                creation_ts => $timestamp,
+                data        => $revision_uri,
+                description => $revision->title,
+                filename    => 'phabricator-D' . $revision->id . '-url.txt',
+                ispatch     => 0,
+                isprivate   => 0,
+                mimetype    => PHAB_CONTENT_TYPE,
+            }
+        );
+
+        # Insert a comment about the new attachment into the database.
+        $bug->add_comment($revision->summary, { type       => CMT_ATTACHMENT_CREATED,
+                                                extra_data => $attachment->id });
+    }
+    catch {
+        die $_;
+    }
+    finally {
+        Bugzilla->set_user($old_user) if $old_user;
+    };
 
     return $attachment;
 }
@@ -132,321 +111,47 @@ sub get_bug_role_phids {
     push(@bug_users, $bug->qa_contact) if $bug->qa_contact;
     push(@bug_users, @{ $bug->cc_users }) if @{ $bug->cc_users };
 
-    return get_members_by_bmo_id(\@bug_users);
-}
-
-sub create_private_revision_policy {
-    my ($bug, $groups) = @_;
-
-    my $data = {
-        objectType => 'DREV',
-        default    => 'deny',
-        policy     => [
-            {
-                action => 'allow',
-                rule   => 'PhabricatorSubscriptionsSubscribersPolicyRule',
-            }
-        ]
-    };
-
-    if(scalar @$groups gt 0) {
-        my $project_phids = [];
-        foreach my $group (@$groups) {
-            my $phid = get_project_phid('bmo-' . $group);
-            push(@$project_phids, $phid) if $phid;
-        }
-
-        ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids;
-
-        push(@{ $data->{policy} },
-            {
-                action => 'allow',
-                rule   => 'PhabricatorProjectsPolicyRule',
-                value  => $project_phids,
-            }
-        );
-    }
-    else {
-        my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
-            name => 'secure-revision'
-        });
-        push(@{ $data->{policy} },
-            {
-                action => 'allow',
-                value  => $secure_revision->phid,
-            }
-        );
-    }
-
-    my $result = request('policy.create', $data);
-    return $result->{result}{phid};
-}
-
-sub make_revision_public {
-    my ($revision_phid) = @_;
-    return request('differential.revision.edit', {
-        transactions => [
-            {
-                type  => 'view',
-                value => 'public'
-            },
-            {
-                type  => 'edit',
-                value => 'users'
-            }
-        ],
-        objectIdentifier => $revision_phid
-    });
-}
-
-sub make_revision_private {
-    my ($revision_phid) = @_;
-
-    my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
-        name => 'secure-revision'
-    });
-
-    return request('differential.revision.edit', {
-        transactions => [
-            {
-                type  => "view",
-                value => $secure_revision->phid
-            },
-            {
-                type  => "edit",
-                value => $secure_revision->phid
-            }
-        ],
-        objectIdentifier => $revision_phid
-    });
-}
-
-sub edit_revision_policy {
-    my ($revision_phid, $policy_phid, $subscribers) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'view',
-                value => $policy_phid
-            },
-            {
-                type  => 'edit',
-                value => $policy_phid
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-
-    if (@$subscribers) {
-        push(@{ $data->{transactions} }, {
-            type  => 'subscribers.set',
-            value => $subscribers
-        });
-    }
-
-    return request('differential.revision.edit', $data);
-}
-
-sub set_revision_subscribers {
-    my ($revision_phid, $subscribers) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'subscribers.set',
-                value => $subscribers
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-
-    return request('differential.revision.edit', $data);
-}
-
-sub add_comment_to_revision {
-    my ($revision_phid, $comment) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'comment',
-                value => $comment
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-    return request('differential.revision.edit', $data);
-}
-
-sub get_project_phid {
-    my $project = shift;
-    my $memcache = Bugzilla->memcached;
-
-    # Check memcache
-    my $project_phid = $memcache->get_config({ key => "phab_project_phid_" . $project });
-    if (!$project_phid) {
-        my $data = {
-            queryKey => 'all',
-            constraints => {
-                name => $project
-            }
-        };
-
-        my $result = request('project.search', $data);
-        return undef
-            unless (exists $result->{result}{data} && @{ $result->{result}{data} });
-
-        $project_phid = $result->{result}{data}[0]{phid};
-        $memcache->set_config({ key => "phab_project_phid_" . $project, data => $project_phid });
-    }
-    return $project_phid;
-}
-
-sub create_project {
-    my ($project, $description, $members) = @_;
-
-    my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
-        name => 'secure-revision'
-    });
-
-    my $data = {
-        transactions => [
-            { type => 'name',  value => $project               },
-            { type => 'description', value => $description     },
-            { type => 'edit',  value => $secure_revision->phid }.
-            { type => 'join',  value => $secure_revision->phid },
-            { type => 'view',  value => $secure_revision->phid },
-            { type => 'icon',  value => 'group'                },
-            { type => 'color', value => 'red'                  }
-        ]
-    };
-
-    my $result = request('project.edit', $data);
-    return $result->{result}{object}{phid};
-}
-
-sub set_project_members {
-    my ($project_id, $phab_user_ids) = @_;
-
-    my $data = {
-        objectIdentifier => $project_id,
-        transactions => [
-            { type => 'members.set',  value => $phab_user_ids }
-        ]
-    };
-
-    my $result = request('project.edit', $data);
-    return $result->{result}{object}{phid};
-}
-
-sub get_members_by_bmo_id {
-    my $users = shift;
-
-    my $result = get_phab_bmo_ids({ ids => [ map { $_->id } @$users ] });
-
-    my @phab_ids;
-    foreach my $user (@$result) {
-        push(@phab_ids, $user->{phid})
-          if ($user->{phid} && $user->{phid} =~ /^PHID-USER/);
-    }
-
-    return \@phab_ids;
-}
-
-sub get_members_by_phid {
-    my $phids = shift;
-
-    my $result = get_phab_bmo_ids({ phids => $phids });
-
-    my @bmo_ids;
-    foreach my $user (@$result) {
-        push(@bmo_ids, $user->{id})
-          if ($user->{phid} && $user->{phid} =~ /^PHID-USER/);
-    }
-
-    return \@bmo_ids;
-}
-
-sub get_phab_bmo_ids {
-    my ($params) = @_;
-    my $memcache = Bugzilla->memcached;
-
-    # Try to find the values in memcache first
-    my @results;
-    if ($params->{ids}) {
-        my @bmo_ids = @{ $params->{ids} };
-        for (my $i = 0; $i < @bmo_ids; $i++) {
-            my $phid = $memcache->get({ key => "phab_user_bmo_id_" . $bmo_ids[$i] });
-            if ($phid) {
-                push(@results, {
-                    id   => $bmo_ids[$i],
-                    phid => $phid
-                });
-                splice(@bmo_ids, $i, 1);
-            }
-        }
-        $params->{ids} = \@bmo_ids;
-    }
-
-    if ($params->{phids}) {
-        my @phids = @{ $params->{phids} };
-        for (my $i = 0; $i < @phids; $i++) {
-            my $bmo_id = $memcache->get({ key => "phab_user_phid_" . $phids[$i] });
-            if ($bmo_id) {
-                push(@results, {
-                    id   => $bmo_id,
-                    phid => $phids[$i]
-                });
-                splice(@phids, $i, 1);
-            }
+    my $phab_users =
+      Bugzilla::Extension::PhabBugz::User->match(
+        {
+          ids => [ map { $_->id } @bug_users ]
         }
-        $params->{phids} = \@phids;
-    }
-
-    my $result = request('bugzilla.account.search', $params);
-
-    # Store new values in memcache for later retrieval
-    foreach my $user (@{ $result->{result} }) {
-        $memcache->set({ key   => "phab_user_bmo_id_" . $user->{id},
-                         value => $user->{phid} });
-        $memcache->set({ key   => "phab_user_phid_" . $user->{phid},
-                         value => $user->{id} });
-        push(@results, $user);
-    }
+    );
 
-    return \@results;
+    return [ map { $_->phid } @{ $phab_users } ];
 }
 
 sub is_attachment_phab_revision {
     my ($attachment) = @_;
-    return ($attachment->contenttype eq PHAB_CONTENT_TYPE
-            && $attachment->attacher->login eq PHAB_AUTOMATION_USER) ? 1 : 0;
+    return $attachment->contenttype eq PHAB_CONTENT_TYPE;
 }
 
 sub get_attachment_revisions {
     my $bug = shift;
 
-    my $revisions;
-
     my @attachments =
       grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
 
-    if (@attachments) {
-        my @revision_ids;
-        foreach my $attachment (@attachments) {
-            my ($revision_id) =
-              ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN );
-            next if !$revision_id;
-            push( @revision_ids, int($revision_id) );
-        }
+    return unless @attachments;
 
-        if (@revision_ids) {
-            $revisions = get_revisions_by_ids( \@revision_ids );
-        }
+    my @revision_ids;
+    foreach my $attachment (@attachments) {
+        my ($revision_id) =
+          ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN );
+        next if !$revision_id;
+        push( @revision_ids, int($revision_id) );
     }
 
-    return @$revisions;
+    return unless @revision_ids;
+
+    my @revisions;
+    foreach my $revision_id (@revision_ids) {
+        push @revisions, Bugzilla::Extension::PhabBugz::Revision->new_from_query({
+            ids => [ $revision_id ]
+        });
+    }
+
+    return \@revisions;
 }
 
 sub request {
@@ -478,7 +183,12 @@ sub request {
       if $response->is_error;
 
     my $result;
-    my $result_ok = eval { $result = decode_json( $response->content); 1 };
+    my $result_ok = eval {
+        my $content = $response->content;
+        untaint($content);
+        $result = decode_json( $content );
+        1;
+    };
     if (!$result_ok || $result->{error_code}) {
         ThrowCodeError('phabricator_api_error',
             { reason => 'JSON decode failure' }) if !$result_ok;
@@ -493,9 +203,8 @@ sub request {
 sub get_security_sync_groups {
     my $bug = shift;
 
-    my $phab_sync_groups = Bugzilla->params->{phabricator_sync_groups}
-        || ThrowUserError('invalid_phabricator_sync_groups');
-    my $sync_group_names = [ split('[,\s]+', $phab_sync_groups) ];
+    my $sync_groups = Bugzilla::Group->match( { isactive => 1, isbuggroup => 1 } );
+    my $sync_group_names = [ map { $_->name } @$sync_groups ]; 
 
     my $bug_groups = $bug->groups_in;
     my $bug_group_names = [ map { $_->name } @$bug_groups ];
@@ -519,7 +228,7 @@ sub add_security_sync_comments {
     my $phab_error_message = 'Revision is being made private due to unknown Bugzilla groups.';
 
     foreach my $revision (@$revisions) {
-        add_comment_to_revision( $revision->{phid}, $phab_error_message );
+        $revision->add_comment($phab_error_message);
     }
 
     my $num_revisions = scalar @$revisions;
@@ -536,4 +245,36 @@ sub add_security_sync_comments {
     Bugzilla->set_user($old_user);
 }
 
+sub get_needs_review {
+    my ($user) = @_;
+    $user //= Bugzilla->user;
+    return unless $user->id;
+
+    my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query(
+      {
+        ids => [ $user->id ]
+      }
+    );
+
+    return [] unless $phab_user;
+
+    my $diffs = request(
+        'differential.revision.search',
+        {
+            attachments => {
+                reviewers => 1,
+            },
+            constraints => {
+                reviewerPHIDs => [$phab_user->phid],
+                statuses      => [qw( needs-review )],
+            },
+            order       => 'newest',
+        }
+    );
+    ThrowCodeError('phabricator_api_error', { reason => 'Malformed Response' })
+        unless exists $diffs->{result}{data};
+
+    return $diffs->{result}{data};
+}
+
 1;
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index 5b6310de6..0239ccf74 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -13,115 +13,44 @@ use warnings;
 
 use base qw(Bugzilla::WebService);
 
-use Bugzilla::Attachment;
-use Bugzilla::Bug;
-use Bugzilla::BugMail;
 use Bugzilla::Constants;
-use Bugzilla::Error;
-use Bugzilla::Extension::Push::Util qw(is_public);
 use Bugzilla::User;
-use Bugzilla::Util qw(detaint_natural);
+use Bugzilla::Util qw(detaint_natural datetime_from time_ago trick_taint);
 use Bugzilla::WebService::Constants;
 
 use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Util qw(
-    add_security_sync_comments
-    create_revision_attachment
-    create_private_revision_policy
-    edit_revision_policy
-    get_bug_role_phids
-    get_project_phid
-    get_revisions_by_ids
-    intersect
-    is_attachment_phab_revision
-    make_revision_public
-    request
-    get_security_sync_groups
+    get_needs_review
 );
 
-use List::Util qw(first);
+use DateTime ();
+use List::Util qw(first uniq);
 use List::MoreUtils qw(any);
 use MIME::Base64 qw(decode_base64);
 
-use constant PUBLIC_METHODS => qw(
+use constant READ_ONLY => qw(
     check_user_permission_for_bug
-    obsolete_attachments
-    revision
-    update_reviewer_statuses
+    needs_review
 );
 
-sub revision {
-    my ($self, $params) = @_;
-
-    # Phabricator only supports sending credentials via HTTP Basic Auth
-    # so we exploit that function to pass in an API key as the password
-    # of basic auth. BMO does not support basic auth but does support
-    # use of API keys.
-    my $http_auth = Bugzilla->cgi->http('Authorization');
-    $http_auth =~ s/^Basic\s+//;
-    $http_auth = decode_base64($http_auth);
-    my ($login, $api_key) = split(':', $http_auth);
-    $params->{'Bugzilla_login'} = $login;
-    $params->{'Bugzilla_api_key'} = $api_key;
-
-    my $user = Bugzilla->login(LOGIN_REQUIRED);
-
-    # Prechecks
-    _phabricator_precheck($user);
-
-    unless (defined $params->{revision} && detaint_natural($params->{revision})) {
-        ThrowCodeError('param_required', { param => 'revision' })
-    }
-
-    # Obtain more information about the revision from Phabricator
-    my $revision_id = $params->{revision};
-    my $revisions = get_revisions_by_ids([$revision_id]);
-    my $revision = $revisions->[0];
-
-    my $revision_phid  = $revision->{phid};
-    my $revision_title = $revision->{fields}{title} || 'Unknown Description';
-    my $bug_id         = $revision->{fields}{'bugzilla.bug-id'};
-
-    my $bug = Bugzilla::Bug->new($bug_id);
-
-    # If bug is public then remove privacy policy
-    my $result;
-    if (is_public($bug)) {
-        $result = make_revision_public($revision_id);
-    }
-    # else bug is private
-    else {
-        my @set_groups = get_security_sync_groups($bug);
-
-        # If bug privacy groups do not have any matching synchronized groups,
-        # then leave revision private and it will have be dealt with manually.
-        if (!@set_groups) {
-            add_security_sync_comments($revisions, $bug);
-        }
-
-        my $policy_phid = create_private_revision_policy($bug, \@set_groups);
-        my $subscribers = get_bug_role_phids($bug);
-        $result = edit_revision_policy($revision_phid, $policy_phid, $subscribers);
-    }
-
-    my $attachment = create_revision_attachment($bug, $revision_id, $revision_title);
-
-    Bugzilla::BugMail::Send($bug_id, { changer => $user });
-
-    return {
-        result          => $result,
-        attachment_id   => $attachment->id,
-        attachment_link => Bugzilla->localconfig->{urlbase} . "attachment.cgi?id=" . $attachment->id
-    };
-}
+use constant PUBLIC_METHODS => qw(
+    check_user_permission_for_bug
+    needs_review
+    set_build_target
+);
 
 sub check_user_permission_for_bug {
     my ($self, $params) = @_;
 
     my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-    # Prechecks
-    _phabricator_precheck($user);
+    # Ensure PhabBugz is on
+    ThrowUserError('phabricator_not_enabled')
+        unless Bugzilla->params->{phabricator_enabled};
+
+    # Validate that the requesting user's email matches phab-bot
+    ThrowUserError('phabricator_unauthorized_user')
+        unless $user->login eq PHAB_AUTOMATION_USER;
 
     # Validate that a bug id and user id are provided
     ThrowUserError('phabricator_invalid_request_params')
@@ -136,184 +65,152 @@ sub check_user_permission_for_bug {
     };
 }
 
-sub update_reviewer_statuses {
+sub needs_review {
     my ($self, $params) = @_;
-
+    ThrowUserError('phabricator_not_enabled')
+        unless Bugzilla->params->{phabricator_enabled};
     my $user = Bugzilla->login(LOGIN_REQUIRED);
-
-    # Prechecks
-    _phabricator_precheck($user);
-
-    my $revision_id = $params->{revision_id};
-    unless (defined $revision_id && detaint_natural($revision_id)) {
-        ThrowCodeError('param_required', { param => 'revision_id' })
-    }
-
-    my $bug_id = $params->{bug_id};
-    unless (defined $bug_id && detaint_natural($bug_id)) {
-        ThrowCodeError('param_required', { param => 'bug_id' })
+    my $dbh  = Bugzilla->dbh;
+
+    my $reviews = get_needs_review();
+
+    my $authors = Bugzilla::Extension::PhabBugz::User->match({
+        phids => [
+            uniq
+            grep { defined }
+            map { $_->{fields}{authorPHID} }
+            @$reviews
+        ]
+    });
+
+    my %author_phab_to_id = map { $_->phid => $_->bugzilla_user->id } @$authors;
+    my %author_id_to_user = map { $_->bugzilla_user->id => $_->bugzilla_user } @$authors;
+
+    # bug data
+    my $visible_bugs = $user->visible_bugs([
+        uniq
+        grep { $_ }
+        map { $_->{fields}{'bugzilla.bug-id'} }
+        @$reviews
+    ]);
+
+    # get all bug statuses and summaries in a single query to avoid creation of
+    # many bug objects
+    my %bugs;
+    if (@$visible_bugs) {
+        #<<<
+        my $bug_rows =$dbh->selectall_arrayref(
+            'SELECT bug_id, bug_status, short_desc ' .
+            '  FROM bugs ' .
+            ' WHERE bug_id IN (' . join(',', ('?') x @$visible_bugs) . ')',
+            { Slice => {} },
+            @$visible_bugs
+        );
+        #>>>
+        %bugs = map { $_->{bug_id} => $_ } @$bug_rows;
     }
 
-    my $accepted_user_ids = $params->{accepted_users};
-    defined $accepted_user_ids
-      || ThrowCodeError('param_required', { param => 'accepted_users' });
-    $accepted_user_ids = [ split(':', $accepted_user_ids) ];
-
-    my $denied_user_ids = $params->{denied_users};
-    defined $denied_user_ids
-      || ThrowCodeError('param_required', { param => 'denied_users' });
-    $denied_user_ids = [ split(':', $denied_user_ids) ];
-
-    my $bug = Bugzilla::Bug->check($bug_id);
-
-    my @attachments =
-      grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
-
-    return { result => [] } if !@attachments;
-
-    my $dbh = Bugzilla->dbh;
-    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
-
-    my @updated_attach_ids;
-    foreach my $attachment (@attachments) {
-        my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
-        next if $revision_id != $curr_revision_id;
-
-        # Clear old flags if no longer accepted
-        my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type);
-        foreach my $flag (@{ $attachment->flags }) {
-            next if $flag->type->name ne 'review';
-            $flag_type = $flag->type if $flag->type->is_active;
-            if (any { $flag->setter->id == $_ } @$denied_user_ids) {
-                push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-            if (any { $flag->setter->id == $_ } @$accepted_user_ids) {
-                $accepted_done{$flag->setter->id}++;
-            }
-            if ($flag->status eq '+'
-                && !any { $flag->setter->id == $_ } (@$accepted_user_ids, @$denied_user_ids)) {
-                push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-        }
-
-        $flag_type ||= first { $_->name eq 'review' && $_->is_active } @{ $attachment->flag_types };
-
-        # Create new flags
-        foreach my $user_id (@$accepted_user_ids) {
-            next if $accepted_done{$user_id};
-            my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
-            push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' });
+    # build result
+    my $datetime_now = DateTime->now(time_zone => $user->timezone);
+    my @result;
+    foreach my $review (@$reviews) {
+        my $review_flat = {
+            id     => $review->{id},
+            title  => $review->{fields}{title},
+            url    => Bugzilla->params->{phabricator_base_uri} . 'D' . $review->{id},
+        };
+
+        # show date in user's timezone
+        my $datetime = DateTime->from_epoch(
+            epoch     => $review->{fields}{dateModified},
+            time_zone => 'UTC'
+        );
+        $datetime->set_time_zone($user->timezone);
+        $review_flat->{updated}       = $datetime->strftime('%Y-%m-%d %T %Z');
+        $review_flat->{updated_fancy} = time_ago($datetime, $datetime_now);
+
+        # review requester
+        if (my $author = $author_id_to_user{$author_phab_to_id{ $review->{fields}{authorPHID} }}) {
+            $review_flat->{author_name}  = $author->name;
+            $review_flat->{author_email} = $author->email;
         }
-
-        # Also add comment to for attachment update showing the user's name
-        # that changed the revision.
-        my $comment;
-        foreach my $flag_data (@new_flags) {
-            $comment .= $flag_data->{setter}->name . " has approved the revision.\n";
-        }
-        foreach my $flag_data (@denied_flags) {
-            $comment .= $flag_data->{setter}->name . " has requested changes to the revision.\n";
-        }
-        foreach my $flag_data (@removed_flags) {
-            $comment .= $flag_data->{setter}->name . " has been removed from the revision.\n";
+        else {
+            $review_flat->{author_name}  = 'anonymous';
+            $review_flat->{author_email} = 'anonymous';
         }
 
-        if ($comment) {
-            $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision_id;
-            # Add transaction_id as anchor if one present
-            $comment .= "#" . $params->{transaction_id} if $params->{transaction_id};
-            $bug->add_comment($comment, {
-                isprivate  => $attachment->isprivate,
-                type       => CMT_ATTACHMENT_UPDATED,
-                extra_data => $attachment->id
-            });
+        # referenced bug
+        if (my $bug_id = $review->{fields}{'bugzilla.bug-id'}) {
+            my $bug = $bugs{$bug_id};
+            $review_flat->{bug_id}      = $bug_id;
+            $review_flat->{bug_status}  = $bug->{bug_status};
+            $review_flat->{bug_summary} = $bug->{short_desc};
         }
 
-        $attachment->set_flags([ @denied_flags, @removed_flags ], \@new_flags);
-        $attachment->update($timestamp);
-        $bug->update($timestamp) if $comment;
-
-        push(@updated_attach_ids, $attachment->id);
+        push @result, $review_flat;
     }
 
-    Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids;
-
-    return { result => \@updated_attach_ids };
+    return { result => \@result };
 }
 
-sub obsolete_attachments {
-    my ($self, $params) = @_;
+sub set_build_target {
+    my ( $self, $params ) = @_;
 
-    my $user = Bugzilla->login(LOGIN_REQUIRED);
-
-    # Prechecks
-    _phabricator_precheck($user);
-
-    my $revision_id = $params->{revision_id};
-    unless (defined $revision_id && detaint_natural($revision_id)) {
-        ThrowCodeError('param_required', { param => 'revision' })
-    }
-
-    my $bug_id= $params->{bug_id};
-    unless (defined $bug_id && detaint_natural($bug_id)) {
-        ThrowCodeError('param_required', { param => 'bug_id' })
-    }
-
-    my $make_obsolete = $params->{make_obsolete};
-    unless (defined $make_obsolete) {
-        ThrowCodeError('param_required', { param => 'make_obsolete' })
-    }
-    $make_obsolete = $make_obsolete ? 1 : 0;
-
-    my $bug = Bugzilla::Bug->check($bug_id);
-
-    my @attachments =
-      grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
-
-    return { result => [] } if !@attachments;
-
-    my $dbh = Bugzilla->dbh;
-    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
-
-    my @updated_attach_ids;
-    foreach my $attachment (@attachments) {
-        my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
-        next if $revision_id != $curr_revision_id;
+    # Phabricator only supports sending credentials via HTTP Basic Auth
+    # so we exploit that function to pass in an API key as the password
+    # of basic auth. BMO does not support basic auth but does support
+    # use of API keys.
+    my $http_auth = Bugzilla->cgi->http('Authorization');
+    $http_auth =~ s/^Basic\s+//;
+    $http_auth = decode_base64($http_auth);
+    my ($login, $api_key) = split(':', $http_auth);
+    $params->{'Bugzilla_login'}   = $login;
+    $params->{'Bugzilla_api_key'} = $api_key;
 
-        $attachment->set_is_obsolete($make_obsolete);
-        $attachment->update($timestamp);
+    my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-        push(@updated_attach_ids, $attachment->id);
-    }
+    # Ensure PhabBugz is on
+    ThrowUserError('phabricator_not_enabled')
+      unless Bugzilla->params->{phabricator_enabled};
+    
+    # Validate that the requesting user's email matches phab-bot
+    ThrowUserError('phabricator_unauthorized_user')
+      unless $user->login eq PHAB_AUTOMATION_USER;
 
-    Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids;
+    my $revision_id  = $params->{revision_id};
+    my $build_target = $params->{build_target};
 
-    return { result => \@updated_attach_ids };
-}
+    ThrowUserError('invalid_phabricator_revision_id')
+      unless detaint_natural($revision_id);
 
-sub _phabricator_precheck {
-    my ($user) = @_;
+    ThrowUserError('invalid_phabricator_build_target')
+      unless $build_target =~ /^PHID-HMBT-[a-zA-Z0-9]+$/;
+    trick_taint($build_target);
 
-    # Ensure PhabBugz is on
-    ThrowUserError('phabricator_not_enabled')
-        unless Bugzilla->params->{phabricator_enabled};
+    Bugzilla->dbh->do(
+        "INSERT INTO phabbugz (name, value) VALUES (?, ?)",
+        undef,
+        'build_target_' . $revision_id,
+        $build_target
+    );
 
-    # Validate that the requesting user's email matches phab-bot
-    ThrowUserError('phabricator_unauthorized_user')
-        unless $user->login eq PHAB_AUTOMATION_USER;
+    return { result => 1 };
 }
 
 sub rest_resources {
     return [
-        # Revision creation
-        qr{^/phabbugz/revision/([^/]+)$}, {
+        # Set build target in Phabricator
+        qr{^/phabbugz/build_target/(\d+)/(PHID-HMBT-.*)$}, {
             POST => {
-                method => 'revision',
+                method => 'set_build_target',
                 params => sub {
-                    return { revision => $_[0] };
+                    return { 
+                        revision_id  => $_[0],
+                        build_target => $_[1]
+                    };
                 }
             }
-        },
+        },        
         # Bug permission checks
         qr{^/phabbugz/check_bug/(\d+)/(\d+)$}, {
             GET => {
@@ -323,17 +220,11 @@ sub rest_resources {
                 }
             }
         },
-        # Update reviewer statuses
-        qr{^/phabbugz/update_reviewer_statuses$}, {
-            PUT => {
-                method => 'update_reviewer_statuses',
-            }
-        },
-        # Obsolete attachments
-        qr{^/phabbugz/obsolete$}, {
-            PUT => {
-                method => 'obsolete_attachments',
-            }
+        # Review requests
+        qw{^/phabbugz/needs_review$}, {
+            GET => {
+                method => 'needs_review',
+            },
         }
     ];
 }