Skip to content

Commit

Permalink
- Address review comments
Browse files Browse the repository at this point in the history 
- Add unit tests
  • Loading branch information
@konstantin-msft
konstantin-msft committed Jul 3, 2024
1 parent c1a0d5b commit 818424a
Show file tree
Hide file tree
Showing 3 changed files with 222 additions and 4 deletions.
8 changes: 5 additions & 3 deletions lib/msal-browser/src/interaction_client/NativeInteractionClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ function setSKUs(messageHandler: NativeMessageHandler): string {

// Report extension SKU
const extensionName =
messageHandler.getExtensionId() === NativeConstants.CHROME_EXTENSION_ID
messageHandler.getExtensionId() ===
NativeConstants.PREFERRED_EXTENSION_ID
? "chrome"
: messageHandler.getExtensionId()?.length
? "unknown"
Expand Down Expand Up @@ -369,7 +370,6 @@ export class NativeInteractionClient extends BaseInteractionClient {
}
}
}
this.serverTelemetryManager.clearNativeBrokerErrorCode();
this.browserStorage.setTemporaryCache(
TemporaryCacheKeys.NATIVE_REQUEST,
JSON.stringify(nativeRequest),
Expand Down Expand Up @@ -458,7 +458,9 @@ export class NativeInteractionClient extends BaseInteractionClient {
reqTimestamp
);
this.browserStorage.setInteractionInProgress(false);
return await result;
const res = await result;
this.serverTelemetryManager.clearNativeBrokerErrorCode();
return res;
} catch (e) {
this.browserStorage.setInteractionInProgress(false);
throw e;
Expand Down
1 change: 0 additions & 1 deletion lib/msal-browser/src/utils/BrowserConstants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export const NativeConstants = {
CHANNEL_ID: "53ee284d-920a-4b59-9d30-a60315b26836",
PREFERRED_EXTENSION_ID: "ppnbnpeolgkicgegkbkbjmhlideopiji",
MATS_TELEMETRY: "MATS",
CHROME_EXTENSION_ID: "ppnbnpeolgkicgegkbkbjmhlideopiji",
};

export const NativeExtensionMethod = {
Expand Down
217 changes: 217 additions & 0 deletions lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { BrowserPerformanceClient, IPublicClientApplication } from "../../src";
import { buildAccountFromIdTokenClaims, buildIdToken } from "msal-test-utils";
import { version } from "../../src/packageMetadata";
import { BrowserConstants } from "../../src/utils/BrowserConstants";
import * as NativeStatusCodes from "../../src/broker/nativeBroker/NativeStatusCodes";

const MOCK_WAM_RESPONSE = {
access_token: TEST_TOKENS.ACCESS_TOKEN,
Expand Down Expand Up @@ -636,6 +637,112 @@ describe("NativeInteractionClient Tests", () => {
});
});

it("does not set native broker error to server telemetry", async () => {
sinon
.stub(NativeMessageHandler.prototype, "sendMessage")
.callsFake((message): Promise<object> => {
return Promise.resolve(MOCK_WAM_RESPONSE);
});

await nativeInteractionClient.acquireToken({
scopes: ["User.Read"],
});
expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
});
});

it("sets native broker error to server telemetry", async () => {
sinon
.stub(NativeMessageHandler.prototype, "sendMessage")
.callsFake((message): Promise<object> => {
return Promise.reject(
new NativeAuthError("test_native_error_code")
);
});
try {
await nativeInteractionClient.acquireToken({
scopes: ["User.Read"],
});
} catch (e) {}
expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
nativeBrokerErrorCode: "test_native_error_code",
});
});

it("resets native broker error in server telemetry", async () => {
const sendMessageStub = sinon.stub(
NativeMessageHandler.prototype,
"sendMessage"
);
sendMessageStub
.onFirstCall()
.callsFake((message): Promise<object> => {
return Promise.reject(
new NativeAuthError(
"test_native_error_code",
"test_error_desc",
{ status: NativeStatusCodes.PERSISTENT_ERROR }
)
);
});
sendMessageStub
.onSecondCall()
.callsFake((message): Promise<object> => {
return Promise.resolve(MOCK_WAM_RESPONSE);
});

try {
await nativeInteractionClient.acquireToken({
scopes: ["User.Read"],
});
} catch (e) {}
expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
nativeBrokerErrorCode: "test_native_error_code",
});

await nativeInteractionClient.acquireToken({
scopes: ["User.Read"],
});
expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
});
});

describe("storeInCache tests", () => {
//here

Expand Down Expand Up @@ -816,6 +923,116 @@ describe("NativeInteractionClient Tests", () => {
perfMeasurement
);
});

it("sets native broker error to server telemetry", (done) => {
sinon
.stub(NavigationClient.prototype, "navigateExternal")
.callsFake((url: string) => {
expect(url).toBe(window.location.href);
expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toHaveProperty(
"nativeBrokerErrorCode",
"test_native_error_code"
);
done();
return Promise.resolve(true);
});
sinon
.stub(NativeMessageHandler.prototype, "sendMessage")
.callsFake((message): Promise<object> => {
return Promise.reject(
new NativeAuthError("test_native_error_code")
);
});
nativeInteractionClient.acquireTokenRedirect(
{
scopes: ["User.Read"],
},
perfMeasurement
);
});

it("resets native broker error in server telemetry", async () => {
sinon
.stub(NavigationClient.prototype, "navigateExternal")
.callsFake((url: string) => {
return Promise.resolve(true);
});
const sendMessageStub = sinon.stub(
NativeMessageHandler.prototype,
"sendMessage"
);
sendMessageStub
.onFirstCall()
.callsFake((message): Promise<object> => {
return Promise.reject(
new NativeAuthError(
"test_native_error_code",
"test_error_desc",
{ status: NativeStatusCodes.PERSISTENT_ERROR }
)
);
});
sendMessageStub
.onSecondCall()
.callsFake((message): Promise<object> => {
return Promise.resolve(MOCK_WAM_RESPONSE);
});
sendMessageStub
.onThirdCall()
.callsFake((message): Promise<object> => {
return Promise.resolve(MOCK_WAM_RESPONSE);
});

try {
await nativeInteractionClient.acquireTokenRedirect(
{
scopes: ["User.Read"],
},
perfMeasurement
);
} catch (e) {}

expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
nativeBrokerErrorCode: "test_native_error_code",
});

await nativeInteractionClient.acquireTokenRedirect(
{
scopes: ["User.Read"],
},
perfMeasurement
);
// @ts-ignore
pca.browserStorage.setInteractionInProgress(true);
await nativeInteractionClient.handleRedirectPromise();

expect(
JSON.parse(
window.sessionStorage.getItem(
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
) || ""
)
).toEqual({
cacheHits: 0,
errors: [],
failedRequests: [],
});
});
});

describe("handleRedirectPromise tests", () => {
Expand Down

0 comments on commit 818424a

Please sign in to comment.