Skip to content

Conversation

@Erwane
Copy link
Contributor

@Erwane Erwane commented Feb 3, 2020

Skip cakephp/authorization plugin for DebugKit requests only

@markstory markstory added this to the 3.x milestone Feb 4, 2020
// try/catch/finally instead of expectExcetion
// to restore `debug` configuration
try {
$controller->beforeFilter($event);
Copy link
Member

Choose a reason for hiding this comment

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

Integration tests generally make requests to the application and check the response contents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm ok with you about testDebugDisabled but for the testSkipAuthorization i should test the AuthorizationService::authorizationChecked() status, and i only can do that thrue controller request.
Any ideas ?

$this->assertInstanceOf('Cake\Http\Exception\NotFoundException', $e);
$this->assertSame('Not available without debug mode on.', $e->getMessage());
} finally {
Configure::write('debug', $oldStatus);
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to do this. The test harness will reset Configure state during its teardown method.

}

// Skip authorization for DebuKit requests
$authorizationService = $this->getRequest()->getAttribute('authorization');
Copy link
Member

Choose a reason for hiding this comment

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

I thought this was going to be a behavior that users could turn on. Something like DebugKit.safeTLD works.

@markstory markstory mentioned this pull request Feb 4, 2020
@Erwane
Copy link
Contributor Author

Erwane commented Feb 4, 2020

Thanks. I thought my code was clean ;)
I will do better.

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Tests look much better now. Thank you for taking the time to update them.

if (Configure::read('DebugKit.ignoreAuthorization')) {
$authorizationService->skipAuthorization();
} else {
Log::info(
Copy link
Member

Choose a reason for hiding this comment

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

👍 This is a nice inclusion.

@markstory markstory merged commit f2ed910 into cakephp:3.x Feb 6, 2020
markstory pushed a commit that referenced this pull request Feb 6, 2020
Add configuration option to skip authorization for DebugKit requests.

Port #734 to 4.x
@Erwane Erwane deleted the 729-v3-authorization-fail branch February 6, 2020 07:05
markstory pushed a commit that referenced this pull request Feb 7, 2020
Add configuration option to skip authorization for DebugKit requests.

Port #734 to 4.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants