Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[msal-node][msal-common] client applications, authorization code and device code implementation #1434

Merged
merged 46 commits into from Apr 9, 2020

Conversation

sangonzal
Copy link
Contributor

Adds implementation public APIs and implementation for authorization code flow and device code flow. Will add a separate PR for unit tests- won't merge this until that is out and reviewed.

Adds to msal-common:

  • AuthorizationCodeClient - creating authorization endpoint URI and then making token request
  • DeviceCodeClient - device code flow
  • BaseClient - will be extended by all oauth clients (refresh token and silent coming next)
  • Configuration - configuration of for oauth clients

response:

  • DeviceCodeResponse - response from first step of device code flow. Returned to user via callback.
  • AuthenticationResult - value returned to the user after a successful token acquisition. Have not finalized if this will be added.

request:

  • AuthorizationCodeRequest - ClientApp.acquireTokenByAuthCode(AuthorizationCodeRequest)
  • AuthorizationCodeUrlRequest - ClientApp.getAuthCodeUrl(AuthorizationCodeUrlRequest)
  • DeviceCodeRequest - PublicClientApp.acquireTokenByDeviceCode(DeviceCodeRequest)
  • RequestValidator - validation utils for requests
  • ParameterBuilder - util for building oauth requests
  • CancellationToken - for cancelling polling in device code flow

Add to msal-node:

  • ClientApplication - abstract base class for PublicClientApp and ConfidentialClientApp. Contains APIs shared by both. Not exported.
  • PublicClientApplication - public client flows.
  • ConfidentialClientApplication - confidential client flows.
  • ClientConfiguration - configuration for PublicClientApplication and ConfidentialClientApplication.
  • HeaderUtils - Injecting node default headers into msal-common

Adds to samples:

  • msal-node-auth-code: express app showing authorization code flow
  • msal-node-device-code: console app showing device code flow

Copy link
Contributor

@pkanher617 pkanher617 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm


// Poll token endpoint while (device code is not expired AND operation has not been cancelled by
// setting CancellationToken.cancel = true). POST request is sent at interval set by pollingIntervalMilli
let intervalId: ReturnType<typeof setTimeout>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a const defined inside the Promise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

clearInterval(intervalId);
reject(ClientAuthError.createDeviceCodeExpiredError());

} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early returns might make this try bit more readable.

lib/msal-common/src/client/AuthorizationCodeClient.ts Outdated Show resolved Hide resolved
lib/msal-node/src/cache/Storage.ts Show resolved Hide resolved
const fs = require('fs');

// Cookie life calculation (hours * minutes * seconds * ms)
const COOKIE_LIFE_MULTIPLIER = 24 * 60 * 60 * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the units to the name? e.g. COOKIE_LIFE_MS

cookieStr += `expires=${expireTime};`;
}

document.cookie = cookieStr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document doesn't exist in node, so I don't think you need the cookie logic.

lib/msal-node/src/network/HttpClient.ts Outdated Show resolved Hide resolved
lib/msal-node/tsconfig.json Outdated Show resolved Hide resolved
…checks

[msal-node] Switch strictNullChecks: true for msal-node
[msal-node][msal-common] unit tests for msal-node changes
@sangonzal sangonzal merged commit 5c48a19 into msal-node-network Apr 9, 2020
@sangonzal sangonzal deleted the sagonzal/device-code branch May 6, 2020 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants