openidc_ensure_discovered_data doesn't work in the face of errors

[bodewig]

openidc_ensure_discovered_data is there to ensure the discovery endpoint has been called. It does so by replacing opts.discovery when it contains the discovery URI with the parsed response of said discovery endpoint.

If calling the discovery endpoint fails for some reason the following code

  if type(opts.discovery) == "string" then
    opts.discovery, err = openidc_discover(opts.discovery, opts.ssl_verify, opts.timeout, opts.jwk_expires_in, opts.proxy_opts,
                                           opts.http_request_decorator)
  end

ends up setting opts.discovery to nil - which not only is not a string but also throws away the original configured uri.

This means a later call to the same function with the same opts table will not try to obtain discovery data at all. One can argue that we assume opts to be a fresh table for each invocation of authenticate but this is not necessarily true - and not documented as a recommendation.

At least I'd like to fix the code to only set opts.discovery if it is not nil. A more invasive - and probably not backwards compatible - change would be to use a different key for the parsed response, I'm not sure whether we want to go that far.

[zandbelt]

yeah, at the time it seemed to be fast and easy but I agree long term it may be better to differentiate; the fix sounds reasonable

[bodewig]

OK, I'll take care of it.

the fix sounds reasonable

including "use a different key in opts to store discovered data"?

[zandbelt]

long term, yes; short term "fix the code to only set opts.discovery if it is not nil"

[bodewig]

see 1fec029 - unfortunately I forgot the magical "closes" commit message