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
21 changes: 17 additions & 4 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,33 @@
*/
class KeyVaultCredential extends KeyVaultCredentials {

String clientId = null;
String clientKey = null;
SQLServerKeyVaultAuthenticationCallback authenticationCallback = null;
String clientId = null;
String clientKey = null;
String accessToken = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting here?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's how mssql-jdbc formatter works here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is what happens when you apply the formatter.

SQLServerKeyVaultAuthenticationCallback authenticationCallback = null;
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();
String accessToken;
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.

if (null == authenticationCallback)

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

private static AuthenticationResult getAccessTokenFromClientCredentials(String authorization,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,26 @@
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
* provider and vice versa.
*
* Envolope Format for the encrypted column encryption key version + keyPathLength + ciphertextLength + keyPath + ciphertext + signature version: A
* Envelope Format for the encrypted column encryption key version + keyPathLength + ciphertextLength + keyPath + ciphertext + signature version: A
* single byte indicating the format version. keyPathLength: Length of the keyPath. ciphertextLength: ciphertext length keyPath: keyPath used to
* encrypt the column encryption key. This is only used for troubleshooting purposes and is not verified during decryption. ciphertext: Encrypted
* column encryption key signature: Signature of the entire byte array. Signature is validated before decrypting the column encryption key.
Expand All @@ -42,6 +49,8 @@ public class SQLServerColumnEncryptionAzureKeyVaultProvider extends SQLServerCol
*/
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 @@ -64,9 +73,58 @@ public String getName() {
}

/**
* Constructor that authenticates to AAD. This is used by KeyVaultClient at runtime to authenticate to Azure Key
* Constructor that takes a callback function to authenticate to AAD. This is used by KeyVaultClient at runtime to authenticate to Azure Key
* Vault.
*
* This constructor is present to maintain backwards compatibility with 6.0 version of the driver. Deprecated for removal in next Stable release.
Copy link
Contributor

Choose a reason for hiding this comment

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

"stable release"

*
* @param authenticationCallback
* - Callback function used for authenticating to AAD.
* @param executorService
* - The ExecutorService, previously used to create the keyVaultClient, but not in use anymore. - This param can be passed as 'null'
Copy link
Contributor

Choose a reason for hiding this comment

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

"This parameter"

* @throws SQLServerException
* when an error occurs
*/
@Deprecated
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 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.
* @throws SQLServerException
* when an error occurs
*/
public SQLServerColumnEncryptionAzureKeyVaultProvider(SQLServerKeyVaultAuthenticationCallback authenticationCallback) throws SQLServerException {
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

If both SQLServerColumnEncryptionAzureKeyVaultProvider constructors do exactly the same thing, maybe refactor the inner part so we don't duplicate code?

...actually, we would have to revert that in our next release when we remove the deprecated API. in that case, just have the SQLServerColumnEncryptionAzureKeyVaultProvider with ExecutorService in its parameter call the other SQLServerColumnEncryptionAzureKeyVaultProvider constructor.

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

}

/**
* Constructor that authenticates to AAD. This is used by KeyVaultClient at runtime to authenticate to Azure Key Vault.
*
* @param clientId
* Identifier of the client requesting the token.
* @param clientKey
Expand All @@ -76,8 +134,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);
}