oauth2: harden the generic OIDC provider from the second review

Key the one-time email cache on the resolved registration instead of the
request domain. verifyOAuthCodeAndGetUser can resolve the code with no
domain while the login that follows lands on a concrete domain; the old key
mixed the two, so the single use authorization code was redeemed a second
time and rejected. The registration is resolved up front and its uuid keys
the cache, which also keeps a code scoped to its own registration.

Make the verified-email requirement a domain setting
(oauth2.oidc.require.verified.email, default true). An explicit
email_verified=false is always rejected; an absent claim is rejected only
when the setting is on, so a trusted provider that does not send it, such as
Microsoft Entra ID, can be allowed per domain.

Stop returning the provider's raw response body on a failed discovery or
JWKS fetch; it now goes to the log and the caller gets a generic message,
matching the token exchange.

Author: rootnomad

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManager.java

@@ -41,6 +41,10 @@ public interface OAuth2AuthManager extends PluggableAPIAuthenticator, PluggableS
             "List of OAuth plugins", true);
     public static final ConfigKey<String> OAuth2PluginsExclude = new ConfigKey<String>("Advanced", String.class, "oauth2.plugins.exclude", "",
             "List of OAuth plugins which are excluded", true);
+    public static final ConfigKey<Boolean> OIDCRequireVerifiedEmail = new ConfigKey<Boolean>("Advanced", Boolean.class, "oauth2.oidc.require.verified.email", "true",
+            "For the generic OIDC provider, require the id_token to assert a verified email (email_verified=true) before the address is used to identify a CloudStack account. " +
+            "When set to false for a trusted identity provider that does not send the claim (such as Microsoft Entra ID), an id_token without the claim is accepted, " +
+            "but one that explicitly marks the email as not verified is always rejected. This can be configured at domain level.", true, ConfigKey.Scope.Domain);
 
     /**
      * Lists user OAuth2 provider plugins

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java

@@ -395,6 +395,6 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {OAuth2IsPluginEnabled, OAuth2Plugins, OAuth2PluginsExclude};
+        return new ConfigKey<?>[] {OAuth2IsPluginEnabled, OAuth2Plugins, OAuth2PluginsExclude, OIDCRequireVerifiedEmail};
     }
 }

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/oidc/GenericOIDCOAuth2Provider.java

@@ -30,6 +30,8 @@
 import javax.ws.rs.core.HttpHeaders;
 
 import org.apache.cloudstack.auth.UserOAuth2Authenticator;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.oauth2.OAuth2AuthManager;
 import org.apache.cloudstack.oauth2.dao.OauthProviderDao;
 import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
 import org.apache.commons.codec.digest.DigestUtils;
@@ -153,9 +155,10 @@ public boolean verifyUser(String email, String secretCode, Long domainId, String
             throw new CloudAuthenticationException("Either email or secret code should not be null/empty");
         }
 
-        String verifiedEmail = verifiedEmailCache.asMap().remove(verifiedEmailKey(providerName, secretCode, domainId));
+        OauthProviderVO registration = findRegistration(providerName, domainId);
+        String verifiedEmail = verifiedEmailCache.asMap().remove(verifiedEmailKey(registration, secretCode));
         if (verifiedEmail == null) {
-            verifiedEmail = resolveEmail(secretCode, domainId, providerName);
+            verifiedEmail = resolveEmail(registration, secretCode);
         }
         if (StringUtils.isBlank(verifiedEmail) || !email.equals(verifiedEmail)) {
             throw new CloudRuntimeException("Unable to verify the email address with the provided secret");
@@ -166,26 +169,26 @@ public boolean verifyUser(String email, String secretCode, Long domainId, String
 
     @Override
     public String verifySecretCodeAndFetchEmail(String secretCode, Long domainId, String providerName) {
-        String email = resolveEmail(secretCode, domainId, providerName);
-        verifiedEmailCache.put(verifiedEmailKey(providerName, secretCode, domainId), email);
+        OauthProviderVO registration = findRegistration(providerName, domainId);
+        String email = resolveEmail(registration, secretCode);
+        verifiedEmailCache.put(verifiedEmailKey(registration, 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);
+    protected String resolveEmail(OauthProviderVO registration, String secretCode) {
+        OIDCMetadata metadata = getMetadata(registration);
+        String idToken = exchangeAuthorizationCode(secretCode, registration, metadata);
 
-        return validateAndExtractEmail(idToken, provider, metadata);
+        return validateAndExtractEmail(idToken, registration, metadata);
     }
 
     @Override
     public String getUserEmailAddress() throws CloudRuntimeException {
         return null;
     }
 
-    private String verifiedEmailKey(String providerName, String secretCode, Long domainId) {
-        return DigestUtils.sha256Hex(providerName + ":" + domainId + ":" + secretCode);
+    private String verifiedEmailKey(OauthProviderVO registration, String secretCode) {
+        return DigestUtils.sha256Hex(registration.getUuid() + ":" + secretCode);
     }
 
     protected OauthProviderVO findRegistration(String providerName, Long domainId) {
@@ -279,18 +282,38 @@ protected String validateAndExtractEmail(String idToken, OauthProviderVO provide
         if (StringUtils.isBlank(email)) {
             throw new CloudAuthenticationException("The id_token carries no email claim");
         }
-        if (!isEmailVerified(claims)) {
-            throw new CloudAuthenticationException("The identity provider has not verified the email address in the id_token");
+        Boolean emailVerified = emailVerifiedClaim(claims);
+        if (Boolean.FALSE.equals(emailVerified)) {
+            throw new CloudAuthenticationException("The identity provider has marked the email address in the id_token as not verified");
+        }
+        if (emailVerified == null && isVerifiedEmailRequired(provider)) {
+            throw new CloudAuthenticationException(String.format(
+                    "The identity provider did not assert that the email address is verified; set %s to false for this domain to accept it",
+                    OAuth2AuthManager.OIDCRequireVerifiedEmail.key()));
         }
         return email;
     }
 
-    private boolean isEmailVerified(JwtClaims claims) {
+    private Boolean emailVerifiedClaim(JwtClaims claims) {
         Object verified = claims.getClaim("email_verified");
         if (verified instanceof Boolean) {
             return (Boolean) verified;
         }
-        return verified instanceof String && Boolean.parseBoolean((String) verified);
+        if (verified instanceof String) {
+            String value = ((String) verified).trim();
+            if ("true".equalsIgnoreCase(value)) {
+                return Boolean.TRUE;
+            }
+            if ("false".equalsIgnoreCase(value)) {
+                return Boolean.FALSE;
+            }
+        }
+        return null;
+    }
+
+    protected boolean isVerifiedEmailRequired(OauthProviderVO provider) {
+        return Boolean.TRUE.equals(
+                OAuth2AuthManager.OIDCRequireVerifiedEmail.valueInScope(ConfigKey.Scope.Domain, provider.getDomainId(), false));
     }
 
     protected void verifySignature(JwsJwtCompactConsumer consumer, OIDCMetadata metadata, OauthProviderVO provider) {
@@ -344,7 +367,8 @@ protected String httpGet(String url, String failureMessage) {
         try (CloseableHttpResponse response = httpClient.execute(new HttpGet(url))) {
             String body = EntityUtils.toString(response.getEntity());
             if (response.getStatusLine().getStatusCode() != 200) {
-                throw new CloudRuntimeException(String.format("%s: %s", failureMessage, body));
+                logger.warn("{} (HTTP {}): {}", failureMessage, response.getStatusLine().getStatusCode(), body);
+                throw new CloudRuntimeException(failureMessage);
             }
             return body;
         } catch (IOException e) {

plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/oidc/GenericOIDCOAuth2ProviderTest.java

@@ -19,8 +19,10 @@
 package org.apache.cloudstack.oauth2.oidc;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
@@ -48,6 +50,10 @@
 import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactProducer;
 import org.apache.cxf.rs.security.jose.jws.JwsUtils;
 import org.apache.cxf.rs.security.jose.jwt.JwtClaims;
+import org.apache.http.StatusLine;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.junit.After;
 import org.junit.Before;
@@ -360,7 +366,7 @@ public void testEveryCallExchangesItsOwnAuthorizationCode() {
     @Test(expected = CloudRuntimeException.class)
     public void testVerifyUserRejectsAnEmailThatDoesNotMatchTheToken() {
         when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
-        doReturn("someone-else@example.com").when(provider).resolveEmail("code", null, REGISTRATION);
+        doReturn("someone-else@example.com").when(provider).resolveEmail(registration, "code");
 
         provider.verifyUser("user@example.com", "code", null, REGISTRATION);
     }
@@ -371,38 +377,64 @@ public void testVerifyUserRejectsAnEmailThatDoesNotMatchTheToken() {
      */
     @Test
     public void testLoginAfterVerificationDoesNotRedeemTheCodeAgain() {
-        doReturn("user@example.com").when(provider).resolveEmail("code", null, REGISTRATION);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
+        doReturn("user@example.com").when(provider).resolveEmail(registration, "code");
 
         assertEquals("user@example.com", provider.verifySecretCodeAndFetchEmail("code", null, REGISTRATION));
         assertTrue(provider.verifyUser("user@example.com", "code", null, REGISTRATION));
 
-        verify(provider, times(1)).resolveEmail("code", null, REGISTRATION);
+        verify(provider, times(1)).resolveEmail(registration, "code");
+    }
+
+    /**
+     * verifyOAuthCodeAndGetUser can resolve the code with no domain while the login that follows lands on a
+     * concrete domain. The cached email therefore has to key on the resolved registration, not the request
+     * domain, or the single use authorization code would be redeemed a second time and rejected.
+     */
+    @Test
+    public void testCachedEmailIsReusedWhenTheLoginResolvesToADifferentDomainScope() {
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, 1L)).thenReturn(registration);
+        doReturn("user@example.com").when(provider).resolveEmail(registration, "code");
+
+        assertEquals("user@example.com", provider.verifySecretCodeAndFetchEmail("code", null, REGISTRATION));
+        assertTrue(provider.verifyUser("user@example.com", "code", 1L, REGISTRATION));
+
+        verify(provider, times(1)).resolveEmail(registration, "code");
     }
 
     @Test
     public void testVerifiedCodeIsServedFromTheCacheOnlyOnce() {
-        doReturn("user@example.com").when(provider).resolveEmail("code", null, REGISTRATION);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
+        doReturn("user@example.com").when(provider).resolveEmail(registration, "code");
 
         provider.verifySecretCodeAndFetchEmail("code", null, REGISTRATION);
         provider.verifyUser("user@example.com", "code", null, REGISTRATION);
         provider.verifyUser("user@example.com", "code", null, REGISTRATION);
 
-        verify(provider, times(2)).resolveEmail("code", null, REGISTRATION);
+        verify(provider, times(2)).resolveEmail(registration, "code");
     }
 
     @Test(expected = CloudRuntimeException.class)
     public void testAnotherCodeIsNeverAnsweredFromTheCache() {
-        doReturn("user@example.com").when(provider).resolveEmail("user-code", null, REGISTRATION);
-        doThrow(new CloudRuntimeException("invalid_grant")).when(provider).resolveEmail("unrelated-code", null, REGISTRATION);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
+        doReturn("user@example.com").when(provider).resolveEmail(registration, "user-code");
+        doThrow(new CloudRuntimeException("invalid_grant")).when(provider).resolveEmail(registration, "unrelated-code");
 
         provider.verifySecretCodeAndFetchEmail("user-code", null, REGISTRATION);
         provider.verifyUser("user@example.com", "unrelated-code", null, REGISTRATION);
     }
 
     @Test(expected = CloudRuntimeException.class)
     public void testCachedCodeIsScopedToItsRegistration() {
-        doReturn("user@example.com").when(provider).resolveEmail("code", null, REGISTRATION);
-        doThrow(new CloudRuntimeException("invalid_grant")).when(provider).resolveEmail("code", null, "other-idp");
+        OauthProviderVO other = new OauthProviderVO();
+        other.setProvider("other-idp");
+        other.setType(GenericOIDCOAuth2Provider.OIDC_PROVIDER_TYPE);
+        other.setIssuerUrl(ISSUER);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback(REGISTRATION, null)).thenReturn(registration);
+        when(oauthProviderDao.findByProviderAndDomainWithGlobalFallback("other-idp", null)).thenReturn(other);
+        doReturn("user@example.com").when(provider).resolveEmail(registration, "code");
+        doThrow(new CloudRuntimeException("invalid_grant")).when(provider).resolveEmail(other, "code");
 
         provider.verifySecretCodeAndFetchEmail("code", null, REGISTRATION);
         provider.verifyUser("user@example.com", "code", null, "other-idp");
@@ -412,4 +444,74 @@ public void testCachedCodeIsScopedToItsRegistration() {
     public void testVerifyUserRejectsEmptyArguments() {
         provider.verifyUser("", "", null, REGISTRATION);
     }
+
+    /**
+     * Some conformant providers, such as Microsoft Entra ID, never send the email_verified claim. The domain
+     * can opt out of requiring it, and then a token without the claim is accepted.
+     */
+    @Test
+    public void testAbsentEmailVerifiedIsAcceptedWhenTheDomainDoesNotRequireIt() throws Exception {
+        KeyPair keys = rsaKeyPair();
+        publishKey(keys, "key-1");
+        doReturn(false).when(provider).isVerifiedEmailRequired(registration);
+
+        assertEquals("user@example.com",
+                provider.validateAndExtractEmail(signedIdToken(keys, "key-1", "user@example.com", null), registration, metadata()));
+    }
+
+    @Test(expected = CloudAuthenticationException.class)
+    public void testAbsentEmailVerifiedIsRejectedWhenTheDomainRequiresIt() throws Exception {
+        KeyPair keys = rsaKeyPair();
+        publishKey(keys, "key-1");
+        doReturn(true).when(provider).isVerifiedEmailRequired(registration);
+
+        provider.validateAndExtractEmail(signedIdToken(keys, "key-1", "user@example.com", null), registration, metadata());
+    }
+
+    /**
+     * An explicit email_verified=false is the provider telling us the address is not verified, so it is
+     * rejected regardless of the domain policy.
+     */
+    @Test(expected = CloudAuthenticationException.class)
+    public void testExplicitlyUnverifiedEmailIsRejectedEvenWhenTheDomainDoesNotRequireVerification() throws Exception {
+        KeyPair keys = rsaKeyPair();
+        publishKey(keys, "key-1");
+        doReturn(false).when(provider).isVerifiedEmailRequired(registration);
+
+        provider.validateAndExtractEmail(signedIdToken(keys, "key-1", "user@example.com", false), registration, metadata());
+    }
+
+    @Test
+    public void testHttpGetDoesNotLeakTheProviderBodyOnFailure() throws Exception {
+        CloseableHttpResponse response = Mockito.mock(CloseableHttpResponse.class);
+        StatusLine statusLine = Mockito.mock(StatusLine.class);
+        when(statusLine.getStatusCode()).thenReturn(500);
+        when(response.getStatusLine()).thenReturn(statusLine);
+        when(response.getEntity()).thenReturn(new StringEntity("sensitive idp body"));
+        when(httpClient.execute(any(HttpGet.class))).thenReturn(response);
+
+        try {
+            provider.httpGet(ISSUER + "/jwks", "Unable to read the signing keys from " + ISSUER + "/jwks");
+            fail("expected the fetch to fail");
+        } catch (CloudRuntimeException e) {
+            assertEquals("Unable to read the signing keys from " + ISSUER + "/jwks", e.getMessage());
+            assertFalse(e.getMessage().contains("sensitive idp body"));
+        }
+    }
+
+    private String signedIdToken(KeyPair keys, String keyId, String email, Boolean emailVerified) {
+        JwtClaims claims = new JwtClaims();
+        claims.setIssuer(ISSUER);
+        claims.setAudiences(Collections.singletonList(CLIENT_ID));
+        claims.setSubject("12345");
+        claims.setExpiryTime(System.currentTimeMillis() / 1000L + 3600);
+        claims.setClaim("email", email);
+        if (emailVerified != null) {
+            claims.setClaim("email_verified", emailVerified);
+        }
+        JwsHeaders headers = new JwsHeaders(SignatureAlgorithm.RS256);
+        headers.setKeyId(keyId);
+        return new JwsJwtCompactProducer(headers, claims)
+                .signWith(JwsUtils.getPrivateKeySignatureProvider(keys.getPrivate(), SignatureAlgorithm.RS256));
+    }
 }