From 8206aeaec5656bab36b6b9bdd25d5f93dc4187c0 Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Tue, 16 Feb 2010 10:42:56 -0600 Subject: [PATCH] enhance format of .wgaccess files to provide security for Gallery files --- lib/WebGUI/Asset/File.pm | 11 +++- lib/WebGUI/Asset/File/GalleryFile.pm | 5 ++ lib/WebGUI/Storage.pm | 45 +++++++++++++-- lib/WebGUI/URL/Uploads.pm | 82 +++++++++++++++++----------- t/Storage.t | 20 +++++-- 5 files changed, 121 insertions(+), 42 deletions(-) diff --git a/lib/WebGUI/Asset/File.pm b/lib/WebGUI/Asset/File.pm index 6ffaf9bc1..d1eca73bd 100644 --- a/lib/WebGUI/Asset/File.pm +++ b/lib/WebGUI/Asset/File.pm @@ -78,10 +78,19 @@ A hash reference of optional parameters. None at this time. sub applyConstraints { my $self = shift; - $self->getStorageLocation->setPrivileges($self->get('ownerUserId'), $self->get('groupIdView'), $self->get('groupIdEdit')); + $self->setPrivileges; $self->setSize; } +sub setPrivileges { + my $self = shift; + $self->getStorageLocation->setPrivileges( + $self->get('ownerUserId'), + $self->get('groupIdView'), + $self->get('groupIdEdit'), + ); +} + #------------------------------------------------------------------- diff --git a/lib/WebGUI/Asset/File/GalleryFile.pm b/lib/WebGUI/Asset/File/GalleryFile.pm index 0c1abe0f0..835fa3281 100644 --- a/lib/WebGUI/Asset/File/GalleryFile.pm +++ b/lib/WebGUI/Asset/File/GalleryFile.pm @@ -1196,5 +1196,10 @@ sub www_view { return "chunked"; } +sub setPrivileges { + my $self = shift; + $self->getStorageLocation->setPrivileges($self); +} + 1; # Who knew the truth would be so obvious? diff --git a/lib/WebGUI/Storage.pm b/lib/WebGUI/Storage.pm index 3303228ed..91a5e0f84 100644 --- a/lib/WebGUI/Storage.pm +++ b/lib/WebGUI/Storage.pm @@ -26,6 +26,7 @@ use Image::Magick; use Path::Class::Dir; use Storable (); use WebGUI::Utility qw(isIn); +use JSON (); =head1 NAME @@ -1665,10 +1666,42 @@ The groupId that is allowed to edit the files in this storage location. =cut sub setPrivileges { - my $self = shift; - my $owner = shift; - my $viewGroup = shift; - my $editGroup = shift; + my $self = shift; + my %privs = ( + users => [], + groups => [], + assets => [], + ); + if (@_ == 3 && !ref $_[0] && !ref $_[1] && !ref $_[0]) { + push @{ $privs{users} }, $_[0]; + push @{ $privs{groups} }, @_[1,2]; + } + else { + for my $object (@_) { + if ($object->isa('WebGUI::User')) { + push @{ $privs{users} }, $object->getId; + } + elsif ($object->isa('WebGUI::Group')) { + push @{ $privs{groups} }, $object->getId; + } + elsif ($object->isa('WebGUI::Asset')) { + push @{ $privs{assets} }, $object->getId; + } + } + } + + my $public; + for my $user (@{ $privs{users} }) { + if ($user eq '1') { + $public = 1; + } + } + for my $group (@{ $privs{groups} }) { + if ($group eq '1' || $group eq '7') { + $public = 1; + } + } + my $accessFile = JSON->new->encode( \%privs ); my $dirObj = $self->getPathClassDir(); return undef if ! defined $dirObj; @@ -1678,11 +1711,11 @@ sub setPrivileges { return unless $obj->is_dir; my $rel = $obj->relative($dirObj); - if ($owner eq '1' || $viewGroup eq '1' || $viewGroup eq '7' || $editGroup eq '1' || $editGroup eq '7') { + if ($public) { $self->deleteFile($rel->file('.wgaccess')->stringify); } else { - $self->addFileFromScalar($rel->file('.wgaccess')->stringify,$owner."\n".$viewGroup."\n".$editGroup); + $self->addFileFromScalar($rel->file('.wgaccess')->stringify, $accessFile); } } ); diff --git a/lib/WebGUI/URL/Uploads.pm b/lib/WebGUI/URL/Uploads.pm index 36ca8470a..aa6138bbd 100644 --- a/lib/WebGUI/URL/Uploads.pm +++ b/lib/WebGUI/URL/Uploads.pm @@ -47,38 +47,58 @@ The Apache request handler for this package. sub handler { my ($request, $server, $config) = @_; - $request->push_handlers(PerlAccessHandler => sub { - if (-e $request->filename) { - my $path = $request->filename; - $path =~ s/^(\/.*\/).*$/$1/; - if (-e $path.".wgaccess") { - my $fileContents; - open(my $FILE, "<" ,$path.".wgaccess"); - while (my $line = <$FILE>) { - $fileContents .= $line; - } - close($FILE); - my @privs = split("\n", $fileContents); - unless ($privs[1] eq "7" || $privs[1] eq "1") { - my $session = $request->pnotes('wgSession'); - unless (defined $session) { - $session = WebGUI::Session->open($server->dir_config('WebguiRoot'), $config->getFilename, $request, $server); - } - my $hasPrivs = ($session->var->get("userId") eq $privs[0] || $session->user->isInGroup($privs[1]) || $session->user->isInGroup($privs[2])); - $session->close(); - if ($hasPrivs) { - return Apache2::Const::OK; - } - else { - return Apache2::Const::AUTH_REQUIRED; - } - } - } - return Apache2::Const::OK; - } + $request->push_handlers(PerlAccessHandler => sub { + my $path = $request->filename; + return Apache2::Const::NOT_FOUND + unless -e $path; + $path =~ s{[^/]*$}{}; + return Apache2::Const::OK + unless -e $path . '.wgaccess'; + + my $fileContents; + open my $FILE, '<' , $path . '.wgaccess'; + my $fileContents = do { local $/; <$FILE> }; + close($FILE); + my @users; + my @groups; + my @assets; + if ($fileContents =~ /\A(?:\d+|[A-Za-z0-9_-]{22})\n(?:\d+|[A-Za-z0-9_-]{22})\n(?:\d+|[A-Za-z0-9_-]{22})/) { + my @privs = split("\n", $fileContents); + push @users, $privs[0]; + push @groups, @privs[1,2]; + } else { - return Apache2::Const::NOT_FOUND; - } + my $privs = JSON->new->decode($fileContents); + @users = @{ $privs->{users} }; + @groups = @{ $privs->{groups} }; + @assets = @{ $privs->{assets} }; + } + + return Apache2::Const::OK + if grep { $_ eq '1' } @users; + + return Apache2::Const::OK + if grep { $_ eq '1' || $_ eq '7' } @groups; + + my $session = $request->pnotes('wgSession'); + unless (defined $session) { + $session = WebGUI::Session->open($server->dir_config('WebguiRoot'), $config->getFilename, $request, $server); + } + + my $userId = $session->var->get('userId'); + + return Apache2::Const::OK + if grep { $_ eq $userId } @users; + + my $user = $session->user; + + return Apache2::Const::OK + if grep { $user->isInGroup($_) } @groups; + + return Apache2::Const::OK + if grep { WebGUI::Asset->new($session, $_)->canView } @assets; + + return Apache2::Const::AUTH_REQUIRED; } ); return Apache2::Const::OK; } diff --git a/t/Storage.t b/t/Storage.t index 5264622a1..37c726f9a 100644 --- a/t/Storage.t +++ b/t/Storage.t @@ -32,7 +32,7 @@ my $cwd = Cwd::cwd(); my ($extensionTests, $fileIconTests) = setupDataDrivenTests($session); -my $numTests = 134; # increment this value for each test you create +my $numTests = 136; # increment this value for each test you create plan tests => $numTests + scalar @{ $extensionTests } + scalar @{ $fileIconTests }; my $uploadDir = $session->config->get('uploadsPath'); @@ -508,7 +508,7 @@ my $shallowDir = $shallowStorage->getPathClassDir(); ok(-e $shallowDir->file('.wgaccess')->stringify, 'setPrivilege: .wgaccess file created in shallow storage'); my $privs; $privs = $shallowStorage->getFileContentsAsScalar('.wgaccess'); -is ($privs, "3\n3\n3", '... correct group contents'); +is ($privs, '{"assets":[],"groups":["3","3"],"users":["3"]}', '... correct group contents'); $shallowStorage->deleteFile('.wgaccess'); my $deepStorage = WebGUI::Storage->create($session); @@ -524,9 +524,21 @@ ok(-e $deepDir->file('.wgaccess')->stringify, '.wgaccess file created in dee ok(-e $deepDeepDir->file('.wgaccess')->stringify, '.wgaccess file created in deep storage subdir'); $privs = $deepStorage->getFileContentsAsScalar('.wgaccess'); -is ($privs, "3\n3\n3", '... correct group contents, deep storage'); +is ($privs, '{"assets":[],"groups":["3","3"],"users":["3"]}', '... correct group contents, deep storage'); $privs = $deepStorage->getFileContentsAsScalar('deep/.wgaccess'); -is ($privs, "3\n3\n3", '... correct group contents, deep storage subdir'); +is ($privs, '{"assets":[],"groups":["3","3"],"users":["3"]}', '... correct group contents, deep storage subdir'); + +{ + my $storage = WebGUI::Storage->create($session); + addToCleanup($storage); + my $asset = WebGUI::Asset->getRoot($session); + $storage->setPrivileges( $asset ); + my $accessFile = $storage->getPathClassDir->file('.wgaccess'); + ok(-e $accessFile, 'setPrivilege: .wgaccess file created for asset permissions'); + my $privs = $accessFile->slurp; + is ($privs, '{"assets":["' . $asset->getId . '"],"groups":[],"users":[]}', '... correct asset contents'); +} + #################################################### #