Skip to content

Commit

Permalink
[core-http] Remove ServiceClientCredentials from ServiceClient API (#…
Browse files Browse the repository at this point in the history
…4367)

* Remove ServiceClientCredentials from ServiceClient API

* Remove additional check added to isTokenCredential

This rolls back the change made in 771614e because it will prevent
forward-compatibility in `ms-rest-nodeauth` and `ms-rest-browserauth`
credentials.

* Improve RawTokenCredential comment

* Improve credential detection logic in ServiceClient

* Add isTokenCredential heuristic to identify TokenClientCredentials

* Simplify ServiceClient constructor logic around credentials

* Move RawTokenCredential from core-http to core-arm

* Delete TokenCredentials, update samples to use RawTokenCredential

* Rename RawTokenCredential to SimpleTokenCredential
  • Loading branch information
daviwil authored and HarshaNalluru committed Aug 2, 2019
1 parent e12f8e5 commit 8e3db7c
Show file tree
Hide file tree
Showing 27 changed files with 143 additions and 196 deletions.
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, SimpleTokenCredential, 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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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, SimpleTokenCredential, 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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential("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 SimpleTokenCredential implements TokenCredential {
constructor(token: string, expiresOn?: Date);
expiresOn: Date;
getToken(_scopes: string | string[], _options?: GetTokenOptions): Promise<AccessToken | null>;
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 { SimpleTokenCredential } from "./simpleTokenCredential";
export { AbortSignalLike } from '@azure/abort-controller';
44 changes: 44 additions & 0 deletions sdk/core/core-auth/src/simpleTokenCredential.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 SimpleTokenCredential 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, SimpleTokenCredential } 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

0 comments on commit 8e3db7c

Please sign in to comment.