Prevent jwks_expired flood in worker 2nd try

[maennchen]

Fixes #359

[asabil]

Overall I think this is error prone. If we could get rid of the maybe_cancel_timer function altogether that would be perfect.

[maennchen]

@asabil There's multiple ways a config / jwk is refreshed.

• Config Lifetime (based on cache headers)
• JWK Lifetime (based on cache headers)
• JWK kid not found triggering refresh
• Manual refresh of both config / jwks

In all cases when a refresh happens, the timeout has to be reset. Additionally, a refresh of the config triggers a refresh of the jwks as well.

Since not all refreshes are triggered by the timeouts, we have to reset the timers, no?

One thing i was thinking about now is if there needs to be a strict dependency between the config and the jwks. If the config does not change the jwk uri, the jwks do not need to be refreshed as long as they haven't expired in the meantime.

Even with that change, we still have to cancel the timers on any not timeout based refresh.

Edit: Something along the line of this:

diff --git a/src/oidcc_provider_configuration_worker.erl b/src/oidcc_provider_configuration_worker.erl
index b0105bb..9e6d459 100644
--- a/src/oidcc_provider_configuration_worker.erl
+++ b/src/oidcc_provider_configuration_worker.erl
@@ -168,15 +168,14 @@ handle_continue(
     load_configuration,
     #state{
         issuer = Issuer,
+        provider_configuration = OldProviderConfiguration,
         provider_configuration_opts = ProviderConfigurationOpts,
-        configuration_refresh_timer = OldConfigurationRefreshTimer,
-        jwks_refresh_timer = OldJwksRefreshTimer,
+        configuration_refresh_timer = RefreshTimer,
         ets_table = EtsTable
     } =
         State
 ) ->
-    maybe_cancel_timer(OldConfigurationRefreshTimer),
-    maybe_cancel_timer(OldJwksRefreshTimer),
+    maybe_cancel_timer(RefreshTimer),
 
     maybe
         {ok, {Configuration, Expiry}} ?=
@@ -184,15 +183,18 @@ handle_continue(
                 Issuer,
                 ProviderConfigurationOpts
             ),
+        #oidcc_provider_configuration{jwks_uri = JwksUri} = Configuration,
         {ok, NewTimer} = timer:send_after(Expiry, configuration_expired),
         ok = store_in_ets(EtsTable, provider_configuration, Configuration),
-        {noreply,
-            State#state{
-                provider_configuration = Configuration,
-                configuration_refresh_timer = NewTimer,
-                jwks_refresh_timer = undefined
-            },
-            {continue, load_jwks}}
+        NewState = State#state{
+            provider_configuration = Configuration,
+            configuration_refresh_timer = NewTimer
+        },
+        case OldProviderConfiguration of
+            undefined -> {noreply, NewState, {continue, load_jwks}};
+            #oidcc_provider_configuration{jwks_uri = JwksUri} -> {noreply, NewState};
+            #oidcc_provider_configuration{} -> {noreply, NewState, {continue, load_jwks}}
+        end
     else
         {error, Reason} -> handle_backoff_retry(configuration_load_failed, Reason, State)
     end;

Edit 2: We could possible store a ref fot the config and the jwks. Instead of triggering refresh_[configuration|jwks], we could trigger {refresh_[configuration|jwks], Ref} this way we could discard timeouts for older loads. But I don't think that would be prettier overall.

[maennchen]

Going with edit 1 of comment.