Skip to content

Commit

Permalink
Temporarily disable saving locale via Fortify UpdateUserProfileInform…
Browse files Browse the repository at this point in the history
…ation.php action because it is currently broken.

Fix an issue where system/database would throw an error when PDO did not support the server version method.
Make admin/get_bu_data return empty values like v5 when an attribute was never set.
Use MachinePolicy to guard access to clients/detail which enforces Business Unit authorization
Make note about the difference between UnitController and AdminController
Change the `delete` authorization for MachinePolicy.php from Business Units v2 back to the legacy BU
Copy/paste the `view` authorization from Machine to ReportData as they have identical policies
Few type hints here and there
User::machineGroups() now uses a DB query instead of the session data from LoginRoleDecider/MachineGroupMembership so that business unit gates can apply to REST API too
authorized_for_serial() leverages gate/policy instead of having its own authZ logic
Legacy Business Units have a machineGroups() scope to return only MGs
Factories have been updated to fit the code style of Laravel 10
Added some notes about architecture, specifically around AuthZ
Delete some remaining assets from the /me/ endpoint which is replaced by jetstream API
Minor updates to JS components to make ESLint happy
blank/unauthenticated layout gets a class so we know when it is being rendered by using the browser inspector
Removed the opt-in blade component dashboard, just made it the default as it is working perfectly.
Listing menu now scrolls when it overflows the viewport
/show/kiss_layout is only available in dev mode
Update several failing tests including the ones which were included with Jetstream
Update PHPUnit config in preparation for v10
  • Loading branch information
mosen committed Jan 2, 2024
1 parent b3605a0 commit ad76d32
Show file tree
Hide file tree
Showing 50 changed files with 442 additions and 247 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
- Laravel 10.x
- PHP 8.1.x now required
- Composer 2.2.x now required

- Blade Component Dashboards now default layout type, old style dashboard.php view has a warning applied.
- Jetstream views are used instead of custom ones for user profile editing and password update.

### [6.0.0-beta.3](https://github.com/munkireport/munkireport-php/compare/v6.0.0-beta.3...wip) (Unreleased)

Expand Down
5 changes: 0 additions & 5 deletions app/Actions/Fortify/UpdateUserProfileInformation.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ public function update(User $user, array $input): void
// session()->set('theme', $input['theme']);
}

if (isset($input['locale'])) {
$user->locale = $input['locale'];
$user->save();
}

// TODO:
// 'locale' => $request->getLocale(),
// 'current_theme' => $request->session()->get('theme', config('_munkireport.default_theme')),
Expand Down
6 changes: 6 additions & 0 deletions app/Http/Controllers/AdminController.php
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,12 @@ public function get_bu_data(): JsonResponse
'managers' => [],
'archivers' => [],
'machine_groups' => [],
'name' => '',
'unitid' => 0,
'address' => '',
'link' => '',
'groupid' => '',
'key' => '',
];
}
switch ($obj->property) {
Expand Down
9 changes: 8 additions & 1 deletion app/Http/Controllers/Api/SystemInformationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,16 @@ public function database()
$connection = DB::connection(DB::getDefaultConnection());
$pdo = DB::getPdo();

// The method PDO::getServerVersion is not always available.
if (method_exists($pdo, 'getServerVersion')) {
$serverVersion = $pdo->getServerVersion();
} else {
$serverVersion = "Unavailable";
}

$response = [
'database_name' => $connection->getDatabaseName(),
'server_version' => $pdo->getServerVersion(),
'server_version' => $serverVersion,
'client_version' => $pdo->getAttribute(\PDO::ATTR_CLIENT_VERSION),
'driver_name' => $pdo->getAttribute(\PDO::ATTR_DRIVER_NAME),
// 'server_info' => $pdo->getAttribute(\PDO::ATTR_SERVER_INFO),
Expand Down
10 changes: 7 additions & 3 deletions app/Http/Controllers/ClientsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
use App\Machine;
use App\ReportData;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
use Illuminate\Support\Str;
use munkireport\lib\Modules;
use Compatibility\Kiss\ConnectDbTrait;
use Illuminate\Http\Request;

class ClientsController extends Controller
{
Expand Down Expand Up @@ -132,7 +131,7 @@ public function get_links(): JsonResponse
* @param string $sn serial number
* @author abn290
**/
public function detail(string $sn = '')
public function detail(Request $request, string $sn = '')
{
$data = array('serial_number' => $sn);
$data['scripts'] = array("clients/client_detail.js");
Expand All @@ -143,6 +142,11 @@ public function detail(string $sn = '')

$reportData = ReportData::where('serial_number', $sn)
->firstOrFail();

if ($request->user()->cannot('view', $machine)) {
abort(403);
}

$data['reportData'] = $reportData;

// Tab list, each item should contain:
Expand Down
5 changes: 4 additions & 1 deletion app/Http/Controllers/UnitController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ public function get_data(Request $request)
}

/**
* Get machine group data for current user
* Get machine group data for current user.
*
* This endpoint is different to the admin endpoint because it provides an "Unassigned" option. It is used to
* populate the filter modal and inside machine_detail.js.
*
* @author
**/
Expand Down
61 changes: 39 additions & 22 deletions app/Policies/MachinePolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use App\User;
use App\Machine;
use Illuminate\Auth\Access\HandlesAuthorization;
use Illuminate\Auth\Access\Response as AccessResponse;
use Illuminate\Support\Facades\DB;

/**
Expand All @@ -30,6 +31,19 @@ public function __construct()
//
}

/**
* Pre-authorization checks: Always allow roles which hold the `global` action (usually the admin role).
*
* @param User $user
* @param string $ability
* @return bool|null
*/
public function before(User $user, string $ability): bool|null
{
if ($user->isAdmin()) return true;
return null;
}

/**
* Determine whether the user can view any Machines.
*
Expand All @@ -39,17 +53,17 @@ public function __construct()
* With business units enabled:
* - Admins can always view.
* - Manager or User can only view their own Business Unit.
* - Nobody role cannot view anything.
* - Nobody role cannot view anything. (Nobody is a user who is part of munkireport but belongs to no business units)
*
*
* @param User $user
* @return mixed
*/
public function viewAny(User $user)
public function viewAny(User $user): AccessResponse
{
// user must be admin or manager
if (!config('_munkireport.enable_business_units', false)) return true;
if ($user->isAdmin()) return true;


// TODO: scan business unit membership

Expand All @@ -68,49 +82,52 @@ public function viewAny(User $user)
* @param Machine $machine
* @return mixed
*/
public function view(User $user, Machine $machine)
public function view(User $user, Machine $machine): AccessResponse
{
// user must be admin or manager
// there are no view restrictions when BU is turned off
if (!config('_munkireport.enable_business_units', false)) return true;
if ($user->isAdmin()) return true;

// TODO: scan business unit membership
// Determine whether the user can view based on their machine groups which was decided by the MachineGroupMembership Auth Listener
$machineGroups = session()->get('machine_groups', []);
$matchMachineGroup = $machine->reportData()->firstOrFail()->machine_group;
if (in_array((string)$matchMachineGroup, $machineGroups)) return true;


return false;
}

/**
* Determine whether the user can delete the model.
*
* With business units enabled:
* - You need to be a manager in the same business unit *OR* a global admin.
* With business units disabled:
* - Be a global manager or admin
*
* @param User $user
* @param Machine $machine
* @return mixed
*/
public function delete(User $user, Machine $machine)
public function delete(User $user, Machine $machine): AccessResponse
{
if ($user->isAdmin()) return true;
if (!config('_munkireport.enable_business_units', false) && $user->isManager()) return true;

$managerOfBusinessUnits = $user->managerOfBusinessUnits()->get('business_units.id');

$managesMachineBusinessUnit = DB::table('machine')
$machineGroupBusinessUnits = DB::table('machine')
->join(
'reportdata',
'machine.serial_number', '=', 'reportdata.serial_number'
)
->join(
'machine_groups',
'reportdata.machine_group', '=', 'machine_groups.id'
'machine_group',
'reportdata.machine_group', '=', 'machine_group.groupid'
)
->join(
'business_units',
'machine_groups.business_unit_id', '=', 'business_units.id'
)
->where('machine.id', '=', $machine->id)
->whereIn('business_units.id', $managerOfBusinessUnits);
'business_unit', function (JoinClause $join) {
$join->on('reportdata.machine_group', '=', 'business_unit.value')
->where('business_unit.property', '=', 'machine_group');
})
->get();

if ($managesMachineBusinessUnit->count() > 0) {
return true;
}

return false;
}
Expand Down
29 changes: 29 additions & 0 deletions app/Policies/ReportDataModelPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,33 @@ public function archive_gate(User $user)
return false;
}
}

/**
* Determine whether the user can view the model.
*
* With business units enabled:
* - Admins can always view.
* - Manager or User can only view their own Business Unit.
* - Nobody role cannot view anything.
*
* @param User $user
* @param ReportData $reportData
* @return mixed
*/
public function view(User $user, ReportData $reportData)
{
if ($user->isAdmin()) return true;

// there are no view restrictions when BU is turned off
if (!config('_munkireport.enable_business_units', false)) return true;

// Determine whether the user can view based on their machine groups which was decided by the MachineGroupMembership Auth Listener
// TODO: cannot use session when ths authz is API key based
$machineGroups = session()->get('machine_groups', []);
$matchMachineGroup = $reportData->machine_group;
if (in_array((string)$matchMachineGroup, $machineGroups)) return true;


return false;
}
}
4 changes: 3 additions & 1 deletion app/Providers/AuthServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Providers;

use App\Extensions\YamlUserProvider;
use App\User;
use Illuminate\Foundation\Application;
use Illuminate\Foundation\Support\Providers\AuthServiceProvider as ServiceProvider;
use Illuminate\Support\Facades\Auth;
Expand Down Expand Up @@ -36,6 +37,7 @@ public function boot(): void
{
$this->registerPolicies();

// The delete action should really be a policy, but the ManagerController::delete_machine action does not resolve a Machine model instance
Gate::define('delete_machine', function (\App\User $user, string $serial_number) {
if ($user->isAdmin()) return true;

Expand Down Expand Up @@ -68,7 +70,7 @@ public function boot(): void
return false;
});

Gate::define('global', function ($user) {
Gate::define('global', function (User $user): bool {
$authorizations = config('_munkireport.authorization', []);
// No archive authorizations defined: it would not be possible to pass this gate.
if (!isset($authorizations['global'])) {
Expand Down
50 changes: 49 additions & 1 deletion app/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Contracts\Translation\HasLocalePreference;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Foundation\Auth\User as Authenticatable;
use Illuminate\Notifications\Notifiable;
Expand Down Expand Up @@ -62,6 +63,20 @@ class User extends Authenticatable implements LegacyUser, HasLocalePreference
'profile_photo_url',
];

//// Business Units V1

/**
* Retrieve legacy business unit membership rows where this user is a member
* (via their username) having a role of manager, archiver, or user.
* @return mixed
*/
public function memberOfLegacyBusinessUnits() {
return LegacyBusinessUnit::members()
->where('value', '=', $this->name);
}

//// Business Units V2 (Alpha)

/**
* Retrieve business units where this user is a member (manager or normal user).
*
Expand Down Expand Up @@ -90,6 +105,15 @@ public function userOfBusinessUnits() {
return $this->memberOfBusinessUnits()->wherePivot('role', 'user');
}

/**
* Retrieve business units where this user holds the given role name.
*
* @param string $role The role name to filter by, only business units will be returned if the user holds a role in them.
*/
public function businessUnitsWithRole(string $role): BelongsToMany {
return $this->memberOfBusinessUnits()->wherePivot('role', $role);
}

//// LegacyUser Interface for MunkiReport

/**
Expand Down Expand Up @@ -156,11 +180,35 @@ public function canAccessMachineGroup($id): bool
* - MunkiReport 5.6.5 sets this information on the session when you log in
* - Backwards compatibility for session-based machine groups is provided by App\Auth\Listeners\MachineGroupMembership
*
* This was moved from a session value to a database query because machine groups need to be queried for authz decisions in
* stateless access such as via REST API.
*
* @return array
*/
public function machineGroups(): array
{
return session('machine_groups') ?? [];
//return session('machine_groups') ?? [];
if (!config('_munkireport.enable_business_units', false) || $this->role === 'admin') {
// Can access all defined groups (from machine_group)
// and used groups (from reportdata)
$mg = new Machine_group;
$reportedMachineGroups = ReportData::select('machine_group')
->groupBy('machine_group')
->get()
->pluck('machine_group')
->toArray();
$reportedMachineGroups = $reportedMachineGroups ? $reportedMachineGroups : [0];
$machineGroups = array_unique(array_merge($reportedMachineGroups, $mg->get_group_ids()));
} else {
$businessUnitMembership = LegacyBusinessUnit::members()->where('value', $this->name)->first();
$machineGroups = LegacyBusinessUnit::where('unitid', $businessUnitMembership->unitid)
->where('property', 'machine_group')
->get()
->pluck('value')
->toArray();
}

return $machineGroups;
}

/**
Expand Down
14 changes: 12 additions & 2 deletions compatibility/BusinessUnit.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use Illuminate\Database\Eloquent\Model;

/**
* Business Unit Model (from v5).
* Business Unit Model (from v5). AKA Legacy Business Unit
*
* Currently business units are implemented as a key - value table containing three things:
*
Expand Down Expand Up @@ -118,7 +118,7 @@ public function businessUnitable() {
//// SCOPES

/**
* Return only BusinessUnit rows with details about Managers and Users.
* Return only BusinessUnit rows with details about members.
*
* @param Builder $query
* @return Builder The amended query
Expand All @@ -131,6 +131,16 @@ public function scopeMembers(Builder $query): Builder {
]);
}

/**
* Return only BusinessUnit rows with details about Machine Groups
*
* @param Builder $query
* @return Builder The amended query
*/
public function scopeMachineGroups(Builder $query): Builder {
return $query->where('property', '=', BusinessUnit::PROP_MACHINE_GROUP);
}

/**
* Return only rows for the given Business Unit identifier.
*
Expand Down
Loading

0 comments on commit ad76d32

Please sign in to comment.