CredScan Reports - clearing noise related to "scope" in request bodies #14954

HarshaNalluru · 2021-04-20T23:44:36Z

CredScan Reports - clearing noise related to "scope" in request bodies

...& Extending `applyRequestBodyTransformations` for browser

Extended the requestBodyTransformations from #14897 to handle browser tests as well.
Adds a default transformation to strip out the scope in the request body with the string "https://sanitized/"
And removed requestBodyTransformations from the setup, users won't be able to pass. The reason being, Nock doesn't support multiple .filteringRequestBody patches in the recordings. The best alternative would be to migrate to JSON recordings for node tests.

Updates after #14897

("scope" in the request body is only present in the login requests.)
For node, as we mock the msal auth already through [Recorder] Filtering request bodies #14897, only thing left is to replace the scope URL in the recordings so that Cred scan doesn't complain with false positives - So, added a default callback at "customizationOnRecordings".
For browser, along with the replacements, we need to mock the new requests - So, added a new default request body transformation - which will be applicable in during the playback mode to match the new request and during the record mode to save the recording by sanitizing the scope URL to avoid false positives from Cred Scan reports.

sdk/test-utils/recorder/src/utils/requestBodyTransform.ts

Co-authored-by: Jeremy Meng <[email protected]>

…to harshan/recorder/filter-scope

…rshaNalluru/azure-sdk-for-js into harshan/recorder/filter-scope

…ported in nock with multiple transforms

HarshaNalluru · 2021-04-21T08:23:20Z

sdk/test-utils/recorder/src/utils/requestBodyTransform.ts

-  } else {
-    // TODO: Browser side - not needed right now since the browser tests are not using the new identity with msal
-    console.log("This feature is not yet supported in the browser");
+  } else if (runtime === "browser" && typeof fixture !== "string") {


jeremymeng · 2021-04-21T16:29:15Z

sdk/test-utils/recorder/src/createRecorder.browser.ts

 // This class overrides requests' 'open', 'send' and 'onreadystatechange' functions, adding our own code to them to deal with requests
 export class NiseRecorder extends BaseRecorder {
-  private recordings: any[] = [];
+  private recordings: { [key: string]: unknown }[] = [];


nit: Record<string, unknown>?

sdk/test-utils/recorder/src/utils/requestBodyTransform.ts

…to harshan/recorder/filter-scope

sdk/test-utils/recorder/CHANGELOG.md

…to harshan/recorder/filter-scope

jeremymeng

LGTM. Thanks a lot for the hard work!

ghost · 2021-05-26T00:30:42Z

Hello @HarshaNalluru!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

Fix path at line 1557 (Azure#14954)

HarshaNalluru added 4 commits April 20, 2021 16:28

starter code - handle browser recordings

9ac07dd

changelog

c508d34

Update matchRequest with applyRequestBodyTransformations

f56a3be

update test

acdb7bb

jeremymeng reviewed Apr 20, 2021

View reviewed changes

sdk/test-utils/recorder/src/utils/requestBodyTransform.ts Outdated Show resolved Hide resolved

HarshaNalluru and others added 10 commits April 20, 2021 16:51

Update sdk/test-utils/recorder/src/utils/requestBodyTransform.ts

60b2d93

Co-authored-by: Jeremy Meng <[email protected]>

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

05d2284

…to harshan/recorder/filter-scope

Merge branch 'harshan/recorder/filter-scope' of https://github.com/Ha…

ce1f236

…rshaNalluru/azure-sdk-for-js into harshan/recorder/filter-scope

fix regexes and minor bugs

43bc9a2

fix test titles and removed .only

19dbec8

remove requestBodyTransformations on the exports as it isn't well sup…

478c7f4

…ported in nock with multiple transforms

InternalRecorderEnvironmentSetup with requestBodyTransformations

2332a82

recording

a139974

reasoning

fe26243

applyRequestBodyTransformationsOnFixture

901e574

HarshaNalluru changed the title ~~[Recorder] Credscan reports - clearing noise & extend applyRequestBodyTransformations for browser~~ Credscan reports - clearing noise & extend applyRequestBodyTransformations for browser Apr 21, 2021

HarshaNalluru changed the title ~~Credscan reports - clearing noise & extend applyRequestBodyTransformations for browser~~ Credscan reports - clearing noise & Extending applyRequestBodyTransformations for browser Apr 21, 2021

HarshaNalluru changed the title ~~Credscan reports - clearing noise & Extending applyRequestBodyTransformations for browser~~ CredScan Reports - clearing noise & Extending applyRequestBodyTransformations for browser Apr 21, 2021

changelog

ff8dd0b

HarshaNalluru changed the title ~~CredScan Reports - clearing noise & Extending applyRequestBodyTransformations for browser~~ CredScan Reports - clearing noise related to "scope" in request bodies Apr 21, 2021

HarshaNalluru added 2 commits April 21, 2021 01:18

minor bug fix

80794b7

record a test from text-analytics

81b58cb

HarshaNalluru commented Apr 21, 2021

View reviewed changes

jeremymeng reviewed Apr 21, 2021

View reviewed changes

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

c6842ca

…to harshan/recorder/filter-scope

HarshaNalluru marked this pull request as ready for review April 21, 2021 21:52

HarshaNalluru requested review from chradek, deyaaeldeen, richardpark-msft and sadasant as code owners April 21, 2021 21:52

HarshaNalluru added 5 commits May 12, 2021 16:40

convert to Record<string, unknown>

051aeed

rush update --recheck

dc46c22

Required<RequestBodyTransformsType>

822ac74

retain older changelog

854ce6d

set the date

07baf37

HarshaNalluru commented May 12, 2021

View reviewed changes

sdk/test-utils/recorder/CHANGELOG.md Outdated Show resolved Hide resolved

HarshaNalluru commented May 12, 2021

View reviewed changes

sdk/test-utils/recorder/CHANGELOG.md Outdated Show resolved Hide resolved

HarshaNalluru added 6 commits May 12, 2021 16:52

Update sdk/test-utils/recorder/CHANGELOG.md

c9d66fd

lock file

0f14c68

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

fedce31

…to harshan/recorder/filter-scope

revert recording

0bdcad2

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

1af5db3

…to harshan/recorder/filter-scope

lock file

b3b00dd

HarshaNalluru force-pushed the harshan/recorder/filter-scope branch from 9cde294 to b3b00dd Compare May 25, 2021 08:08

HarshaNalluru added 8 commits May 25, 2021 01:33

sanitizeScopeUrl

5c1b872

fix the regex

4d8fe5a

regenerate and playback - works

e002369

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

f627108

…to harshan/recorder/filter-scope

rush update

1c80a0f

changelog

bbffc88

tests for browser recordings

f52eb97

tests for defaultCustomizationsOnRecordings

b4a22d7

jeremymeng approved these changes May 25, 2021

View reviewed changes

HarshaNalluru marked this pull request as ready for review May 26, 2021 00:30

HarshaNalluru requested a review from bterlson as a code owner May 26, 2021 00:30

HarshaNalluru added the auto-merge label May 26, 2021

ghost merged commit dd475e1 into Azure:master May 26, 2021

HarshaNalluru mentioned this pull request Jun 16, 2021

Replace the scope values in the recordings to suppress credscan warnings #14349

Closed

openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this pull request Jul 15, 2021

CodeGen from PR 14954 in Azure/azure-rest-api-specs

7bc15ff

Fix path at line 1557 (Azure#14954)

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CredScan Reports - clearing noise related to "scope" in request bodies #14954

CredScan Reports - clearing noise related to "scope" in request bodies #14954

Uh oh!

HarshaNalluru commented Apr 20, 2021 •

edited

Loading

Uh oh!

Uh oh!

HarshaNalluru Apr 21, 2021

Uh oh!

jeremymeng Apr 21, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremymeng left a comment

Uh oh!

ghost commented May 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CredScan Reports - clearing noise related to "scope" in request bodies #14954

CredScan Reports - clearing noise related to "scope" in request bodies #14954

Uh oh!

Conversation

HarshaNalluru commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!