Added check to handle token parsing with claims without "sub". #3287
Conversation
wassafshahzad
force-pushed
the
fix/claim-parsing-oidc-introspection
branch
from
03721d1
to
c7467c4
Compare
|
Can you please add a test case? |
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. |
|
hi @wassafshahzad, thanks for your contribution! You can check skipper/filters/auth/oidc_test.go Line 224 in 8e6c365 & 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 |
Thank you and on it |
…e token subjact if sub is not present in claims Signed-off-by: wassafshahzad <wassafshahzad@gmail.com>
Signed-off-by: wassafshahzad <wassafshahzad@gmail.com>
wassafshahzad
force-pushed
the
fix/claim-parsing-oidc-introspection
branch
from
2f71fb9
to
3cfb9fa
Compare
|
@MustafaSaber Sorry for the delay, I had a bit of a tough time resolving this issue. I tried using 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. |
|
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 ? |
|
ofc feel free to pick one of the issues 😄 |
Description
In the oidc_introspection.py
func (filter *oidcIntrospectionFilter) Request(ctx filters.FilterContext)function get value form the Claim map usingsubkey 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.