Release 2.58 (#5952)
* bump version * fix formatting locale reset after embedded controller sub-requests (#5944) * fix GHSA-c6w6-57jj-62vh * fix GHSA-m492-gv72-xvxj * fix GHSA-jr9p-4h4j-6c58 * make sure to only use JS logic to call API endpoints * fixes GHSA-r8vr-m544-qh4h * make sure to only use JS logic to call API endpoints * fix GHSA-rw46-qg69-vg6h * fix GHSA-pj8j-p4g4-4vw8 - prevent kimai from rendering images via markdown * fix GHSA-pj8j-p4g4-4vw8 - use a safe network client to prevent SSRF via images * fix GHSA-xv4r-4885-gwpg * fix GHSA-pgcc-vfmc-7cw5 - move GET routes to API with POST method to prevent CSRF * fix tooltip survives page reload * updated wizard images * split wizard and password reset subscriber into two classes * relax upper php limit * added zizmor workflow scans and apply findings * user permissions <name>_other_profile now respect teams * move all linting steps to new job * updated docker image version names * use .env.local for storing APP_SECRET * improve build order and use given tag as ref for checkout, not default main branch * improved APP_SECRET handling, see entrypoint.sh * use local code for building the image for more flexibility, added dockerignore
This commit is contained in:
Kevin Papst
@@ -192,19 +192,6 @@ class TimesheetVoterTest extends AbstractVoterTestCase
|
||||
$this->assertVote($other, $timesheet, 'is_owner', VoterInterface::ACCESS_DENIED);
|
||||
}
|
||||
|
||||
public function testIsOwnerUsesObjectIdentityNotId(): void
|
||||
{
|
||||
// The is_owner branch compares with strict identity ($user === $subject->getUser()),
|
||||
// not by id like the permission-based branches. Two distinct User instances that
|
||||
// share the same id are therefore NOT considered the same owner.
|
||||
$tokenUser = self::getUser(1, User::ROLE_USER);
|
||||
$timesheetUser = self::getUser(1, User::ROLE_USER);
|
||||
|
||||
$timesheet = self::getTimesheet($timesheetUser);
|
||||
|
||||
$this->assertVote($tokenUser, $timesheet, 'is_owner', VoterInterface::ACCESS_DENIED);
|
||||
}
|
||||
|
||||
public function testIsOwnerDeniedWhenTimesheetHasNoUser(): void
|
||||
{
|
||||
$user = self::getUser(1, User::ROLE_USER);
|
||||
@@ -653,6 +640,99 @@ class TimesheetVoterTest extends AbstractVoterTestCase
|
||||
$this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reproduces GHSA-c6w6-57jj-62vh.
|
||||
*
|
||||
* After a user loses team access to a project, "restart" (start) and
|
||||
* "duplicate" must NOT be allowed on one of their own historical timesheets:
|
||||
* both operations derive a brand-new record from the old entry and would
|
||||
* therefore create a new write under the now-unauthorized project/activity.
|
||||
*
|
||||
* The "_own_timesheet" branch in the voter currently short-circuits before
|
||||
* checkTeamAccess*() runs, so this test FAILS on vulnerable code (the voter
|
||||
* returns ACCESS_GRANTED) and documents the expected secure behaviour.
|
||||
*/
|
||||
public function testStartAndDuplicateDeniedAfterProjectAccessRevoked(): void
|
||||
{
|
||||
$owner = self::getUser(1, User::ROLE_USER);
|
||||
|
||||
// The project is now restricted to a team the owner is NOT a member of.
|
||||
// This is the post-revocation state from the advisory's PoC.
|
||||
$restrictedTeam = new Team('restricted after revocation');
|
||||
|
||||
$customer = new Customer('Acme');
|
||||
$project = new Project();
|
||||
$project->setCustomer($customer);
|
||||
$project->addTeam($restrictedTeam);
|
||||
|
||||
$activity = new Activity();
|
||||
$activity->setProject($project);
|
||||
|
||||
$timesheet = new Timesheet();
|
||||
$timesheet->setUser($owner);
|
||||
$timesheet->setProject($project);
|
||||
$timesheet->setActivity($activity);
|
||||
|
||||
$this->assertVote($owner, $timesheet, 'start', VoterInterface::ACCESS_DENIED);
|
||||
$this->assertVote($owner, $timesheet, 'duplicate', VoterInterface::ACCESS_DENIED);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reproduces GHSA-c6w6-57jj-62vh for activity-level restriction.
|
||||
*
|
||||
* Even if the project is unrestricted, a restricted activity (a team the
|
||||
* owner is no longer in) must block restart/duplicate.
|
||||
*/
|
||||
public function testStartAndDuplicateDeniedAfterActivityAccessRevoked(): void
|
||||
{
|
||||
$owner = self::getUser(1, User::ROLE_USER);
|
||||
|
||||
$customer = new Customer('Acme');
|
||||
$project = new Project();
|
||||
$project->setCustomer($customer);
|
||||
|
||||
$activity = new Activity();
|
||||
$activity->setProject($project);
|
||||
$activity->addTeam(new Team('restricted after revocation'));
|
||||
|
||||
$timesheet = new Timesheet();
|
||||
$timesheet->setUser($owner);
|
||||
$timesheet->setProject($project);
|
||||
$timesheet->setActivity($activity);
|
||||
|
||||
$this->assertVote($owner, $timesheet, 'start', VoterInterface::ACCESS_DENIED);
|
||||
$this->assertVote($owner, $timesheet, 'duplicate', VoterInterface::ACCESS_DENIED);
|
||||
}
|
||||
|
||||
/**
|
||||
* Positive control for GHSA-c6w6-57jj-62vh: when the owner still has team
|
||||
* access to the project and activity, restart/duplicate stay allowed.
|
||||
* Guards the fix against over-restriction.
|
||||
*/
|
||||
public function testStartAndDuplicateGrantedWhenOwnerStillHasProjectAccess(): void
|
||||
{
|
||||
$owner = self::getUser(1, User::ROLE_USER);
|
||||
|
||||
$team = new Team('still a member');
|
||||
$team->addUser($owner);
|
||||
|
||||
$customer = new Customer('Acme');
|
||||
$project = new Project();
|
||||
$project->setCustomer($customer);
|
||||
$project->addTeam($team);
|
||||
|
||||
$activity = new Activity();
|
||||
$activity->setProject($project);
|
||||
|
||||
$timesheet = new Timesheet();
|
||||
$timesheet->setUser($owner);
|
||||
$timesheet->setProject($project);
|
||||
$timesheet->setActivity($activity);
|
||||
|
||||
$this->assertVote($owner, $timesheet, 'start', VoterInterface::ACCESS_GRANTED);
|
||||
$this->assertVote($owner, $timesheet, 'duplicate', VoterInterface::ACCESS_GRANTED);
|
||||
}
|
||||
|
||||
private static function getTimesheetFor(User $owner, ?Team $customerTeam = null, ?Team $projectTeam = null, ?Team $activityTeam = null): Timesheet
|
||||
{
|
||||
$customer = new Customer('Acme');
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
namespace App\Tests\Voter;
|
||||
|
||||
use App\Entity\InvoiceTemplate;
|
||||
use App\Entity\Team;
|
||||
use App\Entity\User;
|
||||
use App\Voter\UserVoter;
|
||||
use PHPUnit\Framework\Attributes\CoversClass;
|
||||
@@ -119,4 +120,137 @@ class UserVoterTest extends AbstractVoterTestCase
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $sut->vote($token, $user, ['view_team_member']));
|
||||
self::assertEquals(VoterInterface::ACCESS_DENIED, $sut->vote($token, $userMock, ['view_team_member']));
|
||||
}
|
||||
|
||||
/**
|
||||
* Even with the "<attribute>_other_profile" role permission, access to another user's
|
||||
* profile must additionally pass the team-membership check in
|
||||
* RolePermissionManager::checkUserAccess().
|
||||
*/
|
||||
public function testOtherProfileRequiresTeamRelation(): void
|
||||
{
|
||||
$teamlead = self::getUser(10, User::ROLE_TEAMLEAD);
|
||||
$teamlead->setEnabled(true);
|
||||
$foreignUser = self::getUser(11, User::ROLE_USER);
|
||||
$foreignUser->setEnabled(true);
|
||||
|
||||
// give the foreign user a team that the teamlead is NOT part of,
|
||||
// so the special "subject has no teams" fallback does not kick in
|
||||
$team = new Team('foreign team');
|
||||
$foreignUser->addTeam($team);
|
||||
|
||||
$permissions = [
|
||||
'ROLE_TEAMLEAD' => ['view_other_profile', 'edit_other_profile'],
|
||||
];
|
||||
$rpm = $this->getRolePermissionManager($permissions, true);
|
||||
$voter = new UserVoter($rpm);
|
||||
|
||||
$token = new UsernamePasswordToken($teamlead, 'bar', $teamlead->getRoles());
|
||||
|
||||
// the role permission "view_other_profile" exists, but the team relation is missing
|
||||
self::assertEquals(VoterInterface::ACCESS_DENIED, $voter->vote($token, $foreignUser, ['view']));
|
||||
self::assertEquals(VoterInterface::ACCESS_DENIED, $voter->vote($token, $foreignUser, ['edit']));
|
||||
}
|
||||
|
||||
/**
|
||||
* Same setup as above, but the current user IS teamlead of one of the subject's
|
||||
* teams — access should then be granted.
|
||||
*/
|
||||
public function testOtherProfileGrantedWhenUserIsTeamleadOfSubject(): void
|
||||
{
|
||||
$teamlead = self::getUser(20, User::ROLE_TEAMLEAD);
|
||||
$teamlead->setEnabled(true);
|
||||
$member = self::getUser(21, User::ROLE_USER);
|
||||
$member->setEnabled(true);
|
||||
|
||||
$team = new Team('shared team');
|
||||
$team->addUser($member);
|
||||
$team->addTeamlead($teamlead);
|
||||
|
||||
$permissions = [
|
||||
'ROLE_TEAMLEAD' => ['view_other_profile', 'edit_other_profile'],
|
||||
];
|
||||
$rpm = $this->getRolePermissionManager($permissions, true);
|
||||
$voter = new UserVoter($rpm);
|
||||
|
||||
$token = new UsernamePasswordToken($teamlead, 'bar', $teamlead->getRoles());
|
||||
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $member, ['view']));
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $member, ['edit']));
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify that disabled profiles can still be edited (e.g. to reactivate them or check historic data).
|
||||
*/
|
||||
public function testOtherProfileAllowedForDisabledSubject(): void
|
||||
{
|
||||
$teamlead = self::getUser(30, User::ROLE_TEAMLEAD);
|
||||
$teamlead->setEnabled(true);
|
||||
$member = self::getUser(31, User::ROLE_USER);
|
||||
$member->setEnabled(false);
|
||||
|
||||
$team = new Team('shared team');
|
||||
$team->addUser($member);
|
||||
$team->addTeamlead($teamlead);
|
||||
|
||||
$permissions = [
|
||||
'ROLE_TEAMLEAD' => ['view_other_profile', 'edit_other_profile'],
|
||||
];
|
||||
$rpm = $this->getRolePermissionManager($permissions, true);
|
||||
$voter = new UserVoter($rpm);
|
||||
|
||||
$token = new UsernamePasswordToken($teamlead, 'bar', $teamlead->getRoles());
|
||||
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $member, ['view']));
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $member, ['edit']));
|
||||
}
|
||||
|
||||
/**
|
||||
* Special case in checkUserAccess(): if the subject has no teams at all and the
|
||||
* current user is a teamlead/admin, access is granted (small-installation case).
|
||||
*/
|
||||
public function testOtherProfileGrantedForTeamlessSubjectWhenUserIsTeamlead(): void
|
||||
{
|
||||
$teamlead = self::getUser(40, User::ROLE_TEAMLEAD);
|
||||
$teamlead->setEnabled(true);
|
||||
$lonelyUser = self::getUser(41, User::ROLE_USER);
|
||||
$lonelyUser->setEnabled(true);
|
||||
|
||||
$permissions = [
|
||||
'ROLE_TEAMLEAD' => ['view_other_profile', 'edit_other_profile'],
|
||||
];
|
||||
$rpm = $this->getRolePermissionManager($permissions, true);
|
||||
$voter = new UserVoter($rpm);
|
||||
|
||||
$token = new UsernamePasswordToken($teamlead, 'bar', $teamlead->getRoles());
|
||||
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $lonelyUser, ['view']));
|
||||
self::assertEquals(VoterInterface::ACCESS_GRANTED, $voter->vote($token, $lonelyUser, ['edit']));
|
||||
}
|
||||
|
||||
/**
|
||||
* Without the role permission "<attribute>_other_profile" the voter must deny,
|
||||
* regardless of any team relation between user and subject.
|
||||
*/
|
||||
public function testOtherProfileDeniedWithoutRolePermissionEvenWithTeamRelation(): void
|
||||
{
|
||||
$teamlead = self::getUser(50, User::ROLE_TEAMLEAD);
|
||||
$teamlead->setEnabled(true);
|
||||
$member = self::getUser(51, User::ROLE_USER);
|
||||
$member->setEnabled(true);
|
||||
|
||||
$team = new Team('shared team');
|
||||
$team->addUser($member);
|
||||
$team->addTeamlead($teamlead);
|
||||
|
||||
// no "view_other_profile" permission for ROLE_TEAMLEAD
|
||||
$permissions = [
|
||||
'ROLE_TEAMLEAD' => ['view_own_profile'],
|
||||
];
|
||||
$rpm = $this->getRolePermissionManager($permissions, true);
|
||||
$voter = new UserVoter($rpm);
|
||||
|
||||
$token = new UsernamePasswordToken($teamlead, 'bar', $teamlead->getRoles());
|
||||
|
||||
self::assertEquals(VoterInterface::ACCESS_DENIED, $voter->vote($token, $member, ['view']));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user