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

Adopt the MSAL broker to talk to the OS for Microsoft auth #233739

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build/darwin/create-universal-app.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions build/darwin/create-universal-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ async function main(buildDir?: string) {
const filesToSkip = [
'**/CodeResources',
'**/Credits.rtf',
// TODO: Should we consider expanding this to other files in this area?
'**/node_modules/@parcel/node-addon-api/nothing.target.mk'
Comment on lines +31 to +32
Copy link
Member Author

@TylerLeonhardt TylerLeonhardt Nov 14, 2024

Choose a reason for hiding this comment

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

@deepak1556 this fixed the Create Universal App step of the build for me. Let me know if you'd like to expand the scope but until then I've scoped it as small as possible.

];

await makeUniversalApp({
Expand Down
56 changes: 45 additions & 11 deletions build/gulpfile.extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,27 +230,61 @@ exports.compileExtensionMediaBuildTask = compileExtensionMediaBuildTask;

//#region Azure Pipelines

/**
* Cleans the build directory for extensions
*/
const cleanExtensionsBuildTask = task.define('clean-extensions-build', util.rimraf('.build/extensions'));
const compileExtensionsBuildTask = task.define('compile-extensions-build', task.series(
exports.cleanExtensionsBuildTask = cleanExtensionsBuildTask;

/**
* brings in the marketplace extensions for the build
*/
const bundleMarketplaceExtensionsBuildTask = task.define('bundle-marketplace-extensions-build', () => ext.packageMarketplaceExtensionsStream(false).pipe(gulp.dest('.build')));

/**
* Compiles the non-native extensions for the build
* @note this does not clean the directory ahead of it. See {@link cleanExtensionsBuildTask} for that.
*/
const compileNonNativeExtensionsBuildTask = task.define('compile-non-native-extensions-build', task.series(
bundleMarketplaceExtensionsBuildTask,
task.define('bundle-non-native-extensions-build', () => ext.packageNonNativeLocalExtensionsStream().pipe(gulp.dest('.build')))
));
gulp.task(compileNonNativeExtensionsBuildTask);
exports.compileNonNativeExtensionsBuildTask = compileNonNativeExtensionsBuildTask;

/**
* Compiles the native extensions for the build
* @note this does not clean the directory ahead of it. See {@link cleanExtensionsBuildTask} for that.
*/
const compileNativeExtensionsBuildTask = task.define('compile-native-extensions-build', () => ext.packageNativeLocalExtensionsStream().pipe(gulp.dest('.build')));
gulp.task(compileNativeExtensionsBuildTask);
exports.compileNativeExtensionsBuildTask = compileNativeExtensionsBuildTask;

/**
* Compiles the extensions for the build.
* This is essentially a helper task that combines {@link cleanExtensionsBuildTask}, {@link compileNonNativeExtensionsBuildTask} and {@link compileNativeExtensionsBuildTask}
*/
const compileAllExtensionsBuildTask = task.define('compile-extensions-build', task.series(
cleanExtensionsBuildTask,
task.define('bundle-marketplace-extensions-build', () => ext.packageMarketplaceExtensionsStream(false).pipe(gulp.dest('.build'))),
task.define('bundle-extensions-build', () => ext.packageLocalExtensionsStream(false, false).pipe(gulp.dest('.build'))),
bundleMarketplaceExtensionsBuildTask,
task.define('bundle-extensions-build', () => ext.packageAllLocalExtensionsStream(false, false).pipe(gulp.dest('.build'))),
));
gulp.task(compileAllExtensionsBuildTask);
exports.compileAllExtensionsBuildTask = compileAllExtensionsBuildTask;

gulp.task(compileExtensionsBuildTask);
gulp.task(task.define('extensions-ci', task.series(compileExtensionsBuildTask, compileExtensionMediaBuildTask)));
// This task is run in the compilation stage of the CI pipeline. We only compile the non-native extensions since those can be fully built regardless of platform.
// This defers the native extensions to the platform specific stage of the CI pipeline.
gulp.task(task.define('extensions-ci', task.series(compileNonNativeExtensionsBuildTask, compileExtensionMediaBuildTask)));

const compileExtensionsBuildPullRequestTask = task.define('compile-extensions-build-pr', task.series(
cleanExtensionsBuildTask,
task.define('bundle-marketplace-extensions-build', () => ext.packageMarketplaceExtensionsStream(false).pipe(gulp.dest('.build'))),
task.define('bundle-extensions-build-pr', () => ext.packageLocalExtensionsStream(false, true).pipe(gulp.dest('.build'))),
bundleMarketplaceExtensionsBuildTask,
task.define('bundle-extensions-build-pr', () => ext.packageAllLocalExtensionsStream(false, true).pipe(gulp.dest('.build'))),
));

gulp.task(compileExtensionsBuildPullRequestTask);
gulp.task(task.define('extensions-ci-pr', task.series(compileExtensionsBuildPullRequestTask, compileExtensionMediaBuildTask)));


exports.compileExtensionsBuildTask = compileExtensionsBuildTask;
// This task is run in the compilation stage of the PR pipeline. We compile all extensions in it to verify compilation.
gulp.task(task.define('extensions-ci-pr', task.series(compileExtensionsBuildPullRequestTask, compileExtensionMediaBuildTask)));

//#endregion

Expand Down
6 changes: 4 additions & 2 deletions build/gulpfile.reh.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const File = require('vinyl');
const fs = require('fs');
const glob = require('glob');
const { compileBuildTask } = require('./gulpfile.compile');
const { compileExtensionsBuildTask, compileExtensionMediaBuildTask } = require('./gulpfile.extensions');
const { cleanExtensionsBuildTask, compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileExtensionMediaBuildTask } = require('./gulpfile.extensions');
const { vscodeWebResourceIncludes, createVSCodeWebFileContentMapper } = require('./gulpfile.vscode.web');
const cp = require('child_process');
const log = require('fancy-log');
Expand Down Expand Up @@ -468,6 +468,7 @@ function tweakProductForServerWeb(product) {
const destinationFolderName = `vscode-${type}${dashed(platform)}${dashed(arch)}`;

const serverTaskCI = task.define(`vscode-${type}${dashed(platform)}${dashed(arch)}${dashed(minified)}-ci`, task.series(
compileNativeExtensionsBuildTask,
gulp.task(`node-${platform}-${arch}`),
util.rimraf(path.join(BUILD_ROOT, destinationFolderName)),
packageTask(type, platform, arch, sourceFolderName, destinationFolderName)
Expand All @@ -476,7 +477,8 @@ function tweakProductForServerWeb(product) {

const serverTask = task.define(`vscode-${type}${dashed(platform)}${dashed(arch)}${dashed(minified)}`, task.series(
compileBuildTask,
compileExtensionsBuildTask,
cleanExtensionsBuildTask,
compileNonNativeExtensionsBuildTask,
compileExtensionMediaBuildTask,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want compileExtensionsBuildTask here ? I believe this task is only used for local build, so useful to build all targets ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does build all targets. We build the non-native ones here, and then the native ones in the serverTaskCI which is included in this local build task at the bottom

minified ? minifyTask : bundleTask,
serverTaskCI
Expand Down
8 changes: 5 additions & 3 deletions build/gulpfile.vscode.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const { config } = require('./lib/electron');
const createAsar = require('./lib/asar').createAsar;
const minimist = require('minimist');
const { compileBuildTask } = require('./gulpfile.compile');
const { compileExtensionsBuildTask, compileExtensionMediaBuildTask } = require('./gulpfile.extensions');
const { compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileAllExtensionsBuildTask, compileExtensionMediaBuildTask, cleanExtensionsBuildTask } = require('./gulpfile.extensions');
const { promisify } = require('util');
const glob = promisify(require('glob'));
const rcedit = promisify(require('rcedit'));
Expand Down Expand Up @@ -487,6 +487,7 @@ BUILD_TARGETS.forEach(buildTarget => {
const destinationFolderName = `VSCode${dashed(platform)}${dashed(arch)}`;

const tasks = [
compileNativeExtensionsBuildTask,
util.rimraf(path.join(buildRoot, destinationFolderName)),
packageTask(platform, arch, sourceFolderName, destinationFolderName, opts)
];
Expand All @@ -500,7 +501,8 @@ BUILD_TARGETS.forEach(buildTarget => {

const vscodeTask = task.define(`vscode${dashed(platform)}${dashed(arch)}${dashed(minified)}`, task.series(
compileBuildTask,
compileExtensionsBuildTask,
cleanExtensionsBuildTask,
compileNonNativeExtensionsBuildTask,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
compileNonNativeExtensionsBuildTask,
compileExtensionsBuildTask,

?

Copy link
Member Author

Choose a reason for hiding this comment

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

With this suggestion, we will be building the native ones twice. I separated them out so that we didn't double build based on your original feedback.

The non-native ones are built here, and then the native ones are part of vscodeTaskCI which is added to the end of this local build task.

Am I misunderstanding something?

compileExtensionMediaBuildTask,
minified ? minifyVSCodeTask : bundleVSCodeTask,
vscodeTaskCI
Expand Down Expand Up @@ -537,7 +539,7 @@ gulp.task(task.define(
'vscode-translations-export',
task.series(
core,
compileExtensionsBuildTask,
compileAllExtensionsBuildTask,
function () {
const pathToMetadata = './out-build/nls.metadata.json';
const pathToExtensions = '.build/extensions/*';
Expand Down
2 changes: 1 addition & 1 deletion build/gulpfile.vscode.web.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function packageTask(sourceFolderName, destinationFolderName) {

const compileWebExtensionsBuildTask = task.define('compile-web-extensions-build', task.series(
task.define('clean-web-extensions-build', util.rimraf('.build/web/extensions')),
task.define('bundle-web-extensions-build', () => extensions.packageLocalExtensionsStream(true, false).pipe(gulp.dest('.build/web'))),
task.define('bundle-web-extensions-build', () => extensions.packageAllLocalExtensionsStream(true, false).pipe(gulp.dest('.build/web'))),
task.define('bundle-marketplace-web-extensions-build', () => extensions.packageMarketplaceExtensionsStream(true).pipe(gulp.dest('.build/web'))),
task.define('bundle-web-extension-media-build', () => extensions.buildExtensionMedia(false, '.build/web/extensions')),
));
Expand Down
53 changes: 51 additions & 2 deletions build/lib/extensions.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 52 additions & 1 deletion build/lib/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,14 @@ export function fromGithub({ name, version, repo, sha256, metadata }: IExtension
.pipe(packageJsonFilter.restore);
}

/**
* All extensions that are known to have some native component and thus must be built on the
* platform that is being built.
*/
const nativeExtensions = [
'microsoft-authentication',
];

const excludedExtensions = [
'vscode-api-tests',
'vscode-colorize-tests',
Expand Down Expand Up @@ -334,7 +342,49 @@ function isWebExtension(manifest: IExtensionManifest): boolean {
return true;
}

export function packageLocalExtensionsStream(forWeb: boolean, disableMangle: boolean): Stream {
/**
* Package local extensions that are known to not have native dependencies. Mutually exclusive to {@link packageNativeLocalExtensionsStream}.
* @param forWeb build the extensions that have web targets
* @param disableMangle disable the mangler
* @returns a stream
*/
export function packageNonNativeLocalExtensionsStream(forWeb: boolean, disableMangle: boolean): Stream {
return doPackageLocalExtensionsStream(forWeb, disableMangle, false);
}

/**
* Package local extensions that are known to have native dependencies. Mutually exclusive to {@link packageNonNativeLocalExtensionsStream}.
* @note it's possible that the extension does not have native dependencies for the current platform, especially if building for the web,
* but we simplify the logic here by having a flat list of extensions (See {@link nativeExtensions}) that are known to have native
* dependencies on some platform and thus should be packaged on the platform that they are building for.
* @param forWeb build the extensions that have web targets
* @param disableMangle disable the mangler
* @returns a stream
*/
export function packageNativeLocalExtensionsStream(forWeb: boolean, disableMangle: boolean): Stream {
return doPackageLocalExtensionsStream(forWeb, disableMangle, true);
}

/**
* Package all the local extensions... both those that are known to have native dependencies and those that are not.
* @param forWeb build the extensions that have web targets
* @param disableMangle disable the mangler
* @returns a stream
*/
export function packageAllLocalExtensionsStream(forWeb: boolean, disableMangle: boolean): Stream {
return es.merge([
packageNonNativeLocalExtensionsStream(forWeb, disableMangle),
packageNativeLocalExtensionsStream(forWeb, disableMangle)
]);
}

/**
* @param forWeb build the extensions that have web targets
* @param disableMangle disable the mangler
* @param native build the extensions that are marked as having native dependencies
*/
function doPackageLocalExtensionsStream(forWeb: boolean, disableMangle: boolean, native: boolean): Stream {
const nativeExtensionsSet = new Set(nativeExtensions);
const localExtensionsDescriptions = (
(<string[]>glob.sync('extensions/*/package.json'))
.map(manifestPath => {
Expand All @@ -343,6 +393,7 @@ export function packageLocalExtensionsStream(forWeb: boolean, disableMangle: boo
const extensionName = path.basename(extensionPath);
return { name: extensionName, path: extensionPath, manifestPath: absoluteManifestPath };
})
.filter(({ name }) => native ? nativeExtensionsSet.has(name) : !nativeExtensionsSet.has(name))
.filter(({ name }) => excludedExtensions.indexOf(name) === -1)
.filter(({ name }) => builtInExtensions.every(b => b.name !== name))
.filter(({ manifestPath }) => (forWeb ? isWebExtension(require(manifestPath)) : true))
Expand Down
1 change: 1 addition & 0 deletions extensions/microsoft-authentication/.vscodeignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ vsc-extension-quickstart.md
**/tslint.json
**/*.map
**/*.ts
packageMocks/
34 changes: 33 additions & 1 deletion extensions/microsoft-authentication/extension.webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,42 @@
'use strict';

const withDefaults = require('../shared.webpack.config');
const CopyWebpackPlugin = require('copy-webpack-plugin');
const path = require('path');
const { NormalModuleReplacementPlugin } = require('webpack');

const isWindows = process.platform === 'win32';

module.exports = withDefaults({
context: __dirname,
entry: {
extension: './src/extension.ts'
}
},
externals: {
// The @azure/msal-node-runtime package requires this native node module (.node).
// It is currently only included on Windows, but the package handles unsupported platforms
// gracefully.
'./msal-node-runtime': 'commonjs ./msal-node-runtime'
},
plugins: [
...withDefaults.nodePlugins(__dirname),
new CopyWebpackPlugin({
patterns: [
{
// The native files we need to ship with the extension
from: '**/dist/msal*.(node|dll)',
to: '[name][ext]',
// These will only be present on Windows for now
noErrorOnMissing: !isWindows
}
]
}),
// We don't use the feature that uses Dpapi, so we can just replace it with a mock.
// This is a bit of a hack, but it's the easiest way to do it. Really, msal should
// handle when this native node module is not available.
new NormalModuleReplacementPlugin(
Copy link
Member Author

Choose a reason for hiding this comment

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

This can go away when this is released: AzureAD/microsoft-authentication-library-for-js#7412

/\.\.\/Dpapi\.mjs/,
path.resolve(__dirname, 'packageMocks', 'dpapi', 'dpapi.js')
)
]
});
Loading
Loading