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

Adds property baseUri on the ServiceClientOptions #457

Merged
merged 22 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## 2.6.0 - (2021-08-18)

- Added a new property `baseUri` on the `ServiceClientOptions` that is then used to initialize the corresponding `baseUri` protected property on the `ServiceClient`.
- For `baseUri` that happen to be known Azure resource manager endpoints, this allows the instantiating of the `AzureIdentityCredentialAdapter` class with the right scope when a user constructs a `ServiceClient` with a `TokenCredential`. Resolves https://github.com/Azure/azure-sdk-for-js/issues/15945

## 2.5.3 - (2021-07-12)
- Updated the dependency on the uuid package to v8 (PR [456](https://github.com/Azure/ms-rest-js/pull/456))

Expand Down
10 changes: 10 additions & 0 deletions lib/credentials/azureIdentityTokenCredentialAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ import { TokenResponse } from "./tokenResponse";

const DEFAULT_AUTHORIZATION_SCHEME = "Bearer";

/**
* Resource manager endpoints to match in order to specify a valid scope to the AzureIdentityCredentialAdapter.
*/
export const azureResourceManagerEndpoints = [
"https://management.windows.net",
"https://management.chinacloudapi.cn",
"https://management.usgovcloudapi.net",
"https://management.cloudapi.de",
];

/**
* This class provides a simple extension to use {@link TokenCredential} from `@azure/identity` library to
* use with legacy Azure SDKs that accept {@link ServiceClientCredentials} family of credentials for authentication.
Expand Down
6 changes: 5 additions & 1 deletion lib/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,11 @@ function deserializeCompositeType(
// paging
if (Array.isArray(responseBody[key]) && modelProps[key].serializedName === "") {
propertyInstance = responseBody[key];
const arrayInstance = serializer.deserialize(propertyMapper, propertyInstance, propertyObjectName);
const arrayInstance = serializer.deserialize(
propertyMapper,
propertyInstance,
propertyObjectName
);
// Copy over any properties that have already been added into the instance, where they do
// not exist on the newly de-serialized array
for (const [key, value] of Object.entries(instance)) {
Expand Down
33 changes: 29 additions & 4 deletions lib/serviceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ import { agentPolicy } from "./policies/agentPolicy";
import { proxyPolicy, getDefaultProxySettings } from "./policies/proxyPolicy";
import { throttlingRetryPolicy } from "./policies/throttlingRetryPolicy";
import { Agent } from "http";
import { AzureIdentityCredentialAdapter } from "./credentials/azureIdentityTokenCredentialAdapter";
import {
AzureIdentityCredentialAdapter,
azureResourceManagerEndpoints,
} from "./credentials/azureIdentityTokenCredentialAdapter";

/**
* HTTP proxy settings (Node.js only)
Expand Down Expand Up @@ -143,6 +146,16 @@ export interface ServiceClientOptions {
* HTTP and HTTPS agents which will be used for every HTTP request (Node.js only).
*/
agentSettings?: AgentSettings;
/**
* If specified:
* - This `baseUri` becomes the base URI that requests will be made against for this ServiceClient.
* - If the `baseUri` matches a known resource manager endpoint and if a `TokenCredential` was passed through the constructor, this `baseUri` defines the `getToken` scope to be `${options.baseUri}/.default`. Otherwise, the scope would default to "https://management.azure.com/.default".
*
* If it is not specified:
* - All OperationSpecs must contain a baseUrl property.
* - If a `TokenCredential` was passed through the constructor, the `getToken` scope is set to be "https://management.azure.com/.default".
*/
baseUri?: string;
}

/**
Expand All @@ -151,8 +164,12 @@ export interface ServiceClientOptions {
*/
export class ServiceClient {
/**
* If specified, this is the base URI that requests will be made against for this ServiceClient.
* If it is not specified, then all OperationSpecs must contain a baseUrl property.
* The base URI against which requests will be made when using this ServiceClient instance.
*
* This can be set either by setting the `baseUri` in the `options` parameter to the ServiceClient constructor or directly after constructing the ServiceClient.
* If set via the ServiceClient constructor when using the overload that takes the `TokenCredential`, and if it matches a known resource manager endpoint, this base URI sets the scope used to get the AAD token to `${baseUri}/.default` instead of the default "https://management.azure.com/.default"
*
* If it is not specified, all OperationSpecs must contain a baseUrl property.
*/
protected baseUri?: string;

Expand Down Expand Up @@ -185,9 +202,17 @@ export class ServiceClient {
options = {};
}

if (options.baseUri) {
this.baseUri = options.baseUri;
}

let serviceClientCredentials: ServiceClientCredentials | undefined;
if (isTokenCredential(credentials)) {
serviceClientCredentials = new AzureIdentityCredentialAdapter(credentials);
let scope: string | undefined = undefined;
if (options?.baseUri && azureResourceManagerEndpoints.includes(options?.baseUri)) {
scope = `${options.baseUri}/.default`;
}
serviceClientCredentials = new AzureIdentityCredentialAdapter(credentials, scope);
} else {
serviceClientCredentials = credentials;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const Constants = {
* @const
* @type {string}
*/
msRestVersion: "2.5.3",
msRestVersion: "2.6.0",

/**
* Specifies HTTP.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"email": "azsdkteam@microsoft.com",
"url": "https://github.com/Azure/ms-rest-js"
},
"version": "2.5.3",
"version": "2.6.0",
"description": "Isomorphic client Runtime for Typescript/node.js/browser javascript client libraries generated using AutoRest",
"tags": [
"isomorphic",
Expand Down
141 changes: 141 additions & 0 deletions test/serviceClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,147 @@ describe("ServiceClient", function () {
assert.strictEqual(request!.withCredentials, true);
});

it("The behavior of baseUri on ServiceClient children classes should be predictable on management clouds", async function () {
const httpClient: HttpClient = {
sendRequest: (request) => {
return Promise.resolve({ request, status: 200, headers: new HttpHeaders() });
},
};

const scope = "https://management.chinacloudapi.cn";

class ServiceClientChildren extends ServiceClient {
getBaseUri(): string | undefined {
return this.baseUri;
}
}

let receivedScope: string | string[] = "";

const client = new ServiceClientChildren(
{
async getToken(_receivedScope) {
receivedScope = _receivedScope;
return {
token: "token",
expiresOnTimestamp: Date.now(),
};
},
},
{
httpClient,
baseUri: scope,
}
);

const res = await client.sendOperationRequest(
{},
{
serializer: new Serializer(),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: {
type: {
name: "Sequence",
element: {
type: {
name: "Number",
},
},
},
},
},
},
}
);

assert.strictEqual(res._response.status, 200);

assert.strictEqual(
receivedScope,
`${scope}/.default`,
"The baseUri passed through the options should determine the scope used on getToken"
);
assert.strictEqual(
client.getBaseUri(),
scope,
"The baseUri passed through the options should be assigned as the protected baseUri"
);
});

it("The behavior of baseUri on ServiceClient children classes should remain the same if the baseUri is not a management endpoint", async function () {
const httpClient: HttpClient = {
sendRequest: (request) => {
return Promise.resolve({ request, status: 200, headers: new HttpHeaders() });
},
};

const baseUri = "https://original-scope.xyz";
const expectedReceivedScope = "https://management.azure.com/.default";

class ServiceClientChildren extends ServiceClient {
getBaseUri(): string | undefined {
return this.baseUri;
}
}

let receivedScope: string | string[] = "";

const client = new ServiceClientChildren(
{
async getToken(_receivedScope) {
receivedScope = _receivedScope;
return {
token: "token",
expiresOnTimestamp: Date.now(),
};
},
},
{
httpClient,
baseUri,
}
);

const res = await client.sendOperationRequest(
{},
{
serializer: new Serializer(),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: {
type: {
name: "Sequence",
element: {
type: {
name: "Number",
},
},
},
},
},
},
}
);

assert.strictEqual(res._response.status, 200);

assert.strictEqual(
receivedScope,
expectedReceivedScope,
"The baseUri passed through the options should determine the scope used on getToken"
);
assert.strictEqual(
client.getBaseUri(),
baseUri,
"The baseUri passed through the options should be assigned as the protected baseUri"
);
});

it("should deserialize response bodies", async function () {
let request: WebResourceLike;
const httpClient: HttpClient = {
Expand Down