Damans227 opened a new issue, #13854:
URL: https://github.com/apache/cloudstack/issues/13854

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to