JwtAuthProviderReader problem with RequiresAudience

catmanjan commented 2 years ago

Describe the issue

If I call JwtAuthProviderReader.GetValidJwtPayload on my JWT it works as expected.

The first item in the payload is {[aud, api://22a6b621-fc19-4e08-9cad-c4b64ab7105b]}

If I set jwt.Audience to api://22a6b621-fc19-4e08-9cad-c4b64ab7105b and jwt.RequiresAudience to true in my appsettings.json, JwtAuthProviderReader.GetValidJwtPayload now returns null and my services are returning the error:

{ "responseStatus": { "errorCode": "TokenException", "message": "Invalid Audience: api://22a6b621-fc19-4e08-9cad-c4b64ab7105b", "errors": [] } }

Reproduction

Set up JwtAuthProviderReader as normal, check everything is working

Now enable jwt.RequiresAudience and it no longer works

Expected behavior

RequiresAudience should allow the token to be validated

System Info

Servicestack 6, .NET 6

Additional context

This probably isn't the place, but could you please enable my Servicestack forum account?

Validations

[X] Read the docs.
[X] Have an active ServiceStack License (GitHub Username can be registered on Support page).

catmanjan commented 2 years ago

I just had quick look at the repo and I believe the issue is that HasInvalidAudience will never return false when RequiresAudience is true, maybe its meant to be something like this?

public override bool HasInvalidAudience(JsonObject jwtPayload, out string audience)
{
    var hasValidAudience = false;

    if (jwtPayload.TryGetValue("aud", out audience))
    {
        var jwtAudiences = audience.FromJson<List<string>>();
        if (jwtAudiences?.Count > 0 && Audiences.Count > 0)
        {
            var containsAnyAudience = jwtAudiences.Any(x => Audiences.Contains(x));
            if (containsAnyAudience)
                hasValidAudience = true;
            if (!containsAnyAudience)
                return true;
        }
    }
    return hasValidAudience || RequiresAudience;
}

mythz commented 2 years ago

Invalid Audiences should now be validated in this commit.

This change is now available from v6.1.1 on MyGet

catmanjan commented 2 years ago

Is the function meant to return true if any of the audiences don’t match the configured audiences? I think this change will return false if any of the audiences are correct

On Mon, 23 May 2022 at 5:32 pm, Demis Bellot @.***> wrote:

Invalid Audiences should now be validated in this commit https://github.com/ServiceStack/ServiceStack/commit/97a10730028470f6c3f70912b60f7266c3294c0f .

This change is now available from v6.1.1 on MyGet https://docs.servicestack.net/myget

— Reply to this email directly, view it on GitHub https://github.com/ServiceStack/Issues/issues/775#issuecomment-1134291520, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABBRBWXZNU6LBLUFVOXOPTVLMYANANCNFSM5WUUJC3A . You are receiving this because you authored the thread.Message ID: @.***>

mythz commented 2 years ago

I think this change will return false if any of the audiences are correct

Right, multiple aud in JWT's is a niche case that I couldn't find any conclusive determination on what the correct default behavior should be in this case and the spec is also vague:

The interpretation of audience values is generally application specific.

What's your use case for having multiple audiences in a JWT?

catmanjan commented 2 years ago

Good point I just assumed it was part of spec, but I can see why that would be a problem!

ServiceStack / Issues