add test

kivra · Jul 18, 2023 · c0c4f41 · c0c4f41
1 parent b0620b9
commit c0c4f41
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 21 deletions.
diff --git a/elvis.config b/elvis.config
@@ -8,7 +8,7 @@
  ignore => [],
  ruleset => [{elvis_style, state_record_and_type, disable}],
  rules => [ {elvis_text_style, line_length,
- #{ limit => 80,
+ #{ limit => 100,
  skip_comments => false
  }}
  , {elvis_text_style, no_tabs}
@@ -57,7 +57,7 @@
  ],
  filter => "*.erl",
  rules => [ {elvis_text_style, line_length,
- #{ limit => 80,
+ #{ limit => 100,
  skip_comments => false
  }}
  , {elvis_text_style, no_tabs}

diff --git a/src/id_token_provider.erl b/src/id_token_provider.erl
@@ -87,9 +87,11 @@ maybe_refresh(Provider, #{force_refresh := true}) ->
 maybe_refresh(Provider, #{kid := Kid}) ->
  %% check whether Kid has been added to the cache already while the
  %% refresh request was queued in the gen_server inbox
- case cache_has_kid(Kid, Provider) of
- {true, CacheEntry} -> CacheEntry;
- {false, _} -> refresh(Provider)
+ [{Provider, CacheEntry}] = ets:lookup(?ID_TOKEN_CACHE, Provider),
+ #{keys := Keys} = CacheEntry,
+ case lists:search(fun(#{<<"kid">> := K}) -> K =:= Kid end, Keys) of
+ {value, _} -> CacheEntry;
+ false -> refresh(Provider)
  end;
 maybe_refresh(Provider, _Opts) ->
  [{Provider, CacheEntry}] = ets:lookup(?ID_TOKEN_CACHE, Provider),
@@ -137,14 +139,6 @@ handle_error(Message, Provider, Reason) ->
  [{Provider, CacheEntry}] = ets:lookup(?ID_TOKEN_CACHE, Provider),
  CacheEntry.
 
-cache_has_kid(Kid, Provider) ->
- [{Provider, CacheEntry}] = ets:lookup(?ID_TOKEN_CACHE, Provider),
- #{keys := Keys} = CacheEntry,
- case lists:search(fun(#{<<"kid">> := K}) -> K =:= Kid end, Keys) of
- {value, _} -> {true, CacheEntry};
- false -> {false, CacheEntry}
- end.
-
 %%%_* Emacs ============================================================
 %%% Local Variables:
 %%% allout-layout: t

diff --git a/test/id_token_SUITE.erl b/test/id_token_SUITE.erl
@@ -10,7 +10,8 @@
 
 groups() -> [].
 
-all() -> [validate_jwt, keys_are_cached].
+all() -> [ validate_jwt,
+ keys_are_only_refreshed_once_per_kid ].
 
 init_per_suite(Config) ->
  application:ensure_all_started(jose),
@@ -22,9 +23,9 @@ init_per_testcase(_TestCase, Config) ->
  id_token_jws:generate_key_for(<<"RS256">>, #{key_size => 1024}),
  Claims = #{ <<"exp">> => erlang:system_time(second) + 10},
  Jwt = id_token_jws:sign(Claims, Jwk),
- mock_id_provider(PublicKeyMap),
+ mock_id_provider(PublicKeyMap, 0),
  application:ensure_all_started(id_token),
- [{jwt, Jwt} | Config].
+ [{jwt, Jwt}, {pubkeys, [PublicKeyMap]} | Config].
 end_per_testcase(_TestCase, Config) ->
  application:stop(id_token),
  meck:unload([id_token_jwks, hackney]),
@@ -36,19 +37,47 @@ validate_jwt(Config) ->
 
 keys_are_cached(Config) ->
  Jwt = ?config(jwt, Config),
- id_token:validate(?ID_PROVIDER, Jwt),
- id_token:validate(?ID_PROVIDER, Jwt),
+ ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)),
+ ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)),
  1 = meck:num_calls(id_token_jwks, get_pub_keys, 1),
  ok.
 
-mock_id_provider(PublicKeyMap) ->
- meck:expect(id_token_jwks, get_providers, 0,
- [{?ID_PROVIDER, ?WELL_KNOWN_URI}]),
+keys_are_only_refreshed_once_per_kid(Config) ->
+ %% init pubkey cache with config from init_per_testcase
+ CurrentKeyCache = #{ exp_at => id_token_util:now_gregorian_seconds() + 10,
+ keys => ?config(pubkeys, Config)},
+ ok = meck:expect(id_token_provider, get_cached_keys, 1, CurrentKeyCache),
+
+ %% create JWT with kid that's not yet in the pubkey cache
+ {Jwk, NewPubkey} = id_token_jws:generate_key_for(<<"RS256">>, #{key_size => 1024}),
+ Claims = #{ <<"exp">> => erlang:system_time(second) + 10 },
+ Jwt = id_token_jws:sign(Claims, Jwk),
+
+ %% set up provider to return new pubkeys with a 50 ms delay
+ HttpReponseDelay = 50,
+ mock_id_provider(NewPubkey, HttpReponseDelay),
+ ?assertEqual(0, meck:num_calls(id_token_jwks, get_pub_keys, 1)),
+
+ %% try to validate multiple JWTs based on kid that's not yet in the key cache
+ spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end),
+ spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end),
+ spawn(fun() -> id_token:validate(?ID_PROVIDER, Jwt) end),
+ timer:sleep(10 * HttpReponseDelay),
+
+ %% ensure that the pubkey cache was only refreshed once
+ ?assertEqual(1, meck:num_calls(id_token_jwks, get_pub_keys, 1)),
+ ?assertMatch({ok, _}, id_token:validate(?ID_PROVIDER, Jwt)),
+
+ meck:unload([id_token_provider]).
+
+mock_id_provider(PublicKeyMap, HttpReponseDelay) ->
+ meck:expect(id_token_jwks, get_providers, 0, [{?ID_PROVIDER, ?WELL_KNOWN_URI}]),
  meck:expect(hackney, request,
  fun(get, ?WELL_KNOWN_URI, _, _, _) ->
  Body = jsx:encode(#{<<"jwks_uri">> => ?JWKS_URI}),
  {ok, 200, [], Body};
  (get, ?JWKS_URI, _, _, _) ->
+ timer:sleep(HttpReponseDelay),
  Body = jsx:encode(#{<<"keys">> => [PublicKeyMap]}),
  MaxAge = <<"max-age=3600">>,
  Headers = [{<<"Cache-Control">>, MaxAge}],