joken-elixir / joken_jwks Goto Github PK

View Code? Open in Web Editor NEW

29.0 6.0 24.0 129 KB

A Joken 2 hook for fetching the signer from a public JWKS url

License: Apache License 2.0

Elixir 100.00%

joken jwt jwks

joken_jwks's People

Contributors

Stargazers

Watchers

joken_jwks's Issues

Log level not respected, follows default Logger level regardless of configuration

Regardless of either of the documented configurations for log level:

# lib/app/apple_strategy.ex
defmodule App.AppleStrategy do
  use JokenJwks.DefaultStrategyTemplate

  def init_opts(opts) do
    url = "https://appleid.apple.com/auth/keys"
    Keyword.merge(opts, jwks_url: url, log_level: :warn)
  end
end

# config/config.exs
config :joken_jwks,
  log_level: :warn

The logs emitted follow the Logger.level, debug shown below:

[info] GET https://appleid.apple.com/auth/keys -> 200 (572.966 ms)
[debug]
>>> REQUEST >>>
(no query)
(no headers)
(no body)

<<< RESPONSE <<<
server: Apple
date: Thu, 15 Sep 2022 13:27:25 GMT
content-type: application/json;charset=ISO-8859-1
content-length: 1439
connection: keep-alive
cache-control: no-store
pragma: no-cache
access-control-allow-origin: *

{
  "keys": [
    {
      "kty": "RSA",
      "kid": "W6WcOKB",
      "use": "sig",
      "alg": "RS256",
      "n": "2Zc5d0-zkZ5AKmtYTvxHc3vRc41YfbklflxG9SWsg5qXUxvfgpktGAcxXLFAd9Uglzow9ezvmTGce5d3DhAYKwHAEPT9hbaMDj7DfmEwuNO8UahfnBkBXsCoUaL3QITF5_DAPsZroTqs7tkQQZ7qPkQXCSu2aosgOJmaoKQgwcOdjD0D49ne2B_dkxBcNCcJT9pTSWJ8NfGycjWAQsvC8CGstH8oKwhC5raDcc2IGXMOQC7Qr75d6J5Q24CePHj_JD7zjbwYy9KNH8wyr829eO_G4OEUW50FAN6HKtvjhJIguMl_1BLZ93z2KJyxExiNTZBUBQbbgCNBfzTv7JrxMw",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "kid": "YuyXoY",
      "use": "sig",
      "alg": "RS256",
      "n": "1JiU4l3YCeT4o0gVmxGTEK1IXR-Ghdg5Bzka12tzmtdCxU00ChH66aV-4HRBjF1t95IsaeHeDFRgmF0lJbTDTqa6_VZo2hc0zTiUAsGLacN6slePvDcR1IMucQGtPP5tGhIbU-HKabsKOFdD4VQ5PCXifjpN9R-1qOR571BxCAl4u1kUUIePAAJcBcqGRFSI_I1j_jbN3gflK_8ZNmgnPrXA0kZXzj1I7ZHgekGbZoxmDrzYm2zmja1MsE5A_JX7itBYnlR41LOtvLRCNtw7K3EFlbfB6hkPL-Swk5XNGbWZdTROmaTNzJhV-lWT0gGm6V1qWAK2qOZoIDa_3Ud0Gw",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "kid": "fh6Bs8C",
      "use": "sig",
      "alg": "RS256",
      "n": "u704gotMSZc6CSSVNCZ1d0S9dZKwO2BVzfdTKYz8wSNm7R_KIufOQf3ru7Pph1FjW6gQ8zgvhnv4IebkGWsZJlodduTC7c0sRb5PZpEyM6PtO8FPHowaracJJsK1f6_rSLstLdWbSDXeSq7vBvDu3Q31RaoV_0YlEzQwPsbCvD45oVy5Vo5oBePUm4cqi6T3cZ-10gr9QJCVwvx7KiQsttp0kUkHM94PlxbG_HAWlEZjvAlxfEDc-_xZQwC6fVjfazs3j1b2DZWsGmBRdx1snO75nM7hpyRRQB4jVejW9TuZDtPtsNadXTr9I5NjxPdIYMORj9XKEh44Z73yfv0gtw",
      "e": "AQAB"
    }
  ]
}

Soften `hackney` dependency version constraint

This library locks down the hackney dependency to ~> 1.17.4 or ~> 1.18.0. This prevents newer versions of Hackney (current version is 1.20.1).

Why is hackney locked down to the minor release number? By stating ~> 1.17.4 or ~> 1.18, we would lock down only the major version, thus allowing compatible minor version upgrades.

Upgrade `hackney` dependency

This library locks down the hackney dependency to ~> 1.17.4 or ~> 1.18.0. This prevents our application from using newer versions of Hackney (current version is 1.20.1).

Please add newer versions to the constraint.

Add example for multiple JWKS url's

I love that you guys have put the time and effort in to make this easy.

In the case someone has many jwks URL's (many integrations vs one), will it be safe for ets table and supervised task to have the jwks URL be dynamic like:

def init_opts(opts) do
  url = one_of_many_jwks_urls
  Keyword.merge(opts, jwks_url: url)
end

If so, it may be worth mentioning in docs that this can be done since it would be common to re-use an integration strategy with others.

Add support for kid-less jwks signature validation

The documentation states that JWKS mandates a kid claim, but it seems that this is not the case in both the JWS and JWK RFCs, even though there's a statement that says that kid is used to match a JWS, it does not indicate that is a requirement.

I'm implementing application-based access using Teleport and they do not add a kid claim in the header or in JWKS. I'm able to correctly validate JWKS with a stripped-down version of this library.

What do you think about allowing this use case?

JokenJwks.DefaultStrategy module does not exist

The examples in the documentation refer to the JokenJwks.DefaultStrategy module, but it does not exist in the project.

ets table not being cleaned up, causes application crash

Hi, I've noticed that my application is crashing periodically. I've tracked down this library as at least contributing to the crash. For whatever reason my supervisor is attempting to restart my JWK strategy but it cannot because the ETS table that it wants to create already exists:

** (exit) an exception was raised:
    ** (ArgumentError) errors were found at the given arguments:

  * 1st argument: table name already exists

        (stdlib 4.2) :ets.new(MyApp.Auth.JwkStrategy.EtsCache, [:set, :public, :named_table, {:read_concurrency, true}, {:write_concurrency, true}])
        (my_app 0.1.0) lib/my_app/auth/jwk_strategy.ex:11: MyApp.Auth.JwkStrategy.EtsCache.new/0
        ( my_app 0.1.0) lib/my_app/auth/jwk_strategy.ex:11: MyApp.Auth.JwkStrategy.start_link/1

I'm not sure why it's being restarted in the first place (I think it is happening after a recompile in dev), but the supervisor is unable to start the JWK strategy and it reaches the retry policy limits which in turn crashes the parent supervisor and ultimately the whole application.

Looking at the implementation I see that the ets table is being created in the start_link function of the strategy: https://github.com/joken-elixir/joken_jwks/blob/master/lib/joken_jwks/default_strategy_template.ex#L190

Since that start_link is being called by my supervisor, it means that the supervisor owns the table. Therefore whenever the strategy itself restarts under the supervisor, it tries to re-create the ets table and fails. I think the ets table should probably be started by the actual worker process here.

I think a workaround for me is to check if the table exists in an init_opts callback and delete it if it already exists. I would be glad to submit a PR to fix the underlying problem, but looking at the actual code I'm a bit curious why it's not just using genserver? That would make this a bit simpler to solve since the table could be created in the genserver's init callback.

1st argument: the table identifier does not refer to an existing ETS table

Hi guys! I need your help!
I am trying to verify idToken from google user(google sign in)
Here is my implementation

google_strategy.ex

defmodule MyApp.Account.GoogleStrategy do
  use JokenJwks.DefaultStrategyTemplate

  def init_opts(opts) do
    url = "https://www.googleapis.com/oauth2/v3/certs"
    Keyword.merge(opts, jwks_url: url)
  end
end

google_token.ex

defmodule MyApp.Account.GoogleToken do
  use Joken.Config

  add_hook(JokenJwks, strategy: MyApp.Account.GoogleStrategy)

  def token_config do
    auds = [
     .....................
    ]

    # Validate from the token
    default_claims()
    |> add_claim("iss", nil, &(&1 == "https://accounts.google.com"))
    |> add_claim(
      "aud",
      nil,
      &Enum.member?(auds, &1)
    )
  end
end

And trying

GoogleToken.verify_and_validate(id_token)

And got this error.

(stdlib 3.15.1) :ets.lookup(MyApp.Account.GoogleStrategy.EtsCache, :signers)
    (my_app 0.1.0) lib/my_app/account/google_strategy.ex:2: MyApp.Account.GoogleStrategy.EtsCache.get_signers/0
    (my_app 0.1.0) lib/my_app/account/google_strategy.ex:2: MyApp.Account.GoogleStrategy.match_signer_for_kid/2
    (joken_jwks 1.4.1) lib/joken_jwks.ex:54: JokenJwks.before_verify/2
    (joken 2.3.0) lib/joken/hooks.ex:199: anonymous fn/3 in Joken.Hooks.run_before_hook/3
    (elixir 1.13.1) lib/enum.ex:4475: Enumerable.List.reduce/3
    (elixir 1.13.1) lib/enum.ex:2442: Enum.reduce_while/3
    (joken 2.3.0) lib/joken/hooks.ex:196: Joken.Hooks.run_before_hook/3
    (joken 2.3.0) lib/joken.ex:253: Joken.verify/3
    (joken 2.3.0) lib/joken.ex:299: Joken.verify_and_validate/5

I am using joken_jwks 1.4
What am I missing?

Advice on unit testing with Joken JWKS?

I have a project using joken_jwks. I've been writing some unit tests for my business logic, but whenever I try to verify_and_validate a token in a unit test, I get various failures because the tokens I'm testing with aren't valid according to the JWKS signers fetched.

I found some useful testing code that I tried copying:

The one thing I didn't copy was the Tesla mock for the JWKS URL, but maybe that's required?

Even then, when I create a token in a unit test it fails verification/validation. Perhaps there is a way to configure joken_jwks in config/test.exs to make this easier? Or something I can add to the module I'm calling use JokenJwks.DefaultStrategyTemplate in? I could mock/stub my use Joken.Config module and its verify_and_exit/1 call, but I'd prefer not to mock/stub.

Any advice would be appreciated; thanks! 🙏

quick question on pre-fetching from jwks url

I have integrated jwks in to my application and it works quite nicely, however I notice that it does not fetch against the jwks url until the first attempt, so each time I reboot my server, I have to log in once (which does not validate) at which point I see joken/jwks go and fetch the keys and then all subsequent logins work. I have tried variations of the init options (should_start and first_fetch_sync) but they do nothing.

I suppose I could start a Task at application startup to verify a well-known token to jumpstart the process, but was wondering if I was missing something obvious.

thanks

Question about manually refetch the token from public API (e.g. google).

Thank you, for the very nice library!
I have one question for sync retrying logic.
Is it possible synchronously to refetch the public_key on error?
For example,

There is request with JWT TOKEN generated with the new public key (e.g. with google public key).
On validation of the JWT TOKEN with GoogleToken.verify_and_validate(token) there is error for the token.
What kind of error will get one (2)? How could I refetch sync the token and retry manually on point (2)?
At the moment if I understand correctly the flow, the new token will be refetch base on how often the time_interval and all request meanwhile with the new token will be failing.

Implementation is out of sync with Microsoft

Hey!

The google key page looks like this:

{
  "keys": [
    {
      "alg": "RS256",
      "use": "sig",
      "e": "AQAB",
      "n": "t-EAePKVyMaQbjG96EP98IIB7-CJLeo4AZOdc1WVTfMZgdbN_csbY1WP25CrlXvhaIewRNEKTF9WkKGvsxowpYJ_18rtOYnz94mn9s_EvJaBtoEcixXedwMwniw78ayLyi4IGzCLhUopgLnwAFardde9ZxpEAVqMK3q4EdScMLebrdrTu63oZ2EpLLIvuC5tjitFXLtNb5v2yiOElX3nXntOF9OYTtpCRzKRVOZ1Lqcj7G3oWmmBmLrR-fRc5yFpLFRVHu-vdp4BGUh96t2flz95QxhIRF0zcuvRiCPWjdiRZgJ8wiSy627XeINqKaoVycW0TofFcz2xAix9GuNdqQ",
      "kty": "RSA",
      "kid": "e197bf2e87bd1905575f9b6e5eb426eda5d574e3"
    },
    {
      "kty": "RSA",
      "n": "sxnJ-oXmCxTyR_ORbJQB55-VwvQWZfqFAx9eszyQvnbkYFlDFr0Mat89i2563KXANiDypqeAWVbKXQDdnhsAmLdPUHDzfFHWY_LvLDcUiExd_-w-lkXETNEM_-mvFnj3amz8atK1Q0Gy8wfPRf0-we0MjEdJ0cGSNQLOrjghvyyVNSL0nK1wTfJ51vLFPrRpEVXkxvKMVH2hOdBpNoLBvzl1nfgvvZbLhC6CbJ3ATTBzVJoaHGTtBvQizvZhHKLbr72B0VjK6Prt2VKL-1FFC1sDj7JFEP0G1Z3ikI6ffC6OTIsvWNhHE9R4CcPUPO6YVJ1K_i4PSfPMPRKBTITDSw",
      "kid": "6adc101cc7498c09c010dc3d5176fa97c9627ecb",
      "e": "AQAB",
      "alg": "RS256",
      "use": "sig"
    },
    {
      "kty": "RSA",
      "alg": "RS256",
      "e": "AQAB",
      "use": "sig",
      "n": "s44bQ6JmMh-9YBCyCdpbfslwFQ9mloCTgBiX3mwzrBUkliwBRBt5-jJKTXNz_IKERRf43grdSBb3mUiNwq-I6H6EHU0ueyiliGS38rTOrZSK9LM0qy-I8mSNc7p-5MA4Yu-gkBBfvicQ9GZfwlFZpoXt6UIVXywtvNuQNtRsx5oJ8PtbmMPCcA5aFkFl-8YS-4lM6ZNTc9Q6UgWFap3sM9kfCiuISmJs0_SNOzlbLu4FJEA2ZIEqM-aV7kciE4jTeR0W3ks3SotiwitHTvQF89mADa8qEzh5xA0HagKDWnoT0TdF80hdT2lsvggL2r5tllw3gyCVL0LT_pjb12841w",
      "kid": "d4cba25e563660a9009d820a1c02022070574e82"
    }
  ]
}

The MSFT one (both their generic and their Azure AD B2C one) looks like:

{"keys":[{"kty":"RSA","use":"sig","kid":"kg2LYs2T0CTjIfj4rt6JIynen38","x5t":"kg2LYs2T0CTjIfj4rt6JIynen38","n":"yTKa6m5GFOllz7oIHFCkvRJoBv7wLMuKIPLHbFGh5yOiO8o3akoqMhf1x6MxINGhZo6dkIrhVlVfWJhEJZPVaQdvyvVmlIZruhcbz3PGMqPAbjq2JqbB1mMnsyGHx-ovP0Cm5xj8sgI8wm67p3nosqzqFvg6mPKVO-w1QBr5seDU2AwU2DR88LF2v03Zjgn4mGvPdUOXihTQoNlf-nJFduXMDyRgZabnR2HlYHhagHwy1beWW1WtEaPz8iBN_0bGkGw705aDBUHJkdTty1mzsCZRur_n0imqXu9IzoSyiq5d0yKrRA5xkA-K3DMeRMquZ5QvPT9Eee4EZfFL97zBfQ","e":"AQAB","x5c":["MIIDBTCCAe2gAwIBAgIQQiR8gZNKuYpH6cP+KIE5ijANBgkqhkiG9w0BAQsFADAtMSswKQYDVQQDEyJhY2NvdW50cy5hY2Nlc3Njb250cm9sLndpbmRvd3MubmV0MB4XDTIwMDgyODAwMDAwMFoXDTI1MDgyODAwMDAwMFowLTErMCkGA1UEAxMiYWNjb3VudHMuYWNjZXNzY29udHJvbC53aW5kb3dzLm5ldDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMkymupuRhTpZc+6CBxQpL0SaAb+8CzLiiDyx2xRoecjojvKN2pKKjIX9cejMSDRoWaOnZCK4VZVX1iYRCWT1WkHb8r1ZpSGa7oXG89zxjKjwG46tiamwdZjJ7Mhh8fqLz9ApucY/LICPMJuu6d56LKs6hb4OpjylTvsNUAa+bHg1NgMFNg0fPCxdr9N2Y4J+Jhrz3VDl4oU0KDZX/pyRXblzA8kYGWm50dh5WB4WoB8MtW3lltVrRGj8/IgTf9GxpBsO9OWgwVByZHU7ctZs7AmUbq/59Ipql7vSM6EsoquXdMiq0QOcZAPitwzHkTKrmeULz0/RHnuBGXxS/e8wX0CAwEAAaMhMB8wHQYDVR0OBBYEFGcWXwaqmO25Blh2kHHAFrM/AS2CMA0GCSqGSIb3DQEBCwUAA4IBAQDFnKQ98CBnvVd4OhZP0KpaKbyDv93PGukE1ifWilFlWhvDde2mMv/ysBCWAR8AGSb1pAW/ZaJlMvqSN/+dXihcHzLEfKbCPw4/Mf2ikq4gqigt5t6hcTOSxL8wpe8OKkbNCMcU0cGpX5NJoqhJBt9SjoD3VPq7qRmDHX4h4nniKUMI7awI94iGtX/vlHnAMU4+8y6sfRQDGiCIWPSyypIWfEA6/O+SsEQ7vZ/b4mXlghUmxL+o2emsCI1e9PORvm5yc9Y/htN3Ju0x6ElHnih7MJT6/YUMISuyob9/mbw8Vf49M7H2t3AE5QIYcjqTwWJcwMlq5i9XfW2QLGH7K5i8"]},{"kty":"RSA","use":"sig","kid":"5Of9P5F9gCCwCmF2BOHHxDDQ-Dk","x5t":"5Of9P5F9gCCwCmF2BOHHxDDQ-Dk","n":"2y6laZzXOPwGpMOhh0RcZq-Cng12HRv4EHT_Y6w5WOuNWZxzGFjF77qfTKtp_izFIGlr0IwJnbJsDqmTfAXdDMsfRXpWE6DZ6D0s49coNgu-nEFT7UdkuyfUnfPfU8lZLLzxB4fPp0CpUZIacZWb9Ci83dkqS6yEkppftf8bZOW1Cmz6SQuBbZgDyrm7hKBK8NxmSxJvnqUN6CDdOpxJdLSvIon8EUMcA0VEhNx0acgzZmjedZJEGWO6zs8jrRROkX0_fhpjW1BP4nq5OI6JpXMRgV6LuqCdmg9s3Qvw2k27baa97pxAJprMKwBnHSLcbrjkldREZgQ9NweYbLX-JQ","e":"AQAB","x5c":["MIIDBTCCAe2gAwIBAgIQNmD9my1yu4hPh6X2ySAQMjANBgkqhkiG9w0BAQsFADAtMSswKQYDVQQDEyJhY2NvdW50cy5hY2Nlc3Njb250cm9sLndpbmRvd3MubmV0MB4XDTIwMTExNTE4NTMyMFoXDTI1MTExNDE4NTMyMFowLTErMCkGA1UEAxMiYWNjb3VudHMuYWNjZXNzY29udHJvbC53aW5kb3dzLm5ldDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANsupWmc1zj8BqTDoYdEXGavgp4Ndh0b+BB0/2OsOVjrjVmccxhYxe+6n0yraf4sxSBpa9CMCZ2ybA6pk3wF3QzLH0V6VhOg2eg9LOPXKDYLvpxBU+1HZLsn1J3z31PJWSy88QeHz6dAqVGSGnGVm/QovN3ZKkushJKaX7X/G2TltQps+kkLgW2YA8q5u4SgSvDcZksSb56lDegg3TqcSXS0ryKJ/BFDHANFRITcdGnIM2Zo3nWSRBljus7PI60UTpF9P34aY1tQT+J6uTiOiaVzEYFei7qgnZoPbN0L8NpNu22mve6cQCaazCsAZx0i3G645JXURGYEPTcHmGy1/iUCAwEAAaMhMB8wHQYDVR0OBBYEFEhEsQJa9KyDIqzLnHtB0fPwqAv2MA0GCSqGSIb3DQEBCwUAA4IBAQAl4wkCQOD4UCaiuxVCFzs8PXGxhLJJseuN4TbMxvmmzgBaRPeazEmLqvqUW/VxCAWHiCzK6nVXThpAk9SPxN37HnlLt/wzEfQTapVxTzKj5PTzsEm582dDyw+0WDdNtb37a1BoAEpv6J4zVHTQLprfx4ccCQq4m3v6IDuKHrT/U0TlQz0negsUlDr4iwr3lPCdpnbLbgki8JnP5a/Kh/XMs5qXsJRF2IHqEF+yTupEN+fpNvTItfmGAfoGm+6AIBWhH+SEe6sshDpW7rngVJoLAxCrqDlzrXae1pq1pyvfbeMtHkjY7yl74wdFcDD/J0gOA2u30X/2Ld+5AglZIlBm"]},{"kty":"RSA","use":"sig","kid":"18pnMg3UmrWvBK_tkDAbjgM5CmA","x5t":"18pnMg3UmrWvBK_tkDAbjgM5CmA","n":"v3tn90CVkqJ57gTZu8bbC37NX0RloPlEnelHmqobAEiDLRuqw7Hv2M5o9iRFhF4sSw64fr6P33stLWKpzVmm4y6HUi89QeQmYCNYzxQy2V-tBiLxWX3vtVYgUFwfZDz4TIEu_Ia7rgTg8aHJ8t_b6mz_xPaWlLJWSFBlNY22z2KX87ULrE5AVNMr125aaPWLhxCGWYrnk5KdMrDGb1cuOExzX4S-_fQrRAWTpQWhqi0bEn9Y0vIWKD9-2CkLmZlJGgOueICSuKwwWXm87RKergHVS9sEGkSaBwWOtCPWLsv01Nc0sZymNs3BkPZsQKioYkdox6beXSQwYsmXtBZHjQ","e":"AQAB","x5c":["MIIC8TCCAdmgAwIBAgIQSoIG9pq9C4lOtPUfxLmCSDANBgkqhkiG9w0BAQsFADAjMSEwHwYDVQQDExhsb2dpbi5taWNyb3NvZnRvbmxpbmUudXMwHhcNMjAwOTEzMDAwMDAwWhcNMjUwOTEzMDAwMDAwWjAjMSEwHwYDVQQDExhsb2dpbi5taWNyb3NvZnRvbmxpbmUudXMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC/e2f3QJWSonnuBNm7xtsLfs1fRGWg+USd6UeaqhsASIMtG6rDse/Yzmj2JEWEXixLDrh+vo/fey0tYqnNWabjLodSLz1B5CZgI1jPFDLZX60GIvFZfe+1ViBQXB9kPPhMgS78hruuBODxocny39vqbP/E9paUslZIUGU1jbbPYpfztQusTkBU0yvXblpo9YuHEIZZiueTkp0ysMZvVy44THNfhL799CtEBZOlBaGqLRsSf1jS8hYoP37YKQuZmUkaA654gJK4rDBZebztEp6uAdVL2wQaRJoHBY60I9Yuy/TU1zSxnKY2zcGQ9mxAqKhiR2jHpt5dJDBiyZe0FkeNAgMBAAGjITAfMB0GA1UdDgQWBBTuNouNBOGNFuUdRuSpaTYO2ZRL6DANBgkqhkiG9w0BAQsFAAOCAQEAKZD3Hn+79D/S9Xby/8zSHhYlsXM4FrxDUggt29o15EdBdxyLHCwc3bXZI2PMn0u5vBoz9T0U/MnzoUIxdkmSI9qhfpaxrz7LPEFsMLN6uqRfN9XbypaGcHXP4Qb4xLjLqlmqc8TCUAi0MAokCVSLl83lIbF8/Kw4hSYWMs7HdiuJLHwIgWSd9mguaPks+w64268bw7yZnKutE8xj1x3f6f6AqD3yZr6B1IfnVgrWX4kQIF8z0XB4J5jPaCcORRRB1GV/aSKhNFXWbJ7Z2UnX5f27dX1mgrnFb48jAZLuoeRn7M7+gtj/wPzRiYN845c9wyx91HDo4xqvq/dFtRQ3dQ=="]}]}

As you can see, there is no alg or similar on the microsoft one.

I am not able to find spec that says that is required. You just want the n-parameter and match it against the kid after all.

Is there a particular reason that you've hardcoded a dependency on "alg" into the key fetcher?

Joken JWKS version 2.0

This library is around 5 years old currently and not only my understanding of the environment and language has hopefully evolved but also the uses of JWKS and the understanding of the spec itself is better nowadays. So, I think it is time we discuss a bit about a 2.0 version.

Current breaking issues are:

We should support kid-less and duplicated kids in sets as per the discussion on #37
We are simply using process the wrong way and have caused unfortunate side-effects #40
More and more people want to use a fixed set of JWKs sources and even a completely dynamic set of sources #39
Manually re-fetching might help solve some issues to some people during emergencies #30

These have to be tackled either way (breaking or not).

Some others I think are important:

JWKS claims like certificate thumbprints are increasingly important for security
- This has mainly x5t and x5c claims as targets
Better HTTP configuration would also have helped avoid needing to release new versions
More observability would also help here (not only HTTP events per-se but also cache events for example)
Better guides and documentation examples (many issues were open about that in the past)
More options to re-trigger fetching (like cache expiration as mentioned on #35

I will have time to work on these on May. If anybody has any other issues that would like to see added here please join the discussion :)

'alg' is required, but should be optional

According to the specs, 'alg' should technically be optional, but it is required in this library.

Microsoft (both v1 and v2) jwks files do not include "alg" on the keys, instead they have "kty" of "RSA".

See the v2 url as an example: https://login.microsoftonline.com/common/discovery/v2.0/keys

Optional dependency on `:hackney`

Hi,

I was pleased to see that the Tesla adapter in the HTTP Fetcher is user configurable

 defp new(opts) do
    adapter =
      Application.get_env(:tesla, __MODULE__)[:adapter] ||
        Application.get_env(:tesla, :adapter, @default_adapter)

However, in the root mix.exs, the :hackney dependency is a hard requirement, meaning that it has to be pulled in, along with its many sub-dependencies, regardless of choice of adapter. Can it be changed to be an optional requirement in the dependencies instead please? It's a 1 line change :)

...
defp deps do
    [
      {:joken, "~> 2.4"},
      {:jason, "~> 1.2"},
      {:tesla, "~> 1.4"},
      {:hackney, "~> 1.17.4 or ~> 1.18.0", optional: true},
      {:telemetry, "~> 0.4.2 or ~> 1.0"},
...

Support for content-type = 'application/jwk-set+json'

Some OIDC providers specifies the content type = 'application/jwk-set+json' when returning the jwks document.
This means that the returned document will not be decoded to an elixir map, since the Tesla.JSON middleware is not made aware of this content-type, which again results in an error when trying to read the jwks document.

An example of an OIDC provider using this content-type can be found below:
https://demo.identityserver.io/.well-known/openid-configuration/jwks

A solution could be to pass the 'application/jwk-set+json' to the Tesla.JSON middleware as a content-type to be decoded as JSON.

DefaultMatchStrategy does not exist

Unfortunately there's no DefaultMatchStrategy module nowhere in the repo.

Have the default fetcher implementation respect caching headers

Having the only trigger for a refresh of the JWKS be the presentation of a token with an invalid kid is a neat approach, but suffers from the risk that valid tokens will be rejected for some period of time by the resource server.

Instead, it would be handy if the HTTP response headers that control caching behaviour were examined and used as a means of determining a schedule for periodic pre-emptive refreshes of the JWKS. This would allow IdPs to periodically rotate keys, without the risk that a JokenJwks-using resource server would reject valid tokens, as long as they introduce the new key into the JWKS at least one expiry time period before first using it.

Of course, the existing "refresh on unknown kid" behaviour could also stay in place, as a fallback in the event that an IdP makes a mistake with their caching/rotation scheduling, and the "only retry every N milliseconds" prevents an absurdly short expiry period from causing a DoS.

If this is a feature that you'd be willing to accept in the default fetcher, I'd be happy to work up a PR. I'm pretty confident I can make the whole thing backwards-compatible, and could even make the cache-respecting behaviour be opt-in via an option if that would make the feature more palatable.

ETS table does not exist (sometimes)

We use joken and joken_jwks in our application and it works very well.

Our usage is very basic (I believe). We have the following Token implementation:

defmodule App.Accounts.Tokens.Google do
  @moduledoc false
  use Joken.Config, default_signer: nil

  @iss "https://accounts.google.com"

  defp aud, do: Application.fetch_env!(:elixir_auth_google, :client_id)

  add_hook(JokenJwks, strategy: App.Accounts.Tokens.Strategy)

  @impl Joken.Config
  def token_config do
    default_claims()
    |> add_claim("iss", nil, &(&1 == @iss))
    |> add_claim("aud", nil, &(&1 == aud()))
  end
end

And the following strategy:

defmodule App.Accounts.Tokens.Strategy do
  @moduledoc false
  use JokenJwks.DefaultStrategyTemplate

  def init_opts(opts) do
    url = "https://www.googleapis.com/oauth2/v3/certs"
    Keyword.merge(opts, jwks_url: url)
  end
end

We face some issues with our app refusing to start (in staging, mostly) with the following error message:

[error] Task #PID<0.845.0> started from App.Supervisor terminating
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: the table identifier does not refer to an existing ETS table

    (stdlib 5.0) :ets.insert(App.Accounts.Tokens.Strategy.EtsCache, {:signers, %{PRUNED}})
    (app 0.1.0) lib/app/accounts/tokens/strategy.ex:3: App.Accounts.Tokens.Strategy.EtsCache.put_signers/1
    (app 0.1.0) lib/app/accounts/tokens/strategy.ex:3: App.Accounts.Tokens.Strategy.fetch_signers/2
    (elixir 1.15.4) lib/task/supervised.ex:101: Task.Supervised.invoke_mfa/2
Function: #Function<1.47344484/0 in App.Accounts.Tokens.Strategy.start_fetch_signers/2>
    Args: []

It seemed like a race condition because it worked most of the time. We were not able to figure out what went wrong and diving into the joken_jwks code, it seems strange that the ETS table would not be created because we'd expect the process to fail earlier when setting the counter to 0.

After some more investigating, the problem seems to be related to the compilation step, not the runtime. The reason we suspect this is that a build that throws the error above does it every time it's started and a build that does not throw on the first start does not throw at all. Locally it's the same, as soon as the app throws the error at startup each mix phx.server will throw the error. Recompiling the app will solve this, but changing unrelated files (which only trigger a partial recompile) does not.

Hackney version needs updated to fix HTTPS key retrieval

Related: benoitc/hackney#591

The fix has been in the Hackney code for a couple of days, but an actual release was just made. Proposing we update the version dependency in mix.exs to read {:hackney, "~> 1.15.2"}.

joken-elixir / joken_jwks Goto Github PK

joken_jwks's People

Contributors

Stargazers

Watchers

Forkers

joken_jwks's Issues

Recommend Projects

Recommend Topics

Recommend Org