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

fix: allow the extensions to authenticate #339

Merged
merged 29 commits into from Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion .rebase/CHANGELOG.md
Expand Up @@ -2,6 +2,12 @@

The file to keep a list of changed files which will potentionaly help to resolve rebase conflicts.

#### @vitaliy-guliy
https://github.com/che-incubator/che-code/pull/339

- code/src/vs/platform/product/common/product.ts
---

#### @vitaliy-guliy
https://github.com/che-incubator/che-code/pull/331

Expand All @@ -27,7 +33,6 @@ https://github.com/che-incubator/che-code/pull/337/commits/875893566c2acd0bb7031
- code/src/vs/base/common/network.ts
---


#### @benoitf
https://github.com/che-incubator/che-code/commit/eed0a5213ba1b29b810d53f6365aaa2294165845#diff-2735bf66f14ee64b9ce6fdc30355a5e3085ae96a791cd01d65843a8dcef7c166

Expand Down
@@ -0,0 +1,10 @@
[
{
"from": "import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';",
"by": "import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';\\\nimport { loadFromFileSystem } from 'vs/platform/product/common/che/product';"
},
{
"from": "product = { /\\*BUILD->INSERT_PRODUCT_CONFIGURATION\\*/ } as IProductConfiguration;",
"by": "product = { /\\*BUILD->INSERT_PRODUCT_CONFIGURATION\\*/ } as IProductConfiguration;\\\n\\\tproduct = loadFromFileSystem();"
}
]
6 changes: 3 additions & 3 deletions code/extensions/che-api/src/impl/github-service-impl.ts
Expand Up @@ -42,7 +42,7 @@ export class GithubServiceImpl implements GithubService {
@inject(K8SServiceImpl) private readonly k8sService: K8SServiceImpl,
@inject(Symbol.for('AxiosInstance')) private readonly axiosInstance: AxiosInstance
) {
this.iniitializeToken();
this.initializeToken();
}

private checkToken(): void {
Expand Down Expand Up @@ -117,10 +117,10 @@ export class GithubServiceImpl implements GithubService {
}

// another token should be used by the Github Service after removing the Device Authentication token
this.iniitializeToken();
this.initializeToken();
}

private async iniitializeToken(): Promise<void> {
private async initializeToken(): Promise<void> {
this.logger.info('Github Service: extracting token...');

const deviceAuthToken = await this.getDeviceAuthToken();
Expand Down
32 changes: 32 additions & 0 deletions code/src/vs/platform/product/common/che/product.ts
@@ -0,0 +1,32 @@
/**********************************************************************
* Copyright (c) 2024 Red Hat, Inc.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
***********************************************************************/
/* eslint-disable header/header */

import { IProductConfiguration } from 'vs/base/common/product';

export function loadFromFileSystem(): IProductConfiguration {
const href = `./oss-dev/static/product.json`;

try {
var xmlhttp = new XMLHttpRequest();
xmlhttp.open("GET", href, false);
xmlhttp.send();

if (xmlhttp.status == 200 && xmlhttp.readyState == 4) {
return JSON.parse(xmlhttp.responseText);
}

console.log(`Request failed with status: ${xmlhttp.status}, readyState: ${xmlhttp.readyState}`);
} catch (err) {
console.error(err);
}

throw new Error(`Unable to load product.json from ${href}.`);
}
2 changes: 2 additions & 0 deletions code/src/vs/platform/product/common/product.ts
Expand Up @@ -6,6 +6,7 @@
import { env } from 'vs/base/common/process';
import { IProductConfiguration } from 'vs/base/common/product';
import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';
import { loadFromFileSystem } from 'vs/platform/product/common/che/product';

/**
* @deprecated You MUST use `IProductService` if possible.
Expand Down Expand Up @@ -54,6 +55,7 @@ else {

// Built time configuration (do NOT modify)
product = { /*BUILD->INSERT_PRODUCT_CONFIGURATION*/ } as IProductConfiguration;
product = loadFromFileSystem();

// Running out of sources
if (Object.keys(product).length === 0) {
Expand Down
2 changes: 2 additions & 0 deletions launcher/src/main.ts
Expand Up @@ -12,6 +12,7 @@ import { CodeWorkspace } from './code-workspace';
import { DevWorkspaceId } from './devworkspace-id';
import { NodeExtraCertificate } from './node-extra-certificate';
import { OpenVSIXRegistry } from './openvsix-registry';
import { TrustedExtensions } from './trusted-extensions';
import { VSCodeLauncher } from './vscode-launcher';
import { WebviewResources } from './webview-resources';

Expand All @@ -27,6 +28,7 @@ export class Main {
await new OpenVSIXRegistry().configure();
await new WebviewResources().configure();
await new NodeExtraCertificate().configure();
await new TrustedExtensions().configure();

const workspaceFile = await new CodeWorkspace().generate();

Expand Down
12 changes: 12 additions & 0 deletions launcher/src/product-json.ts
Expand Up @@ -12,6 +12,10 @@ import * as fs from './fs-extra';

const PRODUCT_JSON = 'product.json';

export interface Record {
[key: string]: string[];
}

export class ProductJSON {
private json: any;

Expand Down Expand Up @@ -89,4 +93,12 @@ export class ProductJSON {

gallery.itemUrl = url;
}

getTrustedExtensionAuthAccess(): string[] | Record | undefined {
return this.json.trustedExtensionAuthAccess;
}

setTrustedExtensionAuthAccess(trustedExtensionAuthAccess: string[] | Record | undefined) {
this.json.trustedExtensionAuthAccess = trustedExtensionAuthAccess;
}
}
64 changes: 64 additions & 0 deletions launcher/src/trusted-extensions.ts
@@ -0,0 +1,64 @@
/**********************************************************************
* Copyright (c) 2024 Red Hat, Inc.
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
***********************************************************************/

import { env } from 'process';
import { ProductJSON } from './product-json';

export class TrustedExtensions {
async configure(): Promise<void> {
console.log('# Configuring Trusted Extensions...');

if (!env.VSCODE_TRUSTED_EXTENSIONS) {
console.log(' > env.VSCODE_TRUSTED_EXTENSIONS is not defined, skip this step');
return;
}

try {
const extensions: string[] = [];

for (const extension of env.VSCODE_TRUSTED_EXTENSIONS.split(',')) {
if (extension) {
extensions.push(extension);
console.log(` > add ${extension}`);
}
}

if (!extensions.length) {
console.log(' > env.VSCODE_TRUSTED_EXTENSIONS does not specify any extension');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to add the value of the variable to the log
Let's say another separator is applied by a user, then:

  • I agree that extensions array is empty
  • but env.VSCODE_TRUSTED_EXTENSIONS does not specify any extension is NOT true
  • so it makes sense to clarify it in logs, this way it would help to identify and resolve the problem quickly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

User specified env.VSCODE_TRUSTED_EXTENSIONS in his devfile.
Launcher says that env.VSCODE_TRUSTED_EXTENSIONS is wrong.
No need here to duplicate the value.

From the other side, if you have an idea, please share it.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, Launcher says that env.VSCODE_TRUSTED_EXTENSIONS is wrong.
but the launcher says nothing about:

  • what value of the variable the launcher has got
  • what could be wrong
  • what is expected

You use here , as a separator for the VSCODE_TRUSTED_EXTENSIONS env var,
but, for example, PATH env variable uses : as a separator

image

If a user uses : as a separator (just out of habit) for the VSCODE_TRUSTED_EXTENSIONS then:

  • VSCODE_TRUSTED_EXTENSIONS is defined, it's not empty, it contains extensions
  • but it doesn't work and there is nothing helpful in logs - isn't it?

My propose was just clarify it in logs - like - if the logic can not handle the variable - then it should be clear:

  • env variable VSCODE_TRUSTED_EXTENSIONS has some value here =>
  • using , as a separator to parse extensions =>
  • can not parse extensions, so the value of VSCODE_TRUSTED_EXTENSIONS will be ignored =>
  • the expected format of the VSCODE_TRUSTED_EXTENSIONS is: some-value,another-value

============
anyway I've approved your pull request yesterday, these changes are minor, it's up to you
so feel free to merge the pull request as soon as you believe it's good enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have just played with it and would say, that the cheapest way in case of errors is to print the current value of VSCODE_TRUSTED_EXTENSIONS environment variable and explain that it should contain one or several extension identifiers divided by comma.

It's for the first step.
The next, I think we should document the functionality provided by launcher. Among them, ability to configure trusted extensions, location of Open VSX registry, webviews, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output with one failed extension

# Configuring Trusted Extensions...
  > env.VSCODE_TRUSTED_EXTENSIONS is set to [redhat.yaml,redhat.openshift,red hat.java]
  > add redhat.yaml
  > add redhat.openshift
  > failure to add [red hat.java] because of wrong identifier

The output when the variable does not specify anything

# Configuring Trusted Extensions...
  > env.VSCODE_TRUSTED_EXTENSIONS is set to [,,,]
  > ERROR: The variable provided most likely has wrong format. It should specify one or more extensions separated by comma.

return;
}

const productJSON = await new ProductJSON().load();
let productJSONChanged = false;

let access = productJSON.getTrustedExtensionAuthAccess();
if (access === undefined) {
productJSON.setTrustedExtensionAuthAccess([...extensions]);
productJSONChanged = true;
} else if (Array.isArray(access)) {
for (const extension of extensions) {
if (!access.includes(extension)) {
access.push(extension);
productJSONChanged = true;
}
}
} else {
console.log(' > Unexpected type of trustedExtensionAuthAccess in product.json. Skip this step');
return;
}

if (productJSONChanged) {
await productJSON.save();
}
} catch (err) {
console.error(`${err.message} Failure to configure trusted extensions in product.json.`);
}
}
}
8 changes: 8 additions & 0 deletions launcher/tests/main.spec.ts
Expand Up @@ -38,6 +38,13 @@ jest.mock('../src/node-extra-certificate', () => ({
},
}));

const configureTustedExtensions = jest.fn();
jest.mock('../src/trusted-extensions', () => ({
TrustedExtensions: function () {
return { configure: configureTustedExtensions };
},
}));

const generateCodeWorkspace = jest.fn();
jest.mock('../src/code-workspace', () => ({
CodeWorkspace: function () {
Expand All @@ -60,6 +67,7 @@ describe('Test main flow:', () => {
expect(configureOpenVSIXRegistryMock).toBeCalled();
expect(configureWebviewResourcesMock).toBeCalled();
expect(configureNodeExtraCertificate).toBeCalled();
expect(configureTustedExtensions).toBeCalled();

expect(generateCodeWorkspace).toBeCalled();

Expand Down