Added check to handle token parsing with claims without "sub".

[wassafshahzad]

Description

In the oidc_introspection.py func (filter *oidcIntrospectionFilter) Request(ctx filters.FilterContext) function get value form the Claim map using sub key and then casts the value into a string, but this would panic if no sub key is present. I added a check to ensure we only cast values if sub key is present.

Linked Issue

closes #3216

Approach to solution

As describe in the following comment, I dug a bit deeper and found 2 suitable values to use if subject is not present in token. One is the Subject value in tokenContainer struct or UserInfo Struct. I decided to go with the tokenContainer struct. If this is not correct please do guide me if UserInfo Struct would be better.

[szuecs]

Can you please add a test case?
Thanks 🙏🏽

[wassafshahzad]

Can you please add a test case? Thanks 🙏🏽

Could you give me some pointers, I have been looking into multiple filter tests and oidc_intorspection_tests but cant wrap my head around on how to test it.

[MustafaSaber]

hi @wassafshahzad, thanks for your contribution! You can check

skipper/filters/auth/oidc_test.go

Line 224 in 8e6c365

claims := jwt.MapClaims{

&

skipper/filters/auth/oidc_introspection_test.go

Line 295 in 8e6c365

oidcServer := createOIDCServer(proxy.URL+"/redirect", validClient, "mysec", jwt.MapClaims{"groups": []string{"CD-Administrators", "Purchasing-Department", "AppX-Test-Users", "white space"}})

This server creates the token with those claims, I would try to find a way to override those claims and create another testcase where I create the server with the override and this case we expect the fail before the fix and succeed after.

The test case should fail or succeed after triggering

skipper/filters/auth/oidc_introspection_test.go

Line 355 in 8e6c365

resp, err := client.Do(req)

[wassafshahzad]

hi @wassafshahzad, thanks for your contribution! You can check

skipper/filters/auth/oidc_test.go

Line 224 in 8e6c365

claims := jwt.MapClaims{

&

skipper/filters/auth/oidc_introspection_test.go

Line 295 in 8e6c365

oidcServer := createOIDCServer(proxy.URL+"/redirect", validClient, "mysec", jwt.MapClaims{"groups": []string{"CD-Administrators", "Purchasing-Department", "AppX-Test-Users", "white space"}})

This server creates the token with those claims, I would try to find a way to override those claims and create another testcase where I create the server with the override and this case we expect the fail before the fix and succeed after.

The test case should fail or succeed after triggering

skipper/filters/auth/oidc_introspection_test.go

Line 355 in 8e6c365

resp, err := client.Do(req)

Thank you and on it

[wassafshahzad]

@MustafaSaber Sorry for the delay, I had a bit of a tough time resolving this issue. I tried using token.Subject and token.UserInfo.Subject in case sub was missing but this would still cause authentication issues. I still need help to identify the right key as I am not as familiar with the repo yet.

However I stumbled on to this piece of code from the jwt_validation file and decided to use it since we already have it for validation and thought consistency would be the right way to go.

[MustafaSaber]

Thanks for the update! We will review when we have time, we can't merge in the next couple of days also.

[wassafshahzad]

Thanks for the update! We will review when we have time, we can't merge in the next couple of days also.

No issue, Can I pick another issue ?

[MustafaSaber]

ofc feel free to pick one of the issues 😄