diff --git a/ProcessMaker/Http/Middleware/EnsureAccountAllowsAccess.php b/ProcessMaker/Http/Middleware/EnsureAccountAllowsAccess.php index 5cedec09a4..5060cba876 100644 --- a/ProcessMaker/Http/Middleware/EnsureAccountAllowsAccess.php +++ b/ProcessMaker/Http/Middleware/EnsureAccountAllowsAccess.php @@ -3,6 +3,7 @@ namespace ProcessMaker\Http\Middleware; use Closure; +use Illuminate\Contracts\Auth\StatefulGuard; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; use ProcessMaker\Models\User; @@ -16,12 +17,15 @@ class EnsureAccountAllowsAccess private const DENIED_STATUSES = ['BLOCKED', 'INACTIVE']; /** - * If any active guard has a blocked/inactive user, log out and return the appropriate response. - * Used by the web middleware group and by ProcessMakerAuthenticate (covers all auth:api routes, including packages). + * If an authenticated user for one of the supplied guards is blocked/inactive, log out and return the appropriate response. + * + * @param array $guards */ - public static function blockingResponseForRequest(Request $request): ?Response + public static function blockingResponseForRequest(Request $request, array $guards = []): ?Response { - foreach (['web', 'api'] as $guard) { + foreach ($guards ?: [config('auth.defaults.guard')] as $guard) { + $guard = $guard ?: config('auth.defaults.guard'); + if (!Auth::guard($guard)->check()) { continue; } @@ -40,7 +44,7 @@ public static function blockingResponseForRequest(Request $request): ?Response public function handle(Request $request, Closure $next): Response { - $blocked = self::blockingResponseForRequest($request); + $blocked = self::blockingResponseForRequest($request, ['web']); if ($blocked !== null) { return $blocked; } @@ -50,11 +54,15 @@ public function handle(Request $request, Closure $next): Response public static function denyAccess(Request $request, string $guard, User $user): Response { - Auth::guard($guard)->logout(); + $authGuard = Auth::guard($guard); + + if ($authGuard instanceof StatefulGuard) { + $authGuard->logout(); - if ($request->hasSession()) { - $request->session()->invalidate(); - $request->session()->regenerateToken(); + if ($request->hasSession()) { + $request->session()->invalidate(); + $request->session()->regenerateToken(); + } } if ($guard === 'api' || $request->expectsJson()) { diff --git a/ProcessMaker/Http/Middleware/ProcessMakerAuthenticate.php b/ProcessMaker/Http/Middleware/ProcessMakerAuthenticate.php index b64c13812e..5d95579871 100644 --- a/ProcessMaker/Http/Middleware/ProcessMakerAuthenticate.php +++ b/ProcessMaker/Http/Middleware/ProcessMakerAuthenticate.php @@ -18,7 +18,7 @@ public function handle($request, Closure $next, ...$guards) { $this->authenticate($request, $guards); - if ($blocked = EnsureAccountAllowsAccess::blockingResponseForRequest($request)) { + if ($blocked = EnsureAccountAllowsAccess::blockingResponseForRequest($request, $guards)) { return $blocked; } diff --git a/tests/Feature/Http/Middleware/EnsureAccountAllowsAccessTest.php b/tests/Feature/Http/Middleware/EnsureAccountAllowsAccessTest.php new file mode 100644 index 0000000000..e359edefef --- /dev/null +++ b/tests/Feature/Http/Middleware/EnsureAccountAllowsAccessTest.php @@ -0,0 +1,136 @@ + 'ACTIVE']); + $webGuard = Mockery::mock(StatefulGuard::class); + $webGuard->shouldReceive('check')->once()->andReturn(true); + $webGuard->shouldReceive('user')->once()->andReturn($user); + + Auth::shouldReceive('guard')->with('web')->andReturn($webGuard); + Auth::shouldReceive('guard')->never()->with('api'); + + $request = Request::create('/', 'GET', [], [], [], [ + 'HTTP_AUTHORIZATION' => 'Bearer undefined', + ]); + + $response = (new EnsureAccountAllowsAccess())->handle($request, fn () => response('allowed')); + + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('allowed', $response->getContent()); + } + + #[DataProvider('deniedWebStatuses')] + public function test_web_middleware_denies_users_with_a_disallowed_status(string $status): void + { + $user = new User(['status' => $status]); + $webGuard = Mockery::mock(StatefulGuard::class); + $webGuard->shouldReceive('check')->once()->andReturn(true); + $webGuard->shouldReceive('user')->once()->andReturn($user); + $webGuard->shouldReceive('logout')->once(); + + Auth::shouldReceive('guard')->with('web')->andReturn($webGuard); + Auth::shouldReceive('guard')->never()->with('api'); + + $response = (new EnsureAccountAllowsAccess())->handle(Request::create('/', 'GET'), fn () => response('allowed')); + + $this->assertSame(302, $response->getStatusCode()); + $this->assertStringEndsWith('/login', $response->headers->get('Location')); + } + + public static function deniedWebStatuses(): array + { + return [ + 'blocked' => ['BLOCKED'], + 'inactive' => ['INACTIVE'], + ]; + } + + public function test_api_account_check_does_not_resolve_the_web_guard_for_an_active_user(): void + { + $user = new User(['status' => 'ACTIVE']); + $apiGuard = Mockery::mock(Guard::class); + $apiGuard->shouldReceive('check')->once()->andReturn(true); + $apiGuard->shouldReceive('user')->once()->andReturn($user); + + Auth::shouldReceive('guard')->never()->with('web'); + Auth::shouldReceive('guard')->with('api')->andReturn($apiGuard); + + $response = EnsureAccountAllowsAccess::blockingResponseForRequest( + Request::create('/api/test', 'GET'), + ['api'] + ); + + $this->assertNull($response); + } + + #[DataProvider('deniedApiStatuses')] + public function test_api_account_check_denies_users_with_a_disallowed_status(string $status, string $message): void + { + $user = new User(['status' => $status]); + $apiGuard = Mockery::mock(Guard::class); + $apiGuard->shouldReceive('check')->once()->andReturn(true); + $apiGuard->shouldReceive('user')->once()->andReturn($user); + + Auth::shouldReceive('guard')->never()->with('web'); + Auth::shouldReceive('guard')->with('api')->andReturn($apiGuard); + + $response = EnsureAccountAllowsAccess::blockingResponseForRequest( + Request::create('/api/test', 'GET', [], [], [], ['HTTP_ACCEPT' => 'application/json']), + ['api'] + ); + + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame(['message' => $message], json_decode($response->getContent(), true)); + } + + public static function deniedApiStatuses(): array + { + return [ + 'blocked' => ['BLOCKED', 'Account locked after too many failed attempts. Contact administrator.'], + 'inactive' => ['INACTIVE', 'Unauthorized'], + ]; + } + + public function test_processmaker_authenticate_checks_only_the_authenticated_api_guard(): void + { + $user = new User(['status' => 'INACTIVE']); + $apiGuard = Mockery::mock(Guard::class); + $apiGuard->shouldReceive('check')->once()->andReturn(true); + $apiGuard->shouldReceive('user')->once()->andReturn($user); + + Auth::shouldReceive('guard')->never()->with('web'); + Auth::shouldReceive('guard')->with('api')->andReturn($apiGuard); + + $middleware = new class(app('auth')) extends ProcessMakerAuthenticate { + protected function authenticate($request, array $guards) + { + // Authentication is outside the scope of this guard-selection test. + } + }; + + $response = $middleware->handle( + Request::create('/api/test', 'GET', [], [], [], ['HTTP_ACCEPT' => 'application/json']), + fn () => response('allowed'), + 'api' + ); + + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame(['message' => 'Unauthorized'], json_decode($response->getContent(), true)); + } +} diff --git a/tests/k6/etag/README.md b/tests/k6/etag/README.md new file mode 100644 index 0000000000..8c42b32623 --- /dev/null +++ b/tests/k6/etag/README.md @@ -0,0 +1,20 @@ +# ETag k6 tests + +The ETag tests require a valid ProcessMaker OAuth bearer token. They do not run with a missing, `null`, `undefined`, or `FAKE_JWT` token. + +Set the target URL and token through environment variables: + +```bash +PM_K6_BASE_URL=https://performance2-qa.processmaker.net \ +PM_K6_TOKEN='' \ +k6 run tests/k6/etag/performance-test.js +``` + +Use the same variables for the other tests: + +```bash +k6 run tests/k6/etag/long-duration-test.js +k6 run tests/k6/etag/detect-dynamic-endpoints.js +``` + +Do not set an `Authorization` header outside these scripts when the token is unavailable. Test setup must fail instead of sending `Bearer null` or `Bearer undefined`. diff --git a/tests/k6/etag/config.js b/tests/k6/etag/config.js index 843d73bcc4..09455af96f 100644 --- a/tests/k6/etag/config.js +++ b/tests/k6/etag/config.js @@ -1,11 +1,28 @@ +const baseUrl = (__ENV.PM_K6_BASE_URL || '').replace(/\/$/, ''); + export const config = { - token: 'FAKE_JWT', // Replace with a valid token. + token: __ENV.PM_K6_TOKEN || '', endpoints: { - startProcesses: 'https://processmaker.test/api/1.0/start_processes?page=1&per_page=15&filter=&order_by=category.name%2Cname&order_direction=asc%2Casc&include=events%2Ccategories&without_event_definitions=true', + startProcesses: `${baseUrl}/api/1.0/start_processes?page=1&per_page=15&filter=&order_by=category.name%2Cname&order_direction=asc%2Casc&include=events%2Ccategories&without_event_definitions=true`, }, }; -export const headers = { - 'Content-Type': 'application/json', - Authorization: `Bearer ${config.token}`, -}; \ No newline at end of file +export function validateConfiguration() { + if (!baseUrl) { + throw new Error('PM_K6_BASE_URL is required.'); + } + + const token = config.token.trim(); + if (!token || ['null', 'undefined', 'fake_jwt'].includes(token.toLowerCase())) { + throw new Error('PM_K6_TOKEN must contain a valid bearer token.'); + } +} + +export function getHeaders() { + validateConfiguration(); + + return { + 'Content-Type': 'application/json', + Authorization: `Bearer ${config.token}`, + }; +} diff --git a/tests/k6/etag/detect-dynamic-endpoints.js b/tests/k6/etag/detect-dynamic-endpoints.js index 091a3722a1..d259fa0f6f 100644 --- a/tests/k6/etag/detect-dynamic-endpoints.js +++ b/tests/k6/etag/detect-dynamic-endpoints.js @@ -1,5 +1,5 @@ import http from 'k6/http'; -import { config, headers } from './config.js'; +import { config, getHeaders, validateConfiguration } from './config.js'; export let options = { vus: 1, @@ -16,7 +16,13 @@ const etagHistory = {}; // Limit to determine when an endpoint is considered dynamic. const ETAG_HISTORY_LIMIT = 5; +export function setup() { + validateConfiguration(); +} + export default function () { + const headers = getHeaders(); + endpoints.forEach((url) => { const res1 = http.get(url, { headers }); const etag = res1.headers['Etag']; @@ -54,4 +60,4 @@ export default function () { console.log(`Dynamic endpoint detected: ${url}`); } }); -} \ No newline at end of file +} diff --git a/tests/k6/etag/long-duration-test.js b/tests/k6/etag/long-duration-test.js index 5ed1651568..802e04ff47 100644 --- a/tests/k6/etag/long-duration-test.js +++ b/tests/k6/etag/long-duration-test.js @@ -1,7 +1,7 @@ import http from 'k6/http'; import { check, sleep } from 'k6'; import { Trend } from 'k6/metrics'; -import { config, headers } from './config.js'; +import { config, getHeaders, validateConfiguration } from './config.js'; // Metrics to track response durations. let duration200 = new Trend('duration_200'); @@ -18,8 +18,13 @@ export let options = { }, }; +export function setup() { + validateConfiguration(); +} + export default function () { const url = config.endpoints.startProcesses; + const headers = getHeaders(); let res = http.get(url, { headers }); duration200.add(res.timings.duration); check(res, { @@ -40,4 +45,4 @@ export default function () { } sleep(1); -} \ No newline at end of file +} diff --git a/tests/k6/etag/performance-test.js b/tests/k6/etag/performance-test.js index 0805d98ceb..47fd63efdf 100644 --- a/tests/k6/etag/performance-test.js +++ b/tests/k6/etag/performance-test.js @@ -1,7 +1,7 @@ import http from 'k6/http'; import { check, sleep, group } from 'k6'; import { Trend } from 'k6/metrics'; -import { config, headers } from './config.js'; +import { config, getHeaders, validateConfiguration } from './config.js'; // Custom metrics to track request durations. let duration200 = new Trend('duration_200'); @@ -15,8 +15,13 @@ export let options = { ], }; +export function setup() { + validateConfiguration(); +} + export default function () { const baseUrl = config.endpoints.startProcesses; + const headers = getHeaders(); group('ETag Performance', () => { // Add the duration of the 200 response to the custom metric. let res = http.get(baseUrl, { headers }); @@ -43,4 +48,4 @@ export default function () { sleep(1); }); -} \ No newline at end of file +}