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.

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

Adding Old Constructor back to AKV Provider #675

Merged
merged 10 commits into from
May 25, 2018
16 changes: 13 additions & 3 deletions src/main/java/com/microsoft/sqlserver/jdbc/KeyVaultCredential.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,30 @@
*/
class KeyVaultCredential extends KeyVaultCredentials {

SQLServerKeyVaultAuthenticationCallback authenticationCallback = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation.

String clientId = null;
String clientKey = null;
String accessToken = null;

KeyVaultCredential(String clientId,
String clientKey) {
this.clientId = clientId;
this.clientKey = clientKey;
}


KeyVaultCredential(SQLServerKeyVaultAuthenticationCallback authenticationCallback) {
this.authenticationCallback = authenticationCallback;
}

public String doAuthenticate(String authorization,
String resource,
String scope) {
AuthenticationResult token = getAccessTokenFromClientCredentials(authorization, resource, clientId, clientKey);
return token.getAccessToken();
if(authenticationCallback==null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

AuthenticationResult token = getAccessTokenFromClientCredentials(authorization, resource, clientId, clientKey);
return token.getAccessToken();
}else {
return authenticationCallback.getAccessToken(authorization, resource, scope);
}
}

private static AuthenticationResult getAccessTokenFromClientCredentials(String authorization,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,20 @@
import java.security.NoSuchAlgorithmException;
import java.text.MessageFormat;
import java.util.Locale;
import java.util.concurrent.ExecutorService;

import com.microsoft.azure.AzureResponseBuilder;
import com.microsoft.azure.keyvault.KeyVaultClient;
import com.microsoft.azure.keyvault.models.KeyBundle;
import com.microsoft.azure.keyvault.models.KeyOperationResult;
import com.microsoft.azure.keyvault.models.KeyVerifyResult;
import com.microsoft.azure.keyvault.webkey.JsonWebKeyEncryptionAlgorithm;
import com.microsoft.azure.keyvault.webkey.JsonWebKeySignatureAlgorithm;
import com.microsoft.azure.serializer.AzureJacksonAdapter;
import com.microsoft.rest.RestClient;

import okhttp3.OkHttpClient;
import retrofit2.Retrofit;

/**
* Provides implementation similar to certificate store provider. A CEK encrypted with certificate store provider should be decryptable by this
Expand All @@ -41,7 +48,9 @@ public class SQLServerColumnEncryptionAzureKeyVaultProvider extends SQLServerCol
* Column Encryption Key Store Provider string
*/
String name = "AZURE_KEY_VAULT";


private final String baseUrl = "https://{vaultBaseUrl}";

private final String azureKeyVaultDomainName = "vault.azure.net";

private final String rsaEncryptionAlgorithmWithOAEPForAKV = "RSA-OAEP";
Expand All @@ -53,7 +62,7 @@ public class SQLServerColumnEncryptionAzureKeyVaultProvider extends SQLServerCol

private KeyVaultClient keyVaultClient;

private KeyVaultCredential credential;
private KeyVaultCredential credentials;

public void setName(String name) {
this.name = name;
Expand All @@ -62,7 +71,35 @@ public void setName(String name) {
public String getName() {
return this.name;
}


/**
* Constructor that takes a callback function to authenticate to AAD. This is used by KeyVaultClient at runtime to authenticate to Azure Key
* Vault.
*
* @param authenticationCallback
* - Callback function used for authenticating to AAD.
* @param executorService
* - The ExecutorService used to create the keyVaultClient
* @throws SQLServerException
* when an error occurs
*/
public SQLServerColumnEncryptionAzureKeyVaultProvider(SQLServerKeyVaultAuthenticationCallback authenticationCallback,
ExecutorService executorService) throws SQLServerException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is executorService used?

Copy link
Member Author

@cheenamalhotra cheenamalhotra May 4, 2018

Choose a reason for hiding this comment

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

executorService is not consumed here because the KeyVaultClientImpl now manages its own KeyVaultClientService as here. These services are used by Retrofit to make Rest Calls.

The only reason of adding this here was to retain the old constructor params used by current applications in industry to allow no change upgrade of JDBC Driver version, but can be removed as well.

Copy link
Contributor

@ulvii ulvii May 4, 2018

Choose a reason for hiding this comment

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

Could lead to confusions. Let's make sure we discuss this with Jakub.

Copy link
Member

Choose a reason for hiding this comment

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

Since we aren't doing anything with the executor, and the constructor is just here for backwards compatibility, shouldn't we make a call to SQLServerColumnEncryptionAzureKeyVaultProvider(SQLServerKeyVaultAuthenticationCallback authenticationCallback)? This might confuse people who are stepping through the code, thinking there is a very small change somewhere and that's why there are separate functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

if (null == authenticationCallback) {
MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_NullValue"));
Object[] msgArgs1 = {"SQLServerKeyVaultAuthenticationCallback"};
throw new SQLServerException(form.format(msgArgs1), null);
}
credentials = new KeyVaultCredential(authenticationCallback);
RestClient restClient = new RestClient.Builder(new OkHttpClient.Builder(), new Retrofit.Builder())
.withBaseUrl(baseUrl)
.withCredentials(credentials)
.withSerializerAdapter(new AzureJacksonAdapter())
.withResponseBuilderFactory(new AzureResponseBuilder.Factory())
.build();
keyVaultClient = new KeyVaultClient(restClient);
}

/**
* Constructor that authenticates to AAD. This is used by KeyVaultClient at runtime to authenticate to Azure Key
* Vault.
Expand All @@ -76,8 +113,8 @@ public String getName() {
*/
public SQLServerColumnEncryptionAzureKeyVaultProvider(String clientId,
String clientKey) throws SQLServerException {
credential = new KeyVaultCredential(clientId, clientKey);
keyVaultClient = new KeyVaultClient(credential);
credentials = new KeyVaultCredential(clientId, clientKey);
keyVaultClient = new KeyVaultClient(credentials);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Microsoft JDBC Driver for SQL Server
*
* Copyright(c) Microsoft Corporation All rights reserved.
*
* This program is made available under the terms of the MIT License. See the LICENSE file in the project root for more information.
*/

package com.microsoft.sqlserver.jdbc;

public interface SQLServerKeyVaultAuthenticationCallback {

/**
* The authentication callback delegate which is to be implemented by the client code
*
* @param authority
* - Identifier of the authority, a URL.
* @param resource
* - Identifier of the target resource that is the recipient of the requested token, a URL.
* @param scope
* - The scope of the authentication request.
* @return access token
*/
public String getAccessToken(String authority,
String resource,
String scope);
}