Skip to content

Commit

Permalink
Improved how appIds associated with views/windows are validated (#708)
Browse files Browse the repository at this point in the history
  • Loading branch information
johnman authored Apr 19, 2024
1 parent c9e78ce commit d00722d
Show file tree
Hide file tree
Showing 4 changed files with 295 additions and 48 deletions.
1 change: 1 addition & 0 deletions how-to/workspace-platform-starter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## v18.0.0

- More efficient validation of appIds associated with views/windows for faster interop based actions. The appId validation in our wps module (modules directory) has extracted into its own file and tests to ensure it continues to behave as expected across future updates have been added to the test directory.
- Module Helpers now provide a getAnalyticsClient (it is marked as optional and the result could be undefined so it leaves the option for it to be denied to a module or removed). This client supports a ModuleAnalytic event which will have a source of Module assigned to it (you can still specify type and use the data property to provide additional module specific information). This data will be passed to the analyticProviders that receive the Workspace Analytic events. See [How to Configure Analytics](./docs/how-to-configure-analytics.md).
- Added a cloud interop override module so that you can easily test out OpenFin's cloud interop offering. See [How To Add Cloud Interop To Your Interop Broker](./docs/how-to-add-cloud-interop-to-your-interop-broker.md).
- Broke up the build process to make it easier to just build your modules. `npm run build` still builds everything and `npm run build-client` still builds all client related code but now if you change framework files you can use `npm run build-framework`, or if you modify our starter modules you can use `npm run build-starter-modules` or if you just want to build your modules (that are listed in webpack.config.js) then you can use `npm run build-client-modules`. This will let you have a much faster build.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import type OpenFin from "@openfin/core";
import type { PlatformApp } from "../../../../framework/shapes/app-shapes";
import type { Logger } from "../../../../framework/shapes/logger-shapes";
/**
* The AppIdHelper class provides helpful functions related to app ids.
*/
export class AppIdHelper {
private readonly _validatedAppIds: string[] = [];

private readonly _invalidAppIds: string[] = [];

private readonly _unregisteredApp: PlatformApp | undefined;

private readonly _logger: Logger;

private readonly _getApp: (appId: string) => Promise<PlatformApp | undefined>;

private readonly _platformId: string;

/**
* Provides helpful functions related to app ids.
* @param getApp The function to use to get an app for validation.
* @param platformId The platform id that represents the current platform.
* @param logger The logger to use
* @param unregisteredApp The app to use as a placeholder for unregistered apps.
*/
constructor(
getApp: (appId: string) => Promise<PlatformApp | undefined>,
platformId: string,
logger: Logger,
unregisteredApp?: PlatformApp
) {
this._unregisteredApp = unregisteredApp;
this._logger = logger;
this._getApp = getApp;
this._platformId = platformId;
}

/**
* Lookup an application identity.
* @param clientIdentity The client identity to use.
* @returns The application identity.
*/
public async lookupAppId(clientIdentity: OpenFin.ClientIdentity): Promise<string | undefined> {
if (clientIdentity.name.startsWith("internal-generated-")) {
if (clientIdentity.uuid === this._platformId) {
if (this._unregisteredApp) {
this._logger.debug(
`A window or view that is not an app but runs within the platform is running and a placeholder app has been specified ${this._unregisteredApp?.appId}}.`,
clientIdentity
);
return this._unregisteredApp.appId;
}
this._logger.debug(
"A window or view that is not an app but runs within the platform is running and no unregistered placeholder app is specified so no appId will be returned.",
clientIdentity
);
return;
}
this._logger.debug(
"A window or view that follows the runtime generated naming convention is running from another platform. It will not be assigned an appId.",
clientIdentity
);
return;
}
const nameParts = clientIdentity.name.split("/");
let appId: string | undefined;
if (nameParts.length === 1 || nameParts.length === 2) {
appId = nameParts[0];
} else {
appId = `${nameParts[0]}/${nameParts[1]}`;
}

if (this._validatedAppIds.includes(appId)) {
return appId;
}
if (this._invalidAppIds.includes(appId)) {
return;
}

// perform a lookup to validate the appId
const app = await this._getApp(appId);

if (app) {
this._validatedAppIds.push(appId);
return appId;
}
this._invalidAppIds.push(appId);
this._logger.warn(
`AppId ${appId} does not exist in the directory and it isn't a generated view/window that falls under this platform. No app id will be returned as it is unconfirmed.`
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
sanitizeString
} from "workspace-platform-starter/utils";
import { getWindowPositionUsingStrategy } from "workspace-platform-starter/utils-position";
import { AppIdHelper } from "./app-id-helper";
import { AppIntentHelper } from "./app-intent-helper";
import { ClientRegistrationHelper } from "./client-registration-helper";
import { IntentResolverHelper } from "./intent-resolver-helper";
Expand Down Expand Up @@ -70,8 +71,8 @@ export async function getConstructorOverride(
if (helpers?.getEndpointClient) {
endpointClient = await helpers?.getEndpointClient();
}

const launch = helpers.launchApp;

return (Base: OpenFin.Constructor<OpenFin.InteropBroker>) =>
/**
* Extend the InteropBroker to handle intents.
Expand All @@ -89,17 +90,15 @@ export async function getConstructorOverride(

private readonly _metadataKey: Readonly<string>;

private readonly _appIdHelper: AppIdHelper;

/**
* Create a new instance of InteropBroker.
*/
constructor() {
super();
logger.info("Interop Broker Constructor applying settings.");
this._appIntentHelper = new AppIntentHelper(getApps, logger);
this._clientRegistrationHelper = new ClientRegistrationHelper(
async (clientIdentity: OpenFin.ClientIdentity) => this.lookupAppId(clientIdentity),
logger
);
this._metadataKey = `_metadata_${randomUUID()}`;
if (options.intentResolver) {
this._intentResolverHelper = new IntentResolverHelper(
Expand All @@ -114,6 +113,11 @@ export async function getConstructorOverride(
if (!isEmpty(this._unregisteredApp)) {
this._unregisteredApp.manifestType = MANIFEST_TYPES.UnregisteredApp.id;
}
this._appIdHelper = new AppIdHelper(getApp, fin.me.identity.uuid, logger, this._unregisteredApp);
this._clientRegistrationHelper = new ClientRegistrationHelper(
async (clientIdentity: OpenFin.ClientIdentity) => this._appIdHelper.lookupAppId(clientIdentity),
logger
);
}

/**
Expand Down Expand Up @@ -845,7 +849,7 @@ export async function getConstructorOverride(
payload,
clientIdentity
)) as ImplementationMetadata;
const appId = await this.lookupAppId(clientIdentity);
const appId = await this._appIdHelper.lookupAppId(clientIdentity);
if (!isEmpty(appId)) {
const updatedResponse = {
...response,
Expand Down Expand Up @@ -1356,47 +1360,6 @@ export async function getConstructorOverride(
return apiMetadata.type === "fdc3" && apiMetadata.version === "2.0";
}

/**
* Lookup an application identity.
* @param clientIdentity The client identity to use.
* @returns The application identity.
*/
private async lookupAppId(clientIdentity: OpenFin.ClientIdentity): Promise<string | undefined> {
const nameParts = clientIdentity.name.split("/");
let app: PlatformApp | undefined;

if (nameParts.length === 1 || nameParts.length === 2) {
app = await getApp(nameParts[0]);
}
if (nameParts.length > 2) {
app = await getApp(`${nameParts[0]}/${nameParts[1]}`);
}

const appNotFound = isEmpty(app);

if (appNotFound && clientIdentity.uuid !== fin.me.identity.uuid) {
logger.warn(
"Lookup made by a non-registered app that is outside of this platform.",
clientIdentity
);
return;
}

if (appNotFound && isEmpty(this._unregisteredApp)) {
logger.warn(
"Lookup made by a non-registered app that falls under this platform. No unregistered placeholder app is specified.",
clientIdentity
);
return;
}

if (appNotFound) {
app = this._unregisteredApp;
logger.info("Assigned the following unregistered app to represent the app.", app);
}
return app?.appId;
}

/**
* Process a context.
* @param context The context to process.
Expand Down Expand Up @@ -1428,7 +1391,7 @@ export async function getConstructorOverride(
* @returns The context metadata.
*/
private async getContextMetadata(clientIdentity: OpenFin.ClientIdentity): Promise<ContextMetadata> {
const appId = (await this.lookupAppId(clientIdentity)) ?? clientIdentity.name;
const appId = (await this._appIdHelper.lookupAppId(clientIdentity)) ?? "unknown";
return {
source: {
appId,
Expand Down
Loading

0 comments on commit d00722d

Please sign in to comment.