Copilot commented on code in PR #14205: URL: https://github.com/apache/cloudstack/pull/14205#discussion_r4157521279
########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/oidc/GenericOIDCOAuth2Provider.java: ########## @@ -0,0 +1,350 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package org.apache.cloudstack.oauth2.oidc; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import javax.inject.Inject; +import javax.ws.rs.core.HttpHeaders; + +import org.apache.cloudstack.auth.UserOAuth2Authenticator; +import org.apache.cloudstack.oauth2.dao.OauthProviderDao; +import org.apache.cloudstack.oauth2.vo.OauthProviderVO; +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; +import org.apache.cxf.rs.security.jose.jwk.JsonWebKeys; +import org.apache.cxf.rs.security.jose.jwk.JwkUtils; +import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer; +import org.apache.cxf.rs.security.jose.jwt.JwtClaims; +import org.apache.cxf.rs.security.jose.jwt.JwtUtils; +import org.apache.http.NameValuePair; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.util.EntityUtils; + +import com.cloud.exception.CloudAuthenticationException; +import com.cloud.utils.component.AdapterBase; +import com.cloud.utils.exception.CloudRuntimeException; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + +/** + * A single provider implementation for any OIDC compliant identity provider. Unlike the per vendor + * providers it is not bound to one registration: the registration is selected by name on every call, + * so one bean serves any number of registrations of type "oidc". + */ +public class GenericOIDCOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator { + + public static final String OIDC_PROVIDER_TYPE = "oidc"; + + private static final String DISCOVERY_PATH = "/.well-known/openid-configuration"; + private static final int CLOCK_SKEW_SECONDS = 60; + private static final long METADATA_CACHE_MINUTES = 60; + private static final long VERIFIED_EMAIL_CACHE_SECONDS = 60; + private static final int HTTP_TIMEOUT_MILLIS = 10000; + + @Inject + OauthProviderDao oauthProviderDao; + + private CloseableHttpClient httpClient; + + private final Cache<String, OIDCMetadata> metadataCache = + Caffeine.newBuilder() + .expireAfterWrite(METADATA_CACHE_MINUTES, TimeUnit.MINUTES) + .maximumSize(64) + .build(); + + private final Cache<String, String> verifiedEmailCache = + Caffeine.newBuilder() + .expireAfterWrite(VERIFIED_EMAIL_CACHE_SECONDS, TimeUnit.SECONDS) + .maximumSize(1024) + .build(); + + public GenericOIDCOAuth2Provider() { + this(HttpClientBuilder.create() + .setDefaultRequestConfig(RequestConfig.custom() + .setConnectTimeout(HTTP_TIMEOUT_MILLIS) + .setConnectionRequestTimeout(HTTP_TIMEOUT_MILLIS) + .setSocketTimeout(HTTP_TIMEOUT_MILLIS) + .build()) + .build()); + } + + public GenericOIDCOAuth2Provider(CloseableHttpClient httpClient) { + this.httpClient = httpClient; + } + + @Override + public String getName() { + return OIDC_PROVIDER_TYPE; + } + + @Override + public String getDescription() { + return "Generic OpenID Connect Provider Plugin"; + } + + @Override + public boolean verifyUser(String email, String secretCode) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public boolean verifyUser(String email, String secretCode, Long domainId) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public String verifySecretCodeAndFetchEmail(String secretCode) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public String verifySecretCodeAndFetchEmail(String secretCode, Long domainId) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public boolean verifyUser(String email, String secretCode, Long domainId, String providerName) { + if (StringUtils.isAnyEmpty(email, secretCode)) { + throw new CloudAuthenticationException("Either email or secret code should not be null/empty"); + } + + String verifiedEmail = verifiedEmailCache.asMap().remove(verifiedEmailKey(providerName, secretCode)); Review Comment: The one-time cache key omits `domainId`. Because the same registration label may exist globally and in multiple domains, a code verified for domain A can be consumed by `oauthlogin` for domain B without resolving or contacting domain B's registration, bypassing the intended domain-specific IdP boundary. Include the effective domain/registration identity in both cache lookup and insertion keys. ########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java: ########## @@ -168,11 +185,17 @@ public OauthProviderVO registerOauthProvider(RegisterOAuthProviderCmd cmd) { Long domainId = normalizeGlobalScope(resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath())); String authorizeUrl = StringUtils.trim(cmd.getAuthorizeUrl()); String tokenUrl = StringUtils.trim(cmd.getTokenUrl()); + String type = StringUtils.trim(cmd.getType()); Review Comment: Registration accepts the type case-insensitively, but persists the caller's casing. A value such as `OIDC` routes successfully because lookup lowercases it, while list/update responses use case-sensitive plugin checks and the UI filters for exactly `oidc`, so the working registration is reported and rendered as disabled. Normalize the stored type. ########## ui/src/views/auth/Login.vue: ########## @@ -510,6 +522,26 @@ export default { this.handleDomain() this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', 'keycloak') }, + loginWithGenericOidc (provider) { + this.handleDomain() + this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', provider.provider) + const discoveryUrl = provider.issuerurl.replace(/\/$/, '') + '/.well-known/openid-configuration' + return fetch(discoveryUrl).then(response => response.json()).then(config => { + const options = { + client_id: provider.clientid, + redirect_uri: provider.redirecturi, + response_type: 'code', + scope: 'openid email', + state: this.from || 'cloudstack' Review Comment: The `state` value is predictable and the callback path removes it without comparing it to a value created for this login attempt. An attacker can therefore supply a callback containing the attacker's authorization code and cause login CSRF/session confusion. Generate a cryptographically random state, persist it for the attempt, and reject the callback before exchanging the code when it does not match; keep the return path separately. ########## ui/src/views/auth/Login.vue: ########## @@ -510,6 +522,26 @@ export default { this.handleDomain() this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', 'keycloak') }, + loginWithGenericOidc (provider) { + this.handleDomain() + this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', provider.provider) + const discoveryUrl = provider.issuerurl.replace(/\/$/, '') + '/.well-known/openid-configuration' + return fetch(discoveryUrl).then(response => response.json()).then(config => { Review Comment: Discovery is fetched directly by the browser, which adds a CORS requirement that OIDC discovery does not impose. A standards-compliant provider that does not return `Access-Control-Allow-Origin` will always hit this error path even though the management server can reach and use its discovery document. Resolve/expose the authorization endpoint server-side (or store it during registration) so login does not depend on IdP CORS policy. ########## ui/src/views/auth/Login.vue: ########## @@ -510,6 +522,26 @@ export default { this.handleDomain() this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', 'keycloak') }, + loginWithGenericOidc (provider) { + this.handleDomain() + this.$store.commit('SET_OAUTH_PROVIDER_USED_TO_LOGIN', provider.provider) + const discoveryUrl = provider.issuerurl.replace(/\/$/, '') + '/.well-known/openid-configuration' + return fetch(discoveryUrl).then(response => response.json()).then(config => { + const options = { + client_id: provider.clientid, + redirect_uri: provider.redirecturi, + response_type: 'code', + scope: 'openid email', + state: this.from || 'cloudstack' + } + window.location.href = `${config.authorization_endpoint}?${new URLSearchParams(options).toString()}` Review Comment: Appending `?` discards the structure of authorization endpoints that already contain a query component, which OAuth/OIDC metadata permits; the generated URL then has two question marks and the new parameters are parsed incorrectly. Add the parameters through the URL API so existing endpoint parameters are preserved. ########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java: ########## @@ -76,6 +77,16 @@ public class RegisterOAuthProviderCmd extends BaseCmd { @Parameter(name = ApiConstants.TOKEN_URL, type = CommandType.STRING, description = "Token URL for OAuth finalization (only required for keycloak provider)", since = "4.23.0") private String tokenUrl; + @Parameter(name = ApiConstants.TYPE, type = CommandType.STRING, + description = "Type of the provider implementation serving this registration, for example oidc for any OpenID Connect compliant provider. " + + "When set, the name in provider is a label chosen by the administrator rather than a built in provider name.", since = "4.24.0") Review Comment: This feature ships in the 24.0.0 upgrade, and CloudStack's release numbering cuts over from 4.x to 24.x; `4.24.0` identifies a nonexistent release in generated API documentation. Use `24.0.0` for this parameter's `since` metadata. This issue also appears on line 87 of the same file. ########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/UpdateOAuthProviderCmd.java: ########## @@ -67,6 +68,10 @@ public final class UpdateOAuthProviderCmd extends BaseCmd { @Parameter(name = ApiConstants.TOKEN_URL, type = CommandType.STRING, description = "Token URL pre-registered in the specific OAuth provider", since = "4.23.0") private String tokenUrl; + @Parameter(name = ApiConstants.ISSUER_URL, type = CommandType.STRING, + description = "Issuer URL of the OpenID Connect provider, used to read its discovery document", since = "4.24.0") Review Comment: This feature ships in the 24.0.0 upgrade, and CloudStack's release numbering cuts over from 4.x to 24.x; `4.24.0` identifies a nonexistent release in generated API documentation. Use `24.0.0` for this parameter's `since` metadata. ########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/response/OauthProviderResponse.java: ########## @@ -70,6 +70,14 @@ public class OauthProviderResponse extends BaseResponse { @Param(description = "path of the domain the provider belongs to (empty for global)", since = "4.23.0") private String domainPath; + @SerializedName(ApiConstants.TYPE) + @Param(description = "Type of the provider, for example oidc for a generic OpenID Connect provider. Empty for the built in providers", since = "4.24.0") Review Comment: This feature ships in the 24.0.0 upgrade, and CloudStack's release numbering cuts over from 4.x to 24.x; `4.24.0` identifies a nonexistent release in generated API documentation. Use `24.0.0` for the new field's `since` metadata. This issue also appears on line 78 of the same file. ########## ui/src/views/auth/Login.vue: ########## @@ -393,6 +404,7 @@ export default { getAPI('listOauthProvider', params).then(response => { if (response) { const oauthproviders = response.listoauthproviderresponse.oauthprovider || [] + this.oauthGenericProviders = oauthproviders.filter(item => item.type === 'oidc' && (item.enabled === true || item.enabled === 'true')) Review Comment: This overwrites the global generic-provider list when a domain is queried, but the empty-domain branch only restores the three built-in providers. After a user clears the domain field, domain-specific OIDC buttons remain visible and are then invoked with the global domain, causing the wrong registration lookup or a failed login. Preserve the global generic list and restore it when the domain is cleared, as is done for the built-in providers. ########## plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/oidc/GenericOIDCOAuth2Provider.java: ########## @@ -0,0 +1,350 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// +package org.apache.cloudstack.oauth2.oidc; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import javax.inject.Inject; +import javax.ws.rs.core.HttpHeaders; + +import org.apache.cloudstack.auth.UserOAuth2Authenticator; +import org.apache.cloudstack.oauth2.dao.OauthProviderDao; +import org.apache.cloudstack.oauth2.vo.OauthProviderVO; +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; +import org.apache.cxf.rs.security.jose.jwk.JsonWebKeys; +import org.apache.cxf.rs.security.jose.jwk.JwkUtils; +import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer; +import org.apache.cxf.rs.security.jose.jwt.JwtClaims; +import org.apache.cxf.rs.security.jose.jwt.JwtUtils; +import org.apache.http.NameValuePair; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.message.BasicNameValuePair; +import org.apache.http.util.EntityUtils; + +import com.cloud.exception.CloudAuthenticationException; +import com.cloud.utils.component.AdapterBase; +import com.cloud.utils.exception.CloudRuntimeException; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + +/** + * A single provider implementation for any OIDC compliant identity provider. Unlike the per vendor + * providers it is not bound to one registration: the registration is selected by name on every call, + * so one bean serves any number of registrations of type "oidc". + */ +public class GenericOIDCOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator { + + public static final String OIDC_PROVIDER_TYPE = "oidc"; + + private static final String DISCOVERY_PATH = "/.well-known/openid-configuration"; + private static final int CLOCK_SKEW_SECONDS = 60; + private static final long METADATA_CACHE_MINUTES = 60; + private static final long VERIFIED_EMAIL_CACHE_SECONDS = 60; + private static final int HTTP_TIMEOUT_MILLIS = 10000; + + @Inject + OauthProviderDao oauthProviderDao; + + private CloseableHttpClient httpClient; + + private final Cache<String, OIDCMetadata> metadataCache = + Caffeine.newBuilder() + .expireAfterWrite(METADATA_CACHE_MINUTES, TimeUnit.MINUTES) + .maximumSize(64) + .build(); + + private final Cache<String, String> verifiedEmailCache = + Caffeine.newBuilder() + .expireAfterWrite(VERIFIED_EMAIL_CACHE_SECONDS, TimeUnit.SECONDS) + .maximumSize(1024) + .build(); + + public GenericOIDCOAuth2Provider() { + this(HttpClientBuilder.create() + .setDefaultRequestConfig(RequestConfig.custom() + .setConnectTimeout(HTTP_TIMEOUT_MILLIS) + .setConnectionRequestTimeout(HTTP_TIMEOUT_MILLIS) + .setSocketTimeout(HTTP_TIMEOUT_MILLIS) + .build()) + .build()); + } + + public GenericOIDCOAuth2Provider(CloseableHttpClient httpClient) { + this.httpClient = httpClient; + } + + @Override + public String getName() { + return OIDC_PROVIDER_TYPE; + } + + @Override + public String getDescription() { + return "Generic OpenID Connect Provider Plugin"; + } + + @Override + public boolean verifyUser(String email, String secretCode) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public boolean verifyUser(String email, String secretCode, Long domainId) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public String verifySecretCodeAndFetchEmail(String secretCode) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public String verifySecretCodeAndFetchEmail(String secretCode, Long domainId) { + throw new CloudRuntimeException("The generic OIDC provider requires the registered provider name"); + } + + @Override + public boolean verifyUser(String email, String secretCode, Long domainId, String providerName) { + if (StringUtils.isAnyEmpty(email, secretCode)) { + throw new CloudAuthenticationException("Either email or secret code should not be null/empty"); + } + + String verifiedEmail = verifiedEmailCache.asMap().remove(verifiedEmailKey(providerName, secretCode)); + if (verifiedEmail == null) { + verifiedEmail = resolveEmail(secretCode, domainId, providerName); + } + if (StringUtils.isBlank(verifiedEmail) || !email.equals(verifiedEmail)) { + throw new CloudRuntimeException("Unable to verify the email address with the provided secret"); + } + + return true; + } + + @Override + public String verifySecretCodeAndFetchEmail(String secretCode, Long domainId, String providerName) { + String email = resolveEmail(secretCode, domainId, providerName); + verifiedEmailCache.put(verifiedEmailKey(providerName, secretCode), email); + return email; + } + + protected String resolveEmail(String secretCode, Long domainId, String providerName) { + OauthProviderVO provider = findRegistration(providerName, domainId); + OIDCMetadata metadata = getMetadata(provider); + String idToken = exchangeAuthorizationCode(secretCode, provider, metadata); + + return validateAndExtractEmail(idToken, provider, metadata); + } + + @Override + public String getUserEmailAddress() throws CloudRuntimeException { + return null; + } + + private String verifiedEmailKey(String providerName, String secretCode) { + return DigestUtils.sha256Hex(providerName + ":" + secretCode); + } + + protected OauthProviderVO findRegistration(String providerName, Long domainId) { + if (StringUtils.isBlank(providerName)) { + throw new CloudAuthenticationException("The registered provider name is required"); + } + OauthProviderVO provider = oauthProviderDao.findByProviderAndDomainWithGlobalFallback(providerName, domainId); + if (provider == null) { + throw new CloudAuthenticationException(String.format("%s provider is not registered, so user cannot be verified", providerName)); + } + return provider; + } + + protected OIDCMetadata getMetadata(OauthProviderVO provider) { + String issuerUrl = StringUtils.trimToNull(provider.getIssuerUrl()); + if (issuerUrl == null) { + throw new CloudRuntimeException(String.format( + "Provider %s has no issuer URL, so its endpoints and signing keys cannot be discovered", provider.getProvider())); + } + return metadataCache.get(issuerUrl, this::discover); + } + + protected OIDCMetadata discover(String issuerUrl) { + String document = httpGet(StringUtils.removeEnd(issuerUrl, "/") + DISCOVERY_PATH, + String.format("Unable to read the OpenID Connect discovery document from %s", issuerUrl)); + + JsonObject json = JsonParser.parseString(document).getAsJsonObject(); + String issuer = readString(json, "issuer"); + String tokenEndpoint = readString(json, "token_endpoint"); + String jwksUri = readString(json, "jwks_uri"); + if (StringUtils.isAnyBlank(issuer, tokenEndpoint, jwksUri)) { + throw new CloudRuntimeException(String.format( + "The discovery document at %s is missing the issuer, the token endpoint or the JWKS URI", issuerUrl)); + } + if (!StringUtils.removeEnd(issuer, "/").equals(StringUtils.removeEnd(issuerUrl, "/"))) { + throw new CloudRuntimeException(String.format("The discovery document at %s names a different issuer: %s", issuerUrl, issuer)); + } + + return new OIDCMetadata(issuer, readString(json, "authorization_endpoint"), tokenEndpoint, jwksUri); + } + + protected String exchangeAuthorizationCode(String secretCode, OauthProviderVO provider, OIDCMetadata metadata) { + String auth = provider.getClientId() + ":" + provider.getSecretKey(); + String encodedAuth = Base64.getEncoder().encodeToString(auth.getBytes(StandardCharsets.UTF_8)); + + List<NameValuePair> params = new ArrayList<>(); + params.add(new BasicNameValuePair("grant_type", "authorization_code")); + params.add(new BasicNameValuePair("code", secretCode)); + params.add(new BasicNameValuePair("redirect_uri", provider.getRedirectUri())); + + HttpPost post = new HttpPost(metadata.getTokenEndpoint()); + post.setHeader(HttpHeaders.AUTHORIZATION, "Basic " + encodedAuth); + try { + post.setEntity(new UrlEncodedFormEntity(params)); + } catch (UnsupportedEncodingException e) { + throw new CloudRuntimeException("Unable to generate URL parameters: " + e.getMessage()); + } + + try (CloseableHttpResponse response = httpClient.execute(post)) { + String body = EntityUtils.toString(response.getEntity()); + if (response.getStatusLine().getStatusCode() != 200) { + throw new CloudRuntimeException(String.format("%s error during token generation: %s", provider.getProvider(), body)); + } + + JsonElement fetchedIdToken = JsonParser.parseString(body).getAsJsonObject().get("id_token"); + if (fetchedIdToken == null) { + throw new CloudRuntimeException("No id_token found in token"); + } + return fetchedIdToken.getAsString(); + } catch (IOException e) { + throw new CloudRuntimeException(String.format("Unable to connect to the %s token endpoint", provider.getProvider()), e); + } + } + + protected String validateAndExtractEmail(String idToken, OauthProviderVO provider, OIDCMetadata metadata) { + JwsJwtCompactConsumer consumer = new JwsJwtCompactConsumer(idToken); + + verifySignature(consumer, metadata, provider); + + JwtClaims claims = consumer.getJwtClaims(); + if (!metadata.getIssuer().equals(claims.getIssuer())) { + throw new CloudAuthenticationException("Issuer mismatch"); + } + if (!claims.getAudiences().contains(provider.getClientId())) { + throw new CloudAuthenticationException("Audience mismatch"); + } + JwtUtils.validateJwtExpiry(claims, CLOCK_SKEW_SECONDS, true); + + String email = (String) claims.getClaim("email"); + if (StringUtils.isBlank(email)) { + throw new CloudAuthenticationException("The id_token carries no email claim"); + } Review Comment: A signed ID token with `email_verified: false` is accepted and its email is used as the CloudStack account identity. On providers that allow users to add unverified addresses, this lets an IdP user claim another CloudStack user's email. Require a true `email_verified` claim, or make acceptance of unverified/absent claims an explicit administrator policy before using the address for login. -- 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]
