Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refacftored OAuth2 #1324

Closed
wants to merge 4 commits into from
Closed

Refacftored OAuth2 #1324

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
891eeef
refactored OAuth2
ritikk112 Oct 13, 2024
b2872ae
refactored oauth
ritikk112 Oct 13, 2024
ce81c04
Refactored with minor adjustments
ritikk112 Oct 15, 2024
596ec66
fixed structuring in accordance to style guide
ritikk112 Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 23 additions & 34 deletions native/src/main/java/io/ballerina/stdlib/oauth2/OAuth2Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public static Object doHttpRequest(BString url, BMap<BString, Object> clientConf
}

BMap<BString, ?> customHeaders = getBMapValueIfPresent(clientConfig, OAuth2Constants.CUSTOM_HEADERS);
if (customHeaders != null) {
if(customHeaders != null){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these spaces removed?

for (Map.Entry<BString, ?> entry : customHeaders.entrySet()) {
headersList.add(entry.getKey().getValue());
headersList.add(((BString) entry.getValue()).getValue());
Expand All @@ -95,7 +95,6 @@ public static Object doHttpRequest(BString url, BMap<BString, Object> clientConf
String[] flatHeaders = headersList.toArray(String[]::new);
request = buildHttpRequest(uri, flatHeaders, textPayload);
}

if (secureSocket != null) {
try {
SSLContext sslContext = getSslContext(secureSocket);
Expand All @@ -104,20 +103,18 @@ public static Object doHttpRequest(BString url, BMap<BString, Object> clientConf
} catch (Exception e) {
return createError("Failed to init SSL context. " + e.getMessage());
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove additional spaces. You can configure your IDE to remove the trailing spaces when saving a file.

Suggested change
}
}

HttpClient client = buildHttpClient(httpVersion);
return callEndpoint(client, request);
}

private static URI buildUri(String url, BMap<BString, ?> secureSocket) throws IllegalArgumentException {
String[] urlParts = url.split(OAuth2Constants.SCHEME_SEPARATOR, 2);
if (urlParts.length == 1) {
urlParts = secureSocket != null ? new String[]{OAuth2Constants.HTTPS_SCHEME, urlParts[0]} :
urlParts = (secureSocket!=null) ? new String[]{OAuth2Constants.HTTPS_SCHEME, urlParts[0]} :
ritikk112 marked this conversation as resolved.
Show resolved Hide resolved
new String[]{OAuth2Constants.HTTP_SCHEME, urlParts[0]};
} else {
if (urlParts[0].equals(OAuth2Constants.HTTP_SCHEME) && secureSocket != null) {
err.println(OAuth2Constants.RUNTIME_WARNING_PREFIX + OAuth2Constants.HTTPS_RECOMMENDATION_ERROR);
}
} else if (urlParts[0].equals(OAuth2Constants.HTTP_SCHEME) && secureSocket != null){
err.println(OAuth2Constants.RUNTIME_WARNING_PREFIX + OAuth2Constants.HTTPS_RECOMMENDATION_ERROR);
}
urlParts[1] = urlParts[1].replaceAll(OAuth2Constants.DOUBLE_SLASH, OAuth2Constants.SINGLE_SLASH);
url = urlParts[0] + OAuth2Constants.SCHEME_SEPARATOR + urlParts[1];
Expand All @@ -134,44 +131,41 @@ private static SSLContext getSslContext(BMap<BString, ?> secureSocket) throws Ex
if (cert == null) {
throw new Exception("Need to configure 'crypto:TrustStore' or 'cert' with client SSL certificates file.");
}
KeyManagerFactory kmf;
TrustManagerFactory tmf;
KeyManagerFactory kmf = null;
TrustManagerFactory tmf = null;
if (cert instanceof BString) {
if (key != null) {
tmf = getTrustManagerFactory((BString) cert);
if (key.containsKey(OAuth2Constants.CERT_FILE)) {
BString certFile = key.get(OAuth2Constants.CERT_FILE);
BString keyFile = key.get(OAuth2Constants.KEY_FILE);
BString keyPassword = getBStringValueIfPresent(key, OAuth2Constants.KEY_PASSWORD);
kmf = getKeyManagerFactory(certFile, keyFile, keyPassword);
} else {
kmf = getKeyManagerFactory(key);
return buildSslContext(kmf.getKeyManagers(), tmf.getTrustManagers());
}
tmf = getTrustManagerFactory((BString) cert);
kmf = getKeyManagerFactory(key);
return buildSslContext(kmf.getKeyManagers(), tmf.getTrustManagers());
} else {
tmf = getTrustManagerFactory((BString) cert);
return buildSslContext(null, tmf.getTrustManagers());
}
}
tmf = getTrustManagerFactory((BString) cert);
return buildSslContext(null, tmf.getTrustManagers());
}
if (cert instanceof BMap) {
BMap<BString, BString> trustStore = (BMap<BString, BString>) cert;
if (key != null) {
if(key != null){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep these spaces.

I guess if you ran a build these should be identified by the checkstyle plugin. If it doesn't we might have to check what happened there @MohamedSabthar.

Use this styling guide for your IDE.

Suggested change
if(key != null){
if (key != null) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build is failing. @ritikk112 Let's fix it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where can I find the report for the failed build? I want to pinpoint the error but can't find the report, do I have to build and run on my local machine for it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for a detailed report on Checkstyle errors and SpotBugs reports, you should run them locally. Anyway, you should also build and test the project locally before raising the PR. 🙂

tmf = getTrustManagerFactory(trustStore);
if (key.containsKey(OAuth2Constants.CERT_FILE)) {
BString certFile = key.get(OAuth2Constants.CERT_FILE);
BString keyFile = key.get(OAuth2Constants.KEY_FILE);
BString keyPassword = getBStringValueIfPresent(key, OAuth2Constants.KEY_PASSWORD);
kmf = getKeyManagerFactory(certFile, keyFile, keyPassword);
} else {
kmf = getKeyManagerFactory(key);
return buildSslContext(kmf.getKeyManagers(), tmf.getTrustManagers());
}
tmf = getTrustManagerFactory(trustStore);
kmf = getKeyManagerFactory(key);
return buildSslContext(kmf.getKeyManagers(), tmf.getTrustManagers());
} else {
tmf = getTrustManagerFactory(trustStore);
return buildSslContext(null, tmf.getTrustManagers());
}
return buildSslContext(null, tmf.getTrustManagers());
}
return null;
throw new Exception("Failed to initialize SSLContext.");
}

private static HttpClient.Version getHttpVersion(String httpVersion) {
Expand Down Expand Up @@ -209,10 +203,9 @@ private static TrustManagerFactory getTrustManagerFactory(BString cert) throws E
TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
tmf.init(ts);
return tmf;
} else {
throw new Exception("Failed to get the public key from Crypto API. " +
((BError) publicKeyMap).getErrorMessage().getValue());
}
throw new Exception("Failed to get the public key from Crypto API. " +
((BError) publicKeyMap).getErrorMessage().getValue());
}

private static TrustManagerFactory getTrustManagerFactory(BMap<BString, BString> trustStore) throws Exception {
Expand Down Expand Up @@ -250,14 +243,10 @@ private static KeyManagerFactory getKeyManagerFactory(BString certFile, BString
KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
kmf.init(ks, "".toCharArray());
return kmf;
} else {
throw new Exception("Failed to get the private key from Crypto API. " +
((BError) privateKeyMap).getErrorMessage().getValue());
}
} else {
throw new Exception("Failed to get the public key from Crypto API. " +
((BError) publicKey).getErrorMessage().getValue());
}
throw new Exception("Failed to get the public key from Crypto API. " +
((BError) publicKey).getErrorMessage().getValue());
}

private static KeyStore getKeyStore(BString path, BString password) throws Exception {
Expand Down