[Identity] [localhost to cloud proposal] @azure/identity-web #20772

sadasant · 2022-03-10T15:38:06Z

This package is a WORK IN PROGRESS.

This PR introduces the WebRedirectCredential, a credential that aims to simplify the development of web backend applications, from localhost to the Azure cloud.

Before I make the actual proposal, I’ll use this draft PR to ask for feedback, and to build the package for the Playwright-based champion scenarios that I’m making.

Fixes #21045

If you want to try out how the new design looks in comparison to the old design, I’m working on mocked tests here:

KarishmaGhiya · 2022-03-10T20:41:24Z

sdk/identity/identity-web/src/index.ts

+// Licensed under the MIT license.
+
+import { MsalAuthorizationCode } from "../../identity/src/msal/nodeFlows/msalAuthorizationCode";
+import { MsalFlow } from "../../identity/src/msal/flows";


I was wondering let's avoid taking dependencies on source files. Let's import these from what we have exported publicly from identity package. Else we might run into similar issues as the plugin packages. @sadasant

To avoid this is to either copy the code or to make the separate “common” repo that you were working on. To make this PR simpler, I will keep Will’s approach for now.

sadasant · 2022-03-23T00:21:27Z

@jasonnutter brought to me the concern that naming this library and credential “Web” might be confusing as Microsoft already has https://github.com/AzureAD/microsoft-identity-web

sdk/identity/identity-web/LICENSE

sadasant · 2022-04-14T23:44:47Z

sdk/identity/identity-web/README.md

+const state = session.username; // or ID
+
+// Make sure to redirect to this URI to trigger the Authentication Code Flow:
+const authorizeUrl = credential.getRedirectUri(scope, {


(Based on feedback from @willmtemple and @KarishmaGhiya )

Use something similar to this: https://github.com/Azure/azure-sdk-for-js/pull/21028/files#diff-1f63193b2eb9f68e6353d08afcd823edf160f469d3c70c37d6631f98e052ece6R324

The question is: Should our clients expose a scope? If so, it would be much easier for our users to discover the scopes needed to authenticate with different clients:

const client = new ServiceBusClient(serviceBusEndpoint, credential); const scopes = client.scopes; await credential.authenticate(scopes);

Jose suggested - client.authenticationOptions = {scopes: []}

sadasant · 2022-04-14T23:46:16Z

sdk/identity/identity-web/README.md

+    const credential = new WebRedirectCredential(
+      tenantId,
+      clientId,
+      redirectUri


TODO: Mention that this redirect URI should match the route used in the next snippet, meaning /azureResponse

sadasant · 2022-04-14T23:51:19Z

sdk/identity/identity-web/src/index.ts

+    }
+    const params = new URLSearchParams(queryParams as Record<string, string>);
+    const query = params.toString();
+    const authorizeHost = `https://login.microsoftonline.com/${this.tenantId}/oauth2/v2.0/authorize?${query}`;


This login.microsoft.com most likely changes depending on the cloud.

Feedback from @KarishmaGhiya : Use the authority host.

Because getAuthorizeUrl( is part of a credential class, and we’re re-using the code from the main identity package, we can derive the authority host from the MSAL internals, we just need to re-use the code we use there.

sdk/identity/identity-web/src/index.ts

check-enforcer · 2022-04-17T08:00:00Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run js - [service] - ci

KarishmaGhiya · 2022-04-18T22:15:28Z

sdk/identity/identity-web/README.md

+    const state = session.username; // or ID
+
+    // We get the authorize URL.
+    const authorizeUrl = credential.getRedirectUri(scope, {


I think in the code you have this function as getAuthorizeUrl instead of getRedirectUri. We need to update all samples to use the correct method name

sdk/identity/identity-web/src/index.ts

sdk/identity/identity-web/package.json

Co-authored-by: KarishmaGhiya <[email protected]>

ghost · 2022-07-15T10:04:46Z

Hi @sadasant. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

ghost · 2022-07-22T11:04:02Z

Hi @sadasant. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.

[Hub Generated] Review request for Microsoft.MachineLearningServices to add version preview/2022-10-01-preview (Azure#20818) * Adds base for updating Microsoft.MachineLearningServices from version preview/2022-06-01-preview to version 2022-10-01-preview * Updates readme * Updates API version in new specs and examples * Registries public preview 2022-10-01-preview (Azure#20200) * Registries public preview 2022-10-01-preview * Fix validation errors * Fix LintDiff and Avocado errors * Fixing prettier errors * Fixing more LintDiff errors * Another LintDiff error * Another Lint error Co-authored-by: Fahd Kamal <[email protected]> * Added schedule api (Azure#20273) * added schedule api * add updateSchedules to examples parent folder * fix typo * change position of allOf * remove subnet from example compute create * after prettier run * Description change for recurrence freq * point to mfe for common objects * add hash operator * removed triggertype as well * change year * get rid of breaking changes since we didn't change the api Co-authored-by: Naman Agarwal <[email protected]> * Added Compute Instance OS Patching Properties (Azure#20284) * Added CI Os Patching related properties * Updated descriptions as per Dennis's suggestions Co-authored-by: Srivatsa Sinha <[email protected]> * Maulik/sync preview (Azure#20554) * Added changes to sync with main * Fixed Prettier Check Co-authored-by: maulikjoshi <[email protected]> * Remove update schedules REST call for compute instance (Azure#20699) * change post call to put * change put to patch * remove updateschedules rest call Co-authored-by: Naman Agarwal <[email protected]> * Update connection type and auth type (Azure#20493) * Update connection type and auth type * update connection category * update x-ms-secret tag * Add autologger_settings for CI (Azure#20302) * Add autologger_settings for CI Add settings to opt-in/out of default mlflow autologger on CIs. * update machinelearningservices.json * Add for command job * fix comma * Add autologger for command job * fix CI failures * Remove mfe.json from PR * Remove whitespace * Update custom-words.txt * Delete incorrect custom-words.txt * Fixing auto-rest issues in registries (Azure#20772) * Fixing auto-rest issues in registries * Removing location from the required list Co-authored-by: Fahd Kamal <[email protected]> * Maulik/mfe preview changes (Azure#20372) * Added mfe.json preview changes * Added resources to Registries and Workspaces folder * removed unwanted resources * removed unwanted resources * Ran prettier check * removed unwanted resources * removed unwanted resources * Added ManagedOnlineDeployment resource * Added files to OnlineDeplyment * Added Model Validation changes * Added mfe.json changes to handle duplicate operation id changes * Updated mfe to fix underscore error * Update title of mfe.json * Renamed name path parameter * Updated examples * Fixed Semantic validations and examples * Fixed Semantic validations and examples * Fixed Semantic validations and examples * Removed name changes from workspaces * Changes related to Model Validation * Changes related to Model validation * Added consumes amd produces in registries.json * Removed changes from registries * Updated parameter mlFlowAutologger to mlflowAutologger in mfe.json * Changed property name to mlflowAutologger Co-authored-by: maulikjoshi <[email protected]> * Updated custom words to resolve merge conflict (Azure#20833) * Updated custom words to resolve merge conflict * Updated custom words to resolve merge conflict Co-authored-by: maulikjoshi <[email protected]> * Resolved merge conflict (Azure#20845) Co-authored-by: maulikjoshi <[email protected]> * Fixed LintDiff checks (Azure#20847) * Fixed LintDiff checks * Fixed LintDiff checks * Fixed LintDiff checks * Fixed LintDiff checks * Updated examples Co-authored-by: maulikjoshi <[email protected]> * Resolved merge conflict of cutsom-words.txt (Azure#20956) * Resolved merge conflict * Resolved merge conflict Co-authored-by: maulikjoshi <[email protected]> * [Hub Generated] Review request for Microsoft.MachineLearningServices to add version stable/2022-10-01 (Azure#20800) (Azure#20982) * Adds base for updating Microsoft.MachineLearningServices from version stable/2022-05-01 to version 2022-10-01 * Updates readme * Updates API version in new specs and examples * Added readOnly property (Azure#20506) * Added readOnly property * Added changes to sync with main * Fixed Prettier check Co-authored-by: maulikjoshi <[email protected]> * Add schedule related schemas for Compute Instance (Azure#20697) * support for update schedules on Compute Instance * change put to patch * remove update schedules rest call Co-authored-by: Naman Agarwal <[email protected]> * Added mfe.json changes (Azure#20514) * Added mfe.json changes * Added checks related changes * Updated mfe.json to handle online endpoint and deployment changes * Updated mfe.json with AutoRebuild changes Co-authored-by: maulikjoshi <[email protected]> * Removed duplicate definitions and updated with mfe references (Azure#20840) * Removed duplicate definitions and updated with mfe references * Replaced Cron to CronTrigger, Recurrence to RecurrenceTrigger from mfe Co-authored-by: maulikjoshi <[email protected]> Co-authored-by: maulikjoshi <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: maulikjoshi <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> * Revert "[Hub Generated] Review request for Microsoft.MachineLearningS… (Azure#20983) * Revert "[Hub Generated] Review request for Microsoft.MachineLearningServices to add version stable/2022-10-01 (Azure#20800) (Azure#20982)" This reverts commit 649ef8334aca13558e534babdcf53cef6831f22a. * Updated readme.md to resolve merge conflict Co-authored-by: maulikjoshi <[email protected]> * Maulik/fix preview (Azure#20994) * Merged readme from main * Added preview tag changes Co-authored-by: maulikjoshi <[email protected]> * shuffle readme tags (Azure#21005) * conflict resolution (Azure#21007) Co-authored-by: fahdkmsft <[email protected]> Co-authored-by: Fahd Kamal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: Naman Agarwal <[email protected]> Co-authored-by: srivatsasinha <[email protected]> Co-authored-by: Srivatsa Sinha <[email protected]> Co-authored-by: maulikjoshi <[email protected]> Co-authored-by: ZhidaLiu <[email protected]> Co-authored-by: prakharg-msft <[email protected]> Co-authored-by: Teddy Todorov <[email protected]>

ghost added the Azure.Identity label Mar 10, 2022

sadasant self-assigned this Mar 10, 2022

KarishmaGhiya reviewed Mar 10, 2022

View reviewed changes

sadasant commented Mar 24, 2022

View reviewed changes

sdk/identity/identity-web/LICENSE Outdated Show resolved Hide resolved

sadasant commented Apr 14, 2022

View reviewed changes

sdk/identity/identity-web/src/index.ts Outdated Show resolved Hide resolved

KarishmaGhiya reviewed Apr 18, 2022

View reviewed changes

sdk/identity/identity-web/src/index.ts Outdated Show resolved Hide resolved

sadasant commented Apr 18, 2022

View reviewed changes

sdk/identity/identity-web/src/index.ts Outdated Show resolved Hide resolved

KarishmaGhiya reviewed Apr 19, 2022

View reviewed changes

sdk/identity/identity-web/package.json Show resolved Hide resolved

sadasant and others added 8 commits April 19, 2022 11:59

wip

c61dc3a

Update sdk/identity/identity-web/LICENSE

c30a66b

Update sdk/identity/identity-web/src/index.ts

79ffe2e

update pnpm lock file

799d19c

Update sdk/identity/identity-web/src/index.ts

c243e96

Co-authored-by: KarishmaGhiya <[email protected]>

Update sdk/identity/identity-web/src/index.ts

b237ff9

passing clientSecret to MSAL

f5fbdde

update identity version to have the bug fix for including publicApp

e99488a

KarishmaGhiya force-pushed the identity/web-plugin branch from 43392d5 to e99488a Compare April 19, 2022 19:04

KarishmaGhiya added 2 commits April 19, 2022 12:32

make changes to tracing references

b322ae3

add core-tracing and identity as dependencies

23fbffa

ghost added the no-recent-activity There has been no recent activity on this issue. label Jul 15, 2022

ghost closed this Jul 22, 2022

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Identity] [localhost to cloud proposal] @azure/identity-web #20772

[Identity] [localhost to cloud proposal] @azure/identity-web #20772

sadasant commented Mar 10, 2022 •

edited

Loading

KarishmaGhiya Mar 10, 2022

sadasant Mar 10, 2022

sadasant commented Mar 23, 2022

sadasant Apr 14, 2022

KarishmaGhiya May 4, 2022

sadasant Apr 14, 2022

sadasant Apr 14, 2022

sadasant Apr 14, 2022

check-enforcer bot commented Apr 17, 2022

KarishmaGhiya Apr 18, 2022 •

edited

Loading

ghost commented Jul 15, 2022

ghost commented Jul 22, 2022

[Identity] [localhost to cloud proposal] @azure/identity-web #20772

[Identity] [localhost to cloud proposal] @azure/identity-web #20772

Conversation

sadasant commented Mar 10, 2022 • edited Loading

KarishmaGhiya Mar 10, 2022

Choose a reason for hiding this comment

sadasant Mar 10, 2022

Choose a reason for hiding this comment

sadasant commented Mar 23, 2022

sadasant Apr 14, 2022

Choose a reason for hiding this comment

KarishmaGhiya May 4, 2022

Choose a reason for hiding this comment

sadasant Apr 14, 2022

Choose a reason for hiding this comment

sadasant Apr 14, 2022

Choose a reason for hiding this comment

sadasant Apr 14, 2022

Choose a reason for hiding this comment

check-enforcer bot commented Apr 17, 2022

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

KarishmaGhiya Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

ghost commented Jul 15, 2022

ghost commented Jul 22, 2022

sadasant commented Mar 10, 2022 •

edited

Loading

KarishmaGhiya Apr 18, 2022 •

edited

Loading