Skip to content

Commit

Permalink
Merge pull request #6064 from GCTC-NTGC/bug_5926_fix-user-admin-page-…
Browse files Browse the repository at this point in the history
…auth

[Bug] Fix user admin page auth
  • Loading branch information
esizer authored and tristan-orourke committed Mar 28, 2023
1 parent 371c0a9 commit a761586
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 38 deletions.
40 changes: 36 additions & 4 deletions api/app/Models/Pool.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Builder;
use Carbon\Carbon;
use Illuminate\Support\Facades\Auth;

/**
* Class Pool
Expand Down Expand Up @@ -123,14 +124,12 @@ public function getAdvertisementStatusAttribute()
$currentTime = date("Y-m-d H:i:s");
if ($closedDate != null) {
$isClosed = $currentTime >= $closedDate ? true : false;
}
else {
} else {
$isClosed = false;
}
if ($publishedDate != null) {
$isPublished = $currentTime >= $publishedDate ? true : false;
}
else {
} else {
$isPublished = false;
}

Expand All @@ -150,4 +149,37 @@ public function scopeWasPublished(Builder $query, ?array $args)
$query->where('published_at', '<=', Carbon::now()->toDateTimeString());
return $query;
}

public function scopeAuthorizedToView(Builder $query)
{
$userId = Auth::user()->id;
$user = User::find($userId);
if (!$user->isAbleTo("view-any-pool")) {
$query->where(function (Builder $query) use ($user) {

if ($user->isAbleTo("view-team-pool")) {
// Only add teams the user can view pools in to the query for `whereHAs`
$teams = $user->rolesTeams()->get();
$teamIds = [];
foreach ($teams as $team) {
if ($user->isAbleTo("view-team-pool", $team)) {
$teamIds[] = $team->id;
}
}

$query->orWhereHas('team', function (Builder $query) use ($teamIds) {
return $query->whereIn('id', $teamIds);
});
}

if ($user->isAbleTo("view-any-publishedPoolAdvertisement")) {
$query->orWhere('published_at', '<=', Carbon::now()->toDateTimeString());
}

return $query;
});
}

return $query;
}
}
45 changes: 38 additions & 7 deletions api/app/Models/PoolCandidate.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public function scopeClassifications(Builder $query, ?array $classifications): B
// pointing to the classification scope on the User model
// that scope also contains filterByClassificationToSalary and filterByClassificationToGenericJobTitles
$query->whereHas('user', function ($query) use ($classifications) {
User::scopeClassifications($query, $classifications);
});
User::scopeClassifications($query, $classifications);
});
return $query;
}

Expand Down Expand Up @@ -284,10 +284,10 @@ public function scopeNotDraft(Builder $query): Builder
return $query;
}

/* accessor to obtain pool candidate status, additional logic exists to override database field sometimes*/
// pool_candidate_status database value passed into accessor as an argument
public function getPoolCandidateStatusAttribute($candidateStatus)
{
/* accessor to obtain pool candidate status, additional logic exists to override database field sometimes*/
// pool_candidate_status database value passed into accessor as an argument
public function getPoolCandidateStatusAttribute($candidateStatus)
{
// pull info
$submittedAt = $this->submitted_at;
$expiryDate = $this->expiry_date;
Expand Down Expand Up @@ -373,7 +373,7 @@ public function scopePriorityWeight(Builder $query, ?array $priorityWeights): Bu
return $query;
}

public static function scopePositionDuration(Builder $query, ?array $positionDuration) : Builder
public static function scopePositionDuration(Builder $query, ?array $positionDuration): Builder
{

if (empty($positionDuration)) {
Expand Down Expand Up @@ -411,4 +411,35 @@ public function isDraft()
{
return is_null($this->submitted_at) || $this->submitted_at->isFuture();
}

/**
* Scope the query to PoolCandidate's the current user can view
*/
public function scopeAuthorizedToView(Builder $query)
{
$userId = Auth::user()->id;
$user = User::find($userId);
if (!$user->isAbleTo("view-any-application")) {
$query->where(function (Builder $query) use ($user) {
if ($user->isAbleTo("view-any-submittedApplication")) {
$query->orWhere('submitted_at', '<=', Carbon::now()->toDateTimeString());
}

if ($user->isAbleTo("view-team-submittedApplication")) {
$teamIds = $user->rolesTeams()->get()->pluck('id');
$query->orWhereHas('pool', function (Builder $query) use ($teamIds) {
return $query->whereHas('team', function (Builder $query) use ($teamIds) {
return $query->whereIn('id', $teamIds);
});
});
}

if ($user->isAbleTo("view-own-application")) {
$query->orWhere('user_id', $user->id);
}
});
}

return $query;
}
}
6 changes: 6 additions & 0 deletions api/app/Policies/PoolPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ public function viewAny(User $user)
*/
public function view(User $user, Pool $pool)
{
// Guests and Base Users both have permission to view-any-publishedPoolAdvertisement

if ($pool->getAdvertisementStatusAttribute() !== ApiEnums::POOL_ADVERTISEMENT_IS_DRAFT) {
return true;
}

$pool->loadMissing('team');
return $user->isAbleTo("view-any-pool") || $user->isAbleTo("view-team-pool", $pool->team);
}
Expand Down
20 changes: 10 additions & 10 deletions api/app/Policies/UserPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,22 +116,22 @@ public function viewAnyApplicants(User $user)
public function viewApplicant(User $user, User $model)
{
return $user->isAbleTo('view-any-applicantProfile')
|| (
$user->isAbleTo('view-team-applicantProfile')
&& $this->applicantHasAppliedToPoolInTeams($model, $user->rolesTeams()->pluck('id')
) || (
$user->isAbleTo('view-own-applicantProfile')
&& $user->id === $model->id
)
);
|| ($user->isAbleTo('view-team-applicantProfile')
&& $this->applicantHasAppliedToPoolInTeams(
$model,
$user->rolesTeams()->get()->pluck('id')
) || ($user->isAbleTo('view-own-applicantProfile')
&& $user->id === $model->id
)
);
}

protected function applicantHasAppliedToPoolInTeams(User $applicant, $teamIds)
{
return PoolCandidate::where('user_id', $applicant->id)
->notDraft()
->whereExists(function ($query) use ($teamIds) {
return $query->pool()->whereIn('team_id', $teamIds);
->whereHas('pool', function ($query) use ($teamIds) {
return $query->whereIn('team_id', $teamIds);
})
->exists();
}
Expand Down
2 changes: 1 addition & 1 deletion api/database/factories/ApplicantFilterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public function withRelationships(bool $sparse = false)
)->get();
$filter->skills()->saveMany($skills);

$pools = Pool::inRandomOrder()->limit(
$pools = Pool::whereNotNull("published_at")->inRandomOrder()->limit(
$this->faker->numberBetween($minCount, 1)
)->get();
$filter->pools()->saveMany($pools);
Expand Down
2 changes: 1 addition & 1 deletion api/database/factories/PoolCandidateFilterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function configure()
{
return $this->afterCreating(function (PoolCandidateFilter $filter) {
$classifications = Classification::inRandomOrder()->limit(3)->get();
$pools = Pool::inRandomOrder()->limit(1)->get();
$pools = Pool::whereNotNull("published_at")->inRandomOrder()->limit(1)->get();
$filter->classifications()->saveMany($classifications);
$filter->pools()->saveMany($pools);
});
Expand Down
4 changes: 2 additions & 2 deletions api/graphql/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ type Applicant {
@rename(attribute: "job_looking_status")
@deprecated(reason: "Removing with applicantDashboard feature flag")
poolCandidates: [PoolCandidate]
@hasMany(scopes: ["notDraft"])
@hasMany(scopes: ["notDraft", "authorizedToView"])
@can(ability: "view", resolved: true)

hasDiploma: Boolean @rename(attribute: "has_diploma")
Expand Down Expand Up @@ -917,7 +917,7 @@ type Query {
@all(model: "Pool", scopes: ["wasPublished"])
@can(ability: "viewAnyPublishedAdvertisement", model: "Pool")
poolByKey(key: String! @eq): Pool @find @can(ability: "view", query: true)
pools: [Pool]! @all @can(ability: "viewAny")
pools: [Pool]! @all(scopes: ["authorizedToView"]) @can(ability: "view", resolved: true)
poolCandidate(id: UUID! @eq): PoolCandidate
@find
@guard
Expand Down
19 changes: 19 additions & 0 deletions api/tests/Unit/PoolPolicyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ public function testViewAnyPublishedAdvertisements()
*/
public function testView()
{
$this->teamPool->published_at = null;
$this->teamPool->save();
$this->unOwnedPool->published_at = null;
$this->unOwnedPool->save();

$this->assertTrue($this->poolOperatorUser->can('view', $this->teamPool));
$this->assertTrue($this->adminUser->can('view', $this->teamPool));

Expand All @@ -124,6 +129,20 @@ public function testView()
$this->assertFalse($this->requestResponderUser->can('view', $this->teamPool));
// Pool operator cannot view other teams pools
$this->assertFalse($this->poolOperatorUser->can('view', $this->unOwnedPool));

// Note: We now allow everyone to view published pools
$this->teamPool->published_at = config('constants.past_date');
$this->teamPool->save();
$this->unOwnedPool->published_at = config('constants.past_date');
$this->unOwnedPool->save();

$this->assertTrue($this->guestUser->can('view', $this->teamPool));
$this->assertTrue($this->applicantUser->can('view', $this->teamPool));
$this->assertTrue($this->poolOperatorUser->can('view', $this->teamPool));
// Pool operator can view other teams pools if they are published
$this->assertTrue($this->poolOperatorUser->can('view', $this->unOwnedPool));
$this->assertTrue($this->requestResponderUser->can('view', $this->teamPool));
$this->assertTrue($this->adminUser->can('view', $this->teamPool));
}

/**
Expand Down
12 changes: 10 additions & 2 deletions apps/web/src/components/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,11 @@ const createRoute = (locale: Locales, loginPath: string) =>
path: ":userId",
element: (
<RequireAuth
roles={[ROLE_NAME.PlatformAdmin]}
roles={[
ROLE_NAME.PoolOperator,
ROLE_NAME.RequestResponder,
ROLE_NAME.PlatformAdmin,
]}
loginPath={loginPath}
>
<UserLayout />
Expand All @@ -989,7 +993,11 @@ const createRoute = (locale: Locales, loginPath: string) =>
index: true,
element: (
<RequireAuth
roles={[ROLE_NAME.PlatformAdmin]}
roles={[
ROLE_NAME.PoolOperator,
ROLE_NAME.RequestResponder,
ROLE_NAME.PlatformAdmin,
]}
loginPath={loginPath}
>
<UserInformationPage />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from "react";
import { useIntl } from "react-intl";

import { Heading, Well } from "@gc-digital-talent/ui";
import { ROLE_NAME, useAuthorization } from "@gc-digital-talent/auth";

import { JobLookingStatus } from "~/api/generated";

Expand All @@ -11,6 +12,12 @@ import PoolStatusTable from "./PoolStatusSection";

const CandidateStatusSection = ({ user, pools }: UserInformationProps) => {
const intl = useIntl();
const { roleAssignments, isLoaded } = useAuthorization();
const isAdmin =
isLoaded &&
roleAssignments?.some(
(roleAssignment) => roleAssignment.role?.name === ROLE_NAME.PlatformAdmin,
);

return (
<>
Expand Down Expand Up @@ -57,15 +64,19 @@ const CandidateStatusSection = ({ user, pools }: UserInformationProps) => {
})}
</Heading>
<PoolStatusTable user={user} pools={pools} />
<h5 data-h2-margin="base(x2, 0, x1, 0)">
{intl.formatMessage({
defaultMessage: "Add user to pool",
id: "jtEouE",
description:
"Title of the 'Add user to pools' section of the view-user page",
})}
</h5>
<AddToPoolDialog user={user} pools={pools} />
{isAdmin && (
<>
<h5 data-h2-margin="base(x2, 0, x1, 0)">
{intl.formatMessage({
defaultMessage: "Add user to pool",
id: "jtEouE",
description:
"Title of the 'Add user to pools' section of the view-user page",
})}
</h5>
<AddToPoolDialog user={user} pools={pools} />
</>
)}
</>
);
};
Expand Down
6 changes: 5 additions & 1 deletion apps/web/src/pages/Users/UserLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ const UserLayout = () => {
{/* This is above the AdminContentWrapper so it needs its own centering */}
<div data-h2-container="base(center, full, x2)">
<Pending fetching={fetching} error={error}>
{data?.user ? <UserHeader user={data.user} /> : <ThrowNotFound />}
{data?.applicant ? (
<UserHeader user={data.applicant} />
) : (
<ThrowNotFound />
)}
</Pending>
</div>
<Outlet />
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/pages/Users/userOperations.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ query User($id: UUID!) {
}

query UserName($userId: UUID!) {
user(id: $userId) {
applicant(id: $userId) {
id
firstName
lastName
Expand Down

0 comments on commit a761586

Please sign in to comment.