oauth2: address generic OIDC review findings

- require a verified email (email_verified) in the id_token before using it
  as the account identity, so an unverified address cannot claim a user
- key the single-use verified-email cache by domain as well, so a code
  verified for one domain cannot be consumed for another
- store the provider type lowercased so list, update and the UI agree with
  the case-insensitive dispatch lookup
- correct the since metadata on the new type/issuerUrl parameters and
  response fields from 4.24.0 to 24.0.0

Author: nagaboinaramgopal

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

@@ -315,7 +315,7 @@ private OauthProviderVO saveOauthProvider(String provider, String description, S
         oauthProviderVO.setDomainId(domainId);
         oauthProviderVO.setAuthorizeUrl(authorizeUrl);
         oauthProviderVO.setTokenUrl(tokenUrl);
-        oauthProviderVO.setType(type);
+        oauthProviderVO.setType(StringUtils.lowerCase(type));
         oauthProviderVO.setIssuerUrl(issuerUrl);
         oauthProviderVO.setEnabled(true);
 

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java

@@ -79,12 +79,12 @@ public class RegisterOAuthProviderCmd extends BaseCmd {
 
     @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")
+                    + "When set, the name in provider is a label chosen by the administrator rather than a built in provider name.", since = "24.0.0")
     private String type;
 
     @Parameter(name = ApiConstants.ISSUER_URL, type = CommandType.STRING,
             description = "Issuer URL of the OpenID Connect provider, required for type oidc. The token endpoint and the keys that sign "
-                    + "its tokens are read from the issuer's discovery document", since = "4.24.0")
+                    + "its tokens are read from the issuer's discovery document", since = "24.0.0")
     private String issuerUrl;
 
     @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP,

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/UpdateOAuthProviderCmd.java

@@ -69,7 +69,7 @@ public final class UpdateOAuthProviderCmd extends BaseCmd {
     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")
+            description = "Issuer URL of the OpenID Connect provider, used to read its discovery document", since = "24.0.0")
     private String issuerUrl;
 
     @Parameter(name = ApiConstants.ENABLED, type = CommandType.BOOLEAN, description = "OAuth provider will be enabled or disabled based on this value")

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/response/OauthProviderResponse.java

@@ -71,11 +71,11 @@ public class OauthProviderResponse extends BaseResponse {
     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")
+    @Param(description = "Type of the provider, for example oidc for a generic OpenID Connect provider. Empty for the built in providers", since = "24.0.0")
     private String type;
 
     @SerializedName(ApiConstants.ISSUER_URL)
-    @Param(description = "Issuer URL of the OpenID Connect provider, used for discovery", since = "4.24.0")
+    @Param(description = "Issuer URL of the OpenID Connect provider, used for discovery", since = "24.0.0")
     private String issuerUrl;
 
     @SerializedName(ApiConstants.AUTHORIZE_URL)

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

@@ -153,7 +153,7 @@ 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));
+        String verifiedEmail = verifiedEmailCache.asMap().remove(verifiedEmailKey(providerName, secretCode, domainId));
         if (verifiedEmail == null) {
             verifiedEmail = resolveEmail(secretCode, domainId, providerName);
         }
@@ -167,7 +167,7 @@ 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), email);
+        verifiedEmailCache.put(verifiedEmailKey(providerName, secretCode, domainId), email);
         return email;
     }
 
@@ -184,8 +184,8 @@ public String getUserEmailAddress() throws CloudRuntimeException {
         return null;
     }
 
-    private String verifiedEmailKey(String providerName, String secretCode) {
-        return DigestUtils.sha256Hex(providerName + ":" + secretCode);
+    private String verifiedEmailKey(String providerName, String secretCode, Long domainId) {
+        return DigestUtils.sha256Hex(providerName + ":" + domainId + ":" + secretCode);
     }
 
     protected OauthProviderVO findRegistration(String providerName, Long domainId) {
@@ -279,9 +279,20 @@ 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");
+        }
         return email;
     }
 
+    private boolean isEmailVerified(JwtClaims claims) {
+        Object verified = claims.getClaim("email_verified");
+        if (verified instanceof Boolean) {
+            return (Boolean) verified;
+        }
+        return verified instanceof String && Boolean.parseBoolean((String) verified);
+    }
+
     protected void verifySignature(JwsJwtCompactConsumer consumer, OIDCMetadata metadata, OauthProviderVO provider) {
         if (StringUtils.isBlank(metadata.getJwksUri())) {
             throw new CloudAuthenticationException(String.format(

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

@@ -137,6 +137,23 @@ public void testRegisterRejectsBuiltinNameWithType() {
         }
     }
 
+    @Test
+    public void testRegisterNormalizesTypeCasing() {
+        OAuth2AuthManagerImpl.userOAuth2AuthenticationProvidersMap.put("oidc", Mockito.mock(org.apache.cloudstack.auth.UserOAuth2Authenticator.class));
+        when(_authManager.isOAuthPluginEnabled(Mockito.nullable(Long.class))).thenReturn(true);
+        RegisterOAuthProviderCmd cmd = Mockito.mock(RegisterOAuthProviderCmd.class);
+        when(cmd.getProvider()).thenReturn("corp-idp");
+        when(cmd.getType()).thenReturn("OIDC");
+        when(cmd.getDomainId()).thenReturn(null);
+        when(_authManager._oauthProviderDao.findByProviderAndDomain(Mockito.anyString(), Mockito.isNull())).thenReturn(null);
+        org.mockito.ArgumentCaptor<OauthProviderVO> captor = org.mockito.ArgumentCaptor.forClass(OauthProviderVO.class);
+        when(_authManager._oauthProviderDao.persist(captor.capture())).thenReturn(new OauthProviderVO());
+
+        _authManager.registerOauthProvider(cmd);
+
+        assertEquals("oidc", captor.getValue().getType());
+    }
+
     @Test
     public void testUpdateOauthProvider() {
         Long id = 1L;

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

@@ -200,6 +200,7 @@ private String signedIdToken(KeyPair keys, String keyId, String email) {
         claims.setSubject("12345");
         claims.setExpiryTime(System.currentTimeMillis() / 1000L + 3600);
         claims.setClaim("email", email);
+        claims.setClaim("email_verified", true);
         JwsHeaders headers = new JwsHeaders(SignatureAlgorithm.RS256);
         headers.setKeyId(keyId);
         return new JwsJwtCompactProducer(headers, claims)
@@ -244,6 +245,25 @@ public void testSigningKeysAreCachedAcrossVerifications() throws Exception {
         verify(provider, times(1)).httpGet(eq(ISSUER + "/jwks"), anyString());
     }
 
+    @Test(expected = CloudAuthenticationException.class)
+    public void testUnverifiedEmailIsRejected() throws Exception {
+        KeyPair keys = rsaKeyPair();
+        publishKey(keys, "key-1");
+        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", "user@example.com");
+        claims.setClaim("email_verified", false);
+        JwsHeaders headers = new JwsHeaders(SignatureAlgorithm.RS256);
+        headers.setKeyId("key-1");
+        String token = new JwsJwtCompactProducer(headers, claims)
+                .signWith(JwsUtils.getPrivateKeySignatureProvider(keys.getPrivate(), SignatureAlgorithm.RS256));
+
+        provider.validateAndExtractEmail(token, registration, metadata());
+    }
+
     @Test(expected = CloudAuthenticationException.class)
     public void testTokenWithAlteredClaimsIsRejected() throws Exception {
         KeyPair keys = rsaKeyPair();