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

[core-http] Remove ServiceClientCredentials from ServiceClient API #4367

Merged
merged 9 commits into from
Jul 25, 2019
7 changes: 3 additions & 4 deletions sdk/core/core-arm/lib/azureServiceClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

import { HttpOperationResponse, OperationArguments, OperationSpec, RequestOptionsBase, RequestPrepareOptions, ServiceClient, ServiceClientCredentials, ServiceClientOptions, TokenCredential, WebResource, getDefaultUserAgentValue as getDefaultUserAgentValueFromMsRest } from "@azure/core-http";
import { HttpOperationResponse, OperationArguments, OperationSpec, RequestOptionsBase, RequestPrepareOptions, ServiceClient, ServiceClientOptions, TokenCredential, WebResource, getDefaultUserAgentValue as getDefaultUserAgentValueFromMsRest } from "@azure/core-http";
import { createLROPollerFromInitialResponse, createLROPollerFromPollState, LROPoller } from "./lroPoller";
import { LROPollState } from "./lroPollStrategy";
import * as Constants from "./util/constants";
Expand All @@ -27,8 +27,7 @@ export interface AzureServiceClientOptions extends ServiceClientOptions {
* Initializes a new instance of the AzureServiceClient class.
* @constructor
*
* @param {ServiceClientCredentials | TokenCredential} credentials - ApplicationTokenCredentials,
* UserTokenCredentials, or TokenCredential object used for authentication.
* @param {TokenCredential} credentials - The TokenCredential used for authentication.
* @param {AzureServiceClientOptions} options - The parameter options used by AzureServiceClient
*/
export class AzureServiceClient extends ServiceClient {
Expand All @@ -38,7 +37,7 @@ export class AzureServiceClient extends ServiceClient {
*/
public longRunningOperationRetryTimeout?: number;

constructor(credentials: ServiceClientCredentials | TokenCredential, options?: AzureServiceClientOptions) {
constructor(credentials: TokenCredential, options?: AzureServiceClientOptions) {
super(credentials, options = updateOptionsWithDefaultValues(options));

// For convenience, if the credentials have an associated AzureEnvironment,
Expand Down
12 changes: 6 additions & 6 deletions sdk/core/core-arm/samples/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@

<head>
<title>My Todos</title>
<script type="text/javascript" src="../node_modules/ms-rest-ts/msRestBundle.js"></script>
<script type="text/javascript" src="../msRestAzureBundle.js"></script>
<script type="text/javascript" src="../node_modules/@azure/core-http/dist/coreHttp.browser.js"></script>
<script type="text/javascript" src="../dist/coreArm.js"></script>

<script type="text/javascript">
document.write("hello world");
const subscriptionId = "subscriptionId";
const token = "token";
const creds = new msRest.TokenCredentials(token);
const client = new msRestAzure.AzureServiceClient(creds);
const creds = new Azure.Core.HTTP.RawTokenCredential(token);
const client = new Azure.Core.ARM.AzureServiceClient(creds);
const req = {
url: `https://management.azure.com/subscriptions/${subscriptionId}/providers/Microsoft.Storage/storageAccounts?api-version=2015-06-15`,
method: "GET"
Expand All @@ -23,4 +23,4 @@
<body>
</body>

</html>
</html>
2 changes: 1 addition & 1 deletion sdk/core/core-arm/samples/node-sample.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const apiVersion = "2017-06-01";
// 1.5 in the curl tab you will see the actual curl request that has the bearer token in it
// 1.6 copy paste that token here. That token is valid for 1 hour
const token = "token";
const creds = new coreHttp.TokenCredentials(token);
const creds = new coreHttp.RawTokenCredential(token);
const client = new coreArm.AzureServiceClient(creds, clientOptions);
const req: coreHttp.RequestPrepareOptions = {
url: `https://management.azure.com/subscriptions/${subscriptionId}/resourceGroups/${resourceGroupName}/providers/Microsoft.Storage/storageAccounts/${accountName}?api-version=${apiVersion}`,
Expand Down
17 changes: 8 additions & 9 deletions sdk/core/core-arm/test/azureServiceClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

import assert from "assert";
import { HttpHeaders, HttpOperationResponse, RequestOptionsBase, RestError, TokenCredentials, WebResource, OperationArguments, OperationSpec, Serializer } from "@azure/core-http";
import { HttpHeaders, HttpOperationResponse, RequestOptionsBase, RestError, RawTokenCredential, WebResource, OperationArguments, OperationSpec, Serializer } from "@azure/core-http";
import { AzureServiceClient, AzureServiceClientOptions, updateOptionsWithDefaultValues } from "../lib/azureServiceClient";
import * as msAssert from "./msAssert";
import { LROPoller } from "../lib/lroPoller";
Expand All @@ -11,28 +11,27 @@ import { CloudErrorMapper } from "../lib/cloudError";
describe("AzureServiceClient", () => {
describe("constructor", () => {
it("with no options provided", () => {
const client = new AzureServiceClient(new TokenCredentials("my-fake-token"));
const client = new AzureServiceClient(new RawTokenCredential("my-fake-token"));
assert.strictEqual(client.acceptLanguage, "en-us");
assert.strictEqual(client.longRunningOperationRetryTimeout, undefined);
});

it("with acceptLanguage provided", () => {
const client = new AzureServiceClient(new TokenCredentials("my-fake-token"), { acceptLanguage: "my-fake-language" });
const client = new AzureServiceClient(new RawTokenCredential("my-fake-token"), { acceptLanguage: "my-fake-language" });
assert.strictEqual(client.acceptLanguage, "my-fake-language");
assert.strictEqual(client.longRunningOperationRetryTimeout, undefined);
});

it("with longRunningOperationRetryTimeout provided", () => {
const client = new AzureServiceClient(new TokenCredentials("my-fake-token"), { longRunningOperationRetryTimeout: 2 });
const client = new AzureServiceClient(new RawTokenCredential("my-fake-token"), { longRunningOperationRetryTimeout: 2 });
assert.strictEqual(client.acceptLanguage, "en-us");
assert.strictEqual(client.longRunningOperationRetryTimeout, 2);
});

it("should apply the resourceManagerEndpointUrl from credentials", async function() {
const creds = {
signRequest: (request: WebResource) => Promise.resolve(request),
environment: { resourceManagerEndpointUrl: "foo" }
};
const creds = new RawTokenCredential("my-fake-token");
(creds as any).environment = { resourceManagerEndpointUrl: "foo" };

const client = new AzureServiceClient(creds);
// accessing protected property by casting
assert.strictEqual((client as any).baseUri, "foo");
Expand Down Expand Up @@ -2586,7 +2585,7 @@ function createServiceClient(responses: HttpResponse[]): AzureServiceClient {
}

function createServiceClientWithOptions(options: AzureServiceClientOptions, responses: HttpResponse[]): AzureServiceClient {
return new AzureServiceClient(new TokenCredentials("my-fake-token"), Object.assign({
return new AzureServiceClient(new RawTokenCredential("my-fake-token"), Object.assign({
httpClient: {
sendRequest(httpRequest: WebResource): Promise<HttpOperationResponse> {
const response: HttpResponse | undefined = responses.shift();
Expand Down
12 changes: 6 additions & 6 deletions sdk/core/core-arm/test/lroPollStrategyTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

import assert from "assert";
import { HttpHeaders, HttpOperationResponse, TokenCredentials, WebResource } from "@azure/core-http";
import { HttpHeaders, HttpOperationResponse, RawTokenCredential, WebResource } from "@azure/core-http";
import { AzureServiceClient } from "../lib/azureServiceClient";
import { getDelayInSeconds, isFinished } from "../lib/lroPollStrategy";

describe("LROPollStrategy", function () {
describe("getDelayInMilliseconds()", function () {
it("with no AzureServiceClient.longRunningOperationRetryTimeout value and no retry-after header", function () {
const azureServiceClient = new AzureServiceClient(new TokenCredentials("my-fake-token"));
const azureServiceClient = new AzureServiceClient(new RawTokenCredential("my-fake-token"));
const previousResponse: HttpOperationResponse = {
request: new WebResource(),
status: 200,
Expand All @@ -19,7 +19,7 @@ describe("LROPollStrategy", function () {
});

it("with 11 AzureServiceClient.longRunningOperationRetryTimeout and no retry-after header", function () {
const azureServiceClient = new AzureServiceClient(new TokenCredentials("my-fake-token"), { longRunningOperationRetryTimeout: 11 });
const azureServiceClient = new AzureServiceClient(new RawTokenCredential("my-fake-token"), { longRunningOperationRetryTimeout: 11 });
const previousResponse: HttpOperationResponse = {
request: new WebResource(),
status: 200,
Expand All @@ -29,7 +29,7 @@ describe("LROPollStrategy", function () {
});

it("with no AzureServiceClient.longRunningOperationRetryTimeout value and 12 retry-after header", function () {
const azureServiceClient = new AzureServiceClient(new TokenCredentials("my-fake-token"));
const azureServiceClient = new AzureServiceClient(new RawTokenCredential("my-fake-token"));
const previousResponse: HttpOperationResponse = {
request: new WebResource(),
status: 200,
Expand All @@ -39,7 +39,7 @@ describe("LROPollStrategy", function () {
});

it("with no AzureServiceClient.longRunningOperationRetryTimeout value and spam retry-after header", function () {
const azureServiceClient = new AzureServiceClient(new TokenCredentials("my-fake-token"));
const azureServiceClient = new AzureServiceClient(new RawTokenCredential("my-fake-token"));
const previousResponse: HttpOperationResponse = {
request: new WebResource(),
status: 200,
Expand All @@ -49,7 +49,7 @@ describe("LROPollStrategy", function () {
});

it("with 11 AzureServiceClient.longRunningOperationRetryTimeout and 12 retry-after header", function () {
const azureServiceClient = new AzureServiceClient(new TokenCredentials("my-fake-token"), { longRunningOperationRetryTimeout: 11 });
const azureServiceClient = new AzureServiceClient(new RawTokenCredential("my-fake-token"), { longRunningOperationRetryTimeout: 11 });
const previousResponse: HttpOperationResponse = {
request: new WebResource(),
status: 200,
Expand Down
8 changes: 8 additions & 0 deletions sdk/core/core-auth/review/core-auth.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ export interface GetTokenOptions {
// @public
export function isTokenCredential(credential: any): credential is TokenCredential;

// @public
export class RawTokenCredential implements TokenCredential {
constructor(token: string, expiresOn?: Date);
expiresOn: Date;
getToken(_scopes: string | string[], _options?: GetTokenOptions): Promise<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.

Why are the params having underscore here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are unused in the scope of this function, adding the _ prefix tells the linter to ignore the fact that they aren't used. I can look into using an eslint rule disable comment instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

nah, not worth it
this is fine

token: string;
}

// @public
export interface TokenCredential {
getToken(scopes: string | string[], options?: GetTokenOptions): Promise<AccessToken | null>;
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-auth/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ export {
isTokenCredential
} from "./tokenCredential";

export { RawTokenCredential } from "./rawTokenCredential";
export { AbortSignalLike } from '@azure/abort-controller';
44 changes: 44 additions & 0 deletions sdk/core/core-auth/src/rawTokenCredential.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { TokenCredential, GetTokenOptions, AccessToken } from "./tokenCredential";

/**
* A TokenCredential that always returns the given token. This class can be
* used when the access token is already known or can be retrieved from an
* outside source.
*/
export class RawTokenCredential implements TokenCredential {
/**
* The raw token string. Can be changed when the token needs to be updated.
*/
public token: string;

/**
* The Date at which the token expires. Can be changed to update the expiration time.
*/
public expiresOn: Date;

/**
* Creates an instance of TokenCredential.
* @param {string} token
*/
constructor(token: string, expiresOn?: Date) {
this.token = token;
this.expiresOn = expiresOn ? expiresOn : new Date(Date.now() + 60*60*1000);
}

/**
* Retrieves the token stored in this RawTokenCredential.
*
* @param _scopes Ignored since token is already known.
* @param _options Ignored since token is already known.
* @returns {AccessToken} The access token details.
*/
async getToken(_scopes: string | string[], _options?: GetTokenOptions): Promise<AccessToken | null> {
return {
token: this.token,
expiresOnTimestamp: this.expiresOn.getTime()
};
}
}
7 changes: 4 additions & 3 deletions sdk/core/core-auth/src/tokenCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ export interface AccessToken {
* @param credential The assumed TokenCredential to be tested.
*/
export function isTokenCredential(credential: any): credential is TokenCredential {
// Check for an object with a 'getToken' function but without
// Check for an object with a 'getToken' function and possibly with
// a 'signRequest' function. We do this check to make sure that
// a ServiceClientCredentials implementor (like TokenClientCredentials
// in ms-rest-nodeauth) doesn't get mistaken for a TokenCredential.
// in ms-rest-nodeauth) doesn't get mistaken for a TokenCredential if
// it doesn't actually implement TokenCredential also.
return credential
&& typeof credential.getToken === "function"
&& credential.signRequest === undefined;
&& (credential.signRequest === undefined || credential.getToken.length > 0);
}
9 changes: 8 additions & 1 deletion sdk/core/core-auth/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,17 @@ describe("isTokenCredential", function () {
}), false);
});

it("should return false for an object that has a 'signRequest' field", () => {
it("should return false for an object that has a 'signRequest' field and getToken that takes no parameters", () => {
assert.strictEqual(isTokenCredential({
getToken: function () { },
signRequest: function() { },
}), false);
})

it("should return true for an object that has a 'signRequest' field and getToken that takes parameters", () => {
assert.strictEqual(isTokenCredential({
getToken: function (scope: string) { },
signRequest: function() { },
}), true);
})
});
3 changes: 1 addition & 2 deletions sdk/core/core-http/lib/coreHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ export {
export { URLBuilder, URLQuery } from "./url";

// Credentials
export { TokenCredential, GetTokenOptions, AccessToken, isTokenCredential } from "@azure/core-auth";
export { TokenCredential, GetTokenOptions, AccessToken, isTokenCredential, RawTokenCredential } from "@azure/core-auth";
export { AccessTokenCache, ExpiringAccessTokenCache } from "./credentials/accessTokenCache";
export { TokenCredentials } from "./credentials/tokenCredentials";
export { BasicAuthenticationCredentials } from "./credentials/basicAuthenticationCredentials";
export { ApiKeyCredentials, ApiKeyCredentialOptions } from "./credentials/apiKeyCredentials";
export { ServiceClientCredentials } from "./credentials/serviceClientCredentials";
Expand Down
45 changes: 0 additions & 45 deletions sdk/core/core-http/lib/credentials/tokenCredentials.ts

This file was deleted.

Loading