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

Add type to JWT decode header #119

Merged
merged 5 commits into from
Jan 16, 2024
Merged

Add type to JWT decode header #119

merged 5 commits into from
Jan 16, 2024

Conversation

AlyHKafoury
Copy link
Contributor

jws.go Show resolved Hide resolved
Copy link
Member

@taik0 taik0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AlyHKafoury can you add the requested changes?

Thank you!

@AlyHKafoury
Copy link
Contributor Author

Hi @AlyHKafoury can you add the requested changes?

Thank you!

I have added the required changes, can I ask for your review please ?

@AlyHKafoury
Copy link
Contributor Author

I am fixing the tests problems

@AlyHKafoury
Copy link
Contributor Author

@kpacha is there a correct method to generate the signed JWT payloads for tests ?

@kpacha
Copy link
Member

kpacha commented Oct 4, 2023

There are some keys at the fixtures folder. you can use symmetric or asymmetric alg. you can instantiate a Signer (https://github.com/krakend/krakend-jose/blob/master/gin/jose.go#L24) and use it to sign your test token

@alombarte
Copy link
Member

Hi @AlyHKafoury, we have selected this feature to release with KrakenD 2.5, which will be out really soon. Will you be able to complete the tests?

@AlyHKafoury
Copy link
Contributor Author

AlyHKafoury commented Oct 23, 2023 via email

@alombarte
Copy link
Member

Hello @AlyHKafoury,

We will be preparing the release of KrakenD CE 2.5 early next week. I just wanted to ping you to see if you can complete this. Otherwise we will introduce this functionality in krakenD 2.6 (2024)

@alombarte
Copy link
Member

alombarte commented Jan 11, 2024

Hi @AlyHKafoury,

Are you still interested in finishing this for the next release 2.6?

@AlyHKafoury
Copy link
Contributor Author

Yes I will do my best to fix the unit tests I am just not very familiar with the key generation

@AlyHKafoury
Copy link
Contributor Author

@alombarte I made changes to the test case encryption once it is reviewed I believe the code should pass the test case passes locally now

@AlyHKafoury
Copy link
Contributor Author

@kpacha can you please re-review

@AlyHKafoury
Copy link
Contributor Author

@alombarte Will re run the signer

@kpacha
Copy link
Member

kpacha commented Jan 16, 2024

@AlyHKafoury I've created a PR on your fork fixing the failling tests. Once you approve and merge it, we'll be able to merge and close this PR

fix unit tests after extending the token header
@kpacha kpacha merged commit 0b6a550 into krakend:master Jan 16, 2024
3 checks passed
@kpacha
Copy link
Member

kpacha commented Jan 16, 2024

Thanks for the contribution!!!

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

Successfully merging this pull request may close these issues.

4 participants