FEATURE: Allow private themes to be partially installed (#17644)

A public key must be added to GitHub when installing private themes.
When the process happens asynchronously (for example if the admin does
not have admin permissions to the GitHub repository), installing
private themes becomes very difficult.

In this case, the Discourse admin can partially install the theme by
letting Discourse save the private key, create a placeholder theme and
give the admin a public key to be used as a deploy key. After the key
is installed, the admin can finish theme installation by pressing a
button on the theme page.
This commit is contained in:
Bianca Nenciu
2022-08-10 13:30:18 +03:00
committed by GitHub
parent cc84ea2444
commit e029a9b36c
10 changed files with 617 additions and 261 deletions

View File

@ -167,6 +167,15 @@ export default Controller.extend({
return errorMessage && !updating;
},
@discourseComputed(
"model.remote_theme.remote_url",
"model.remote_theme.local_version",
"model.remote_theme.commits_behind"
)
finishInstall(remoteUrl, localVersion, commitsBehind) {
return remoteUrl && !localVersion && !commitsBehind;
},
editedFieldsForTarget(target) {
return this.get("model.editedFields").filter(
(field) => field.target === target

View File

@ -119,8 +119,12 @@ export default Controller.extend(ModalFunctionality, {
}
},
@discourseComputed("selection")
submitLabel(selection) {
@discourseComputed("selection", "themeCannotBeInstalled")
submitLabel(selection, themeCannotBeInstalled) {
if (themeCannotBeInstalled) {
return "admin.customize.theme.create_placeholder";
}
return `admin.customize.theme.${
selection === "create" ? "create" : "install"
}`;
@ -216,6 +220,12 @@ export default Controller.extend(ModalFunctionality, {
}
}
// User knows that theme cannot be installed, but they want to continue
// to force install it.
if (this.themeCannotBeInstalled) {
options.data["force"] = true;
}
if (this.get("model.user_id")) {
// Used by theme-creator
options.data["user_id"] = this.get("model.user_id");
@ -231,7 +241,16 @@ export default Controller.extend(ModalFunctionality, {
.then(() => {
this.setProperties({ privateKey: null, publicKey: null });
})
.catch(popupAjaxError)
.catch((error) => {
if (!this.privateKey || this.themeCannotBeInstalled) {
return popupAjaxError(error);
}
this.set(
"themeCannotBeInstalled",
I18n.t("admin.customize.theme.force_install")
);
})
.finally(() => this.set("loading", false));
},
},

View File

@ -15,6 +15,36 @@
<div class="alert alert-error">{{error}}</div>
{{/each}}
{{#if this.finishInstall}}
<div class="control-unit">
{{#if this.sourceIsHttp}}
<a class="remote-url" href={{this.remoteThemeLink}}>{{i18n "admin.customize.theme.source_url"}}{{d-icon "link"}}</a>
{{else}}
<div class="remote-url">
<code>{{this.model.remote_theme.remote_url}}</code>
{{#if this.model.remote_theme.branch}}
(<code>{{this.model.remote_theme.branch}}</code>)
{{/if}}
</div>
{{/if}}
{{#if this.showRemoteError}}
<div class="error-message">
{{d-icon "exclamation-triangle"}} {{i18n "admin.customize.theme.repo_unreachable"}}
</div>
<div class="raw-error">
<code>{{this.model.remoteError}}</code>
</div>
{{/if}}
<DButton @action={{action "updateToLatest"}} @icon="download" @class="btn-primary finish-install" @label="admin.customize.theme.finish_install" />
<DButton @action={{action "destroy"}} @label="admin.customize.delete" @icon="trash-alt" @class="btn-danger" />
<span class="status-message">
{{i18n "admin.customize.theme.last_attempt"}} {{format-date this.model.remote_theme.updated_at leaveAgo="true"}}
</span>
</div>
{{else}}
{{#unless this.model.supported}}
<div class="alert alert-error">
{{i18n "admin.customize.theme.required_version.error"}}
@ -291,5 +321,6 @@
<DButton @action={{action "destroy"}} @label="admin.customize.delete" @icon="trash-alt" @class="btn-danger" />
</div>
{{/if}}
</div>

View File

@ -111,7 +111,12 @@
⚠️ {{this.duplicateRemoteThemeWarning}}
</div>
{{/if}}
<DButton @action={{action "installTheme"}} @disabled={{this.installDisabled}} @class="btn btn-primary" @label={{this.submitLabel}} />
{{#if this.themeCannotBeInstalled}}
<div class="install-theme-warning">
⚠️ {{this.themeCannotBeInstalled}}
</div>
{{/if}}
<DButton @action={{action "installTheme"}} @disabled={{this.installDisabled}} @class="btn {{if this.themeCannotBeInstalled "btn-danger" "btn-primary"}}" @label={{this.submitLabel}} />
<DModalCancel @close={{route-action "closeModal"}} />
</div>
{{/unless}}

View File

@ -0,0 +1,243 @@
import { click, fillIn, visit } from "@ember/test-helpers";
import {
acceptance,
exists,
query,
} from "discourse/tests/helpers/qunit-helpers";
import I18n from "I18n";
import { test } from "qunit";
acceptance("Theme", function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.get("/admin/themes", () => {
return helper.response(200, {
themes: [
{
id: 42,
name: "discourse-incomplete-theme",
created_at: "2022-01-01T12:00:00.000Z",
updated_at: "2022-01-01T12:00:00.000Z",
component: false,
color_scheme: null,
color_scheme_id: null,
user_selectable: false,
auto_update: true,
remote_theme_id: 42,
settings: [],
supported: true,
description: null,
enabled: true,
user: {
id: 1,
username: "foo",
name: null,
avatar_template:
"/letter_avatar_proxy/v4/letter/f/3be4f8/{size}.png",
title: "Tester",
},
theme_fields: [],
child_themes: [],
parent_themes: [],
remote_theme: {
id: 42,
remote_url:
"git@github.com:discourse/discourse-incomplete-theme.git",
remote_version: null,
local_version: null,
commits_behind: null,
branch: null,
remote_updated_at: null,
updated_at: "2022-01-01T12:00:00.000Z",
last_error_text: null,
is_git: true,
license_url: null,
about_url: null,
authors: null,
theme_version: null,
minimum_discourse_version: null,
maximum_discourse_version: null,
},
translations: [],
},
],
});
});
server.post("/admin/themes/import", (request) => {
const data = helper.parsePostData(request.requestBody);
if (!data.force) {
return helper.response(422, {
errors: [
"Error cloning git repository, access is denied or repository is not found",
],
});
}
return helper.response(201, {
theme: {
id: 42,
name: "discourse-inexistent-theme",
created_at: "2022-01-01T12:00:00.000Z",
updated_at: "2022-01-01T12:00:00.000Z",
component: false,
color_scheme: null,
color_scheme_id: null,
user_selectable: false,
auto_update: true,
remote_theme_id: 42,
settings: [],
supported: true,
description: null,
enabled: true,
user: {
id: 1,
username: "foo",
name: null,
avatar_template:
"/letter_avatar_proxy/v4/letter/f/3be4f8/{size}.png",
},
theme_fields: [],
child_themes: [],
parent_themes: [],
remote_theme: {
id: 42,
remote_url:
"git@github.com:discourse/discourse-inexistent-theme.git",
remote_version: null,
local_version: null,
commits_behind: null,
branch: null,
remote_updated_at: null,
updated_at: "2022-01-01T12:00:00.000Z",
last_error_text: null,
is_git: true,
license_url: null,
about_url: null,
authors: null,
theme_version: null,
minimum_discourse_version: null,
maximum_discourse_version: null,
},
translations: [],
},
});
});
server.put("/admin/themes/42", () => {
return helper.response(200, {
theme: {
id: 42,
name: "discourse-complete-theme",
created_at: "2022-01-01T12:00:00.000Z",
updated_at: "2022-01-01T12:00:00.000Z",
component: false,
color_scheme: null,
color_scheme_id: null,
user_selectable: false,
auto_update: true,
remote_theme_id: 42,
settings: [],
supported: true,
description: null,
enabled: true,
user: {
id: 1,
username: "foo",
name: null,
avatar_template:
"/letter_avatar_proxy/v4/letter/f/3be4f8/{size}.png",
},
theme_fields: [],
child_themes: [],
parent_themes: [],
remote_theme: {
id: 42,
remote_url:
"git@github.com:discourse-org/discourse-incomplete-theme.git",
remote_version: "0000000000000000000000000000000000000000",
local_version: "0000000000000000000000000000000000000000",
commits_behind: 0,
branch: null,
remote_updated_at: "2022-01-01T12:00:30.000Z",
updated_at: "2022-01-01T12:00:30.000Z",
last_error_text: null,
is_git: true,
license_url: "URL",
about_url: "URL",
authors: null,
theme_version: null,
minimum_discourse_version: null,
maximum_discourse_version: null,
},
translations: [],
},
});
});
});
test("can force install themes", async function (assert) {
await visit("/admin/customize/themes");
await click(".themes-list .create-actions button");
await click(".install-theme-items #remote");
await fillIn(
".install-theme-content .repo input",
"git@github.com:discourse/discourse-inexistent-theme.git"
);
await click(".install-theme-content button.advanced-repo");
await click(".install-theme-content .check-private input");
assert.notOk(
exists(".admin-install-theme-modal .modal-footer .install-theme-warning"),
"no Git warning is displayed"
);
await click(".admin-install-theme-modal .modal-footer .btn-primary");
assert.ok(
exists(".admin-install-theme-modal .modal-footer .install-theme-warning"),
"Git warning is displayed"
);
await click(".admin-install-theme-modal .modal-footer .btn-danger");
assert.notOk(
exists(".admin-install-theme-modal:visible"),
"modal is closed"
);
});
test("can continue installation", async function (assert) {
await visit("/admin/customize/themes");
await click(".themes-list-container .themes-list-item");
assert.ok(
query(".control-unit .status-message").innerText.includes(
I18n.t("admin.customize.theme.last_attempt")
),
"it says that theme is not completely installed"
);
await click(".control-unit .btn-primary.finish-install");
assert.equal(
query(".show-current-style .title span").innerText,
"discourse-complete-theme",
"it updates theme title"
);
assert.notOk(
query(".metadata.control-unit").innerText.includes(
I18n.t("admin.customize.theme.last_attempt")
),
"it does not say that theme is not completely installed"
);
assert.notOk(
query(".control-unit .btn-primary.finish-install"),
"it does not show finish install button"
);
});
});

View File

@ -27,9 +27,12 @@
.admin-container {
padding: 0;
}
.error-message {
.error-message,
.raw-error {
margin-top: 5px;
margin-bottom: 5px;
}
.error-message {
.fa {
color: var(--danger);
}

View File

@ -104,8 +104,24 @@ class Admin::ThemesController < Admin::AdminController
@theme = RemoteTheme.import_theme(remote, theme_user, private_key: params[:private_key], branch: branch)
render json: @theme, status: :created
rescue RemoteTheme::ImportError => e
if params[:force]
theme_name = params[:remote].gsub(/.git$/, "").split("/").last
remote_theme = RemoteTheme.new
remote_theme.private_key = params[:private_key]
remote_theme.branch = params[:branch] ? params[:branch] : nil
remote_theme.remote_url = params[:remote]
remote_theme.save!
@theme = Theme.new(user_id: theme_user&.id || -1, name: theme_name)
@theme.remote_theme = remote_theme
@theme.save!
render json: @theme, status: :created
else
render_json_error e.message
end
end
elsif params[:bundle] || (params[:theme] && THEME_CONTENT_TYPES.include?(params[:theme].content_type))
ban_in_allowlist_mode!

View File

@ -171,6 +171,13 @@ class RemoteTheme < ActiveRecord::Base
end
end
# Update all theme attributes if this is just a placeholder
if self.remote_url.present? && !self.local_version && !self.commits_behind
self.theme.name = theme_info["name"]
self.theme.component = [true, "true"].include?(theme_info["component"])
self.theme.child_components = theme_info["components"].presence || []
end
METADATA_PROPERTIES.each do |property|
self.public_send(:"#{property}=", theme_info[property.to_s])
end

View File

@ -4719,6 +4719,8 @@ en:
import_web_advanced: "Advanced..."
import_file_tip: ".tar.gz, .zip, or .dcstyle.json file containing theme"
is_private: "Theme is in a private git repository"
finish_install: "Finish Theme Installation"
last_attempt: "Installation process did not finish, last attempted:"
remote_branch: "Branch name (optional)"
public_key: "Grant the following public key access to the repo:"
public_key_note: "After entering a valid private repository URL above, an SSH key will be generated and displayed here."
@ -4729,6 +4731,8 @@ en:
install_git_repo: "From a git repository"
install_create: "Create new"
duplicate_remote_theme: "The theme component “%{name}” is already installed, are you sure you want to install another copy?"
force_install: "The theme cannot be installed because the Git repository is inaccessible. Are you sure you want to continue installing it?"
create_placeholder: "Create Placeholder"
about_theme: "About"
license: "License"
version: "Version:"

View File

@ -151,6 +151,25 @@ RSpec.describe Admin::ThemesController do
expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
end
it 'can fail if theme is not accessible' do
post "/admin/themes/import.json", params: {
remote: 'git@github.com:discourse/discourse-inexistent-theme.git'
}
expect(response.status).to eq(422)
expect(response.parsed_body["errors"]).to contain_exactly(I18n.t("themes.import_error.git"))
end
it 'can force install theme' do
post "/admin/themes/import.json", params: {
remote: 'git@github.com:discourse/discourse-inexistent-theme.git',
force: true
}
expect(response.status).to eq(201)
expect(response.parsed_body["theme"]["name"]).to eq("discourse-inexistent-theme")
end
it 'fails to import with an error if uploads are not allowed' do
SiteSetting.theme_authorized_extensions = "nothing"