[Bps-public-commit] rt-extension-rest2 branch, master, updated. 1.07-28-g70462de

? sunnavy sunnavy at bestpractical.com
Fri May 1 10:57:05 EDT 2020


The branch, master has been updated
       via  70462dec601086b3e3fbc1cfc0bc15a4b5b6195d (commit)
       via  ff0b37d5fcccc4ab672c65e95e813d6ad8be91fe (commit)
       via  b48bd780ca8b07d8159b3dc9a86d741b1a78f102 (commit)
       via  5bf88756d3c0ca3f780956903a3150a264ce9cec (commit)
      from  332f0ba05c5e540574f36a58318d37fb81a7e17e (commit)

Summary of changes:
 META.yml                                           |   3 +-
 lib/RT/Extension/REST2/Resource/Record/WithETag.pm |   7 +-
 lib/RT/Extension/REST2/Resource/Record/Writable.pm |  18 +++
 lib/RT/Extension/REST2/Resource/User.pm            |  45 +++++-
 lib/RT/Extension/REST2/Resource/Users.pm           |  25 +++-
 xt/users.t                                         | 166 +++++++++++++++++++++
 6 files changed, 247 insertions(+), 17 deletions(-)
 create mode 100644 xt/users.t

- Log -----------------------------------------------------------------
commit 5bf88756d3c0ca3f780956903a3150a264ce9cec
Author: Emmanuel Lacour <elacour at easter-eggs.com>
Date:   Mon Sep 23 10:50:49 2019 +0200

    Improved access and authorisation for user endpoints
    
    Allow any privileged user to search for and view summary of users. If
    without corresponding rights, don't show user history/groups links

diff --git a/lib/RT/Extension/REST2/Resource/Record/WithETag.pm b/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
index 9f05ad3..798367f 100644
--- a/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
@@ -20,7 +20,12 @@ sub generate_etag {
     if ($record->can('Transactions')) {
         my $txns = $record->Transactions;
         $txns->OrderByCols({ FIELD => 'id', ORDER => 'DESC' });
-        return $txns->First->Id if $txns->Count;
+
+        # $txns->Count could be inaccurate because of ACL,
+        # checking ->First directly is more reliable.
+        if ( my $first = $txns->First ) {
+            return $first->Id;
+        }
     }
 
     # fall back to last-modified time, which is commonly accepted even
diff --git a/lib/RT/Extension/REST2/Resource/User.pm b/lib/RT/Extension/REST2/Resource/User.pm
index 8a74630..33a9ea7 100644
--- a/lib/RT/Extension/REST2/Resource/User.pm
+++ b/lib/RT/Extension/REST2/Resource/User.pm
@@ -40,6 +40,13 @@ around 'serialize' => sub {
     my $self = shift;
     my $data = $self->$orig(@_);
     $data->{Privileged} = $self->record->Privileged ? 1 : 0;
+
+    unless ( $self->record->CurrentUserHasRight("AdminUsers") or $self->record->id == $self->current_user->id ) {
+        my $summary = { map { $_ => $data->{$_} }
+                ( '_hyperlinks', 'Privileged', split( /\s*,\s*/, RT->Config->Get('UserSummaryExtraInfo') ) ) };
+        return $summary;
+    }
+
     $data->{Disabled}   = $self->record->PrincipalObj->Disabled;
     $data->{Memberships} = [
         map { expand_uid($_->UID) }
@@ -52,20 +59,44 @@ sub forbidden {
     my $self = shift;
     return 0 if not $self->record->id;
     return 0 if $self->record->id == $self->current_user->id;
-    return 0 if $self->record->CurrentUserHasRight("AdminUsers");
+    return 0 if $self->current_user->Privileged;
     return 1;
 }
 
 sub hypermedia_links {
     my $self = shift;
     my $links = $self->_default_hypermedia_links(@_);
-    push @$links, $self->_transaction_history_link;
 
-    my $id = $self->record->id;
-    push @$links,
-      { ref  => 'memberships',
-        _url => RT::Extension::REST2->base_uri . "/user/$id/groups",
-      };
+    if (   $self->record->CurrentUserHasRight('ShowUserHistory')
+        or $self->record->CurrentUserHasRight("AdminUsers")
+        or $self->record->id == $self->current_user->id )
+    {
+        push @$links, $self->_transaction_history_link;
+    }
+
+    # TODO Use the same right in UserGroups.pm.
+    # Maybe loose it a bit so user can see groups?
+    if ((   $self->current_user->HasRight(
+                Right  => "ModifyOwnMembership",
+                Object => RT->System,
+            )
+            && $self->current_user->id == $self->record->id
+        )
+        || $self->current_user->HasRight(
+            Right  => 'AdminGroupMembership',
+            Object => RT->System
+        )
+       )
+    {
+
+        my $id = $self->record->id;
+        push @$links,
+            {
+            ref  => 'memberships',
+            _url => RT::Extension::REST2->base_uri . "/user/$id/groups",
+            };
+    }
+
     return $links;
 }
 
diff --git a/lib/RT/Extension/REST2/Resource/Users.pm b/lib/RT/Extension/REST2/Resource/Users.pm
index aa6d6f9..f3ccedd 100644
--- a/lib/RT/Extension/REST2/Resource/Users.pm
+++ b/lib/RT/Extension/REST2/Resource/Users.pm
@@ -16,18 +16,27 @@ sub dispatch_rules {
 }
 
 sub searchable_fields {
-    my $class = $_[0]->collection->RecordClass;
-    grep {
-        $class->_Accessible($_ => "public")
-    } $class->ReadableAttributes
+    my $self = shift;
+    my $class = $self->collection->RecordClass;
+    my @fields;
+    if ($self->current_user->HasRight(
+            Right   => "AdminUsers",
+            Object  => RT->System,
+        )) {
+        @fields = grep {
+            $class->_Accessible($_ => "public")
+        } $class->ReadableAttributes;
+    }
+    else {
+        @fields = split(/\s*\,\s*/, RT->Config->Get('UserSummaryExtraInfo'));
+    }
+    return @fields
 }
 
 sub forbidden {
     my $self = shift;
-    return 0 if $self->current_user->HasRight(
-        Right   => "AdminUsers",
-        Object  => RT->System,
-    );
+
+    return 0 if $self->current_user->Privileged;
     return 1;
 }
 

commit b48bd780ca8b07d8159b3dc9a86d741b1a78f102
Author: Aaron Trevena <ast at bestpractical.com>
Date:   Fri Apr 10 14:53:53 2020 +0100

    Added tests for user API endpoints

diff --git a/META.yml b/META.yml
index 65f84bc..7cab39d 100644
--- a/META.yml
+++ b/META.yml
@@ -20,6 +20,7 @@ name: RT-Extension-REST2
 no_index:
   directory:
     - inc
+    - t
     - xt
   package:
     - RT::Extension::REST2::Test
@@ -33,7 +34,7 @@ requires:
   Moose: 0
   MooseX::NonMoose: 0
   MooseX::Role::Parameterized: 0
-  Path::Dispatcher: 0
+  Path::Dispatcher: '1.07'
   Plack::Builder: 0
   Pod::POM: 0
   Scalar::Util: 0
diff --git a/xt/users.t b/xt/users.t
new file mode 100644
index 0000000..2621eb3
--- /dev/null
+++ b/xt/users.t
@@ -0,0 +1,166 @@
+use strict;
+use warnings;
+use RT::Extension::REST2::Test tests => undef;
+use Test::Deep;
+#use Test::Warn;
+
+use Data::Dumper;
+my $mech = RT::Extension::REST2::Test->mech;
+
+my $auth = RT::Extension::REST2::Test->authorization_header;
+
+my $rest_base_path = '/REST/2.0';
+my $test_user = RT::Extension::REST2::Test->user;
+$test_user->PrincipalObj->RevokeRight(Right => 'ShowUserHistory');
+
+my $user_foo = RT::Test->load_or_create_user(
+    Name     => 'foo',
+    RealName => 'Foo Jr III',
+    EmailAddress => 'test at test.test',
+    Password => 'password',
+);
+my $user_bar = RT::Test->load_or_create_user( Name => 'bar', RealName => 'Bar Jr III', );
+my $user_baz = RT::Test->load_or_create_user( Name => 'baz' );
+my $user_quuz = RT::Test->load_or_create_user( Name => 'quuz' );
+
+$user_baz->SetDisabled(1);
+
+my $group1 = RT::Group->new(RT->SystemUser);
+$group1->CreateUserDefinedGroup(Name => 'Group 1');
+
+my $group2 = RT::Group->new(RT->SystemUser);
+$group2->CreateUserDefinedGroup(Name => 'Group 2');
+
+my ($ok, $msg) = $group1->AddMember($user_bar->id);
+($ok, $msg) = $group1->AddMember($user_foo->id);
+($ok, $msg) = $group2->AddMember($user_foo->id);
+($ok, $msg) = $group2->AddMember($user_quuz->id);
+
+# user search
+{
+    my $user_id = $test_user->id;
+    my $res = $mech->get("$rest_base_path/users", 'Authorization' => $auth );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+
+    my $items = delete $content->{items};
+    is_deeply( $content,
+               {
+                   'count' => 7,
+                   'total' => 7,
+                   'per_page' => 20,
+                   'page' => 1
+               });
+    is(scalar(@$items), 7);
+    foreach my $username (qw(foo bar quuz root test)) {
+        my ($item) = grep { $_->{id} eq $username } @$items;
+        like($item->{_url}, qr{$rest_base_path/user/$username$});
+        is($item->{type}, 'user');
+    }
+    ok(not grep { $_->{id} eq 'baz' } @$items);
+}
+
+
+# basic user request, own user details
+{
+    my $user_id = $test_user->id;
+    my $res = $mech->get("$rest_base_path/user/$user_id", 'Authorization' => $auth );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+    cmp_deeply(
+        $content,
+        superhashof({
+            'Privileged' => 1,
+            'Name' => 'test',
+            'Disabled' => '0',
+        }),'basic summary for own user ok'
+    );
+
+    my $links = $content->{_hyperlinks};
+    my ($history_link) = grep { $_->{ref} eq 'history' } @$links;
+    like($history_link->{_url}, qr{$rest_base_path/user/$user_id/history$});
+}
+
+# basic user request, no rights
+{
+    my $user_id = $user_foo->id;
+    my $res = $mech->get("$rest_base_path/user/$user_id",
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+    my $links = delete $content->{_hyperlinks};
+    is(scalar(@$links),1, 'only 1 link, should be self');
+    my ($self_link) = grep { $_->{ref} eq 'self' } @$links;
+    like($self_link->{_url}, qr{$rest_base_path/user/$user_id$});
+    is_deeply($content, {
+        Name     => 'foo',
+        RealName => 'Foo Jr III',
+        EmailAddress => 'test at test.test',
+        'Privileged' => 1
+    }, 'basic summary for user ok, no extra fields or hyperlinks');
+}
+
+# showuserhistory authorised User with hypermedia links
+$test_user->PrincipalObj->GrantRight(Right => 'ShowUserHistory');
+{
+    my $user_id = $user_foo->id;
+    my $res = $mech->get("$rest_base_path/user/$user_id",
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+    my $links = delete $content->{_hyperlinks};
+    is(scalar(@$links),2);
+    my @memberships_links = grep { $_->{ref} eq 'memberships' } @$links;
+    is(scalar(@memberships_links), 0);
+    my ($history_link) = grep { $_->{ref} eq 'history' } @$links;
+    like($history_link->{_url}, qr{$rest_base_path/user/$user_id/history$});
+
+    is_deeply($content, {
+        Name     => 'foo',
+        RealName => 'Foo Jr III',
+        EmailAddress => 'test at test.test',
+        'Privileged' => 1
+    }, 'basic summary for user ok, no extra fields');
+}
+
+# useradmin authorised User with hypermedia links
+$test_user->PrincipalObj->GrantRight(Right => 'AdminUsers');
+$test_user->PrincipalObj->GrantRight(Right => 'AdminGroupMembership');
+{
+    my $user_id = $user_foo->id;
+    my $res = $mech->get("$rest_base_path/user/$user_id",
+        'Authorization' => $auth,
+    );
+    is($res->code, 200);
+    my $content = $mech->json_response;
+
+    # test hyperlinks
+    my $links = delete $content->{_hyperlinks};
+    is(scalar(@$links),3);
+    my ($history_link) = grep { $_->{ref} eq 'history' } @$links;
+    like($history_link->{_url}, qr{$rest_base_path/user/$user_id/history$});
+    my @memberships_links = grep { $_->{ref} eq 'memberships' } @$links;
+    is(scalar(@memberships_links), 1);
+    like($memberships_links[0]->{_url}, qr{$rest_base_path/user/$user_id/groups$});
+    my ($self_link) = grep { $_->{ref} eq 'self' } @$links;
+    like($self_link->{_url}, qr{$rest_base_path/user/$user_id$});
+
+    cmp_deeply($content,
+        superhashof({
+            'RealName' => 'Foo Jr III',
+            'Privileged' => 1,
+            'Memberships' => [],
+            'Disabled' => '0',
+            'Name' => 'foo',
+            'EmailAddress' => 'test at test.test',
+            'CustomFields' => []
+        })
+    );
+}
+
+$test_user->PrincipalObj->RevokeRight(Right => 'ShowUserHistory');
+$test_user->PrincipalObj->RevokeRight(Right => 'AdminUsers');
+
+done_testing;

commit ff0b37d5fcccc4ab672c65e95e813d6ad8be91fe
Author: Matt Brennan <brennanma at gmail.com>
Date:   Mon Mar 25 12:17:10 2019 -0400

    Allow setting privileged flag on user endpoint
    
    Update user management call (PUT /user/:id) to support
     setting and unsetting a user as privileged

diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index acff3dc..de38a97 100644
--- a/lib/RT/Extension/REST2/Resource/Record/Writable.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
@@ -56,6 +56,8 @@ sub update_record {
     push @results, $self->_update_role_members($data);
     push @results, $self->_update_disabled($data->{Disabled})
       unless grep { $_ eq 'Disabled' } $self->record->WritableAttributes;
+    push @results, $self->_update_privileged($data->{Privileged})
+      unless grep { $_ eq 'Privileged' } $self->record->WritableAttributes;
 
     # XXX TODO: Figure out how to return success/failure?  Core RT::Record's
     # ->Update will need to be replaced or improved.
@@ -196,6 +198,22 @@ sub _update_disabled {
     return @results;
 }
 
+sub _update_privileged {
+    my $self = shift;
+    my $data = shift;
+    my @results;
+
+    my $record = $self->record;
+    return unless defined $data and $data =~ /^[01]$/;
+
+    return unless $record->can('SetPrivileged');
+
+    my ($ok, $msg) = $record->SetPrivileged($data);
+    push @results, $msg;
+
+    return @results;
+}
+
 sub update_resource {
     my $self = shift;
     my $data = shift;

commit 70462dec601086b3e3fbc1cfc0bc15a4b5b6195d
Merge: 332f0ba ff0b37d
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Fri May 1 22:34:26 2020 +0800

    Merge branch 'improve-admin-user'


-----------------------------------------------------------------------


More information about the Bps-public-commit mailing list