Fix findings from Copilot review

This commit is contained in:
joel.tazzari 2026-04-17 09:16:01 +02:00
parent beb64d409c
commit cc18f82d09
4 changed files with 15 additions and 16 deletions

View File

@ -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");
}
}

View File

@ -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)) {

View File

@ -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

View File

@ -186,7 +186,7 @@
:href="getGitHubUrl(from)"
class="auth-btn github-auth"
style="height: 38px; width: 185px; padding: 0; margin-bottom: 5px;" >
<img src="/assets/github.svg" style="width: 32px; padding: 5px" />
<img src="/assets/github.svg" alt="Google" style="width: 32px; padding: 5px" />
<a-typography-text>Sign in with Github</a-typography-text>
</a-button>
</div>
@ -198,7 +198,7 @@
:href="getGoogleUrl(from)"
class="auth-btn google-auth"
style="height: 38px; width: 185px; padding: 0" >
<img src="/assets/google.svg" style="width: 32px; padding: 5px" />
<img src="/assets/google.svg" alt="Github" style="width: 32px; padding: 5px" />
<a-typography-text>Sign in with Google</a-typography-text>
</a-button>
</div>
@ -210,7 +210,7 @@
:href="getKeycloakUrl(from)"
class="auth-btn keycloak-auth"
style="height: 38px; width: 185px; padding: 0" >
<img src="/assets/keycloak.svg" style="width: 32px; padding: 5px" />
<img src="/assets/keycloak.svg" alt="Keycloak" style="width: 32px; padding: 5px" />
<a-typography-text>Sign in with Keycloak</a-typography-text>
</a-button>
</div>