Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ devhub/pm-font/dist
test-db-snapshot.db
snapshot_*.db
storage/transitions
.envrc
6 changes: 4 additions & 2 deletions ProcessMaker/Console/Commands/CreateDataLakeViews.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,10 @@ protected function getTableColumns(string $tableName): array
*/
protected function getTables(): array
{
$database = \DB::connection()->getDatabaseName();
$tables = array_map(function ($item) {
return $item['name'];
}, Schema::getTables());
}, Schema::getTables($database));
Copy link

Choose a reason for hiding this comment

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

Schema methods don't accept database parameter

High Severity

Schema::getTables() and Schema::getViews() methods in Laravel 12 don't accept a database name parameter. The code passes $database to these methods, but they take no arguments. This would cause an ArgumentCountError at runtime when the processmaker:create-data-lake-views artisan command is executed.

Additional Locations (1)

Fix in Cursor Fix in Web


return $tables;
}
Expand All @@ -193,9 +194,10 @@ protected function getTables(): array
*/
protected function getViews(): array
{
$database = \DB::connection()->getDatabaseName();
$views = array_map(function ($item) {
return $item['name'];
}, Schema::getViews());
}, Schema::getViews($database));
Copy link

Choose a reason for hiding this comment

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

getViews returns incompatible data structure breaking view logic

High Severity

The getViews() method now returns a numerically-indexed array of view name strings, but consumers expect an associative array keyed by view name with objects having a getSql() method. In shouldCreate(), the check isset($views[$viewName]) will always fail since the array uses numeric keys, causing views to always be recreated unnecessarily. In the up() method's foreach loop, $viewName becomes numeric indices (0, 1, 2...) instead of actual view names, breaking the dropped table detection logic entirely.

Additional Locations (2)

Fix in Cursor Fix in Web


return $views;
}
Expand Down
66 changes: 0 additions & 66 deletions ProcessMaker/Console/Commands/CreateTestDBs.php

This file was deleted.

2 changes: 1 addition & 1 deletion ProcessMaker/Http/Controllers/Admin/DevLinkController.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function getOauthClient(Request $request)

if (!$client) {
$clientRepository = app('Laravel\Passport\ClientRepository');
$client = $clientRepository->create(null, 'devlink', $redirectUri);
$client = $clientRepository->createAuthorizationCodeGrantClient('devlink', [$redirectUri]);
}

$query = http_build_query([
Expand Down
6 changes: 3 additions & 3 deletions ProcessMaker/Http/Controllers/Api/UserTokenController.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public function index(Request $request, User $user)
throw new AuthorizationException(__('Not authorized to update this user.'));
}

$tokens = $this->tokenRepository->forUser($user->id);
$tokens = $this->tokenRepository->forUser($user);

$results = $tokens->load('client')->filter(function ($token) {
return $token->client->personal_access_client && !$token->revoked;
Expand Down Expand Up @@ -202,7 +202,7 @@ public function show(Request $request, User $user, $tokenId)

$token = $this->tokenRepository->findForUser(
$tokenId,
$user->getKey()
$user
);

if (is_null($token)) {
Expand Down Expand Up @@ -256,7 +256,7 @@ public function destroy(Request $request, User $user, $tokenId)

$token = $this->tokenRepository->findForUser(
$tokenId,
$user->getKey()
$user
);

if (is_null($token)) {
Expand Down
95 changes: 43 additions & 52 deletions ProcessMaker/Http/Controllers/Auth/ClientController.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,39 @@
namespace ProcessMaker\Http\Controllers\Auth;

use Illuminate\Http\Request;
use Laravel\Passport\Http\Controllers\ClientController as PassportClientController;
use Illuminate\Http\Response;
use Laravel\Passport\ClientRepository;
use ProcessMaker\Events\AuthClientCreated;
use ProcessMaker\Events\AuthClientDeleted;
use ProcessMaker\Events\AuthClientUpdated;
use ProcessMaker\Http\Resources\AuthClient as AuthClientResource;

class ClientController extends PassportClientController
class ClientController
{
/**
* List auth clients
*
* @param \Illuminate\Http\Request $request
* @return array
*/
public function __construct(
protected ClientRepository $clients,
protected \Illuminate\Contracts\Validation\Factory $validation,
) {
}

public function index(Request $request)
{
$clients = \Laravel\Passport\Client::where('revoked', false)->get();

return AuthClientResource::collection($clients);
}

/**
* Get an individual auth client
*
* @param \Illuminate\Http\Request $request
* @param string $clientId
* @return array
*/
public function show(Request $request, $clientId)
{
// $client = $this->clients->find($clientId);
$client = parent::show($request, $clientId);
$client = $this->clients->findForUser($clientId, $request->user());

if (!$client) {
return new Response('', 404);
}

return new AuthClientResource($client);
}

/**
* Store a new client.
*
* @param \Illuminate\Http\Request $request
Copy link

Choose a reason for hiding this comment

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

Duplicated type-extraction logic in store and update methods

Low Severity

The store() and update() methods contain identical code for extracting $personalAccess, $password, and $redirect from $request->types. These three lines are duplicated verbatim between the two methods. This logic could be extracted to a private helper method like parseClientTypes(Request $request) to reduce duplication and make future maintenance easier.

Additional Locations (1)

Fix in Cursor Fix in Web

* @return \Laravel\Passport\Client
*/
public function store(Request $request)
{
$this->validate($request);
Expand All @@ -53,36 +44,41 @@ public function store(Request $request)
$password = in_array('password_client', $request->types);
$redirect = in_array('authorization_code_grant', $request->types) ? $request->redirect : '';

$client = $this->clients->create(
$request->user()->getKey(),
$request->name,
$redirect,
null,
$personalAccess,
$password
)->makeVisible('secret');
// Use ClientRepository methods based on type
if ($personalAccess) {
$client = $this->clients->createPersonalAccessGrantClient(
$request->name
);
} elseif ($password) {
$client = $this->clients->createPasswordGrantClient(
$request->name,
null, // provider
true // confidential
);
Copy link

Choose a reason for hiding this comment

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

OAuth clients not associated with user when created

High Severity

When creating personal access or password grant clients via store(), the new code uses createPersonalAccessGrantClient() and createPasswordGrantClient() which don't associate the client with the authenticated user. The old code passed $request->user()->getKey() to link all client types to the user. Since show(), update(), and destroy() all use findForUser($clientId, $request->user()) to retrieve clients, users can no longer access, modify, or delete personal access and password grant clients they create through this API.

Additional Locations (2)

Fix in Cursor Fix in Web

} else {
// Authorization code grant
$client = $this->clients->createAuthorizationCodeGrantClient(
$request->name,
$redirect ? explode(',', $redirect) : [],
true, // confidential
$request->user()
);
}

$client->makeVisible('secret');
AuthClientCreated::dispatch($client->getAttributes());

return new AuthClientResource($client);
}

/**
* Update the given client.
*
* @param \Illuminate\Http\Request $request
* @param string $clientId
* @return \Illuminate\Http\Response|\Laravel\Passport\Client
*/
public function update(Request $request, $clientId)
{
$client = $this->clients->find($clientId);
$client = $this->clients->findForUser($clientId, $request->user());
Copy link

Choose a reason for hiding this comment

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

OAuth client update/destroy restricted to owner only

Medium Severity

The update and destroy methods changed from $this->clients->find($clientId) to $this->clients->findForUser($clientId, $request->user()). This restricts operations to only clients owned by the requesting user, while the index method still returns ALL clients. Users with edit-auth_clients or delete-auth_clients permissions will see clients in the list but receive 404 errors when attempting to modify clients they don't own, breaking admin management functionality.

Additional Locations (1)

Fix in Cursor Fix in Web


if (!$client) {
return new Response('', 404);
}

$original_values = $client->getAttributes();

$this->validate($request);

$personalAccess = in_array('personal_access_client', $request->types);
Expand All @@ -97,33 +93,26 @@ public function update(Request $request, $clientId)
]);

$original = array_intersect_key($client->getOriginal(), $client->getDirty());

$client->save();

AuthClientUpdated::dispatch($clientId, $original, $client->getChanges(), $request->name);

return new AuthClientResource($client);
}

/**
* Delete the given client.
*
* @param \Illuminate\Http\Request $request
* @param string $clientId
* @return null
*/
public function destroy(Request $request, $clientId)
{
$client = $this->clients->find($clientId);
$client = $this->clients->findForUser($clientId, $request->user());

if (!$client) {
return new Response('', 404);
}

$attributes = $client->getAttributes();
$this->clients->delete($client);
AuthClientDeleted::dispatch($client->getAttributes());
AuthClientDeleted::dispatch($attributes);

return response('', 204);
return new Response('', 204);
}

private function validate($request)
Expand All @@ -133,9 +122,11 @@ private function validate($request)
'types' => 'array|min:1|required',
'types.*' => 'in:authorization_code_grant,password_client,personal_access_client',
];

if (is_array($request->types) && in_array('authorization_code_grant', $request->types)) {
$rules['redirect'] = 'required|url|max:2000';
}

$this->validation->make($request->all(), $rules, [
'min' => __('The Auth-Client must have at least :min item chosen.'),
])->validate();
Expand Down
Loading
Loading