All--
Thanks to Stian's recommendations for similar features in the codebase,
I've developed a proposal for how to implement this feature, and sketched
out an initial implementation. As he suggested, I'd like to get feedback
before opening up a PR.
1. We introduce a new subclass of the OIDCIdentityProvider / Factory called
OIDCDiscoveryIdentityProvider. The new class adds a discoverConfig method
that can be used to discover and set all relevant endpoints in a
configuration except the issuer. Issuer then becomes the sole required
element of the config. (So the GoogleIdentityProvider, for example, will
then just call discoverConfig with its hard-coded issuer).
-> During the parseConfig call, the Factory will raise a
RuntimeException if the config cannot be obtained, since if the config can
never be obtained there is no way to have a valid configuration at all.
-> During the discoverConfig call, we return the cached value, and
optionally refresh it based on when we last obtained the configuration.
-> In the event of a failure in the *refresh* I'm imagining the best
behavior is to log a warning but then return the cached config (last known
good config).
2. We introduce an SPI and interface called
OIDCDiscoveryRepresentationProvider / Factory and implementations called
Infinispan... which obtains OIDCConfigurationRepresentations using the
/.well-known/openid-configuration endpoint on the issuer.
I have a couple questions:
1. I began an implementation of this, but the @JsonProperty annotations on
the existing OIDCCOnfigurationRepresentation class seem to be ignored when
I try to have it read from the openid-configuration docuement, e.g.:
- OIDCConfigurationRepresentation rep = SimpleHttp.doGet(issuer +
"/.well-known/openid-configuration",
session).asJson(OIDCConfigurationRepresentation.class); // FAILS with
"Unrecognized field ..." Perhaps a mismatch of the jackson annotation
version in the keycloak-model-infinispan submodule?
2. Assuming the design above makes sense, what is the right way to plug
this into the testing framework? Is one of the core developers interested
in tackling that aspect if I push a PR with the above features implemented?
James
On Sun, Jan 27, 2019 at 11:15 AM James Campbell <james.p.campbell(a)gmail.com>
wrote:
Got it; I checked that code out and it does look like a pretty
direct
model for the refreshing feature. I'll begin working on a PR.
On Mon, Jan 21, 2019 at 3:25 AM Stian Thorgersen <sthorger(a)redhat.com>
wrote:
> JWKs used by identity brokering and client authentication.
>
> On Sat, 19 Jan 2019 at 23:29, James Campbell <james.p.campbell(a)gmail.com>
> wrote:
>
>> Stian--
>>
>> Thanks for confirming. Which keys are you referring to--I'll take a look
>> at that in the code and try to follow that pattern closely.
>>
>> James
>>
>> On Wed, Jan 16, 2019 at 1:52 AM Stian Thorgersen <sthorger(a)redhat.com>
>> wrote:
>>
>>> It would be good to get these changes included. We do something similar
>>> for the keys today and they are cached in an Infinispan local cache. Could
>>> do something similar for the OIDC discovery/config.
>>>
>>> On Tue, 15 Jan 2019 at 17:44, James Campbell <
>>> james.p.campbell(a)gmail.com> wrote:
>>>
>>>> Tomas--
>>>>
>>>> Thanks--it certainly seems close, you're right! It looks, however
like
>>>> an
>>>> OIDC provider still uses a static configuration even though it loads it
>>>> from the discovery URL--that is, once it's loaded at configuraiton
>>>> time, it
>>>> doesn't discover new changes, and there isn't an option to
>>>> refresh/store
>>>> that discovery endpoint outside of configuration.
>>>>
>>>> It's not clear to me how important that feature is--on one hand, it
>>>> seems
>>>> unlikely that we should expect frequent changes; on the other, in the
>>>> short
>>>> time since I started exploring this setup, I have encountered three
>>>> changes
>>>> in google's OIDC endpoints between what was hard-coded into
keycloak,
>>>> what
>>>> is in their documentation, and what their current discovery endpoint
>>>> provides. (And, the google docs specifically suggest refreshing from
>>>> the
>>>> endpoint periodically).
>>>>
>>>> Thoughts?
>>>>
>>>> James
>>>>
>>>>
>>>> On Tue, Jan 15, 2019 at 10:31 AM Tomas Kyjovsky
<tkyjovsk(a)redhat.com>
>>>> wrote:
>>>>
>>>> > Please disregard my previous message.
>>>> >
>>>> > Looking at the docs [1] and the Admin Console UI this should be
>>>> already
>>>> > possible with the OIDC identity provider.
>>>> > When creating a OIDC identity provider in the Admin Console there
is
>>>> an
>>>> > option at the bottom of the page to import OIDC configuration
>>>> metadata from
>>>> > URL.
>>>> >
>>>> > Does this cover your use case?
>>>> >
>>>> >
>>>> > Regards,
>>>> > Tomas
>>>> >
>>>> > [1]
>>>> >
>>>>
https://www.keycloak.org/docs/latest/server_admin/index.html#openid-conne...
>>>> >
>>>> >
>>>> > ----- Original Message -----
>>>> > > Hello James,
>>>> > >
>>>> > > See my 2 cents inline..
>>>> > >
>>>> > > ----- Original Message -----
>>>> > > > All--
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > After observing that the Google Social Identity Provider
in
>>>> Keycloak
>>>> > was
>>>> > > > using a deprecated userprofile endpoint [
>>>> > > >
<
https://issues.jboss.org/projects/KEYCLOAK/issues/KEYCLOAK-9179
>>>> >
>>>> > > > Keycloak-9179,
<
https://issues.jboss.org/browse/KEYCLOAK-9169>
>>>> > > > Keycloak-9169], I wanted to propose the creation of an
>>>> IdentityProvider
>>>> > > > that
>>>> > > > will use the OIDC Discovery mechanism to dynamically build
a
>>>> config [
>>>> > > > <
https://issues.jboss.org/browse/KEYCLOAK-9194>
Keycloak-9194].
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > I see a few decision points along the way that I wanted to
ask
>>>> about
>>>> > before
>>>> > > > an implementation, since I'm very new to keycloak and
just
>>>> starting to
>>>> > > > understand the codebase. In particular, I wondered if this
group
>>>> could
>>>> > > > share
>>>> > > > insight into these couple issues so I can make a more
informed
>>>> design:
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > 1. It looks to me like the actual IdentityProviders
are
>>>> instantiated
>>>> > > > just
>>>> > > > as they're being used, but that the models are
persisted in the
>>>> > RealmModel.
>>>> > > > It's not clear to me where the separation of concerns
is
>>>> supposed to be
>>>> > > > between the IdentityProvider and the
IdentityProviderModel-in
>>>> > particular
>>>> > > > since the GoogeIdentityProvider, say, immediately sets
endpoints
>>>> in its
>>>> > > > constructor. Normatively, where *should* social identity
>>>> providers'
>>>> > model
>>>> > > > configuration be set (and, e.g., where are the models
first
>>>> added to
>>>> > the
>>>> > > > RealmModel)?
>>>> > >
>>>> > > Provider classes are being instantiated per transaction by
their
>>>> > > corresponding ProviderFactories and then left to be
>>>> garbage-collected
>>>> > after
>>>> > > Provider.close() is called.
>>>> > > The Provider class is given its configuration
>>>> (IdentityProviderModel in
>>>> > this
>>>> > > case) by its factory which I believe loads it from cache/jpa
>>>> layer. Any
>>>> > > class extending AbstractIdentityProvider should then be able
to
>>>> access
>>>> > its
>>>> > > config via getConfig() method but I don't think it will be
able to
>>>> > > update/persist it back. The provider configuration/model itself
is
>>>> > managed
>>>> > > by the IdentityProviderResource (REST endpoint accessible via
REST
>>>> or
>>>> > admin
>>>> > > console UI) in the keycloak/services module so I think the
>>>> > > auto-configuration logic would have to be placed somewhere
there.
>>>> > >
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > 2. I see that there is logic to parse OIDC Discovery
>>>> configuration
>>>> > as
>>>> > > > part of configuring Keycloak itself as an OIDC provider /
>>>> implementer
>>>> > of
>>>> > > > OIDC protocol (including building and parsing the
.well-known
>>>> config
>>>> > > > elements), but that logic seems not to be used in any
setting
>>>> > currently as
>>>> > > > a
>>>> > > > client. Should I plan to reuse, say, the
>>>> > OIDCConfigurationRepresentation
>>>> > > > and
>>>> > > > OIDCWellKnownProvider classes for their logic in handling
such
>>>> configs?
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > Currently, I'm imagining something along the lines of
extending
>>>> > > > OIDCIdentityProvider with a new
OIDCDiscoveryIdentityProvider
>>>> that
>>>> > adds a
>>>> > > > discoverConfig method which can be used by an implementing
class
>>>> (such
>>>> > as
>>>> > > > GoogleIdentityProvider) to discover and cache endpoints
such
>>>> that they
>>>> > are
>>>> > > > not hard-coded into the implementing class. Each
implementing
>>>> class
>>>> > would
>>>> > > > then have a public static final DISCOVERY_URL that it
passes to
>>>> > > > discoverConfig.
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > My main hangup, as suggested above, is that to implement
the
>>>> caching, I
>>>> > > > want
>>>> > > > to ensure that the model configuration is stored/set in
the right
>>>> > place.
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > Thanks for bearing with me as I come up to speed on this!
>>>> > > >
>>>> > > >
>>>> > > >
>>>> > > > James
>>>> > > >
>>>> > > > _______________________________________________
>>>> > > > keycloak-dev mailing list
>>>> > > > keycloak-dev(a)lists.jboss.org
>>>> > > >
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>> > > >
>>>> > >
>>>> > >
>>>> > > Regards,
>>>> > > Tomas
>>>> > >
>>>> >
>>>>
>>>>
>>>> --
>>>> -... . / - .-. ..- . .-.-.- / -... . / .--. .-. . ... . -. - .-.-.- /
>>>> -...
>>>> . / ..-. .. -
>>>> 07:d0:2d:0b:d6:9d:27:c6:a7:c1:ec:e3:b1:65:f4:70
>>>> _______________________________________________
>>>> keycloak-dev mailing list
>>>> keycloak-dev(a)lists.jboss.org
>>>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>
>>>
>>
>> --
>> -... . / - .-. ..- . .-.-.- / -... . / .--. .-. . ... . -. - .-.-.- /
>> -... . / ..-. .. -
>> 07:d0:2d:0b:d6:9d:27:c6:a7:c1:ec:e3:b1:65:f4:70
>>
>
--
-... . / - .-. ..- . .-.-.- / -... . / .--. .-. . ... . -. - .-.-.- / -...
. / ..-. .. -
07:d0:2d:0b:d6:9d:27:c6:a7:c1:ec:e3:b1:65:f4:70
--
-... . / - .-. ..- . .-.-.- / -... . / .--. .-. . ... . -. - .-.-.- / -...
. / ..-. .. -
07:d0:2d:0b:d6:9d:27:c6:a7:c1:ec:e3:b1:65:f4:70