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

MSI expiry date format fix for Azure Functions #1308

Merged
merged 2 commits into from
Apr 30, 2020

Conversation

peterbae
Copy link
Contributor

@peterbae peterbae commented Apr 9, 2020

Fixes issue #1135.

The format in which the expires_on property is sent to the driver from Azure server used to be in the form of a string, but starting from the latest API version (2019-08-01), this value is now an integer value, which is consistent across all regions and operating systems (Windows/Linux).

Note that this fix only applies to retrieving access tokens for MSI authentication from Azure Functions (and Azure Web Apps), and it doesn't affect the usual Azure IMDS routine for MSI authentication.

@peterbae
Copy link
Contributor Author

peterbae commented Apr 9, 2020

github-1135.zip

Attaching the latest driver jars with this fix included.

@ulvii ulvii added this to the 8.3.1 milestone Apr 16, 2020
@codecov-io
Copy link

Codecov Report

Merging #1308 into dev will decrease coverage by 0.15%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1308      +/-   ##
============================================
- Coverage     58.43%   58.27%   -0.16%     
+ Complexity     3827     3786      -41     
============================================
  Files           133      133              
  Lines         30096    30087       -9     
  Branches       4979     4977       -2     
============================================
- Hits          17586    17533      -53     
- Misses        10076    10095      +19     
- Partials       2434     2459      +25     
Impacted Files Coverage Δ Complexity Δ
...osoft/sqlserver/jdbc/SQLServerSecurityUtility.java 48.68% <66.66%> (+2.72%) 15.00 <0.00> (ø)
...icrosoft/sqlserver/jdbc/SQLServerXADataSource.java 37.50% <0.00%> (-18.75%) 4.00% <0.00%> (-6.00%)
...icrosoft/sqlserver/jdbc/SQLServerXAConnection.java 67.44% <0.00%> (-6.98%) 4.00% <0.00%> (-3.00%)
...a/com/microsoft/sqlserver/jdbc/PLPInputStream.java 59.76% <0.00%> (-2.37%) 34.00% <0.00%> (-2.00%)
...n/java/com/microsoft/sqlserver/jdbc/tdsparser.java 76.85% <0.00%> (-1.66%) 0.00% <0.00%> (ø%)
...m/microsoft/sqlserver/jdbc/SQLServerException.java 75.71% <0.00%> (-1.43%) 35.00% <0.00%> (ø%)
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 64.83% <0.00%> (-0.66%) 0.00% <0.00%> (ø%)
...ft/sqlserver/jdbc/SQLServerAASEnclaveProvider.java 75.00% <0.00%> (-0.56%) 15.00% <0.00%> (ø%)
...ncurrentlinkedhashmap/ConcurrentLinkedHashMap.java 39.22% <0.00%> (-0.44%) 44.00% <0.00%> (-1.00%)
...om/microsoft/sqlserver/jdbc/SQLServerBulkCopy.java 51.77% <0.00%> (-0.43%) 261.00% <0.00%> (-11.00%)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 947b537...e9faea2. Read the comment docs.

@peterbae peterbae merged commit 3b4f0a2 into microsoft:dev Apr 30, 2020
@sgarcia2101
Copy link

sgarcia2101 commented May 6, 2020

Hi! I found that you introduce a new bug. When library use Azure function or App Service, the parameter should be clientid, without underline, but in the fix it is missed as you can see in the image below:
image
If you pass client_id parameter instead of clientid, you receive a 400 Bad Request response.

@peterbae
Copy link
Contributor Author

peterbae commented May 6, 2020

Hi @sgarcia2101, thank you for your input. The API version for Azure function / App Service that the driver uses has been updated to 2019-08-01 version, and in this version Azure accepts clientid instead of client_id. Please let me know if you encounter any issues using the driver related to this change.

@sgarcia2101
Copy link

I dont know why, but if I use "clientid" paramete, it works, and if I use "client_id" parameter, I receive this error: {"statusCode":400,"message":"No UserAssigned Managed Identity found for specified ClientId/ResourceId/PrincipalId.","correlationId":"xxxxxxxx"}

@peterbae
Copy link
Contributor Author

peterbae commented May 6, 2020

I think the older versions of the API versions still accept "clientId", but the latest API version accepts "client_id". Could you show me how you're sending your HTTP GET request to the Azure endpoint? (but this would be unrelated to the driver)

Also, you could refer to this page for details: https://docs.microsoft.com/en-us/azure/app-service/overview-managed-identity?tabs=dotnet#using-the-rest-protocol

@sgarcia2101
Copy link

Yes, sure.

I have this code in Java deployed in an App Service with System Assigned Managed Identity enabled. The parameters are the clientId generated by System Assigned MI and the resource is "https://database.windows.net/".

private String getMSIToken(String clientId, String resource) throws UnsupportedEncodingException {

	Configuration configuration = Configuration.getGlobalConfiguration().clone();

	String msiEndpoint = configuration.get(Configuration.PROPERTY_MSI_ENDPOINT);
	String msiSecret = configuration.get(Configuration.PROPERTY_MSI_SECRET);
	

	StringBuilder uriString = new StringBuilder();
	uriString.append(String.format("%s?resource=%s&api-version=%s", msiEndpoint,
			URLEncoder.encode(resource, "UTF-8"), URLEncoder.encode("2019-08-01", "UTF-8")));
	
	if(clientId != null) {
		uriString.append(String.format("&clientid=%s", URLEncoder.encode(clientId, "UTF-8")));
	}

	URI uri = URI.create(uriString.toString());

	HttpClient client = HttpClient.newHttpClient();
	HttpRequest request = HttpRequest.newBuilder()
			.uri(uri)
			.header("X-IDENTITY-HEADER", msiSecret)
			.GET().build();

	HttpResponse<String> respuesta;
	try {
		respuesta = client.send(request, BodyHandlers.ofString());
		return respuesta.body();
	} catch (Exception e) {
		logger.error("Error getting a token", e);
	}
	return null;
}

Then the final request is:

GET /msi/token?resource=https://database.windows.net/&api-version=2019-08-01&client_id=xxxxxxx HTTP/1.1
X-IDENTITY-HEADER: xxxxxxxxx

@peterbae
Copy link
Contributor Author

peterbae commented May 7, 2020

This is an example of my GET request that works for me:

http://127.0.0.1:41539/MSI/token/?api-version=2019-08-01&resource=https://database.windows.net/&client_id=xxxxxxx

I think your GET request looks correct, I'm not sure why it's failing for you. Perhaps it's because in your code you showed me, you're still sending clientid instead of client_id along with X-IDENTITY-HEADER for your secret header? If you intend to use the latest api version of 2019-08-01, you have to use clientid along with X-IDENTITY-HEADER, but if you use the older version, you use client_id along with Secret. Hope this helps.

@sgarcia2101
Copy link

Sorry, I pasted the code that is working, because I use clientid with X-IDENTITY-HEADER.

As you said, the latest api version 2019-08-01 works with client_id and X-IDENTITY-HEADER, but if I test with this, it is not working.

@peterbae
Copy link
Contributor Author

peterbae commented May 7, 2020

I see. I'm not sure if I can help you further on this front, because I think your example looks like it should work to me. You can submit a support ticket to Azure regarding this issue, if you want more help on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants