-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Identity] Reverting ClientSecretCredential and UsernamePasswordCrede…
…ntial to v1 only for browsers, and fixing Karma browser bundles (#14910) This PR makes it so `ClientSecretCredential` behaves like how it did on v1 only for browsers. It also ensures browser bundles can be used when built with Karma. --- ## ClientSecretCredential (and also UsernamePasswordCredential) Since the inception of Identity, we've been relying on `ClientSecretCredential` to be available in browsers to make it easier for us to automate bowser tests. Whether `ClientSecretCredential` should continue being available on browsers in the future, is something yet to be decided. We started a conversation on the matter here: #14879 Our current plan is to switch master back to v1, as you can see here: #14909 Since the v2 changes to `ClientSecretCredential` won't be reverted, that means that as soon as we move master back to v1, all of our current clients will immediately start working with the latest `ClientSecretCredential`. `ClientSecretCredential` was rewritten to use MSAL, specifically `@azure/msal-node`, which is not intended to work on browsers. This PR makes it so `ClientSecretCredential` is kept as it was on v1 only for browsers. This will: - Allow us to keep our current browser recordings. - Allow us to continue using this credential for browser tests. - On v1, dropping this credential on browsers would be a breaking change, so this technically reduces the differences with v1. **Important:** Keep in mind that while we've always supported this credential in browsers, it can only work if browser security is disabled. This isn't recommended outside of our tests, and it's not something we should be advertising. (Later we also did the same to `UsernamePasswordCredential`). --- ## Browser bundles While my initial change was scoped to one credential, I asked Harsha to test this PR and he found that the browser bundles would not work with Identity v2. We decided to fix it right away since this PR wouldn't be very useful with broken browser bundles.
- Loading branch information
Showing
8 changed files
with
312 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 109 additions & 0 deletions
109
sdk/identity/identity/src/credentials/clientSecretCredential.browser.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import qs from "qs"; | ||
| import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-http"; | ||
| import { TokenCredentialOptions, IdentityClient } from "../client/identityClient"; | ||
| import { createSpan } from "../util/tracing"; | ||
| import { SpanStatusCode } from "@azure/core-tracing"; | ||
| import { credentialLogger, formatError, formatSuccess } from "../util/logging"; | ||
| import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; | ||
|
|
||
| const logger = credentialLogger("ClientSecretCredential"); | ||
|
|
||
| // This credential is exported on browser bundles for development purposes. | ||
| // For this credential to work in browsers, browsers would need to have security features disabled. | ||
| // Please do not disable your browser security features. | ||
|
|
||
| /** | ||
| * Enables authentication to Azure Active Directory using a client secret | ||
| * that was generated for an App Registration. More information on how | ||
| * to configure a client secret can be found here: | ||
| * | ||
| * https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-configure-app-access-web-apis#add-credentials-to-your-web-application | ||
| * | ||
| */ | ||
| export class ClientSecretCredential implements TokenCredential { | ||
| private identityClient: IdentityClient; | ||
| private tenantId: string; | ||
| private clientId: string; | ||
| private clientSecret: string; | ||
|
|
||
| /** | ||
| * Creates an instance of the ClientSecretCredential with the details | ||
| * needed to authenticate against Azure Active Directory with a client | ||
| * secret. | ||
| * | ||
| * @param tenantId - The Azure Active Directory tenant (directory) ID. | ||
| * @param clientId - The client (application) ID of an App Registration in the tenant. | ||
| * @param clientSecret - A client secret that was generated for the App Registration. | ||
| * @param options - Options for configuring the client which makes the authentication request. | ||
| */ | ||
| constructor( | ||
| tenantId: string, | ||
| clientId: string, | ||
| clientSecret: string, | ||
| options?: TokenCredentialOptions | ||
| ) { | ||
| this.identityClient = new IdentityClient(options); | ||
| this.tenantId = tenantId; | ||
| this.clientId = clientId; | ||
| this.clientSecret = clientSecret; | ||
| } | ||
|
|
||
| /** | ||
| * Authenticates with Azure Active Directory and returns an access token if | ||
| * successful. If authentication cannot be performed at this time, this method may | ||
| * return null. If an error occurs during authentication, an {@link AuthenticationError} | ||
| * containing failure details will be thrown. | ||
| * | ||
| * @param scopes - The list of scopes for which the token will have access. | ||
| * @param options - The options used to configure any requests this | ||
| * TokenCredential implementation might make. | ||
| */ | ||
| public async getToken( | ||
| scopes: string | string[], | ||
| options?: GetTokenOptions | ||
| ): Promise<AccessToken | null> { | ||
| const { span, updatedOptions: newOptions } = createSpan( | ||
| "ClientSecretCredential-getToken", | ||
| options | ||
| ); | ||
| try { | ||
| const urlSuffix = getIdentityTokenEndpointSuffix(this.tenantId); | ||
| const webResource = this.identityClient.createWebResource({ | ||
| url: `${this.identityClient.authorityHost}/${this.tenantId}/${urlSuffix}`, | ||
| method: "POST", | ||
| disableJsonStringifyOnBody: true, | ||
| deserializationMapper: undefined, | ||
| body: qs.stringify({ | ||
| response_type: "token", | ||
| grant_type: "client_credentials", | ||
| client_id: this.clientId, | ||
| client_secret: this.clientSecret, | ||
| scope: typeof scopes === "string" ? scopes : scopes.join(" ") | ||
| }), | ||
| headers: { | ||
| Accept: "application/json", | ||
| "Content-Type": "application/x-www-form-urlencoded" | ||
| }, | ||
| abortSignal: options && options.abortSignal, | ||
| spanOptions: newOptions.tracingOptions && newOptions.tracingOptions.spanOptions, | ||
| tracingContext: newOptions.tracingOptions && newOptions.tracingOptions.tracingContext | ||
| }); | ||
|
|
||
| const tokenResponse = await this.identityClient.sendTokenRequest(webResource); | ||
| logger.getToken.info(formatSuccess(scopes)); | ||
| return (tokenResponse && tokenResponse.accessToken) || null; | ||
| } catch (err) { | ||
| span.setStatus({ | ||
| code: SpanStatusCode.ERROR, | ||
| message: err.message | ||
| }); | ||
| logger.getToken.info(formatError(scopes, err)); | ||
| throw err; | ||
| } finally { | ||
| span.end(); | ||
| } | ||
| } | ||
| } |
111 changes: 111 additions & 0 deletions
111
sdk/identity/identity/src/credentials/usernamePasswordCredential.browser.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import qs from "qs"; | ||
| import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-http"; | ||
| import { TokenCredentialOptions, IdentityClient } from "../client/identityClient"; | ||
| import { createSpan } from "../util/tracing"; | ||
| import { SpanStatusCode } from "@azure/core-tracing"; | ||
| import { credentialLogger, formatSuccess, formatError } from "../util/logging"; | ||
| import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; | ||
| import { checkTenantId } from "../util/checkTenantId"; | ||
|
|
||
| const logger = credentialLogger("UsernamePasswordCredential"); | ||
|
|
||
| /** | ||
| * Enables authentication to Azure Active Directory with a user's | ||
| * username and password. This credential requires a high degree of | ||
| * trust so you should only use it when other, more secure credential | ||
| * types can't be used. | ||
| */ | ||
| export class UsernamePasswordCredential implements TokenCredential { | ||
| private identityClient: IdentityClient; | ||
| private tenantId: string; | ||
| private clientId: string; | ||
| private username: string; | ||
| private password: string; | ||
|
|
||
| /** | ||
| * Creates an instance of the UsernamePasswordCredential with the details | ||
| * needed to authenticate against Azure Active Directory with a username | ||
| * and password. | ||
| * | ||
| * @param tenantIdOrName - The Azure Active Directory tenant (directory) ID or name. | ||
| * @param clientId - The client (application) ID of an App Registration in the tenant. | ||
| * @param username - The user account's e-mail address (user name). | ||
| * @param password - The user account's account password | ||
| * @param options - Options for configuring the client which makes the authentication request. | ||
| */ | ||
| constructor( | ||
| tenantIdOrName: string, | ||
| clientId: string, | ||
| username: string, | ||
| password: string, | ||
| options?: TokenCredentialOptions | ||
| ) { | ||
| checkTenantId(logger, tenantIdOrName); | ||
|
|
||
| this.identityClient = new IdentityClient(options); | ||
| this.tenantId = tenantIdOrName; | ||
| this.clientId = clientId; | ||
| this.username = username; | ||
| this.password = password; | ||
| } | ||
|
|
||
| /** | ||
| * Authenticates with Azure Active Directory and returns an access token if | ||
| * successful. If authentication cannot be performed at this time, this method may | ||
| * return null. If an error occurs during authentication, an {@link AuthenticationError} | ||
| * containing failure details will be thrown. | ||
| * | ||
| * @param scopes - The list of scopes for which the token will have access. | ||
| * @param options - The options used to configure any requests this | ||
| * TokenCredential implementation might make. | ||
| */ | ||
| public async getToken( | ||
| scopes: string | string[], | ||
| options?: GetTokenOptions | ||
| ): Promise<AccessToken | null> { | ||
| const { span, updatedOptions: newOptions } = createSpan( | ||
| "UsernamePasswordCredential-getToken", | ||
| options | ||
| ); | ||
| try { | ||
| const urlSuffix = getIdentityTokenEndpointSuffix(this.tenantId); | ||
| const webResource = this.identityClient.createWebResource({ | ||
| url: `${this.identityClient.authorityHost}/${this.tenantId}/${urlSuffix}`, | ||
| method: "POST", | ||
| disableJsonStringifyOnBody: true, | ||
| deserializationMapper: undefined, | ||
| body: qs.stringify({ | ||
| response_type: "token", | ||
| grant_type: "password", | ||
| client_id: this.clientId, | ||
| username: this.username, | ||
| password: this.password, | ||
| scope: typeof scopes === "string" ? scopes : scopes.join(" ") | ||
| }), | ||
| headers: { | ||
| Accept: "application/json", | ||
| "Content-Type": "application/x-www-form-urlencoded" | ||
| }, | ||
| abortSignal: options && options.abortSignal, | ||
| spanOptions: newOptions.tracingOptions && newOptions.tracingOptions.spanOptions, | ||
| tracingContext: newOptions.tracingOptions && newOptions.tracingOptions.tracingContext | ||
| }); | ||
|
|
||
| const tokenResponse = await this.identityClient.sendTokenRequest(webResource); | ||
| logger.getToken.info(formatSuccess(scopes)); | ||
| return (tokenResponse && tokenResponse.accessToken) || null; | ||
| } catch (err) { | ||
| span.setStatus({ | ||
| code: SpanStatusCode.ERROR, | ||
| message: err.message | ||
| }); | ||
| logger.getToken.info(formatError(scopes, err)); | ||
| throw err; | ||
| } finally { | ||
| span.end(); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 23 additions & 0 deletions
23
sdk/identity/identity/test/public/browser/clientSecretCredential.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { ClientSecretCredential } from "../../../src"; | ||
| import { MockAuthHttpClient, assertClientCredentials } from "../../authTestUtils"; | ||
|
|
||
| describe("ClientSecretCredential", function() { | ||
| it("sends an authorization request with the given credentials", async () => { | ||
| const mockHttpClient = new MockAuthHttpClient(); | ||
|
|
||
| const credential = new ClientSecretCredential( | ||
| "tenant", | ||
| "client", | ||
| "secret", | ||
| mockHttpClient.tokenCredentialOptions | ||
| ); | ||
|
|
||
| await credential.getToken("scope"); | ||
|
|
||
| const authRequest = mockHttpClient.requests[0]; | ||
| assertClientCredentials(authRequest, "tenant", "client", "secret"); | ||
| }); | ||
| }); |
48 changes: 48 additions & 0 deletions
48
sdk/identity/identity/test/public/browser/usernamePasswordCredential.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import assert from "assert"; | ||
| import { UsernamePasswordCredential } from "../../../src"; | ||
| import { MockAuthHttpClient } from "../../authTestUtils"; | ||
|
|
||
| describe("UsernamePasswordCredential", function() { | ||
| it("sends an authorization request with the given username and password", async () => { | ||
| const mockHttpClient = new MockAuthHttpClient(); | ||
|
|
||
| const credential = new UsernamePasswordCredential( | ||
| "tenant", | ||
| "client", | ||
| "user@domain.com", | ||
| "p4s$w0rd", | ||
| mockHttpClient.tokenCredentialOptions | ||
| ); | ||
|
|
||
| await credential.getToken("scope"); | ||
|
|
||
| const authRequest = await mockHttpClient.requests[0]; | ||
| if (!authRequest) { | ||
| assert.fail("No authentication request was intercepted"); | ||
| } else { | ||
| assert.strictEqual( | ||
| authRequest.url.startsWith(`https://authority/tenant`), | ||
| true, | ||
| "Request body doesn't contain expected tenantId" | ||
| ); | ||
| assert.strictEqual( | ||
| authRequest.body.indexOf(`client_id=client`) > -1, | ||
| true, | ||
| "Request body doesn't contain expected clientId" | ||
| ); | ||
| assert.strictEqual( | ||
| authRequest.body.indexOf(`username=user%40domain.com`) > -1, | ||
| true, | ||
| "Request body doesn't contain expected username" | ||
| ); | ||
| assert.strictEqual( | ||
| authRequest.body.indexOf(`password=p4s%24w0rd`) > -1, | ||
| true, | ||
| "Request body doesn't contain expected username" | ||
| ); | ||
| } | ||
| }); | ||
| }); |