[Identity] [localhost to cloud proposal] @azure/identity-spa #21028

sadasant · 2022-03-24T23:53:26Z

This package is a WORK IN PROGRESS.

This PR introduces two credentials: RedirectCredential and PopupCredential through a new package, @azure/identity-spa. This effort 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 #21050

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

check-enforcer · 2022-03-25T21:45:31Z

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

sdk/identity/identity-spa/README.md

sadasant · 2022-04-08T22:46:37Z

sdk/identity/identity-spa/README.md

+  const pageLoadResult = await credential.onPageLoad();
+  if (pageLoadResult) {
+    state = JSON.parse(pageLoadResult.state);
+  }


State and pageLoadResult need to move up.

sdk/identity/identity-spa/README.md

sadasant · 2022-04-15T00:02:13Z

sdk/identity/identity-spa/README.md

+  // A browser application can keep track of its own state and resume after authenticating.
+  let state = {
+    application: "state"
+  };


State might not be useful in popup mode.

sadasant · 2022-04-15T00:02:18Z

sdk/identity/identity-spa/README.md

+      // Interactive authentication will happen via a popup window that will appear when the authenticate() method is called.
+      await credential.authenticate(scopes, {
+        // The authenticate() method supports sending the browser application state.
+        state: JSON.stringify(state)


State might not be useful in popup mode.

sadasant · 2022-04-15T00:06:18Z

sdk/identity/identity-spa/src/popupCredential.browser.ts

+      code: hash.get("code") || undefined,
+      state: hash.get("state") || undefined,
+    };
+  }


This isn’t useful for the PopupCredential, but it is useful for the RedirectCredential.

sadasant · 2022-04-15T00:08:35Z

sdk/identity/identity-spa/src/redirectCredential.browser.ts

+   */
+  async authenticate(
+    scopes: string | string[],
+    options: GetTokenOptions = {}


Options here should include a state property.

sadasant · 2022-04-15T00:08:48Z

sdk/identity/identity-spa/src/redirectCredential.browser.ts

+  ): Promise<AuthenticationRecord | undefined> {
+    return trace(`${this.constructor.name}.authenticate`, options, async (newOptions) => {
+      const arrayScopes = Array.isArray(scopes) ? scopes : [scopes];
+      await this.msalFlow.getToken(arrayScopes, newOptions);


The MSAL flow should pass the state property to MSAL.

sdk/identity/identity-spa/README.md

KarishmaGhiya · 2022-04-21T18:32:02Z

sdk/identity/identity-spa/README.md

+
+  // On page load, developers can retrieve the state of the previous authentication.
+  const pageLoadResult = await credential.onPageLoad();
+  if (pageLoadResult) {


Suggested change

if (pageLoadResult) {

if (pageLoadResult.state) {

Suggested change

if (pageLoadResult) {

if (pageLoadResult) {

azure-sdk · 2022-04-21T18:45:28Z

API change check for @azure/core-client

API changes have been detected in @azure/core-client. You can review API changes here

API changes

+     readonly scopes?: string[];

…re-sdk-for-js into identity/spa-plugin

KarishmaGhiya · 2022-04-27T22:35:18Z

sdk/core/core-client/review/core-client.api.md

@@ -321,6 +321,7 @@ export interface SerializerOptions {
 export class ServiceClient {
    constructor(options?: ServiceClientOptions);
    readonly pipeline: Pipeline;
+    readonly scopes?: string[];


The authenticate method requires user to pass in the scope

this is a way for the user to retrieve the scope from the client.

KarishmaGhiya · 2022-04-27T22:47:24Z

sdk/identity/identity-spa/review/identity-spa.api.md

+
+// @public
+export interface SPACredentialOptions extends InteractiveCredentialOptions, CredentialPersistenceOptions {
+    clientId?: string;


CredentialPersistenceOptions not needed, since the persistence is already done by the MSAL browser

ghost · 2022-07-01T10:04:45Z

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.

KarishmaGhiya · 2022-07-02T02:44:57Z

Please leave this PR open. Don't close Pleaseee

ghost · 2022-09-02T10:05:02Z

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-09-09T11:04:08Z

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.

sadasant self-assigned this Mar 24, 2022

ghost added the Azure.Identity label Mar 24, 2022

sadasant commented Apr 8, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Outdated Show resolved Hide resolved

sadasant commented Apr 8, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Outdated Show resolved Hide resolved

sadasant commented Apr 8, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Show resolved Hide resolved

sadasant commented Apr 8, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Show resolved Hide resolved

sadasant commented Apr 8, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Outdated Show resolved Hide resolved

sadasant commented Apr 15, 2022

View reviewed changes

sadasant and others added 8 commits April 20, 2022 16:07

wip

835f5cb

pnpm-lock

da36bcb

builds and has documentation

1b542b0

no tabs

d4bae94

I was missing the onPageLoad functions

27e27a1

some updates after revisiting

c8fe103

cleaned up tabs

9ded720

add tracing as a dependency

622423a

KarishmaGhiya force-pushed the identity/spa-plugin branch from 5fd06a7 to 622423a Compare April 20, 2022 23:10

KarishmaGhiya added 2 commits April 20, 2022 16:18

update tracing client for identity-spa

8d08750

checking

4af32f2

KarishmaGhiya reviewed Apr 21, 2022

View reviewed changes

sdk/identity/identity-spa/README.md Outdated Show resolved Hide resolved

KarishmaGhiya reviewed Apr 21, 2022

View reviewed changes

Update sdk/identity/identity-spa/README.md

b488229

removing disableAutomaticAuthentication from the constructor

c683c70

sadasant and others added 2 commits April 21, 2022 22:05

formatting

f0363ff

Merge branch 'identity/spa-plugin' of https://github.com/sadasant/azu…

526f68f

…re-sdk-for-js into identity/spa-plugin

KarishmaGhiya reviewed Apr 27, 2022

View reviewed changes

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

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

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

ghost closed this Sep 9, 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-spa #21028

[Identity] [localhost to cloud proposal] @azure/identity-spa #21028

sadasant commented Mar 24, 2022 •

edited

check-enforcer bot commented Mar 25, 2022

sadasant Apr 8, 2022

sadasant Apr 15, 2022

sadasant Apr 15, 2022

sadasant Apr 15, 2022

sadasant Apr 15, 2022

sadasant Apr 15, 2022

KarishmaGhiya Apr 21, 2022

azure-sdk commented Apr 21, 2022

KarishmaGhiya Apr 27, 2022 •

edited

KarishmaGhiya Apr 27, 2022

ghost commented Jul 1, 2022

KarishmaGhiya commented Jul 2, 2022

ghost commented Sep 2, 2022

ghost commented Sep 9, 2022

[Identity] [localhost to cloud proposal] @azure/identity-spa #21028

[Identity] [localhost to cloud proposal] @azure/identity-spa #21028

Conversation

sadasant commented Mar 24, 2022 • edited

check-enforcer bot commented Mar 25, 2022

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azure-sdk commented Apr 21, 2022

KarishmaGhiya Apr 27, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 1, 2022

KarishmaGhiya commented Jul 2, 2022

ghost commented Sep 2, 2022

ghost commented Sep 9, 2022

sadasant commented Mar 24, 2022 •

edited

KarishmaGhiya Apr 27, 2022 •

edited