Replace per-vendor OIDC providers (Keycloak, ForgeRock, ...) with a generic OIDC provider type
- Lenguaje dominante
- Java
- Estrellas
- 3.1k
- Forks
- 1.4k
- Merge medio
- 6 d 19 h
- PR fusionados (30 d)
- 32
Descripción
### 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
- #13499 (adds ForgeRock, extracts the shared `AbstractOIDCOAuth2Provider` base this proposal builds on)
Guía de contribución
Línea de trabajo
Comienza con AbstractOIDCOAuth2Provider y OAuth2AuthManagerImpl.getUserOAuth2AuthenticationProvider; después, sigue el esquema oauth_provider, los parámetros de registro/actualización, OauthProviderResponse y Login.vue. Se considera terminado cuando los registros OIDC arbitrarios usan un proveedor genérico y botones de inicio de sesión dinámicos, mientras que los registros existentes de Google, GitHub y Keycloak siguen usando sus beans heredados.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- java, javascript
- Área
- authentication, backend, database, frontend
- Tipo de issue
- Nueva funcionalidad
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Activo
- Claridad
- Bien especificado
- Aptitud para principiantes
- 35/100