apache / apache/cloudstack

Replace per-vendor OIDC providers (Keycloak, ForgeRock, ...) with a generic OIDC provider type

オープン
#13,854 コメント 2 件 リアクション 1 件 担当者 0 名 GitHub で見る
type:enhancement
主要言語
Java
スター
3.1k
フォーク
1.4k
平均マージ
6日 19時間
マージ済み PR(30日)
32

説明

### 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)

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

AbstractOIDCOAuth2Provider と OAuth2AuthManagerImpl.getUserOAuth2AuthenticationProvider から始め、oauth_provider スキーマ、登録/更新パラメーター、OauthProviderResponse、Login.vue を追跡します。任意の OIDC 登録で 1 つの汎用プロバイダーと動的なログインボタンが使用され、既存の Google、GitHub、Keycloak の登録では引き続き従来の bean が使用されれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
java, javascript
領域
authentication, backend, database, frontend
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。