Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of an NSAC emergency info role #330

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+117 −32
Open

Addition of an NSAC emergency info role #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
121c2e6
Updated constants.php to include a role for NSAC
ThijsEgberts Jul 6, 2023
8b6f05a
Updated the db seeded to include a user with the new NSAC role
ThijsEgberts Jul 6, 2023
97755cb
Added conditions to hide tabs from NSAC user
ThijsEgberts Jul 6, 2023
62f98ce
Made it so that the current members menu option is visible to NSAC role
ThijsEgberts Jul 6, 2023
b966e2f
Added acces for NSAC user to user index and show pages and fixed typo
ThijsEgberts Jul 6, 2023
c7554c1
Only the emergency info tab is now visible for the NSAC role
ThijsEgberts Jul 7, 2023
411bea4
modified: app/Http/Middleware/Authorize.php
MiniNick Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/Http/Controllers/UserController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ public function __construct(RepositorieFactory $repositoryFactory)
{
$this->middleware('auth');
// The edit, update, and show methods check the authorization themselves, so we don't apply a role middleware there.
// Only the index method is accessible by both Administrators and Certificate admins, so we apply a different middleware there.
// Only the index method is accessible by both Administrators, Certificate admins and NSAC emergency info users, so we apply a different middleware there.
$this->middleware('authorize:'.\Config::get('constants.Administrator'))->except(['edit', 'update', 'show', 'index']);
$this->middleware('authorize:'.\Config::get('constants.Administrator') .',' . \Config::get('constants.Certificate_administrator'))->only(['index']);
$this->middleware('authorize:'.\Config::get('constants.Administrator') .',' . \Config::get('constants.Certificate_administrator') .',' .\Config::get('constants.NSAC_emergency_info_administrator'))->only(['index']);
$this->_userRepository = $repositoryFactory->getRepositorie(RepositorieFactory::$USERREPOKEY);
}

Expand Down Expand Up @@ -75,7 +75,7 @@ public function store(Request $request){
}

public function show(Request $request, User $user){
if(Auth::user()->id != $user->id && !Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator'))){
if(Auth::user()->id != $user->id && !Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator'),Config::get('constants.NSAC_emergency_info_administrator'))){
Copy link
Contributor

@indiePeeters indiePeeters Jul 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Het lijkt erop dat de rol hier toegang heeft tot het gehele User object. Wat stuurt de API terug naar de frontend? of filtered de API de informatie uit het user object waardoor enkel de info overblijft waar de NSAC role recht op heeft? e.g:
{ "name": "indie peeters", "emergencyInfo": { "emergencyPhonenumber": "phonenumber", "emergencyAddress": "adress", "emergencyCity": "city" } }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oeh dat is waar ja, zoals ik het had gemaakt filtert de frontend de data maar word het idd wel allemaal gestuurd volgens mij. Ik kan kijken of ik het zo kan krijgen dat het in de API al gefilterd word

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prima! laat maar weten als je hulp nodig hebt :)

abort(403, trans('validation.Unauthorized'));
}
return view('beheer.user.show', compact('user'));
Expand Down
4 changes: 2 additions & 2 deletions app/Http/Middleware/Authorize.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ class Authorize
* @param \Closure $next
* @return mixed
*/
public function handle($request, Closure $next, $role,$role2 = "-1")
public function handle($request, Closure $next, $role,$role2="-1",$role3 = "-1")
{
if(Auth::user()->hasRole($role,$role2)){
if(Auth::user()->hasRole($role,$role2,$role3)){
return $next($request);
} else {
abort(403, trans('validation.Unauthorized'));
Expand Down
5 changes: 3 additions & 2 deletions config/constants.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"Administrator" => 1,
"Content_administrator" => 2,
"Activity_administrator" => 3,
"Certificate_administrator" => 4

"Certificate_administrator" => 4,
"NSAC_emergency_info_administrator" => 5

];
4 changes: 4 additions & 0 deletions database/seeders/RolTableSeeder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,9 @@ public function run()
$text->save();
$rol = new \App\Rol(['name' => $text->id]);
$rol->save();
$text = new \App\Text(['NL_text' => 'NSAC noodgegevens beheerder', 'EN_text' => 'NSAC emergency info administrator']);
$text->save();
$rol = new \App\Rol(['name' => $text->id]);
$rol->save();
}
}
33 changes: 33 additions & 0 deletions database/seeders/UsersTableSeeder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,39 @@ public function run()
//add rol
$user->roles()->attach(['1','2','3']); //id for Administrator
$user->save();

//inserting test data
$user = new \App\User();
$user->email = "[email protected]";
$user->password = bcrypt("test");
$user->firstname = "Board";
$user->preposition = "of";
$user->lastname = "NSAC";
$user->street = "Kerkstraat";
$user->houseNumber = 34;
$user->city = "test";
$user->zipcode = "5301jh";
$user->country = "NL";
$user->phonenumber = "123456789";
$user->phonenumber_alt = "987654321";
$user->emergencyNumber = "147258369";
$user->emergencyHouseNumber = "19";
$user->emergencystreet = "Kerk straat";
$user->emergencycity = "Eindhoven";
$user->emergencyzipcode = "3633IK";
$user->emergencycountry = "NL";
$user->birthDay = Carbon::now()->subYear(20);
$user->gender = "man";
$user->kind_of_member = "relationship"; //not 100% sure if this is the correct kind of member for nsac board
$user->IBAN = "NL55 RABO 0107331020";
$user->BIC = "";
$user->incasso = false;
$user->remark = "Ik ben een test NSAC bestuur gebruiker";
$user->save();

//add rol
$user->roles()->attach(['5']); //id for nsac emergency info access
$user->save();

//inserting test data
$user = new \App\User();
Expand Down
89 changes: 67 additions & 22 deletions resources/views/beheer/user/show.blade.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@
<button type="submit" class="btn btn-success"><em class="ion-plus"></em> {{trans("user.makeActiveMember")}}</button>
{{ Form::close() }}
@endif

@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator')) || \Illuminate\Support\Facades\Auth::user()->id === $user->id)
<a href="{{url('/users/'.$user->id . '/edit' )}}" class="btn btn-primary">
<span title="{{trans("menu.edit")}}" class="ion-edit" aria-hidden="true"></span>
{{trans("menu.edit")}}
</a>
@endif

@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator')))
<a href="{{url('/users/'.$user->id . '/addCertificate' )}}" class="btn btn-primary">
<span title="{{trans("user.addCertificate")}}" class="ion-plus" aria-hidden="true"></span>
Expand Down Expand Up @@ -70,30 +74,37 @@
</div>
<div class="card-body">
<ul class="nav nav-tabs" id="myTab" role="tablist">
<li class="nav-item">
<a class="nav-link active" id="tab1" data-toggle="tab" href="#tab1-content" role="tab" aria-controls="general" aria-selected="true">{{trans('user.personal')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab2" data-toggle="tab" href="#tab2-content" role="tab" aria-controls="billing" aria-selected="false">{{trans('user.financial')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#tab3-content" role="tab" aria-controls="security" aria-selected="false">{{trans('user.emergencyInfo')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#rols" role="tab" aria-controls="security" aria-selected="false">{{trans('user.rols')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#certifications" role="tab" aria-controls="security" aria-selected="false">{{trans('certificate.certificates') }}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#registrations" role="tab" aria-controls="security" aria-selected="false">{{trans('user.registrations') }}</a>
</li>
@if($user->registrationInfo !== null)
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.NSAC_emergency_info_administrator')) && \Illuminate\Support\Facades\Auth::user()->id !== $user->id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wat hier gebeurd is dat je conditioneel data toont wanneer een gebruiker een bepaalde rol heeft. Wat ik zou verwachten is dat deze data niet beschikbaar is voor de frontend wanneer een geberuiker geen rechten heeft tot de data. In dat geval zou ik alleen not null checks verwachten

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dat is wel een betere manier idd. Een deel van die checks moeten er volgens mij wel zijn aangezien een deel van de UI anders word als je niet alle data wilt laten zien, dus je moet bepalen welke tabs je laat zien en welke de start tab is based op welke rol de user heeft.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Klopt, Het is toelaatbaar als je conditioneel een deel van de template laat. maar conditioneel data uit de data base tonen is niet persee veilig. (er zijn manieren om dan toch de data in te kunnen zien)

<li class="nav-item">
<a class="nav-link active" id="tab3" data-toggle="tab" href="#tab3-content" role="tab" aria-controls="security" aria-selected="true">{{trans('user.emergencyInfo')}}</a>
</li>
@else
<li class="nav-item">
<a class="nav-link active" id="tab1" data-toggle="tab" href="#tab1-content" role="tab" aria-controls="general" aria-selected="true">{{trans('user.personal')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab2" data-toggle="tab" href="#tab2-content" role="tab" aria-controls="billing" aria-selected="false">{{trans('user.financial')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#tab3-content" role="tab" aria-controls="security" aria-selected="false">{{trans('user.emergencyInfo')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" data-toggle="tab" href="#registration_info" role="tab" aria-controls="security" aria-selected="false">{{trans('user.registrationInfo') }}</a>
<a class="nav-link" id="tab3" data-toggle="tab" href="#rols" role="tab" aria-controls="security" aria-selected="false">{{trans('user.rols')}}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#certifications" role="tab" aria-controls="security" aria-selected="false">{{trans('certificate.certificates') }}</a>
</li>
<li class="nav-item">
<a class="nav-link" id="tab3" data-toggle="tab" href="#registrations" role="tab" aria-controls="security" aria-selected="false">{{trans('user.registrations') }}</a>
</li>
@if($user->registrationInfo !== null)
<li class="nav-item">
<a class="nav-link" data-toggle="tab" href="#registration_info" role="tab" aria-controls="security" aria-selected="false">{{trans('user.registrationInfo') }}</a>
</li>
@endif
@endif
</ul>

@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator')) || \Illuminate\Support\Facades\Auth::user()->id === $user->id)
<div class="tab-content space-sm">
<div class="tab-pane fade show active" id="tab1-content" role="tabpanel" aria-labelledby="tab1-content">
Expand Down Expand Up @@ -192,6 +203,7 @@
</tr>
</table>
</div>
{{-- This tab should be visible to the NSAC emergency info role --}}
<div class="tab-pane fade" id="tab3-content" role="tabpanel" aria-labelledby="tab3-content">
<table class="table table-striped" style="width:100%">
<tr>
Expand Down Expand Up @@ -235,7 +247,7 @@
@endif
</table>
</div>
@endif
{{-- @endif --}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lege comment

<div class="tab-pane fade" id="certifications" role="tabpanel" aria-labelledby="tab3-content">
<table class="table table-striped" style="width:100%">
<thead>
Expand Down Expand Up @@ -304,12 +316,45 @@
</tbody>
</table>
</div>
@if($user->registrationInfo !== null)

@if($user->registrationInfo !== null && (\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator')) || \Illuminate\Support\Facades\Auth::user()->id === $user->id))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hetzelfde hier als comment er boven

<div class="tab-pane fade" id="registration_info" role="tabpanel">
@include('beheer.user.partials.intro-info')
</div>
@endif
</div>
@elseif(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.NSAC_emergency_info_administrator')) && \Illuminate\Support\Facades\Auth::user()->id !== $user->id)
<div class="tab-content space-sm">
<div class="tab-pane fade show active" id="tab3-content" role="tabpanel" aria-labelledby="tab3-content">
<table class="table table-striped" style="width:100%">
<tr>
<td>{{trans('user.emergencystreet')}}</td>
<td>{{$user->emergencystreet}}</td>
</tr>
<tr>
<td>{{trans('user.emergencyHouseNumber')}}</td>
<td>{{$user->emergencyHouseNumber}}</td>
</tr>
<tr>
<td>{{trans('user.emergencyzipcode')}}</td>
<td>{{$user->emergencyzipcode}}</td>
</tr>
<tr>
<td>{{trans('user.emergencycity')}}</td>
<td>{{$user->emergencycity}}</td>
</tr>
<tr>
<td>{{trans('user.emergencycountry')}}</td>
<td>{{trans('countries.' . $user->emergencycountry)}}</td>
</tr>
<tr>
<td>{{trans('user.emergencyNumber')}}</td>
<td>{{$user->emergencyNumber}}</td>
</tr>
</table>
</div>
</div>
@endif
</div>
</div>
@endsection
Expand Down
8 changes: 5 additions & 3 deletions resources/views/layouts/beheer.blade.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
</a>
</li>
@else
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Activity_administrator'),Config::get('constants.Content_administrator'),Config::get('constants.Certificate_administrator')))
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Activity_administrator'),Config::get('constants.Content_administrator'),Config::get('constants.Certificate_administrator'),Config::get('constants.NSAC_emergency_info_administrator')))
<hr class="my-3">

<li class="nav-item active">
Expand All @@ -54,7 +54,7 @@
</a>
</li>
@endif
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator')))
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator'),Config::get('constants.NSAC_emergency_info_administrator')))
<li class="nav-item dropdown">
<a class="nav-link dropdown-toggle" href="#" id="navbarDropdownMenuLink" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
{{trans("menu.leden")}}
Expand All @@ -66,7 +66,9 @@
<a class="dropdown-item" href="{{ url('users/pending_members') }}">{{trans("user.pending_members")}}</a>
<a class="dropdown-item" href="{{ url('rols') }}">{{trans("menu.rols")}}</a>
@endif
<a class="dropdown-item" href="{{ url('certificates') }}">{{trans("menu.certificate")}}</a>
@if(\Illuminate\Support\Facades\Auth::user()->hasRole(Config::get('constants.Administrator'),Config::get('constants.Certificate_administrator')))
<a class="dropdown-item" href="{{ url('certificates') }}">{{trans("menu.certificate")}}</a>
@endif
</div>
</li>
@endif
Expand Down