From cc18f82d09e904c91762747406cb31c8a2d0daaa Mon Sep 17 00:00:00 2001 From: "joel.tazzari" Date: Fri, 17 Apr 2026 09:16:01 +0200 Subject: [PATCH] Fix findings from Copilot review --- .../oauth2/api/command/RegisterOAuthProviderCmd.java | 10 +++++----- .../oauth2/keycloak/KeycloakOAuth2Provider.java | 5 ++++- .../oauth2/keycloak/KeycloakOAuth2ProviderTest.java | 10 +++------- ui/src/views/auth/Login.vue | 6 +++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java index 96fce48b7a9..496933ba32b 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/RegisterOAuthProviderCmd.java @@ -31,8 +31,8 @@ import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.oauth2.OAuth2AuthManager; import org.apache.cloudstack.oauth2.api.response.OauthProviderResponse; import org.apache.cloudstack.oauth2.vo.OauthProviderVO; -import org.apache.commons.codec.binary.StringUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; import com.cloud.exception.ConcurrentOperationException; @@ -58,7 +58,7 @@ public class RegisterOAuthProviderCmd extends BaseCmd { @Parameter(name = ApiConstants.REDIRECT_URI, type = CommandType.STRING, description = "Redirect URI pre-registered in the specific OAuth provider", required = true) private String redirectUri; - @Parameter(name = ApiConstants.AUTHORIZE_URL, type = CommandType.STRING, description = "Authorize URL for OAuth initialization (only required for keyloack provider)") + @Parameter(name = ApiConstants.AUTHORIZE_URL, type = CommandType.STRING, description = "Authorize URL for OAuth initialization (only required for keycloack provider)") private String authorizeUrl; @Parameter(name = ApiConstants.TOKEN_URL, type = CommandType.STRING, description = "Token URL for OAuth finalization (only required for keycloak provider)") @@ -115,10 +115,10 @@ public class RegisterOAuthProviderCmd extends BaseCmd { @Override public void execute() throws ServerApiException, ConcurrentOperationException, EntityExistsException { if (StringUtils.equals("keycloak", getProvider())) { - if (getAuthorizeUrl() == null || "".equals(getAuthorizeUrl())) { - throw new ServerApiException(ApiErrorCode.BAD_REQUEST, "Parameter authorizationurl is mandatory for keycloak OAuth Provider"); + if (StringUtils.isBlank(getAuthorizeUrl())) { + throw new ServerApiException(ApiErrorCode.BAD_REQUEST, "Parameter authorizeurl is mandatory for keycloak OAuth Provider"); } - if (getTokenUrl() == null || "".equals(getTokenUrl())) { + if (StringUtils.isBlank(getTokenUrl())) { throw new ServerApiException(ApiErrorCode.BAD_REQUEST, "Parameter tokenurl is mandatory for keycloak OAuth Provider"); } } diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2Provider.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2Provider.java index 0d028cfc4a1..881858b34b1 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2Provider.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2Provider.java @@ -99,6 +99,9 @@ public class KeycloakOAuth2Provider extends AdapterBase implements UserOAuth2Aut @Override public String verifyCodeAndFetchEmail(String secretCode) { OauthProviderVO provider = oauthProviderDao.findByProvider(getName()); + if (provider == null) { + throw new CloudAuthenticationException("Keycloak provider is not registered, so user cannot be verified"); + } if (StringUtils.isBlank(idToken)) { String auth = provider.getClientId() + ":" + provider.getSecretKey(); @@ -115,7 +118,7 @@ public class KeycloakOAuth2Provider extends AdapterBase implements UserOAuth2Aut try { post.setEntity(new UrlEncodedFormEntity(params)); } catch (UnsupportedEncodingException e) { - throw new CloudRuntimeException("Unable to generating URL parameters: " + e.getMessage()); + throw new CloudRuntimeException("Unable to generate URL parameters: " + e.getMessage()); } try (CloseableHttpResponse response = httpClient.execute(post)) { diff --git a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2ProviderTest.java b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2ProviderTest.java index c1822c81bab..6346efa9f13 100644 --- a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2ProviderTest.java +++ b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/keycloak/KeycloakOAuth2ProviderTest.java @@ -143,9 +143,7 @@ public class KeycloakOAuth2ProviderTest { when(httpClient.execute(any(HttpPost.class))).thenReturn(response); - boolean result = provider.verifyUser("user@example.com", secretCode); - - assertTrue("L'utilisateur devrait être vérifié avec succès", result); + provider.verifyUser("user@example.com", secretCode); } @Test(expected = CloudRuntimeException.class) @@ -180,9 +178,7 @@ public class KeycloakOAuth2ProviderTest { when(httpClient.execute(any(HttpPost.class))).thenReturn(response); - boolean result = provider.verifyUser(testEmail, secretCode); - - assertTrue("L'utilisateur devrait être vérifié avec succès", result); + provider.verifyUser(testEmail, secretCode); } @Test @@ -219,7 +215,7 @@ public class KeycloakOAuth2ProviderTest { boolean result = provider.verifyUser(testEmail, secretCode); - assertTrue("L'utilisateur devrait être vérifié avec succès", result); + assertTrue("User successfully verified", result); } @Test diff --git a/ui/src/views/auth/Login.vue b/ui/src/views/auth/Login.vue index 691fd75cf34..d968b2487a6 100644 --- a/ui/src/views/auth/Login.vue +++ b/ui/src/views/auth/Login.vue @@ -186,7 +186,7 @@ :href="getGitHubUrl(from)" class="auth-btn github-auth" style="height: 38px; width: 185px; padding: 0; margin-bottom: 5px;" > - + Google Sign in with Github @@ -198,7 +198,7 @@ :href="getGoogleUrl(from)" class="auth-btn google-auth" style="height: 38px; width: 185px; padding: 0" > - + Github Sign in with Google @@ -210,7 +210,7 @@ :href="getKeycloakUrl(from)" class="auth-btn keycloak-auth" style="height: 38px; width: 185px; padding: 0" > - + Keycloak Sign in with Keycloak