Skip to content

Commit

Permalink
Merge commit 'refs/changes/79/25979/8' of ssh://gerrit.tuleap.net:294…
Browse files Browse the repository at this point in the history
…18/tuleap into HEAD

* ssh://gerrit.tuleap.net:29418/tuleap:
  request #26816 Resources of private projects can be accessed by non project members
  • Loading branch information
nterray committed Jun 6, 2022
2 parents b5d8e04 + 7e221a9 commit cc38bcc
Show file tree
Hide file tree
Showing 26 changed files with 384 additions and 54 deletions.
Expand Up @@ -489,7 +489,7 @@ public function testManipulateFeature(): void
$this->updateArtifactLinks($program_increment['id'], [], $program_increment['artifact_link_id']);

// US1 is linked in top backlog (linked into sprint), US2 is no longer present
$this->checkLinksArePresentInReleaseTopBacklog($team_id, [$user_story1['id']]);
$this->checkLinksArePresentInReleaseTopBacklog($release_mirror['id'], [$user_story1['id']]);
}

/**
Expand Down
12 changes: 10 additions & 2 deletions site-content/fr_FR/LC_MESSAGES/tuleap-core.po
Expand Up @@ -2657,8 +2657,8 @@ msgid "Project"
msgstr "Projet"

#, php-format
msgid "Project #%d is not active and is not a template"
msgstr "Le project #%d n'est pas actif et n'est pas un modèle"
msgid "Project #%d is not active."
msgstr "Le project #%d n'est pas actif."

msgid "Project (read only)"
msgstr "Projet (lecture seule)"
Expand Down Expand Up @@ -3470,6 +3470,14 @@ msgstr "Tabulation"
msgid "Template #%d is not valid"
msgstr "Le modèle #%d n'est pas valide"

#, php-format
msgid ""
"Template %d is not valid: you don't have the permission to the access the "
"project."
msgstr ""
"Le modèle %d n'est pas valide : vous n'avez pas la permission d'accéder au "
"projet."

msgid "Template used by project"
msgstr "Modèle utilisé par ce projet"

Expand Down
10 changes: 8 additions & 2 deletions site-content/pt_BR/LC_MESSAGES/tuleap-core.po
Expand Up @@ -2593,8 +2593,8 @@ msgid "Project"
msgstr "Projeto"

#, php-format
msgid "Project #%d is not active and is not a template"
msgstr "O projeto #%d não está ativo e não é um modelo"
msgid "Project #%d is not active."
msgstr "O projeto #%d não está ativo."

msgid "Project (read only)"
msgstr "Projeto (somente leitura)"
Expand Down Expand Up @@ -3385,6 +3385,12 @@ msgstr ""
msgid "Template #%d is not valid"
msgstr "O modelo #%d não é válido"

#, php-format
msgid ""
"Template %d is not valid: you don't have the permission to the access the "
"project."
msgstr ""

msgid "Template used by project"
msgstr "Modelo utilizado pelo projeto"

Expand Down
Expand Up @@ -37,6 +37,7 @@
use Tuleap\Project\Registration\Template\TemplateFactory;
use Tuleap\Project\Registration\Template\TemplateFromProjectForCreation;
use Tuleap\Project\XML\XMLFileContentRetriever;
use URLVerification;
use XML_RNGValidator;

class ProjectCreationDataPOSTProjectBuilder
Expand All @@ -55,6 +56,7 @@ public function __construct(
ServiceManager $service_manager,
ProjectCreationDataServiceFromXmlInheritor $from_xml_inheritor,
LoggerInterface $logger,
private URLVerification $url_verification,
) {
$this->project_manager = $project_manager;
$this->template_factory = $template_factory;
Expand Down Expand Up @@ -93,7 +95,8 @@ public function buildProjectCreationDataFromPOSTRepresentation(
TemplateFromProjectForCreation::fromRESTRepresentation(
$post_representation,
$user,
$this->project_manager
$this->project_manager,
$this->url_verification
),
$data
);
Expand Down
12 changes: 9 additions & 3 deletions src/common/Project/REST/v1/ProjectResource.class.php
Expand Up @@ -282,7 +282,8 @@ protected function post(ProjectPostRepresentation $post_representation, bool $dr
new ProjectCreationDataServiceFromXmlInheritor(
ServiceManager::instance(),
),
$this->getBackendLogger()
$this->getBackendLogger(),
$this->getURLVerification()
);

$creation_data = $creation_data_post_project_builder->buildProjectCreationDataFromPOSTRepresentation(
Expand Down Expand Up @@ -603,7 +604,7 @@ private function getProjectForUser($id)
$project = $this->project_manager->getProject($id);
$user = $this->user_manager->getCurrentUser();

ProjectAuthorization::userCanAccessProject($user, $project, new URLVerification());
ProjectAuthorization::userCanAccessProject($user, $project, $this->getURLVerification());

return $project;
}
Expand Down Expand Up @@ -909,7 +910,7 @@ private function userCanSeeUserGroups($project_id)
{
$project = $this->project_manager->getProject($project_id);
$user = $this->user_manager->getCurrentUser();
ProjectAuthorization::userCanAccessProject($user, $project, new URLVerification());
ProjectAuthorization::userCanAccessProject($user, $project, $this->getURLVerification());

return true;
}
Expand Down Expand Up @@ -1434,4 +1435,9 @@ private function getAdditionalFields(Project $project): array

return $project_field_representations;
}

private function getURLVerification(): URLVerification
{
return new URLVerification();
}
}
@@ -0,0 +1,37 @@
<?php
/**
* Copyright (c) Enalean, 2022-Present. All Rights Reserved.
*
* This file is a part of Tuleap.
*
* Tuleap is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* Tuleap is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with Tuleap. If not, see <http://www.gnu.org/licenses/>.
*/

namespace Tuleap\Project\Registration\Template;

use Project;
use Project_Creation_Exception;

final class InsufficientPermissionToUseCompanyTemplateException extends Project_Creation_Exception implements InvalidTemplateException
{
public function __construct(private Project $project)
{
parent::__construct(sprintf(_("Template %d is not valid: you don't have the permission to the access the project."), $this->project->getId()));
}

public function getI18NMessage(): string
{
return $this->message;
}
}
Expand Up @@ -31,12 +31,12 @@ final class ProjectTemplateNotActiveException extends Project_Creation_Exception

public function __construct(Project $project)
{
parent::__construct('Project #' . $project->getID() . ' is not active and is not a template');
parent::__construct('Project #' . $project->getID() . ' is not active');
$this->project = $project;
}

public function getI18NMessage(): string
{
return sprintf(_('Project #%d is not active and is not a template'), $this->project->getID());
return sprintf(_('Project #%d is not active.'), $this->project->getID());
}
}
29 changes: 23 additions & 6 deletions src/common/Project/Registration/Template/TemplateFactory.php
Expand Up @@ -23,6 +23,8 @@

namespace Tuleap\Project\Registration\Template;

use Project;
use Project_AccessException;
use ProjectManager;
use Psr\EventDispatcher\EventDispatcherInterface;
use Tuleap\Glyph\GlyphFinder;
Expand All @@ -31,6 +33,8 @@
use Tuleap\Project\XML\ServiceEnableForXmlImportRetriever;
use Tuleap\Project\XML\XMLFileContentRetriever;
use Tuleap\XML\ProjectXMLMerger;
use URLVerification;
use UserManager;

class TemplateFactory
{
Expand Down Expand Up @@ -63,14 +67,16 @@ public function __construct(
TemplateDao $template_dao,
ProjectManager $project_manager,
EventDispatcherInterface $event_dispatcher,
private UserManager $user_manager,
private URLVerification $url_verification,
) {
$this->template_dao = $template_dao;
$this->templates = [
AgileALMTemplate::NAME => new AgileALMTemplate($glyph_finder, $project_xml_merger, $consistency_checker),
ScrumTemplate::NAME => new ScrumTemplate($glyph_finder, $project_xml_merger, $consistency_checker),
KanbanTemplate::NAME => new KanbanTemplate($glyph_finder, $project_xml_merger, $consistency_checker),
IssuesTemplate::NAME => new IssuesTemplate($glyph_finder, $consistency_checker, $event_dispatcher),
EmptyTemplate::NAME => new EmptyTemplate($glyph_finder),
ScrumTemplate::NAME => new ScrumTemplate($glyph_finder, $project_xml_merger, $consistency_checker),
KanbanTemplate::NAME => new KanbanTemplate($glyph_finder, $project_xml_merger, $consistency_checker),
IssuesTemplate::NAME => new IssuesTemplate($glyph_finder, $consistency_checker, $event_dispatcher),
EmptyTemplate::NAME => new EmptyTemplate($glyph_finder),
];
$this->project_manager = $project_manager;
$this->glyph_finder = $glyph_finder;
Expand All @@ -96,7 +102,9 @@ public static function build(): self
),
new TemplateDao(),
\ProjectManager::instance(),
$event_manager
$event_manager,
\UserManager::instance(),
new URLVerification()
);
}

Expand Down Expand Up @@ -166,7 +174,7 @@ public function getCompanyTemplateList(): array
$project_templates = $this->project_manager->getSiteTemplates();

foreach ($project_templates as $project_template) {
if ((int) $project_template->getGroupId() === \Project::ADMIN_PROJECT_ID) {
if ((int) $project_template->getGroupId() === \Project::ADMIN_PROJECT_ID || ! $this->userCanAccessTemplate($project_template)) {
continue;
}
$company_templates[] = new CompanyTemplate($project_template, $this->glyph_finder);
Expand Down Expand Up @@ -198,4 +206,13 @@ private static function getExternalTemplatesByName(EventDispatcherInterface $eve

return $templates_by_name;
}

private function userCanAccessTemplate(Project $project_template): bool
{
try {
return $this->url_verification->userCanAccessProject($this->user_manager->getCurrentUser(), $project_template);
} catch (Project_AccessException $exception) {
return false;
}
}
}
Expand Up @@ -22,10 +22,12 @@

namespace Tuleap\Project\Registration\Template;

use Exception;
use PFUser;
use Project;
use ProjectManager;
use Tuleap\Project\REST\v1\ProjectPostRepresentation;
use URLVerification;

class TemplateFromProjectForCreation
{
Expand All @@ -45,9 +47,14 @@ private function __construct(Project $project)
* @throws ProjectIDTemplateNotProvidedException
* @throws ProjectTemplateIDInvalidException
* @throws ProjectTemplateNotActiveException
* @throws InsufficientPermissionToUseCompanyTemplateException
*/
private static function fromData(ProjectManager $project_manager, PFUser $user_requesting_creation, ?int $project_id): self
{
private static function fromData(
ProjectManager $project_manager,
PFUser $user_requesting_creation,
?int $project_id,
URLVerification $url_verification,
): self {
if ($project_id === null) {
throw new ProjectIDTemplateNotProvidedException();
}
Expand All @@ -58,13 +65,11 @@ private static function fromData(ProjectManager $project_manager, PFUser $user_r
throw new ProjectTemplateIDInvalidException($project_id);
}

if (! $project->isActive() && ! $project->isTemplate()) {
if (! self::doesProjectStatusAllowUsageAsTemplate($project)) {
throw new ProjectTemplateNotActiveException($project);
}

if (! $project->isTemplate() && ! $user_requesting_creation->isAdmin($project->getID())) {
throw new InsufficientPermissionToUseProjectAsTemplateException($project, $user_requesting_creation);
}
self::checkIfUserHasPermissionsToUseTemplate($project, $url_verification, $user_requesting_creation);

return new self($project);
}
Expand All @@ -79,15 +84,45 @@ public static function fromRESTRepresentation(
ProjectPostRepresentation $representation,
PFUser $user_requesting_creation,
ProjectManager $project_manager,
URLVerification $url_verification,
): self {
return self::fromData($project_manager, $user_requesting_creation, $representation->template_id);
return self::fromData(
$project_manager,
$user_requesting_creation,
$representation->template_id,
$url_verification
);
}

public static function fromGlobalProjectAdminTemplate(): self
{
return new self(new Project(['group_id' => Project::ADMIN_PROJECT_ID, 'status' => Project::STATUS_SYSTEM]));
}

private static function doesProjectStatusAllowUsageAsTemplate(Project $project): bool
{
return $project->isActive() || $project->isSystem();
}

/**
* @throws InsufficientPermissionToUseCompanyTemplateException
* @throws InsufficientPermissionToUseProjectAsTemplateException
*/
private static function checkIfUserHasPermissionsToUseTemplate(
Project $project,
URLVerification $url_verification,
PFUser $user_requesting_creation,
): void {
if ($project->isTemplate()) {
try {
$url_verification->userCanAccessProject($user_requesting_creation, $project);
} catch (Exception $exception) {
throw new InsufficientPermissionToUseCompanyTemplateException($project);
}
} elseif (! $user_requesting_creation->isAdmin($project->getID())) {
throw new InsufficientPermissionToUseProjectAsTemplateException($project, $user_requesting_creation);
}
}

public function getProject(): Project
{
Expand Down
2 changes: 2 additions & 0 deletions tests/lib/TestDataBuilder.php
Expand Up @@ -72,6 +72,8 @@ class TestDataBuilder // phpcs:ignore PSR1.Classes.ClassDeclaration.MissingNames
public const PROJECT_SERVICES = 'project-services';
public const PROJECT_PUBLIC_WITH_MEMBERSHIP_SHORTNAME = 'public-sync-project-member';
public const PROJECT_FUTURE_RELEASES = 'current-future-releases';
public const PROJECT_PUBLIC_TEMPLATE = 'public-template';
public const PROJECT_PRIVATE_TEMPLATE = 'private-template';

public const STATIC_UGROUP_1_ID = 101;
public const STATIC_UGROUP_1_LABEL = 'static_ugroup_1';
Expand Down
7 changes: 7 additions & 0 deletions tests/rest/_fixtures/16-public-template/project.xml
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<project
unix-name="public-template" full-name="Public Template" description="For template test" access="public">
<long-description>A public template test project</long-description>
<services>
</services>
</project>
1 change: 1 addition & 0 deletions tests/rest/_fixtures/16-public-template/user_map.csv
@@ -0,0 +1 @@
name,action,comment
3 changes: 3 additions & 0 deletions tests/rest/_fixtures/16-public-template/users.xml
@@ -0,0 +1,3 @@
<?xml version="1.0" encoding="UTF-8"?>
<users>
</users>
20 changes: 20 additions & 0 deletions tests/rest/_fixtures/17-private-template/project.xml
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
<project
unix-name="private-template" full-name="Private Template" description="For template test" access="private">
<long-description>A private template test project</long-description>
<ugroups>
<ugroup name="project_members" description="">
<members>
<member format="username">rest_api_tester_3</member>
<member format="username">rest_api_tester_5</member>
</members>
</ugroup>
<ugroup name="project_admins" description="">
<members>
<member format="username">rest_api_tester_3</member>
</members>
</ugroup>
</ugroups>
<services>
</services>
</project>
1 change: 1 addition & 0 deletions tests/rest/_fixtures/17-private-template/user_map.csv
@@ -0,0 +1 @@
name,action,comment

0 comments on commit cc38bcc

Please sign in to comment.