Skip to content

Commit

Permalink
fix: review
Browse files Browse the repository at this point in the history
  • Loading branch information
vincentchalamon committed Mar 18, 2024
1 parent b5a44b9 commit 62f4a99
Show file tree
Hide file tree
Showing 16 changed files with 159 additions and 92 deletions.
1 change: 0 additions & 1 deletion api/composer.json
Expand Up @@ -34,7 +34,6 @@
"symfony/validator": "7.0.*",
"symfony/yaml": "7.0.*",
"web-token/jwt-bundle": "^3.3",
"web-token/jwt-library": "^3.3",
"webonyx/graphql-php": "^15.8",
"zenstruck/foundry": "^1.36"
},
Expand Down
2 changes: 1 addition & 1 deletion api/composer.lock

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

1 change: 1 addition & 0 deletions api/config/packages/framework.yaml
Expand Up @@ -26,6 +26,7 @@ framework:

http_client:
scoped_clients:
# use scoped client to ease mock on functional tests
security.authorization.client:
base_uri: '%env(OIDC_SERVER_URL_INTERNAL)%/'

Expand Down
8 changes: 4 additions & 4 deletions api/config/packages/security.yaml
Expand Up @@ -29,13 +29,13 @@ when@prod: &prod
main:
access_token:
token_handler: App\Security\Http\AccessToken\Oidc\OidcDiscoveryTokenHandler
# todo support Discovery in Symfony
# oidc:
# claim: 'email'
# todo support Discovery in Symfony
# oidc:
# claim: 'email'
# base_uri: '%env(OIDC_SERVER_URL)%'
# audience: '%env(OIDC_AUD)%'
# cache: '@cache.app' # default
# cache_ttl: 3600 # default
# cache_ttl: 600 # default

when@dev: *prod

Expand Down
2 changes: 1 addition & 1 deletion api/src/Entity/Book.php
Expand Up @@ -71,7 +71,7 @@
AbstractNormalizer::GROUPS => ['Book:write'],
],
collectDenormalizationErrors: true,
security: 'is_granted("ADMIN")'
security: 'is_granted("OIDC_ADMIN")'
)]
#[ApiResource(
types: ['https://schema.org/Book', 'https://schema.org/Offer'],
Expand Down
4 changes: 2 additions & 2 deletions api/src/Entity/Bookmark.php
Expand Up @@ -36,7 +36,7 @@
operations: [
new GetCollection(),
new Delete(
security: 'object.user == user'
security: 'object.user === user'
),
new Post(
processor: BookmarkPersistProcessor::class
Expand All @@ -54,7 +54,7 @@
],
collectDenormalizationErrors: true,
mercure: true,
security: 'is_granted("USER")'
security: 'is_granted("OIDC_USER")'
)]
#[ORM\Entity(repositoryClass: BookmarkRepository::class)]
#[ORM\UniqueConstraint(fields: ['user', 'book'])]
Expand Down
8 changes: 4 additions & 4 deletions api/src/Entity/Review.php
Expand Up @@ -76,7 +76,7 @@
AbstractNormalizer::GROUPS => ['Review:write', 'Review:write:admin'],
],
collectDenormalizationErrors: true,
security: 'is_granted("ADMIN")'
security: 'is_granted("OIDC_ADMIN")'
)]
#[ApiResource(
types: ['https://schema.org/Review'],
Expand All @@ -98,7 +98,7 @@
]
),
new Post(
security: 'is_granted("USER")',
security: 'is_granted("OIDC_USER")',
// Mercure publish is done manually in MercureProcessor through ReviewPersistProcessor
processor: ReviewPersistProcessor::class,
provider: CreateProvider::class,
Expand All @@ -111,7 +111,7 @@
'bookId' => new Link(toProperty: 'book', fromClass: Book::class),
'id' => new Link(fromClass: Review::class),
],
security: 'object.user == user or is_granted("ADMIN")',
security: 'object.user === user',
// Mercure publish is done manually in MercureProcessor through ReviewPersistProcessor
processor: ReviewPersistProcessor::class
),
Expand All @@ -121,7 +121,7 @@
'bookId' => new Link(toProperty: 'book', fromClass: Book::class),
'id' => new Link(fromClass: Review::class),
],
security: 'object.user == user or is_granted("ADMIN")',
security: 'object.user === user',
// Mercure publish is done manually in MercureProcessor through ReviewRemoveProcessor
processor: ReviewRemoveProcessor::class
),
Expand Down
6 changes: 3 additions & 3 deletions api/src/Entity/User.php
Expand Up @@ -30,17 +30,17 @@
new GetCollection(
uriTemplate: '/admin/users{._format}',
itemUriTemplate: '/admin/users/{id}{._format}',
security: 'is_granted("ADMIN")',
security: 'is_granted("OIDC_ADMIN")',
filters: ['app.filter.user.admin.name'],
paginationClientItemsPerPage: true
),
new Get(
uriTemplate: '/admin/users/{id}{._format}',
security: 'is_granted("ADMIN")'
security: 'is_granted("OIDC_ADMIN")'
),
new Get(
uriTemplate: '/users/{id}{._format}',
security: 'user.sub === object.sub'
security: 'object === user'
),
],
normalizationContext: [
Expand Down
Expand Up @@ -33,7 +33,7 @@ public function __construct(
private JWSLoader $jwsLoader,
private readonly HttpClientInterface $securityAuthorizationClient,
private string $claim = 'email',
private int $ttl = 3600,
private int $ttl = 600,
private ?LoggerInterface $logger = null,
) {
}
Expand Down
34 changes: 0 additions & 34 deletions api/src/Security/Voter/OIDCVoterTrait.php

This file was deleted.

59 changes: 59 additions & 0 deletions api/src/Security/Voter/OidcRoleVoter.php
@@ -0,0 +1,59 @@
<?php

declare(strict_types=1);

namespace App\Security\Voter;

use Jose\Component\Signature\Serializer\JWSSerializerManager;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\AccessToken\AccessTokenExtractorInterface;

/**
* Check user roles from token.
*
* @see https://www.keycloak.org/docs/latest/authorization_services/index.html#obtaining-information-about-an-rpt
*/
final class OidcRoleVoter extends OidcVoter
{
public function __construct(
RequestStack $requestStack,
#[Autowire('@security.access_token_extractor.header')]
AccessTokenExtractorInterface $accessTokenExtractor,
#[Autowire('@jose.jws_serializer.oidc')]
private readonly JWSSerializerManager $jwsSerializerManager,
) {
parent::__construct($requestStack, $accessTokenExtractor);
}

protected function supports(string $attribute, mixed $subject): bool
{
return str_starts_with($attribute, 'OIDC_') && empty($subject);
}

protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
if (!empty($subject)) {
throw new \InvalidArgumentException(sprintf('Invalid subject type, expected empty string or "null", got "%s".', get_debug_type($subject)));
}

// ensure user is authenticated
if (!$token->getUser() instanceof UserInterface) {
return false;
}

$accessToken = $this->getToken();
if (!$accessToken) {
return false;
}

// OIDC server doesn't seem to answer: check roles in token (if present)
$jws = $this->jwsSerializerManager->unserialize($accessToken);
$claims = json_decode($jws->getPayload(), true);
$roles = array_map(static fn (string $role): string => strtolower($role), $claims['realm_access']['roles'] ?? []);

return \in_array(strtolower(substr($attribute, 5)), $roles, true);
}
}
Expand Up @@ -4,55 +4,56 @@

namespace App\Security\Voter;

use Jose\Component\Signature\Serializer\JWSSerializerManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\AccessToken\AccessTokenExtractorInterface;
use Symfony\Contracts\HttpClient\Exception\ExceptionInterface;
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
use Symfony\Contracts\HttpClient\HttpClientInterface;

/**
* Check user roles.
* Check user roles from OIDC server.
*
* @see https://www.keycloak.org/docs/latest/authorization_services/index.html#obtaining-information-about-an-rpt
*/
final class OIDCRoleVoter extends Voter
final class OidcTokenIntrospectRoleVoter extends OidcVoter
{
use OIDCVoterTrait;

public function __construct(
RequestStack $requestStack,
#[Autowire('@security.access_token_extractor.header')]
AccessTokenExtractorInterface $accessTokenExtractor,
#[Autowire('%env(OIDC_API_CLIENT_ID)%')]
private readonly string $oidcClientId,
#[Autowire('%env(OIDC_API_CLIENT_SECRET)%')]
private readonly string $oidcClientSecret,
private readonly HttpClientInterface $securityAuthorizationClient,
private readonly RequestStack $requestStack,
#[Autowire('@security.access_token_extractor.header')]
private readonly AccessTokenExtractorInterface $accessTokenExtractor,
#[Autowire('@jose.jws_serializer.oidc')]
private readonly JWSSerializerManager $jwsSerializerManager,
private ?LoggerInterface $logger = null,
) {
parent::__construct($requestStack, $accessTokenExtractor);
}

protected function supports(string $attribute, mixed $subject): bool
{
return empty($subject);
return str_starts_with($attribute, 'OIDC_INTROSPECT_') && empty($subject);
}

protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool
{
$accessToken = $this->getToken($token);
if (!$accessToken) {
if (!empty($subject)) {
throw new \InvalidArgumentException(sprintf('Invalid subject type, expected empty string or "null", got "%s".', get_debug_type($subject)));
}

// ensure user is authenticated
if (!$token->getUser() instanceof UserInterface) {
return false;
}

if (!empty($subject)) {
throw new \InvalidArgumentException(sprintf('Invalid subject type, expected empty string or "null", got "%s".', get_debug_type($subject)));
$accessToken = $this->getToken();
if (!$accessToken) {
return false;
}

try {
Expand All @@ -65,21 +66,17 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter
]);

$roles = array_map(static fn (string $role): string => strtolower($role), $response->toArray()['realm_access']['roles'] ?? []);
} catch (HttpExceptionInterface $e) {

return \in_array(strtolower(substr($attribute, 5)), $roles, true);
} catch (HttpExceptionInterface) {
// OIDC server said no!
return false;
} catch (ExceptionInterface $e) {
$this->logger?->error('An error occurred while checking the roles on OIDC server.', [
'error' => $e->getMessage(),
'trace' => $e->getTraceAsString(),
]);

// OIDC server doesn't seem to answer: check roles in token (if present)
$jws = $this->jwsSerializerManager->unserialize($accessToken);
$claims = json_decode($jws->getPayload(), true);
$roles = array_map(static fn (string $role): string => strtolower($role), $claims['realm_access']['roles'] ?? []);
}

return \in_array(strtolower($attribute), $roles, true);
return false;
}
}

0 comments on commit 62f4a99

Please sign in to comment.