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

Implements input validation for the AplicationId #135

Merged
merged 3 commits into from
Apr 15, 2020
Merged
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
6 changes: 6 additions & 0 deletions packages/cli/package-lock.json

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

5 changes: 4 additions & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"scripts": {
"build": "tsc",
"lint": "eslint \"src/**/*.{js,ts}\"",
"test": ""
"test": "tsc && jasmine --config=jasmine.json"
},
"files": [
"dist",
Expand All @@ -21,6 +21,9 @@
},
"author": "",
"license": "Apache-2.0",
"devDependencies": {
"@types/jasmine": "^3.5.0"
},
"dependencies": {
"@bubblewrap/core": "^0.6.0",
"@bubblewrap/validator": "^0.6.0",
Expand Down
6 changes: 3 additions & 3 deletions packages/cli/src/lib/cmds/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {AndroidSdkTools, Config, GradleWrapper, JdkHelper, Log, TwaManifest}
from '@bubblewrap/core';
import * as inquirer from 'inquirer';
import * as path from 'path';
import {validatePassword} from '../inputHelpers';
import {validateKeyPassword} from '../inputHelpers';
import {PwaValidator, PwaValidationResult} from '@bubblewrap/validator';
import {printValidationResult} from '../pwaValidationHelper';
import {ParsedArgs} from 'minimist';
Expand Down Expand Up @@ -55,13 +55,13 @@ async function getPasswords(log: Log): Promise<SigningKeyPasswords> {
name: 'password',
type: 'password',
message: 'KeyStore password:',
validate: validatePassword,
validate: validateKeyPassword,
mask: '*',
}, {
name: 'keypassword',
type: 'password',
message: 'Key password:',
validate: validatePassword,
validate: validateKeyPassword,
mask: '*',
},
]);
Expand Down
41 changes: 25 additions & 16 deletions packages/cli/src/lib/cmds/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import * as fs from 'fs';
import Color = require('color');
import * as inquirer from 'inquirer';
import {Config, JdkHelper, KeyTool, Log, TwaGenerator, TwaManifest} from '@bubblewrap/core';
import {validateColor, validatePassword, validateUrl, notEmpty} from '../inputHelpers';
import {Config, JdkHelper, KeyTool, Log, TwaGenerator, TwaManifest, util} from '@bubblewrap/core';
import {validateColor, validateKeyPassword, validateUrl, notEmpty} from '../inputHelpers';
import {ParsedArgs} from 'minimist';
import {APP_NAME} from '../constants';

Expand All @@ -36,19 +36,19 @@ async function confirmTwaConfig(twaManifest: TwaManifest): Promise<TwaManifest>
type: 'input',
message: 'Domain being opened in the TWA:',
default: twaManifest.host,
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'host'),
}, {
name: 'name',
type: 'input',
message: 'Name of the application:',
default: twaManifest.name,
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'name'),
}, {
name: 'launcherName',
type: 'input',
message: 'Name to be shown on the Android Launcher:',
default: twaManifest.launcherName,
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'Launcher name'),
}, {
name: 'themeColor',
type: 'input',
Expand All @@ -66,7 +66,7 @@ async function confirmTwaConfig(twaManifest: TwaManifest): Promise<TwaManifest>
type: 'input',
message: 'Relative path to open the TWA:',
default: twaManifest.startUrl,
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'URL'),
}, {
name: 'iconUrl',
type: 'input',
Expand All @@ -80,7 +80,7 @@ async function confirmTwaConfig(twaManifest: TwaManifest): Promise<TwaManifest>
'maskable icons',
default: twaManifest.maskableIconUrl,
filter: (input): string | undefined => input.length === 0 ? undefined : input,
validate: (input): boolean => input === undefined || validateUrl(input),
validate: async (input): Promise<boolean> => input === undefined || await validateUrl(input),
}, {
name: 'shortcuts',
type: 'confirm',
Expand All @@ -91,19 +91,26 @@ async function confirmTwaConfig(twaManifest: TwaManifest): Promise<TwaManifest>
type: 'input',
message: 'Android Package Name (or Application ID):',
default: twaManifest.packageId,
validate: notEmpty,
validate: async (input): Promise<boolean> => {
if (!util.validatePackageId(input)) {
throw new Error('Invalid Application Id. Check requiements at ' +
'https://developer.android.com/studio/build/application-id');
}
return true;
},
}, {
name: 'keyPath',
type: 'input',
message: 'Location of the Signing Key:',
default: twaManifest.signingKey.path,
validate: notEmpty,
validate: async (input): Promise<boolean> =>
notEmpty(input, 'KeyStore location'),
}, {
name: 'keyAlias',
type: 'input',
message: 'Key name:',
default: twaManifest.signingKey.alias,
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'Key alias'),
},
]);

Expand Down Expand Up @@ -154,32 +161,32 @@ async function createSigningKey(twaManifest: TwaManifest, config: Config): Promi
name: 'fullName',
type: 'input',
message: 'First and Last names (eg: John Doe):',
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'First and Last names'),
}, {
name: 'organizationalUnit',
type: 'input',
message: 'Organizational Unit (eg: Engineering Dept):',
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'Organizational Unit'),
}, {
name: 'organization',
type: 'input',
message: 'Organization (eg: Company Name):',
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'Organization'),
}, {
name: 'country',
type: 'input',
message: 'Country (2 letter code):',
validate: notEmpty,
validate: async (input): Promise<boolean> => notEmpty(input, 'Country'),
}, {
name: 'password',
type: 'password',
message: 'Password for the Key Store:',
validate: validatePassword,
validate: validateKeyPassword,
}, {
name: 'keypassword',
type: 'password',
message: 'Password for the Key:',
validate: validatePassword,
validate: validateKeyPassword,
},
]);

Expand All @@ -204,5 +211,7 @@ export async function init(args: ParsedArgs, config: Config): Promise<boolean> {
await twaManifest.saveToFile('./twa-manifest.json');
await twaGenerator.createTwaProject(targetDirectory, twaManifest);
await createSigningKey(twaManifest, config);
log.info('');
log.info('Project generated successfully. Build it by running "@bubblewrap/cli build"');
return true;
}
3 changes: 2 additions & 1 deletion packages/cli/src/lib/cmds/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ async function updateVersions(twaManifest: TwaManifest, appVersionNameArg: strin
type: 'input',
message: 'versionName for the new App version:',
default: twaManifest.appVersionName,
validate: notEmpty,
validate: async (input): Promise<boolean> =>
notEmpty(input, 'versionName'),
}]);

return {
Expand Down
27 changes: 19 additions & 8 deletions packages/cli/src/lib/inputHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,34 @@
import Color = require('color');
import {isWebUri} from 'valid-url';

export function validatePassword(input: string): boolean {
return input.length > 0;
const MIN_KEY_PASSWORD_LENGTH = 6;

export async function validateKeyPassword(input: string): Promise<boolean> {
if (input.trim().length < MIN_KEY_PASSWORD_LENGTH) {
throw new Error(`Password must be at least ${MIN_KEY_PASSWORD_LENGTH} characters long`);
}
return true;
}

export function notEmpty(input: string): boolean {
return input.trim().length > 0;
export async function notEmpty(input: string, fieldName: string): Promise<boolean> {
if (input.trim().length > 0) {
return true;
}
throw new Error(`${fieldName} cannot be empty`);
}

export function validateColor(color: string): boolean {
export async function validateColor(color: string): Promise<boolean> {
try {
new Color(color);
return true;
} catch (_) {
return false;
throw new Error(`Invalid Color ${color}. Try using hexadecimal representation. eg: #FF3300`);
}
};

export function validateUrl(url: string): boolean {
return isWebUri(url) !== undefined;
export async function validateUrl(url: string): Promise<boolean> {
if (isWebUri(url) === undefined) {
throw new Error(`${url} is not an URL`);
}
return true;
}
64 changes: 64 additions & 0 deletions packages/cli/src/spec/inputHelpersSpec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2019 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as inputHelpers from '../lib/inputHelpers';

describe('inputHelpers', () => {
describe('#notEmpty', () => {
it('throws Error for empty strings', async () => {
await expectAsync(inputHelpers.notEmpty('', 'Error')).toBeRejectedWithError();
await expectAsync(inputHelpers.notEmpty(' ', 'Error')).toBeRejectedWithError();
});

it('returns true for non-empty input', async () => {
expect(await inputHelpers.notEmpty('a', 'Error')).toBeTrue();
});
});

describe('#validateKeyPassword', () => {
it('throws Error for empty string', async () => {
await expectAsync(inputHelpers.validateKeyPassword('')).toBeRejectedWithError();
await expectAsync(inputHelpers.validateKeyPassword(' ')).toBeRejectedWithError();
});

it('throws Error input with less than 6 characters', async () => {
await expectAsync(inputHelpers.validateKeyPassword('a')).toBeRejectedWithError();
await expectAsync(inputHelpers.validateKeyPassword('abc')).toBeRejectedWithError();
await expectAsync(inputHelpers.validateKeyPassword('abcde')).toBeRejectedWithError();
await expectAsync(inputHelpers.validateKeyPassword('abcde ')).toBeRejectedWithError();
});

it('returns true for valid input', async () => {
expect(await inputHelpers.validateKeyPassword('abcdef')).toBeTrue();
expect(await inputHelpers.validateKeyPassword('abcdef ')).toBeTrue();
});
});

describe('#validateColor', () => {
it('returns true for valid colors', async () => {
expect(await inputHelpers.validateColor('#FF0033'));
expect(await inputHelpers.validateColor('blue'));
expect(await inputHelpers.validateColor('rgb(255, 0, 30)'));
});

it('throws Error for invalid colors', async () => {
await expectAsync(inputHelpers.validateColor('')).toBeRejectedWithError();
await expectAsync(inputHelpers.validateColor('abc')).toBeRejectedWithError();
await expectAsync(
inputHelpers.validateColor('rgb(23, 0 30')).toBeRejectedWithError();
});
});
});
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {JdkHelper} from './lib/jdk/JdkHelper';
import {KeyTool} from './lib/jdk/KeyTool';
import {TwaManifest} from './lib/TwaManifest';
import {TwaGenerator} from './lib/TwaGenerator';
import * as util from './lib/util';

export {AndroidSdkTools,
Config,
Expand All @@ -31,4 +32,5 @@ export {AndroidSdkTools,
Log,
TwaGenerator,
TwaManifest,
util,
};
31 changes: 31 additions & 0 deletions packages/core/src/lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const extractZipPromise = promisify(extractZip);
// Regex for disallowed characters on Android Packages, as per
// https://developer.android.com/guide/topics/manifest/manifest-element.html#package
const DISALLOWED_ANDROID_PACKAGE_CHARS_REGEX = /[^a-zA-Z0-9_\.]/g;
const VALID_PACKAGE_ID_SEGMENT_REGEX = /^[a-zA-Z][A-Za-z0-9_]*$/;

export async function execute(cmd: string[], env: NodeJS.ProcessEnv): Promise<void> {
await execPromise(cmd.join(' '), {env: env});
Expand Down Expand Up @@ -134,3 +135,33 @@ export function generatePackageId(host: string): string {
parts.push('twa');
return parts.join('.').replace(DISALLOWED_ANDROID_PACKAGE_CHARS_REGEX, '_');
}

/**
* Validates a Package Id, according to the documentation at:
* https://developer.android.com/studio/build/application-id
*
* Rules summary for the Package Id:
* - It must have at least two segments (one or more dots).
* - Each segment must start with a leter.
* - All characters must be alphanumeric or an underscore [a-zA-Z0-9_].
*
* @param {string} input the package name to be validated
*/
export function validatePackageId(input: string): boolean {
if (input.length <= 0) {
return false;
}

const parts = input.split('.');
if (parts.length < 2) {
return false;
}

for (const part of parts) {
if (part.match(VALID_PACKAGE_ID_SEGMENT_REGEX) === null) {
return false;
}
}

return true;
}
32 changes: 32 additions & 0 deletions packages/core/src/spec/lib/utilSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,36 @@ describe('util', () => {
expect(result).toBe('com.appspot.pwa_directory_test.twa');
});
});

describe('#validatePackageId', () => {
it('returns true for valid packages', () => {
expect(util.validatePackageId('com.pwa_directory.appspot.com')).toBeTrue();
expect(util.validatePackageId('com.pwa1directory.appspot.com')).toBeTrue();
});

it('returns false for packages with invalid characters', () => {
expect(util.validatePackageId('com.pwa-directory.appspot.com')).toBeFalse();
expect(util.validatePackageId('com.pwa@directory.appspot.com')).toBeFalse();
expect(util.validatePackageId('com.pwa*directory.appspot.com')).toBeFalse();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's worth adding an example with some weird unicode character (maybe a unicode character that looks like a full stop but isn't).

Copy link
Member Author

Choose a reason for hiding this comment

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

done

expect(util.validatePackageId('com․pwa-directory.appspot.com')).toBeFalse();
});

it('returns false for packages empty sections', () => {
expect(util.validatePackageId('com.example.')).toBeFalse();
expect(util.validatePackageId('.com.example')).toBeFalse();
expect(util.validatePackageId('com..example')).toBeFalse();
});

it('packages with less than 2 sections return false', () => {
expect(util.validatePackageId('com')).toBeFalse();
expect(util.validatePackageId('')).toBeFalse();
});

it('packages starting with non-letters return false', () => {
expect(util.validatePackageId('com.1char.twa')).toBeFalse();
expect(util.validatePackageId('1com.char.twa')).toBeFalse();
expect(util.validatePackageId('com.char.1twa')).toBeFalse();
Copy link
Collaborator

Choose a reason for hiding this comment

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

One that starts with an underscore?

Copy link
Member Author

Choose a reason for hiding this comment

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

\u2024 () should do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

expect(util.validatePackageId('_com.char.1twa')).toBeFalse();
});
});
});