Skip to content

Commit

Permalink
refs #2917 Restrict excessively long URLs in modal windows.
Browse files Browse the repository at this point in the history
  • Loading branch information
tabuna committed Nov 1, 2024
1 parent 130620a commit 5c8dd63
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 32 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## 14.42.0 - 2024-11-01

### Changed
- Restrict excessively long URLs in modal windows [#2917](https://github.com/orchidsoftware/platform/issues/2917)

## 14.41.0 - 2024-10-29

### Added
- Template for grid columns on `Group` [#2913](https://github.com/orchidsoftware/platform/pull/2913)

## 14.40.0 - 2024-10-28

### Changed
Expand Down
2 changes: 1 addition & 1 deletion public/css/orchid.rtl.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion public/css/orchid.rtl.css.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion public/js/orchid.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion public/js/orchid.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions public/mix-manifest.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions resources/js/controllers/modal_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export default class extends ApplicationController {
type: String,
default: ''
},
parameters: {
type: Object,
},
open: {
type: Boolean,
default: false
Expand Down Expand Up @@ -112,7 +115,7 @@ export default class extends ApplicationController {
}

// Load deferred data if URL is specified and no validation errors are present
if (this.urlValue && !options.validateError) {
if (Object.keys(this.parametersValue).length !== 0 && !options.validateError) {
this.asyncLoadData(JSON.parse(options.params));
}

Expand Down Expand Up @@ -153,7 +156,8 @@ export default class extends ApplicationController {

// Load data via stream and update modal state
this.loadStream(`${this.urlValue}?${query}`, {
'_state': document.getElementById('screen-state')?.value || null
'_state': document.getElementById('screen-state')?.value || null,
...this.parametersValue
})
.then(() => this.element.classList.remove('modal-loading'));
}
Expand Down
9 changes: 6 additions & 3 deletions resources/views/layouts/modal.blade.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
role="dialog"
aria-labelledby="screen-modal-{{$key}}"
data-controller="modal"
data-modal-slug-value="{{$templateSlug}}"
data-modal-url-value="{{$deferredRoute}}"
data-modal-slug-value="{{ $templateSlug }}"
data-modal-url-value="{{ $deferredRoute }}"
@if(!empty($deferrerParams))
data-modal-parameters-value='@json($deferrerParams)'
@endif
data-modal-open-value="{{ var_export($open) }}"
{{$staticBackdrop ? "data-bs-backdrop=static" : ''}}
>
Expand Down Expand Up @@ -62,7 +65,7 @@
<div class="modal-body layout-wrapper">
<x-orchid-stream target="{{$templateSlug}}">
<div id="{{ $templateSlug }}">
@if(!empty($deferredRoute) == \Orchid\Support\Facades\Dashboard::isPartialRequest())
@if(!empty($deferrerParams) == \Orchid\Support\Facades\Dashboard::isPartialRequest())
@foreach($manyForms as $formKey => $modal)
@foreach($modal as $item)
{!! $item ?? '' !!}
Expand Down
2 changes: 1 addition & 1 deletion routes/dashboard.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
->push(__('Search'))
->push($query));

Route::post('async/{screen}/{method?}/{template?}', [AsyncController::class, 'load'])
Route::post('async', [AsyncController::class, 'load'])
->name('async');

Route::post('listener/{screen}/{layout}', [AsyncController::class, 'listener'])
Expand Down
2 changes: 1 addition & 1 deletion src/Platform/Dashboard.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Dashboard
*
* @deprecated Use `Dashboard::version()` instead.
*/
public const VERSION = '14.41.0';
public const VERSION = '14.42.0';

/**
* @deprecated
Expand Down
17 changes: 14 additions & 3 deletions src/Platform/Http/Controllers/AsyncController.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,25 @@ class AsyncController extends Controller
*
* @return mixed
*/
public function load(string $screen, string $method, string $layout)
public function load(Request $request)
{
$screen = Crypt::decryptString($screen);
$request->validate([
'_call' => 'required|string',
'_screen' => 'required|string',
'_template' => 'required|string',
]);

$screen = Crypt::decryptString(
$request->input('_screen')
);

/** @var Screen $screen */
$screen = app($screen);

return $screen->asyncBuild($method, $layout);
return $screen->asyncBuild(
$request->input('_call'),
$request->input('_template')
);
}

/**
Expand Down
21 changes: 11 additions & 10 deletions src/Screen/Layouts/Modal.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ public function getSlug(): string
public function build(Repository $repository)
{
$this->variables = array_merge($this->variables, [
'deferredRoute' => $this->getDataLoadingUrl(),
'deferredRoute' => route('platform.async'),
'deferrerParams' => $this->getDeferrerDataLoadingParameters(),
]);

return $this->buildAsDeep($repository);
Expand Down Expand Up @@ -240,24 +241,24 @@ public function deferred(string $method): self
}

/**
* Return the URL for loading data from the browser.
* Return the deferrer parameters for loading data from the browser.
*/
protected function getDataLoadingUrl(): ?string
protected function getDeferrerDataLoadingParameters(): array
{
$screen = Dashboard::getCurrentScreen();

if (! $screen) {
return null;
return [];
}

if (empty($this->dataLoadingMethod)) {
return null;
return [];
}

return route('platform.async', [
'screen' => Crypt::encryptString(get_class($screen)),
'method' => $this->dataLoadingMethod,
'template' => $this->getSlug(),
]);
return [
'_screen' => Crypt::encryptString(get_class($screen)),
'_call' => $this->dataLoadingMethod,
'_template' => $this->getSlug(),
];
}
}
6 changes: 5 additions & 1 deletion src/Screen/Screen.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ public function asyncBuild(string $method, string $slug)
{
Dashboard::setCurrentScreen($this, true);

abort_unless(method_exists($this, $method), 404, "Async method: {$method} not found");
abort_unless(
static::getAvailableMethods()->contains($method),
Response::HTTP_BAD_REQUEST,
"Async method '{$method}' is unavailable."
);

abort_unless($this->checkAccess(request()), static::unaccessed());

Expand Down
40 changes: 35 additions & 5 deletions tests/Feature/Platform/AsyncScreenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Orchid\Tests\Feature\Platform;

use Illuminate\Http\Response;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Str;
use Orchid\Tests\App\Layouts\DependentSumListener;
Expand All @@ -12,6 +13,10 @@

class AsyncScreenTest extends TestFeatureCase
{
/**
* Test that the asynchronous dependent listener screen returns
* the correct calculation result.
*/
public function testAsyncDependentListenerScreen(): void
{
$response = $this
Expand All @@ -30,22 +35,47 @@ public function testAsyncDependentListenerScreen(): void
$this->assertStringContainsString('The result of adding', $response->getContent());
}

/**
* Test that an asynchronous request to a non-existing method
* returns a 400 Bad Request status.
*/
public function testAsyncMethodNotFoundScreen(): void
{
/** @var DependentSumListener $layout */
$layout = $this->app->make(DependentSumListener::class);

$response = $this
->actingAs($this->createAdminUser())
->post(route('platform.async'), [
'_screen' => Crypt::encryptString(DependentListenerScreen::class),
'_call' => Str::random(),
'_template' => $layout->getSlug(),
]);

$response->assertStatus(Response::HTTP_BAD_REQUEST);
}

/**
* Test that attempting to call a denied method asynchronously
* returns a 400 Bad Request status.
*/
public function testAsyncDeniedMethodScreen(): void
{
/** @var DependentSumListener $layout */
$layout = $this->app->make(DependentSumListener::class);

$response = $this
->actingAs($this->createAdminUser())
->post(route('platform.async', [
'screen' => Crypt::encryptString(DependentListenerScreen::class),
'method' => Str::random(),
'template' => $layout->getSlug(),
]), [
'first' => 2,
'second' => 3,
]), [
'_screen' => Crypt::encryptString(DependentListenerScreen::class),
'_call' => 'validate',
'_template' => $layout->getSlug(),
]);

$response->assertNotFound();
$response
->assertStatus(Response::HTTP_BAD_REQUEST);
}
}

0 comments on commit 5c8dd63

Please sign in to comment.