Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security manager update #292

Open
wants to merge 17 commits into
base: v3.x
Choose a base branch
from
Open
Changes from 4 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
65 changes: 1 addition & 64 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
@@ -79,48 +79,9 @@ private function createAuthorizationServerNode(): NodeDefinition
->cannotBeEmpty()
->defaultValue('P1M')
->end()

// @TODO Remove in v4 start

->scalarNode('auth_code_ttl')
->info("How long the issued authorization code should be valid for.\nThe value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters")
->cannotBeEmpty()
->setDeprecated('"%path%.%node%" is deprecated, use "%path%.grant_types.authorization_code.auth_code_ttl" instead.')
->beforeNormalization()
->ifNull()
->thenUnset()
->end()
->end()
->booleanNode('require_code_challenge_for_public_clients')
->info('Whether to require code challenge for public clients for the authorization code grant.')
->setDeprecated('"%path%.%node%" is deprecated, use "%path%.grant_types.authorization_code.require_code_challenge_for_public_clients" instead.')
->beforeNormalization()
->ifNull()
->thenUnset()
->end()
->end()
->end()
;

foreach (OAuth2Grants::ALL as $grantType => $grantTypeName) {
$oldGrantType = 'authorization_code' === $grantType ? 'auth_code' : $grantType;

$node
->children()
->booleanNode(sprintf('enable_%s_grant', $oldGrantType))
->info(sprintf('Whether to enable the %s grant.', $grantTypeName))
->setDeprecated(sprintf('"%%path%%.%%node%%" is deprecated, use "%%path%%.grant_types.%s.enable" instead.', $grantType))
->beforeNormalization()
->ifNull()
->thenUnset()
->end()
->end()
->end()
;
}

// @TODO Remove in v4 end

$node->append($this->createAuthorizationServerGrantTypesNode());

$node
@@ -134,33 +95,9 @@ private function createAuthorizationServerNode(): NodeDefinition
if (isset($grantTypesWithRefreshToken[$grantType])) {
$grantTypeConfig['refresh_token_ttl'] = $grantTypeConfig['refresh_token_ttl'] ?? $v['refresh_token_ttl'];
}

// @TODO Remove in v4 start
$oldGrantType = 'authorization_code' === $grantType ? 'auth_code' : $grantType;

$grantTypeConfig['enable'] = $v[sprintf('enable_%s_grant', $oldGrantType)] ?? $grantTypeConfig['enable'];

if ('authorization_code' === $grantType) {
$grantTypeConfig['auth_code_ttl'] = $v['auth_code_ttl'] ?? $grantTypeConfig['auth_code_ttl'];
$grantTypeConfig['require_code_challenge_for_public_clients'] = $v['require_code_challenge_for_public_clients']
?? $grantTypeConfig['require_code_challenge_for_public_clients'];
}
// @TODO Remove in v4 end
}

unset(
$v['access_token_ttl'],
$v['refresh_token_ttl'],
// @TODO Remove in v4 start
$v['enable_auth_code_grant'],
$v['enable_client_credentials_grant'],
$v['enable_implicit_grant'],
$v['enable_password_grant'],
$v['enable_refresh_token_grant'],
$v['auth_code_ttl'],
$v['require_code_challenge_for_public_clients']
// @TODO Remove in v4 end
);
unset($v['access_token_ttl'], $v['refresh_token_ttl']);

return $v;
})
10 changes: 0 additions & 10 deletions OAuth2Grants.php
Original file line number Diff line number Diff line change
@@ -54,14 +54,4 @@ final class OAuth2Grants
self::PASSWORD => 'password',
self::REFRESH_TOKEN => 'refresh token',
];

/**
* @deprecated Will be removed in v4, use {@see OAuth2Grants::ALL} instead
*
* @TODO Remove in v4.
*/
public static function has(string $grant): bool
{
return isset(self::ALL[$grant]);
}
}
22 changes: 0 additions & 22 deletions README.md
Original file line number Diff line number Diff line change
@@ -68,28 +68,6 @@ This package is currently in the active development.
# The value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters
refresh_token_ttl: P1M

# How long the issued authorization code should be valid for.
# The value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters
auth_code_ttl: ~ # Deprecated ("trikoder_oauth2.authorization_server.auth_code_ttl" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.auth_code_ttl" instead.)

# Whether to require code challenge for public clients for the authorization code grant.
require_code_challenge_for_public_clients: ~ # Deprecated ("trikoder_oauth2.authorization_server.require_code_challenge_for_public_clients" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.require_code_challenge_for_public_clients" instead.)

# Whether to enable the authorization code grant.
enable_auth_code_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_auth_code_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.authorization_code.enable" instead.)

# Whether to enable the client credentials grant.
enable_client_credentials_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_client_credentials_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.client_credentials.enable" instead.)

# Whether to enable the implicit grant.
enable_implicit_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_implicit_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.implicit.enable" instead.)

# Whether to enable the password grant.
enable_password_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_password_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.password.enable" instead.)

# Whether to enable the refresh token grant.
enable_refresh_token_grant: ~ # Deprecated ("trikoder_oauth2.authorization_server.enable_refresh_token_grant" is deprecated, use "trikoder_oauth2.authorization_server.grant_types.refresh_token.enable" instead.)

# Enable and configure grant types.
grant_types:
authorization_code:
5 changes: 3 additions & 2 deletions Security/Exception/OAuth2AuthenticationFailedException.php
Original file line number Diff line number Diff line change
@@ -5,14 +5,15 @@
namespace Trikoder\Bundle\OAuth2Bundle\Security\Exception;

use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Throwable;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class OAuth2AuthenticationFailedException extends AuthenticationException
{
public static function create(string $message): self
public static function create(string $message, ?Throwable $previous = null): self
{
return new self($message, 401);
return new self($message, 401, $previous);
}
}
2 changes: 1 addition & 1 deletion Security/Firewall/OAuth2Listener.php
Original file line number Diff line number Diff line change
@@ -68,7 +68,7 @@ public function __invoke(RequestEvent $event)
/** @var OAuth2Token $authenticatedToken */
$authenticatedToken = $this->authenticationManager->authenticate($this->oauth2TokenFactory->createOAuth2Token($request, null, $this->providerKey));
} catch (AuthenticationException $e) {
throw new OAuth2AuthenticationFailedException($e->getMessage(), 401, $e);
throw OAuth2AuthenticationFailedException::create($e->getMessage(), $e);
}

if (!$this->isAccessToRouteGranted($event->getRequest(), $authenticatedToken)) {
71 changes: 7 additions & 64 deletions Tests/Unit/ExtensionTest.php
Original file line number Diff line number Diff line change
@@ -265,44 +265,12 @@ public function grantsProvider(): iterable
yield 'Refresh token grant can be disabled' => [
RefreshTokenGrant::class, 'refresh_token.enable', false,
];

yield 'Legacy authorization code grant can be enabled' => [
AuthCodeGrant::class, 'enable_auth_code_grant', true,
];
yield 'Legacy authorization code grant can be disabled' => [
AuthCodeGrant::class, 'enable_auth_code_grant', false,
];
yield 'Legacy client credentials grant can be enabled' => [
ClientCredentialsGrant::class, 'enable_client_credentials_grant', true,
];
yield 'Legacy client credentials grant can be disabled' => [
ClientCredentialsGrant::class, 'enable_client_credentials_grant', false,
];
yield 'Legacy implicit grant can be enabled' => [
ImplicitGrant::class, 'enable_implicit_grant', true,
];
yield 'Legacy implicit grant can be disabled' => [
ImplicitGrant::class, 'enable_implicit_grant', false,
];
yield 'Legacy password grant can be enabled' => [
PasswordGrant::class, 'enable_password_grant', true,
];
yield 'Legacy password grant can be disabled' => [
PasswordGrant::class, 'enable_password_grant', false,
];
yield 'Legacy refresh token grant can be enabled' => [
RefreshTokenGrant::class, 'enable_refresh_token_grant', true,
];
yield 'Legacy refresh token grant can be disabled' => [
RefreshTokenGrant::class, 'enable_refresh_token_grant', false,
];
}

/**
* @dataProvider requireCodeChallengeForPublicClientsProvider
*/
public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConfig(
string $configKey,
?bool $requireCodeChallengeForPublicClients,
bool $shouldTheRequirementBeDisabled
): void {
@@ -313,7 +281,7 @@ public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConf
$extension = new TrikoderOAuth2Extension();

$configuration = $this->getValidConfiguration([
$configKey => $requireCodeChallengeForPublicClients,
'authorization_code.require_code_challenge_for_public_clients' => $requireCodeChallengeForPublicClients,
]);

$extension->load($configuration, $container);
@@ -336,31 +304,20 @@ public function testAuthCodeGrantDisableRequireCodeChallengeForPublicClientsConf
public function requireCodeChallengeForPublicClientsProvider(): iterable
{
yield 'When not requiring code challenge for public clients the requirement should be disabled' => [
'authorization_code.require_code_challenge_for_public_clients', false, true,
false, true,
];
yield 'When code challenge for public clients is required the requirement should not be disabled' => [
'authorization_code.require_code_challenge_for_public_clients', true, false,
true, false,
];
yield 'With the default value the requirement should not be disabled' => [
'authorization_code.require_code_challenge_for_public_clients', null, false,
];

yield 'Legacy when not requiring code challenge for public clients the requirement should be disabled' => [
'require_code_challenge_for_public_clients', false, true,
];
yield 'Legacy when code challenge for public clients is required the requirement should not be disabled' => [
'require_code_challenge_for_public_clients', true, false,
];
yield 'Legacy with the default value the requirement should not be disabled' => [
'require_code_challenge_for_public_clients', null, false,
null, false,
];
}

/**
* @dataProvider authCodeTTLProvider
*/
public function testAuthCodeTTLConfig(
string $configKey,
?string $authCodeTTL,
string $expectedAuthCodeTTL
): void {
@@ -371,7 +328,7 @@ public function testAuthCodeTTLConfig(
$extension = new TrikoderOAuth2Extension();

$configuration = $this->getValidConfiguration([
$configKey => $authCodeTTL,
'authorization_code.auth_code_ttl' => $authCodeTTL,
]);

$extension->load($configuration, $container);
@@ -384,17 +341,10 @@ public function testAuthCodeTTLConfig(
public function authCodeTTLProvider(): iterable
{
yield 'Authorization code TTL can be set' => [
'authorization_code.auth_code_ttl', 'PT20M', 'PT20M',
'PT20M', 'PT20M',
];
yield 'When no authorization code TTL is set, the default is used' => [
'authorization_code.auth_code_ttl', null, 'PT10M',
];

yield 'Legacy authorization code TTL can be set' => [
'auth_code_ttl', 'PT20M', 'PT20M',
];
yield 'Legacy when no authorization code TTL is set, the default is used' => [
'auth_code_ttl', null, 'PT10M',
null, 'PT10M',
];
}

@@ -405,15 +355,8 @@ private function getValidConfiguration(array $options = []): array
'authorization_server' => [
'private_key' => 'foo',
'encryption_key' => 'foo',
'enable_auth_code_grant' => $options['enable_auth_code_grant'] ?? null,
'access_token_ttl' => $options['access_token_ttl'] ?? 'PT1H',
'refresh_token_ttl' => $options['refresh_token_ttl'] ?? 'P1M',
'enable_client_credentials_grant' => $options['enable_client_credentials_grant'] ?? null,
'enable_implicit_grant' => $options['enable_implicit_grant'] ?? null,
'enable_password_grant' => $options['enable_password_grant'] ?? null,
'enable_refresh_token_grant' => $options['enable_refresh_token_grant'] ?? null,
'require_code_challenge_for_public_clients' => $options['require_code_challenge_for_public_clients'] ?? null,
'auth_code_ttl' => $options['auth_code_ttl'] ?? null,
'grant_types' => [
'authorization_code' => [
'enable' => $options['authorization_code.enable'] ?? true,