Addition of an NSAC emergency info role #330
Conversation
Codecov Report
@@ Coverage Diff @@
## master #330 +/- ##
=========================================
Coverage 30.90% 30.90%
Complexity 645 645
=========================================
Files 96 96
Lines 2443 2443
=========================================
Hits 755 755
Misses 1688 1688
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -235,7 +247,7 @@ | |||
| @endif | |||
| </table> | |||
| </div> | |||
| @endif | |||
| {{-- @endif --}} | |||
| @@ -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'))){ | |||
There was a problem hiding this comment.
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" } }
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Prima! laat maar weten als je hulp nodig hebt :)
| <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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
| @@ -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)) | |||
There was a problem hiding this comment.
Hetzelfde hier als comment er boven
Adds an extra role which allows a member to only see the emergency info of members in the administrator dashboard.