Identity application credential by KarishmaGhiya · Pull Request #16787 · Azure/azure-sdk-for-js

KarishmaGhiya · 2021-08-05T21:29:10Z

Create Application Credential (consists of Environment Credential and Managed Identity Credential)
Public tests for Application Credential (testing Env Cred scenarios)
Internal tests for Application Credential (testing Managed Identity Cred Scenarios)
Fixes issue - Create AzureApplicationCredential #15805
Details can be found here
Updated changelog

chradek

This looks like a pretty clean implementation overall! Looks like there's a test failing the CI.

chradek · 2021-08-06T02:52:04Z

sdk/identity/identity/src/credentials/applicationCredential.ts

It doesn't look like the EnvironmentCredential or ManagedIdentityCredential uses the persistence options (if I understand correctly...they don't end up using the msal node flow), so I think we could get away with just extending TokenCredentialOptions.

@willmtemple Am I correct that those credentials don't use persistence?

EnvironmentCredential passes the options to credentials that support persistence. ManagedIdentityCredential does not currently support persistence, but it will eventually.

chradek · 2021-08-06T02:52:55Z

sdk/identity/identity/src/credentials/chainedTokenCredential.ts

Was there an issue for this or did you happen to notice it was wrong?

I’d say, let’s not make this change here unless we make it everywhere in Identity. An issue could help.

On second thought, @KarishmaGhiya can we fix this everywhere else that you find it? Or is this the only place?

so I found inconsitencies in use of "." and "-" with getToken @sadasant

#16806 Filed an issue. May be I can address this later? @chradek @sadasant

@chradek I happened to notice the inconsistencies

Yes, let’s thing about this later. Also, we should align with what other clients are doing! Or align other packages to something that makes sense, (either - or .).

sdk/identity/identity/src/credentials/defaultAzureCredential.ts

sdk/identity/identity/src/credentials/applicationCredential.ts

sdk/identity/identity/test/internal/node/applicationCredential.spec.ts

sadasant · 2021-08-06T14:24:30Z

It’s going really well! Now the final effort comes: Making sure CI is happy 😊

sdk/identity/identity/test/authTestUtils.ts

sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts

sdk/identity/identity/test/public/node/applicationCredential.spec.ts

sdk/identity/identity/test/public/node/environmentCredential.spec.ts

KarishmaGhiya · 2021-08-07T00:37:09Z

This test passes on my local though. Seems that the error messages for Managed Identity Credential are slightly different for local and CI, I updated the test to check just whether it is coming as "not authenticated" instead of verifying the complete message for ManagedIdentityCredential under the "AggregateAuthenticationError"

joshfree · 2021-08-09T17:14:09Z

@sadasant @chlowell can you review/approve this change so it can be merged with this month's ship cycle?

sadasant

Good job! 👏

Microsoft.SecurityInsights 2021-09-01-preview (Azure#16933) * Adds base for updating Microsoft.SecurityInsights from version preview/2021-03-01-preview to version 2021-09-01-preview * Updates readme * Updates API version in new specs and examples * Microsoft.security insights 2021 09 01 preview add missing resources (Azure#15531) * Copy missing resources specs and examples from 2019-01-01-preview * Update added resources specs and examples and extract common types * Update readme * Extract ClientInfo, UserInfo and Lable to common types * Fix SpellCheck and Avocado * Return ThreatIntelligence to readme * Fix broken refs in Watchlists * Resolve duplicate schema errors * Run prettier * Make common types prettier * Add required property to operations according to ARM requirments * Fix readme * Add file separators to readme * Rename example file * Supress OBJECT_ADDITIONAL_PROPERTIES * Add 'where' to OBJECT_ADDITIONAL_PROPERTIES supression * Move OBJECT_ADDITIONAL_PROPERTIES supression under general Supression section. * Copy dataConnectors from 2021-03-01-preview * Update version of dataConnectors (this was done as there were errors when trying to generate C# client. Copying and changing version again fixed the problem). * Add dataConnectorsCheckRequirments path, parameters and definitions from 2019-01-01-preveiw Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Use newest common types in new 2021-09-01-preview API version (Azure#15778) * Use newest common types in AlertRules * Use newest common types in AutomationRules * Use newest common types in Bookmarks * Use newest common types in dataConnectors * Use newest common types in Enrichment * Use newest common types in Entities * Use newest common types in EntityQueries * Use newest common types in Incidents * Use newest common types in Metadata * Use newest common types in OfficeConsents * Use newest common types in OnboardingStates * Use newest common types in operations * Use newest common types in Settings * Use newest common types in SourceControls * Use newest common types in ThreatIntelligence * Use newest common types in Watchlist * Use newest common types in EntityTypes * Use newest common types in RelationTypes * Fix ThreatIntelligence Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Add template version to the scheduled alert rule + scheduled template (Azure#15919) * Add template version to the scheduled alert rule * Update AlertRules.json * Update AlertRules.json * Update AlertRules.json * Update AlertRules.json * Update GetAlertRuleTemplates.json * Update GetAlertRuleTemplateById.json * add aws s3 connector (Azure#15844) * Add a new kind of alert rules - NRT (Azure#15980) * add NRT rule * add NRT rule * add NRT rule * add NRT rule * fix typo * fix typo * fix * Align new Metadata feature with 2021-03-01-preview (Azure#16304) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Add fixes from 2021-03-01-preview (Azure#16238) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Add entity query templates (Azure#16269) * Add entity query templates from 2021-03-01-preview * Update version * Use newest common types and update readme * Fix conflicting common types Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Fix bookmark relations operatinIds to be consistent with other operationIds. (Azure#16519) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Add corrections from 2021-03-01-preview (Azure#16490) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Remove unused parameters (Azure#16619) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Update readme default readme tag for client generation (Azure#16620) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Use CloudError instead of ErrorResponse to avoid breaking change (Azure#16691) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Add data connectors polling ccp api support (Azure#16293) * adding dataConnectors polling CCP api Support. (witout tests validations) * azure sentinel dataconnectors update examples * azure sentinel dataConnectors examples update and fix * azure sentinel dataConnectors prettier * azure sentinel dataConnectors add connect disconnect examples update path * azure sentinel dataConnectors add connect disconnect examples fix * azure sentinel dataConnectors add connect disconnect examples fix 2 * azure sentinel dataConnectors rebase dataConnectors dev * azure sentinel dataconnectors - fix put to post on connect and disconnect endpoints * azure sentinel dataconnectors - adding x-ms-secret to password on connect * azure sentinel dataconnectors - connect/disconnect endpoint remove unnedded 201 return * azure sentinel dataConnectors - remove empty body DataConnectorDisconnectBody Co-authored-by: Alon Danoch <adanoch@microsoft.com> * Add office IRM Connector (Azure#16764) * Add office IRM * fix * fix * fix * fix Co-authored-by: omerhaimov <omer.haimovich@gmail.com> * Add teamInformation to IncidentProperties 2021-09-01-preview (Azure#16787) * Fix Swagger for SecurityInsights - Add teamInformation to IncidentProperties * Try change description as advised by Swagger reviewer Yuchao Yan to fix the validation error. * Revert change in ntDomain description as it has nothing to do with validations Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Make CloudError and CloudErrorBody external resources (already exist under Microsoft.Rest.Azure namespace) (Azure#16872) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Remove operational insights parameter 2021 09 01 preview (Azure#16891) * Remove operationalInsightsResourceProvider parameter from specs * Remove parameter from examples Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> * Update EntityTypes.json (Azure#16972) Co-authored-by: Anat Gilenson <anatgilenson@microsoft.com> Co-authored-by: Amit Bergman <38046493+Amitbergman@users.noreply.github.com> Co-authored-by: sagamzu <52034287+sagamzu@users.noreply.github.com> Co-authored-by: necoh <53861229+necoh@users.noreply.github.com> Co-authored-by: alondanoch <alondanoch@hotmail.com> Co-authored-by: Alon Danoch <adanoch@microsoft.com> Co-authored-by: omerhaimov <55688621+omerhaimov@users.noreply.github.com> Co-authored-by: omerhaimov <omer.haimovich@gmail.com> Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>

KarishmaGhiya requested review from bterlson, chradek, sadasant, schaabs, witemple-msft and xirzec as code owners August 5, 2021 21:29

ghost added the Azure.Identity label Aug 5, 2021

KarishmaGhiya linked an issue Aug 5, 2021 that may be closed by this pull request

Create AzureApplicationCredential #15805

Closed

chradek reviewed Aug 6, 2021

View reviewed changes

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/src/credentials/applicationCredential.ts Outdated Show resolved Hide resolved

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/test/internal/node/applicationCredential.spec.ts Outdated Show resolved Hide resolved

KarishmaGhiya added 10 commits August 6, 2021 10:42

application credential

00827b4

application credential

c8c4c88

application credential creation

5c57432

export ApplicationCredential and update api review

ae0e1e3

application credential tests

cac1927

default management credential

bf87ffe

add all tests and recordings for public tests

ea0a74e

internal tests

d5bcd10

debug tests

6baa62a

add all tests

af9f607

KarishmaGhiya force-pushed the identity-application-credential branch from 438364e to af9f607 Compare August 6, 2021 22:37

update test and recording for that test

0e68e58

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/test/authTestUtils.ts Outdated Show resolved Hide resolved

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/test/internal/node/managedIdentityCredential.spec.ts Outdated Show resolved Hide resolved

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/test/public/node/applicationCredential.spec.ts Outdated Show resolved Hide resolved

sadasant reviewed Aug 6, 2021

View reviewed changes

sdk/identity/identity/test/public/node/environmentCredential.spec.ts Outdated Show resolved Hide resolved

KarishmaGhiya added 2 commits August 6, 2021 16:41

remove assert refs

1137499

fix lint

04e4676

KarishmaGhiya added 2 commits August 6, 2021 16:50

remove unnecessary spacing

599b288

fix tracing and add internal to defaultMangaedIdentityCred

f11b550

KarishmaGhiya and others added 4 commits August 6, 2021 17:42

update error message

2ac198a

added changelog

1a75abc

Merge branch 'main' into identity-application-credential

2f053d5

added changelog

8ea2281

joshfree requested a review from chlowell August 9, 2021 17:13

sadasant approved these changes Aug 9, 2021

View reviewed changes

KarishmaGhiya merged commit 71a793e into Azure:main Aug 9, 2021

Conversation

KarishmaGhiya commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chradek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sadasant commented Aug 6, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KarishmaGhiya commented Aug 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshfree commented Aug 9, 2021

Uh oh!

sadasant left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KarishmaGhiya commented Aug 5, 2021 •

edited

Loading

KarishmaGhiya commented Aug 7, 2021 •

edited

Loading