FIX: Keep private theme key secret from user (#18106)

The generate RSA key and import theme routes worked separate from each
other. The RSA key returned both the public and private key and it was
the frontend which posted the private key back to the server. With this
commit, only the public key is necessary as the server keeps a map of
public and private keys that is used to get the private key back from
a public key.

app/assets/javascripts/admin/addon/controllers/modals/admin-install-theme.js

@@ -93,10 +93,7 @@ export default Controller.extend(ModalFunctionality, {
       this._keyLoading = true;
       ajax(this.keyGenUrl, { type: "POST" })
         .then((pair) => {
-          this.setProperties({
-            privateKey: pair.private_key,
-            publicKey: pair.public_key,
-          });
+          this.set("publicKey", pair.public_key);
         })
         .catch(popupAjaxError)
         .finally(() => {
@@ -139,7 +136,6 @@ export default Controller.extend(ModalFunctionality, {
     this.setProperties({
       duplicateRemoteThemeWarning: null,
       privateChecked: false,
-      privateKey: null,
       localFile: null,
       uploadUrl: null,
       publicKey: null,
@@ -216,7 +212,7 @@ export default Controller.extend(ModalFunctionality, {
         };
 
         if (this.privateChecked) {
-          options.data.private_key = this.privateKey;
+          options.data.public_key = this.publicKey;
         }
       }
 
@@ -239,10 +235,10 @@ export default Controller.extend(ModalFunctionality, {
           this.send("closeModal");
         })
         .then(() => {
-          this.setProperties({ privateKey: null, publicKey: null });
+          this.set("publicKey", null);
         })
         .catch((error) => {
-          if (!this.privateKey || this.themeCannotBeInstalled) {
+          if (!this.publicKey || this.themeCannotBeInstalled) {
             return popupAjaxError(error);
           }
 

app/controllers/admin/themes_controller.rb

@@ -35,11 +35,8 @@ class Admin::ThemesController < Admin::AdminController
   def generate_key_pair
     require 'sshkey'
     k = SSHKey.generate
-
-    render json: {
-      private_key: k.private_key,
-      public_key: k.ssh_public_key
-    }
+    Discourse.redis.setex("ssh_key_#{k.ssh_public_key}", 1.hour, k.private_key)
+    render json: { public_key: k.ssh_public_key }
   end
 
   THEME_CONTENT_TYPES ||= %w{
@@ -101,14 +98,17 @@ class Admin::ThemesController < Admin::AdminController
 
       begin
         branch = params[:branch] ? params[:branch] : nil
-        @theme = RemoteTheme.import_theme(remote, theme_user, private_key: params[:private_key], branch: branch)
+        private_key = params[:public_key] ? Discourse.redis.get("ssh_key_#{params[:public_key]}") : nil
+        return render_json_error I18n.t("themes.import_error.ssh_key_gone") if params[:public_key].present? && private_key.blank?
+
+        @theme = RemoteTheme.import_theme(remote, theme_user, private_key: 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.private_key = private_key
           remote_theme.branch = params[:branch] ? params[:branch] : nil
           remote_theme.remote_url = params[:remote]
           remote_theme.save!

config/locales/server.en.yml

@@ -82,6 +82,7 @@ en:
       file_too_big: "The uncompressed file is too big."
       unknown_file_type: "The file you uploaded does not appear to be a valid Discourse theme."
       not_allowed_theme: "`%{repo}` is not in the list of allowed themes (check `allowed_theme_repos` global setting)."
+      ssh_key_gone: "You waited too long to install the theme and SSH key expired. Please try again."
     errors:
       component_no_user_selectable: "Theme components can't be user-selectable"
       component_no_default: "Theme components can't be default theme"

spec/requests/admin/themes_controller_spec.rb

@@ -16,8 +16,9 @@ RSpec.describe Admin::ThemesController do
       post "/admin/themes/generate_key_pair.json"
       expect(response.status).to eq(200)
       json = response.parsed_body
-      expect(json["private_key"]).to include("RSA PRIVATE KEY")
+      expect(json["private_key"]).to eq(nil)
       expect(json["public_key"]).to include("ssh-rsa ")
+      expect(Discourse.redis.get("ssh_key_#{json["public_key"]}")).not_to eq(nil)
     end
   end
 
@@ -139,6 +140,26 @@ RSpec.describe Admin::ThemesController do
       expect(response.status).to eq(201)
     end
 
+    it 'can lookup a private key by public key' do
+      Discourse.redis.setex('ssh_key_abcdef', 1.hour, 'rsa private key')
+
+      ThemeStore::GitImporter.any_instance.stubs(:import!)
+      RemoteTheme.stubs(:extract_theme_info).returns(
+        'name' => 'discourse-brand-header',
+        'component' => true
+      )
+      RemoteTheme.any_instance.stubs(:update_from_remote)
+
+      post '/admin/themes/import.json', params: {
+        remote: '    https://github.com/discourse/discourse-brand-header.git       ',
+        public_key: 'abcdef',
+      }
+
+      expect(RemoteTheme.last.private_key).to eq('rsa private key')
+
+      expect(response.status).to eq(201)
+    end
+
     it 'imports a theme' do
       post "/admin/themes/import.json", params: { theme: theme_json_file }
       expect(response.status).to eq(201)