[Bps-public-commit] rt-extension-rest2 branch, rest2_api_improved_user_admin, created. 1.07-4-ga5869b8
Aaron Trevena
ast at bestpractical.com
Fri Apr 10 11:50:04 EDT 2020
The branch, rest2_api_improved_user_admin has been created
at a5869b833f1f836654f0843b6c72ea240dc0d7e3 (commit)
- Log -----------------------------------------------------------------
commit 0744f3d68624384cba96ddd49ac1b58d3fbb462e
Author: Emmanuel Lacour <elacour at easter-eggs.com>
Date: Mon Sep 23 10:50:49 2019 +0200
Improved access and authorisation for REST2 user endpoints
Allow any privileged user to search for and view summary of users
Restrict access to user history to users with useradmin or showuserhistory rights
diff --git a/Changes b/Changes
index c756d8c..ef0b5a8 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,7 @@
Revision history for RT-Extension-REST2
+ - Improved access and authorisation for user endpoints
+
1.07 2019-05-24
- Accept 'Content' as a parameter on create. The documentation previously showed
this in examples, but it wasn't yet supported. Now it works as documented.
diff --git a/lib/RT/Extension/REST2/Resource/Record/WithETag.pm b/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
index 9f05ad3..3b00052 100644
--- a/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
+++ b/lib/RT/Extension/REST2/Resource/Record/WithETag.pm
@@ -20,7 +20,10 @@ sub generate_etag {
if ($record->can('Transactions')) {
my $txns = $record->Transactions;
$txns->OrderByCols({ FIELD => 'id', ORDER => 'DESC' });
- return $txns->First->Id if $txns->Count;
+
+ if ($txns->Count && $txns->First) {
+ return $txns->First->Id;
+ }
}
# fall back to last-modified time, which is commonly accepted even
diff --git a/lib/RT/Extension/REST2/Resource/Transactions.pm b/lib/RT/Extension/REST2/Resource/Transactions.pm
index 3c3bfd4..aa324c5 100644
--- a/lib/RT/Extension/REST2/Resource/Transactions.pm
+++ b/lib/RT/Extension/REST2/Resource/Transactions.pm
@@ -8,6 +8,12 @@ use namespace::autoclean;
extends 'RT::Extension::REST2::Resource::Collection';
with 'RT::Extension::REST2::Resource::Collection::QueryByJSON';
+has tx_object_type => (
+ is => 'ro',
+ isa => 'ClassName',
+);
+
+
sub dispatch_rules {
Path::Dispatcher::Rule::Regex->new(
regex => qr{^/transactions/?$},
@@ -37,7 +43,7 @@ sub dispatch_rules {
}
$record->Load($id);
- return { collection => $record->Transactions };
+ return { collection => $record->Transactions, tx_object_type => ref($record) };
},
),
Path::Dispatcher::Rule::Regex->new(
@@ -55,11 +61,28 @@ sub dispatch_rules {
}
$record->Load($id);
- return { collection => $record->Transactions };
+ return { collection => $record->Transactions, tx_object_type => ref($record) };
},
)
}
+sub forbidden {
+ my $self = shift;
+ # check user history only visible to appropriate users
+ if ( $self->tx_object_type && $self->tx_object_type eq 'RT::User') {
+ return 0 if $self->current_user->HasRight(
+ Right => "AdminUsers",
+ Object => RT->System,
+ );
+ return 0 if $self->current_user->HasRight(
+ Right => 'ShowUserHistory',
+ Object => RT->System,
+ );
+ return 1;
+ }
+ return 0;
+}
+
__PACKAGE__->meta->make_immutable;
1;
diff --git a/lib/RT/Extension/REST2/Resource/User.pm b/lib/RT/Extension/REST2/Resource/User.pm
index 8a74630..21c08e1 100644
--- a/lib/RT/Extension/REST2/Resource/User.pm
+++ b/lib/RT/Extension/REST2/Resource/User.pm
@@ -35,37 +35,58 @@ sub dispatch_rules {
),
}
+
around 'serialize' => sub {
my $orig = shift;
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) }
- @{ $self->record->OwnGroups->ItemsArrayRef }
- ];
+ @{ $self->record->OwnGroups->ItemsArrayRef }
+ ];
return $data;
};
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",
- };
+ my $show_history = 0;
+ $show_history = 1 if ( $self->current_user->HasRight(
+ Right => 'ShowUserHistory',
+ Object => RT->System,
+ ));
+
+ if ( $self->record->CurrentUserHasRight("AdminUsers") or $self->record->id == $self->current_user->id) {
+ $show_history = 1;
+ my $id = $self->record->id;
+ push @$links,
+ { ref => 'memberships',
+ _url => RT::Extension::REST2->base_uri . "/user/$id/groups",
+ };
+ }
+
+ push @$links, $self->_transaction_history_link if ($show_history);
+
return $links;
}
diff --git a/lib/RT/Extension/REST2/Resource/Users.pm b/lib/RT/Extension/REST2/Resource/Users.pm
index aa6d6f9..ba9f213 100644
--- a/lib/RT/Extension/REST2/Resource/Users.pm
+++ b/lib/RT/Extension/REST2/Resource/Users.pm
@@ -15,19 +15,29 @@ 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 587ddcbc5f38b3f79d4e38e8146b1f3cc2ab3d33
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/xt/users.t b/xt/users.t
new file mode 100644
index 0000000..66aebd4
--- /dev/null
+++ b/xt/users.t
@@ -0,0 +1,167 @@
+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 ($membership_link) = grep { $_->{ref} eq 'memberships' } @$links;
+ like($membership_link->{_url}, qr{$rest_base_path/user/$user_id/groups$});
+ 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');
+{
+ 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 a5869b833f1f836654f0843b6c72ea240dc0d7e3
Author: Matt Brennan <brennanma at gmail.com>
Date: Mon Mar 25 12:17:10 2019 -0400
Allow setting privileged flag on user in rest2 user endpoint
Update user management call (PUT /user/:id) to support
setting and unsetting a user as privileged
Based on public github PR #30
diff --git a/Changes b/Changes
index ef0b5a8..68c10d1 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,6 @@
Revision history for RT-Extension-REST2
+ - Allow setting privileged flag on user in rest2 user endpoint (Thanks Matt Brennan!)
- Improved access and authorisation for user endpoints
1.07 2019-05-24
diff --git a/lib/RT/Extension/REST2/Resource/Record/Writable.pm b/lib/RT/Extension/REST2/Resource/Record/Writable.pm
index 6d04d3b..6ed3092 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.
@@ -266,6 +268,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;
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list