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] Added logger parameters to bearerTokenAuthenticationPolicy #18467

Merged
merged 7 commits into from
Nov 25, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { AuthorizeRequestOnChallengeOptions } from "@azure/core-rest-pipeline";
import { createClientLogger } from "@azure/logger";
import { decodeStringToString } from "./base64";

const logger = createClientLogger("authorizeRequestOnClaimChallenge");
const defaultLogger = createClientLogger("authorizeRequestOnClaimChallenge");

/**
* Converts: `Bearer a="b", c="d", Bearer d="e", f="g"`.
Expand Down Expand Up @@ -64,6 +64,7 @@ export async function authorizeRequestOnClaimChallenge(
onChallengeOptions: AuthorizeRequestOnChallengeOptions
): Promise<boolean> {
const { scopes, response } = onChallengeOptions;
const logger = onChallengeOptions.logger || defaultLogger;

const challenge = response.headers.get("WWW-Authenticate");
if (!challenge) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,88 @@ describe("authorizeRequestOnClaimChallenge", function() {
assert.deepEqual(finalSendRequestHeaders, [undefined, `Bearer ${getTokenResponse.token}`]);
});
});

it(`a custom logger should log a reasonable message if no challenge is received`, async function() {
const request = createPipelineRequest({ url: "https://example.com" });
const getAccessTokenParameters: {
scopes: string | string[];
getTokenOptions: GetTokenOptions;
}[] = [];

const allParams: any[] = [];
const logger: any = {
info: (...params: any) => allParams.push(params)
};

const result = await authorizeRequestOnClaimChallenge({
async getAccessToken(scopes, getTokenOptions) {
getAccessTokenParameters.push({ scopes, getTokenOptions });
return {
token: "accessToken",
expiresOnTimestamp: new Date().getTime()
};
},
scopes: [],
response: {
headers: createHttpHeaders(),
request,
status: 401
},
request,
logger
});

assert.isFalse(result, "We provided no challenge, so it should return false.");

assert.equal(
allParams.map((x) => x.join(" ")).join("\n"),
`The WWW-Authenticate header was missing. Failed to perform the Continuous Access Evaluation authentication flow.`
);
});

it(`a custom logger should log a reasonable message if a bad challenge is received`, async function() {
const request = createPipelineRequest({ url: "https://example.com" });
const getAccessTokenParameters: {
scopes: string | string[];
getTokenOptions: GetTokenOptions;
}[] = [];

const allParams: any[] = [];
const logger: any = {
info: (...params: any) => allParams.push(params)
};

const result = await authorizeRequestOnClaimChallenge({
async getAccessToken(scopes, getTokenOptions) {
getAccessTokenParameters.push({ scopes, getTokenOptions });
return {
token: "accessToken",
expiresOnTimestamp: new Date().getTime()
};
},
scopes: [],
response: {
headers: createHttpHeaders({
"WWW-Authenticate": [
`Bearer authorization_uri="https://login.windows-ppe.net/", error="invalid_token"`,
`error_description="User session has been revoked"`,
`scope="https://endpoint/.default"`,
// Bad challenge
`claims=""`
].join(", ")
}),
request,
status: 401
},
request,
logger
});

assert.isFalse(result, "We provided a bad challenge, so it should return false.");

assert.equal(
allParams.map((x) => x.join(" ")).join("\n"),
`The WWW-Authenticate header was missing the necessary "claims" to perform the Continuous Access Evaluation authentication flow.`
);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xirzec I have added unit tests 🎉

});
2 changes: 2 additions & 0 deletions sdk/core/core-rest-pipeline/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features Added

- The `bearerTokenAuthenticationPolicy` now accepts a logger.

### Breaking Changes

### Bugs Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { AbortSignalLike } from '@azure/abort-controller';
import { AccessToken } from '@azure/core-auth';
import { AzureLogger } from '@azure/logger';
import { Debugger } from '@azure/logger';
import { GetTokenOptions } from '@azure/core-auth';
import { OperationTracingOptions } from '@azure/core-tracing';
Expand All @@ -33,6 +34,7 @@ export interface Agent {
// @public
export interface AuthorizeRequestOnChallengeOptions {
getAccessToken: (scopes: string[], options: GetTokenOptions) => Promise<AccessToken | null>;
logger?: AzureLogger;
request: PipelineRequest;
response: PipelineResponse;
scopes: string[];
Expand All @@ -41,6 +43,7 @@ export interface AuthorizeRequestOnChallengeOptions {
// @public
export interface AuthorizeRequestOptions {
getAccessToken: (scopes: string[], options: GetTokenOptions) => Promise<AccessToken | null>;
logger?: AzureLogger;
request: PipelineRequest;
scopes: string[];
}
Expand All @@ -55,6 +58,7 @@ export const bearerTokenAuthenticationPolicyName = "bearerTokenAuthenticationPol
export interface BearerTokenAuthenticationPolicyOptions {
challengeCallbacks?: ChallengeCallbacks;
credential?: TokenCredential;
logger?: AzureLogger;
scopes: string | string[];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";
import { AzureLogger } from "@azure/logger";
import { PipelineResponse, PipelineRequest, SendRequest } from "../interfaces";
import { PipelinePolicy } from "../pipeline";
import { createTokenCycler } from "../util/tokenCycler";
Expand All @@ -27,6 +28,10 @@ export interface AuthorizeRequestOptions {
* Request that the policy is trying to fulfill.
*/
request: PipelineRequest;
/**
* A logger, if one was sent through the HTTP pipeline.
*/
logger?: AzureLogger;
}

/**
Expand All @@ -49,6 +54,10 @@ export interface AuthorizeRequestOnChallengeOptions {
* Response containing the challenge.
*/
response: PipelineResponse;
/**
* A logger, if one was sent through the HTTP pipeline.
*/
logger?: AzureLogger;
}

/**
Expand Down Expand Up @@ -86,6 +95,10 @@ export interface BearerTokenAuthenticationPolicyOptions {
* If provided, after a request is sent, if it has a challenge, it can be processed to re-send the original request with the relevant challenge information.
*/
challengeCallbacks?: ChallengeCallbacks;
/**
* A logger can be sent for debugging purposes.
*/
logger?: AzureLogger;
}

/**
Expand Down Expand Up @@ -123,7 +136,7 @@ function getChallenge(response: PipelineResponse): string | undefined {
export function bearerTokenAuthenticationPolicy(
options: BearerTokenAuthenticationPolicyOptions
): PipelinePolicy {
const { credential, scopes, challengeCallbacks } = options;
const { credential, scopes, challengeCallbacks, logger } = options;
const callbacks = {
authorizeRequest: challengeCallbacks?.authorizeRequest ?? defaultAuthorizeRequest,
authorizeRequestOnChallenge: challengeCallbacks?.authorizeRequestOnChallenge,
Expand Down Expand Up @@ -164,7 +177,8 @@ export function bearerTokenAuthenticationPolicy(
await callbacks.authorizeRequest({
scopes: Array.isArray(scopes) ? scopes : [scopes],
request,
getAccessToken
getAccessToken,
logger
});

let response: PipelineResponse;
Expand All @@ -186,7 +200,8 @@ export function bearerTokenAuthenticationPolicy(
scopes: Array.isArray(scopes) ? scopes : [scopes],
request,
response,
getAccessToken
getAccessToken,
logger
});

if (shouldSendRequest) {
Expand Down