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

Multi-value for audience param #79

Open
smehdux opened this issue Nov 20, 2023 · 2 comments
Open

Multi-value for audience param #79

smehdux opened this issue Nov 20, 2023 · 2 comments
Assignees
Labels

Comments

@smehdux
Copy link

smehdux commented Nov 20, 2023

Hello,

We are currently developing an Angular frontend to call our API Symfony where OIDC is used to call REST Controller (with JWT included in the headers).

As the frontend calls several APIs which are secured with the same strategy as done with Symfony, the user (Angular) send a JWT Token with a audience value set by our IDP, let's say "myAngular".

Now the issue with lexik-jose-bridge, if the audience is not set in the configuration, the server_name value is used by default and fall in 401 ERROR because the value "myAngular" <> "serveur_name_value" !

I think using a single value in the configuration (audience) leads to a big issue in case where this API could be called by many clients (SPA, Java, etc...) as the clients cannot have the same audience value.

How to manage to force Symfony API accepting many values under "audience" param ? is any workaround ?

I noticed that the class AudienceChecker from the bundle "web-token/jwt-checker": "^3.2" accepts an array to declare many values (ie. the checkValue method) , so why in the bridge you've change this behavior ?

<?php

declare(strict_types=1);

namespace Jose\Component\Checker;

use function in_array;
use function is_array;
use function is_string;

/**
 * This class is a header parameter and claim checker. When the "aud" header parameter or claim is present, it will
 * check if the value is within the allowed ones.
 */
final class AudienceChecker implements ClaimChecker, HeaderChecker
{
    private const CLAIM_NAME = 'aud';

    public function __construct(
        private readonly string $audience,
        private readonly bool $protectedHeader = false
    ) {
    }

    /**
     * {@inheritdoc}
     */
    public function checkClaim(mixed $value): void
    {
        $this->checkValue($value, InvalidClaimException::class);
    }

    /**
     * {@inheritdoc}
     */
    public function checkHeader(mixed $value): void
    {
        $this->checkValue($value, InvalidHeaderException::class);
    }

    public function supportedClaim(): string
    {
        return self::CLAIM_NAME;
    }

    public function supportedHeader(): string
    {
        return self::CLAIM_NAME;
    }

    public function protectedHeaderOnly(): bool
    {
        return $this->protectedHeader;
    }

    private function checkValue(mixed $value, string $class): void
    {
        if (is_string($value) && $value !== $this->audience) {
            throw new $class('Bad audience.', self::CLAIM_NAME, $value);
        }
        if (is_array($value) && ! in_array($this->audience, $value, true)) {
            throw new $class('Bad audience.', self::CLAIM_NAME, $value);
        }
        if (! is_array($value) && ! is_string($value)) {
            throw new $class('Bad audience.', self::CLAIM_NAME, $value);
        }
    }
}

Thanks

Mehdi

@Spomky
Copy link
Member

Spomky commented Nov 23, 2023

Hi,

It's a good question. I can't remember exactly why it was designed that way, but you're right, the audience could be a list and the bundle should reflect that.

@Spomky Spomky self-assigned this Nov 23, 2023
@Spomky Spomky added the bug label Nov 23, 2023
@smehdux
Copy link
Author

smehdux commented Nov 23, 2023

Hi,

It's a good question. I can't remember exactly why it was designed that way, but you're right, the audience could be a list and the bundle should reflect that.

Hi,
For the moment our backend is intended to be called from only one client, but if we had to cope with many clients we'll be blocked as it's not recommended to use the same audience for all clients.

I'll request for this feature to be added as implemented in the "web-token/jwt-checker" bundle's AudienceChecker class .

Mehdi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants