Commit e45e9280 authored by Jason Lin's avatar Jason Lin Committed by Commit Bot

crostini: impose restrictions on container username in installer

This CL imposes restrictions on the username in crostini installer to
ensure the username conforms with linux convention and does not conflict
with other default users. If the username is invalid, an error message
will be shown and the install button will be disabled.

The CL also adds new browser tests for this feature. Since the flag
crostini-username is turned on for all installer browser tests now,
existing browser tests are modified to click the "next" button to go
into the configuration page before clicking the "install" button.

Bug: 1016195
Test: browsertest; manual test with flag crostini-username flag on
Change-Id: I546e0c56a3a41defb2de3e224992cebf5824dabc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032485
Commit-Queue: Jason Lin <lxj@google.com>
Reviewed-by: default avatarJoel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737686}
parent 45c2ff13
......@@ -3620,6 +3620,15 @@
<message name="IDS_CROSTINI_INSTALLER_USERNAME_MESSAGE" desc="Text shown in the Crostini installer dialog prompting the user to pick their username">
Select a username
</message>
<message name="IDS_CROSTINI_INSTALLER_USERNAME_INVALID_FIRST_CHARACTER_ERROR" desc="Text shown in the Crostini installer dialog when the user picks a username starting with an invalid character">
Must start with a lowercase character or underscore
</message>
<message name="IDS_CROSTINI_INSTALLER_USERNAME_INVALID_CHARACTERS_ERROR" desc="Text shown in the Crostini installer dialog when the user picks a username containing an invalid character">
Lowercase characters, digits, underscores, or dashes only
</message>
<message name="IDS_CROSTINI_INSTALLER_USERNAME_NOT_AVAILABLE_ERROR" desc="Text shown in the Crostini installer dialog when the user picks a username that is not available">
Username "<ph name="USERNAME">$1<ex>root</ex></ph>" is not available
</message>
<message name="IDS_CROSTINI_UNINSTALLER_TITLE" desc="Title of the Crostini uninstaller, a dialog for uninstalling Linux, the associated VM and Linux files.">
Delete Linux (Beta)
</message>
......
......@@ -77,16 +77,18 @@
<!-- Message containers. Depending on the current state, only one of them
is visible. -->
<div id="prompt-message" hidden="[[!isState_(state_, State.PROMPT)]]">
<div id="prompt-message" hidden="[[!eq_(state_, State.PROMPT)]]">
<span>$i18n{promptMessage} </span>
<a href="$i18n{learnMoreUrl}" target="_blank">$i18n{learnMore}</a>
</div>
<div id="configure-message" hidden="[[!isState_(state_, State.CONFIGURE)]]">
<div id="configure-message" hidden="[[!eq_(state_, State.CONFIGURE)]]">
<div>$i18n{configureMessage}</div>
<div hidden="[[!showUsernameSelection_()]]">
<div>$i18n{usernameMessage}</div>
<cr-input label="Username" id="username" type="text"
value="{{username_}}">
value="{{username_}}" maxlength="[[MAX_USERNAME_LENGTH]]"
invalid="[[!eq_(usernameError_, '')]]"
error-message="[[usernameError_]]">
</cr-input>
</div>
<div hidden="[[!showDiskResizing_()]]">
......@@ -105,15 +107,15 @@
</div>
</div>
<div id="installing-message"
hidden="[[!isState_(state_, State.INSTALLING)]]">
hidden="[[!eq_(state_, State.INSTALLING)]]">
<div>[[getProgressMessage_(installerState_)]]</div>
<paper-progress class="progress-bar" value="[[installerProgress_]]">
</paper-progress>
</div>
<div id="error-message" hidden="[[!isState_(state_, State.ERROR)]]">
<div id="error-message" hidden="[[!eq_(state_, State.ERROR)]]">
<div>[[getErrorMessage_(error_)]]</div>
</div>
<div id="canceling-message" hidden="[[!isState_(state_, State.CANCELING)]]">
<div id="canceling-message" hidden="[[!eq_(state_, State.CANCELING)]]">
<div>$i18n{cancelingMessage}</div>
<paper-progress class="progress-bar" indeterminate></paper-progress>
</div>
......@@ -121,7 +123,7 @@
<img id="img-linux-illustration" src="images/linux_illustration.png" alt="">
<div id="button-container">
<cr-button class="cancel-button" on-click="onCancelButtonClick_"
disabled="[[isState_(state_, State.CANCELING)]]">
disabled="[[eq_(state_, State.CANCELING)]]">
$i18n{cancel}
</cr-button>
<cr-button id="next" class="action-button" on-click="onNextButtonClick_"
......@@ -131,7 +133,8 @@
</cr-button>
<cr-button id="install" class="action-button"
on-click="onInstallButtonClick_" aria-describedby="title"
aria-details="prompt-message" hidden="[[!canInstall_(state_)]]">
aria-details="prompt-message" hidden="[[!showInstallButton_(state_)]]"
disabled="[[disableInstallButton_(state_, usernameError_)]]">
[[getInstallButtonLabel_(state_)]]
</cr-button>
</div>
......
......@@ -28,9 +28,43 @@ const State = {
CANCELING: 'canceling',
};
const MAX_USERNAME_LENGTH = 32;
const InstallerState = crostini.mojom.InstallerState;
const InstallerError = crostini.mojom.InstallerError;
const UNAVAILABLE_USERNAMES = [
'root',
'daemon',
'bin',
'sys',
'sync',
'games',
'man',
'lp',
'mail',
'news',
'uucp',
'proxy',
'www-data',
'backup',
'list',
'irc',
'gnats',
'nobody',
'_apt',
'systemd-timesync',
'systemd-network',
'systemd-resolve',
'systemd-bus-proxy',
'messagebus',
'sshd',
'rtkit',
'pulse',
'android-root',
'chronos-access',
'android-everybody'
];
Polymer({
is: 'crostini-installer-app',
......@@ -98,9 +132,17 @@ Polymer({
username_: {
type: String,
notify: true,
value: loadTimeData.getString('defaultContainerUsername'),
value: loadTimeData.getString('defaultContainerUsername')
.substring(0, MAX_USERNAME_LENGTH),
observer: 'onUsernameChanged_',
},
usernameError_: {
type: String,
},
/* Enable the html template to access the length */
MAX_USERNAME_LENGTH: {type: Number, value: MAX_USERNAME_LENGTH},
},
/** @override */
......@@ -166,7 +208,7 @@ Polymer({
/** @private */
onInstallButtonClick_() {
assert(this.canInstall_(this.state_));
assert(this.showInstallButton_(this.state_));
var diskSize = 0;
if (loadTimeData.getBoolean('diskResizingEnabled')) {
diskSize = this.diskSizeTicks_[this.$.diskSlider.value].value;
......@@ -234,13 +276,13 @@ Polymer({
},
/**
* @param {State} state1
* @param {State} state2
* @param {*} value1
* @param {*} value2
* @returns {boolean}
* @private
*/
isState_(state1, state2) {
return state1 === state2;
eq_(value1, value2) {
return value1 === value2;
},
/**
......@@ -248,7 +290,7 @@ Polymer({
* @returns {boolean}
* @private
*/
canInstall_(state) {
showInstallButton_(state) {
if (this.configurePageAccessible_()) {
return state === State.CONFIGURE || state === State.ERROR;
} else {
......@@ -256,6 +298,16 @@ Polymer({
}
},
/**
* @param {State} state
* @param {string} usernameError
* @returns {boolean}
* @private
*/
disableInstallButton_(state, usernameError) {
return state === State.CONFIGURE && usernameError !== '';
},
/**
* @param {State} state
* @returns {boolean}
......@@ -417,4 +469,20 @@ Polymer({
showUsernameSelection_() {
return loadTimeData.getBoolean('crostiniCustomUsername');
},
/** @private */
onUsernameChanged_(username, oldUsername) {
if (UNAVAILABLE_USERNAMES.includes(username)) {
this.usernameError_ =
loadTimeData.getStringF('usernameNotAvailableError', username);
} else if (!/^[a-z_]/.test(username)) {
this.usernameError_ =
loadTimeData.getString('usernameInvalidFirstCharacterError');
} else if (!/^[a-z0-9_-]*$/.test(username)) {
this.usernameError_ =
loadTimeData.getString('usernameInvalidCharactersError');
} else {
this.usernameError_ = '';
}
},
});
......@@ -73,6 +73,12 @@ void AddStringResources(content::WebUIDataSource* source) {
{"configureMessage", IDS_CROSTINI_INSTALLER_CONFIGURE_MESSAGE},
{"diskSizeMessage", IDS_CROSTINI_INSTALLER_DISK_SIZE_MESSAGE},
{"usernameMessage", IDS_CROSTINI_INSTALLER_USERNAME_MESSAGE},
{"usernameInvalidFirstCharacterError",
IDS_CROSTINI_INSTALLER_USERNAME_INVALID_FIRST_CHARACTER_ERROR},
{"usernameInvalidCharactersError",
IDS_CROSTINI_INSTALLER_USERNAME_INVALID_CHARACTERS_ERROR},
{"usernameNotAvailableError",
IDS_CROSTINI_INSTALLER_USERNAME_NOT_AVAILABLE_ERROR},
};
AddLocalizedStringsBulk(source, kStrings);
......
......@@ -21,7 +21,7 @@ class FakePageHandler extends TestBrowserProxy {
/** @override */
install(diskSize, username) {
this.methodCalled('install');
this.methodCalled('install', [diskSize, username]);
}
/** @override */
......@@ -89,6 +89,10 @@ suite('<crostini-installer-app>', () => {
return app.$$('.cancel-button');
};
const clickNext = async () => {
await clickButton(app.$.next);
};
const clickInstall = async () => {
await clickButton(getInstallButton());
};
......@@ -101,7 +105,13 @@ suite('<crostini-installer-app>', () => {
expectFalse(app.$$('#prompt-message').hidden);
expectEquals(fakeBrowserProxy.handler.getCallCount('install'), 0);
await clickNext();
await clickInstall();
await fakeBrowserProxy.handler.whenCalled('install').then(
([diskSize, username]) => {
assertEquals(
username, loadTimeData.getString('defaultContainerUsername'));
});
expectFalse(app.$$('#installing-message').hidden);
expectEquals(fakeBrowserProxy.handler.getCallCount('install'), 1);
expectTrue(getInstallButton().hidden);
......@@ -121,7 +131,42 @@ suite('<crostini-installer-app>', () => {
expectEquals(fakeBrowserProxy.handler.getCallCount('close'), 1);
});
test('configUsername', async () => {
await clickNext();
expectEquals(
app.$.username.value,
loadTimeData.getString('defaultContainerUsername'));
const invalidUsernames = [
'root', // Unavailable.
'0abcd', // Invalid first character.
'aBcd', // Invalid (uppercase) character.
];
for (const username of invalidUsernames) {
app.$.username.value = username;
await flushTasks();
expectTrue(app.$.username.invalid);
expectTrue(!!app.$.username.errorMessage);
expectTrue(app.$.install.disabled);
}
const validUsername = 'totally-valid_username';
app.$.username.value = validUsername;
await flushTasks();
expectFalse(app.$.username.invalid);
clickInstall();
await fakeBrowserProxy.handler.whenCalled('install').then(
([diskSize, username]) => {
assertEquals(username, validUsername);
});
expectEquals(fakeBrowserProxy.handler.getCallCount('install'), 1);
});
test('errorCancel', async () => {
await clickNext();
await clickInstall();
fakeBrowserProxy.page.onInstallFinished(InstallerError.kErrorOffline);
await flushTasks();
......@@ -137,6 +182,7 @@ suite('<crostini-installer-app>', () => {
});
test('errorRetry', async () => {
await clickNext();
await clickInstall();
fakeBrowserProxy.page.onInstallFinished(InstallerError.kErrorOffline);
await flushTasks();
......@@ -157,6 +203,7 @@ suite('<crostini-installer-app>', () => {
});
test('cancelAfterStart', async () => {
await clickNext();
await clickInstall();
await clickCancel();
expectEquals(fakeBrowserProxy.handler.getCallCount('cancel'), 1);
......
......@@ -24,7 +24,8 @@ CrostiniInstallerBrowserTest.prototype = {
featureList: {
enabled: [
'network::features::kOutOfBlinkCors'
'network::features::kOutOfBlinkCors',
'chromeos::features::kCrostiniUsername',
]
},
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment