Problem
CloudStack's OAuth2 plugin currently ships one hardcoded Java class and one hardcoded UI block per OIDC vendor (KeycloakOAuth2Provider, ForgeRockOAuth2Provider). Neither class has any vendor-specific logic. Both just implement the standard OIDC authorization-code flow: hit an authorize URL, exchange the code at a token URL, parse the returned id_token JWT, read the email claim. Any OIDC-compliant IdP (Okta, Auth0, Azure AD, etc.) would work against this same code unchanged.
This was called out directly in #13499, which extracted the duplicated Keycloak logic into AbstractOIDCOAuth2Provider so ForgeRock could reuse it as a thin subclass. From that PR's own description:
Perhaps in the future this should be handled as an unbound provider (just a generic OIDC provider, pluggable with any OIDC-compliant server), but for now, this'll do.
As it stands, every new OIDC IdP someone wants means a new Java class plus a new hardcoded block in Login.vue, forever, for zero actual behavior difference. It also has a real limit today: since provider is both the display name and the routing key, and dispatch is a fixed name-to-bean map, a domain can only ever register one keycloak and one forgerock. It can't run two different OIDC IdPs under arbitrary names.
Proposal
Make OIDC a generic provider type instead of one class per vendor.
- Add a
type field distinct from provider (oauth_provider table + registerOauthProvider/updateOauthProvider params + OauthProviderResponse). provider stays a free-text, admin-chosen label (forgerock, okta, hr-corp-idp); type says which code runs it (e.g. oidc).
- One concrete generic OIDC bean instead of one subclass per vendor.
- Decouple provider identity from
getName(). Right now it's a fixed, parameterless string baked into each bean and used for both dispatch and the bean's own DB lookups. For a shared bean serving many registrations, the provider name needs to be a parameter threaded through verifyUser/verifySecretCodeAndFetchEmail, not a compile-time constant.
- Dispatch fallback in
OAuth2AuthManagerImpl.getUserOAuth2AuthenticationProvider: if no fixed bean matches a name, look up the DB row; if type=oidc, hand off to the generic bean instead of throwing.
- Move the authorizeUrl/tokenUrl-required check off the hardcoded name list (
equalsAny(provider, "keycloak", "forgerock")) onto type == oidc, so it applies to any future name automatically.
Login.vue: render OAuth buttons from the registered provider list instead of one hardcoded block per vendor. Needs a display name/icon per row (admin-supplied, or a generic OIDC icon as fallback).
- Keep
google/github/keycloak legacy beans working unchanged. No forced migration, existing rows keep dispatching to their own classes. Only new arbitrary-name registrations go through the generic path.
Non-goals
- No change to Google/GitHub, they aren't OIDC and keep their own dedicated implementations.
- No forced migration of existing
keycloak registrations.
Related
Problem
CloudStack's OAuth2 plugin currently ships one hardcoded Java class and one hardcoded UI block per OIDC vendor (
KeycloakOAuth2Provider,ForgeRockOAuth2Provider). Neither class has any vendor-specific logic. Both just implement the standard OIDC authorization-code flow: hit an authorize URL, exchange the code at a token URL, parse the returnedid_tokenJWT, read theemailclaim. Any OIDC-compliant IdP (Okta, Auth0, Azure AD, etc.) would work against this same code unchanged.This was called out directly in #13499, which extracted the duplicated Keycloak logic into
AbstractOIDCOAuth2Providerso ForgeRock could reuse it as a thin subclass. From that PR's own description:As it stands, every new OIDC IdP someone wants means a new Java class plus a new hardcoded block in
Login.vue, forever, for zero actual behavior difference. It also has a real limit today: sinceprovideris both the display name and the routing key, and dispatch is a fixed name-to-bean map, a domain can only ever register onekeycloakand oneforgerock. It can't run two different OIDC IdPs under arbitrary names.Proposal
Make OIDC a generic provider type instead of one class per vendor.
typefield distinct fromprovider(oauth_providertable +registerOauthProvider/updateOauthProviderparams +OauthProviderResponse).providerstays a free-text, admin-chosen label (forgerock,okta,hr-corp-idp);typesays which code runs it (e.g.oidc).getName(). Right now it's a fixed, parameterless string baked into each bean and used for both dispatch and the bean's own DB lookups. For a shared bean serving many registrations, the provider name needs to be a parameter threaded throughverifyUser/verifySecretCodeAndFetchEmail, not a compile-time constant.OAuth2AuthManagerImpl.getUserOAuth2AuthenticationProvider: if no fixed bean matches a name, look up the DB row; iftype=oidc, hand off to the generic bean instead of throwing.equalsAny(provider, "keycloak", "forgerock")) ontotype == oidc, so it applies to any future name automatically.Login.vue: render OAuth buttons from the registered provider list instead of one hardcoded block per vendor. Needs a display name/icon per row (admin-supplied, or a generic OIDC icon as fallback).google/github/keycloaklegacy beans working unchanged. No forced migration, existing rows keep dispatching to their own classes. Only new arbitrary-name registrations go through the generic path.Non-goals
keycloakregistrations.Related
AbstractOIDCOAuth2Providerbase this proposal builds on)