Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

Implementing JWKs #57

Merged
merged 46 commits into from
Sep 13, 2021
Merged

Implementing JWKs #57

merged 46 commits into from
Sep 13, 2021

Conversation

shytikov
Copy link
Contributor

Looks like a lot of changes, but I've tried to structure it logically:

  • code from jwt.go was split – Config were moved to config.go and the main middleware call was moved to main.go;
  • added jwks.go file which contains code from https://github.com/MicahParks/keyfunc
  • rest are support files from keyfunc.

I haven't modified logic from the initial jwt.go. All configurations which were added are optional, and jwks will kick in only if KeySetUrl will be provided. That will replace the original keyfunc with the one that is able to work with jwks.

I believe I could get away with smaller changes if I would reference https://github.com/MicahParks/keyfunc directly. But that would bring additional unused legacy dependencies to this middleware and to the rest of the projects which will be using it, so I decided to copy files instead.

I have tested it on my JWKs server, and it works (on my machine :), including background updates of the keys.

I'm open to any suggestions that would make this PR better. Many thanks in advance!

shytikov and others added 25 commits August 30, 2021 11:26
…artup should not be dependent on third-party server availability
@shytikov shytikov changed the title Issue 48 jwks Implementing JWKs Aug 31, 2021
@shytikov
Copy link
Contributor Author

shytikov commented Sep 6, 2021

@ReneWerner87 could you please check is it LGTM?

I can continue covering with tests these pieces of code. But I am afraid that I will delay the code and I'm also forced into refactoring as I write the tests, so I wanted to put a comma somewhere.

@shytikov
Copy link
Contributor Author

shytikov commented Sep 8, 2021

ping @ReneWerner87 :)

@ReneWerner87
Copy link
Member

Jo, sure.. Will check it on the weekend) currently to much other stuff)

@shytikov
Copy link
Contributor Author

shytikov commented Sep 8, 2021

Thank you! :)

@ReneWerner87 ReneWerner87 self-requested a review September 13, 2021 06:16
@ReneWerner87
Copy link
Member

@shytikov can you please correct the three comments of the automated test and the linter

@shytikov
Copy link
Contributor Author

On it! @ReneWerner87 :)

@shytikov
Copy link
Contributor Author

@ReneWerner87 Hopefully now everything shoudl be fine!

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

Successfully merging this pull request may close these issues.

Did this middleware support JWKS?
2 participants