From 6b920acd2c8254b48306639c73deac0382707eae Mon Sep 17 00:00:00 2001 From: Fernando Valverde Date: Fri, 2 Dec 2022 10:03:39 -0600 Subject: [PATCH] Remove Vault Secrets page from admin interface (#18795) --- app/controllers/admin/secrets_controller.rb | 41 ------------- app/lib/app_secrets.rb | 6 -- app/models/admin_menu.rb | 1 - app/views/admin/secrets/index.html.erb | 28 --------- config/routes/admin.rb | 2 - spec/requests/admin/secrets_spec.rb | 64 --------------------- 6 files changed, 142 deletions(-) delete mode 100644 app/controllers/admin/secrets_controller.rb delete mode 100644 app/views/admin/secrets/index.html.erb delete mode 100644 spec/requests/admin/secrets_spec.rb diff --git a/app/controllers/admin/secrets_controller.rb b/app/controllers/admin/secrets_controller.rb deleted file mode 100644 index fadaa4902..000000000 --- a/app/controllers/admin/secrets_controller.rb +++ /dev/null @@ -1,41 +0,0 @@ -module Admin - class SecretsController < Admin::ApplicationController - layout "admin" - - before_action :validate_settable_secret, only: [:update] - after_action only: [:update] do - Audit::Logger.log(:internal, current_user, params.dup) - end - - def index - @vault_enabled = AppSecrets.vault_enabled? - @secrets = AppSecrets::SETTABLE_SECRETS.map do |key| - secret_value = AppSecrets[key] - secret_value = if secret_value.present? - I18n.t("admin.secrets_controller.value", - first8: secret_value.first(8)) - else - I18n.t("admin.secrets_controller.not_in_vault") - end - [key, secret_value] - end - end - - def update - AppSecrets[params[:key_name]] = params[:key_value] - - flash[:success] = - I18n.t("admin.secrets_controller.updated", key: params[:key_name]) - redirect_to admin_secrets_path - end - - private - - def validate_settable_secret - update_param = params.permit(*AppSecrets::SETTABLE_SECRETS) - params[:key_name], params[:key_value] = update_param.to_h.to_a.first - - bad_request unless update_param.present? && params[:key_name].is_a?(String) && params[:key_value].is_a?(String) - end - end -end diff --git a/app/lib/app_secrets.rb b/app/lib/app_secrets.rb index 9929d9335..e4dbb125f 100644 --- a/app/lib/app_secrets.rb +++ b/app/lib/app_secrets.rb @@ -1,10 +1,4 @@ class AppSecrets - SETTABLE_SECRETS = %w[ - SLACK_CHANNEL - SLACK_DEPLOY_CHANNEL - SLACK_WEBHOOK_URL - ].freeze - def self.[](key) result = Vault.kv(namespace).read(key)&.data&.fetch(:value) if vault_enabled? result ||= ApplicationConfig[key] diff --git a/app/models/admin_menu.rb b/app/models/admin_menu.rb index b3ddded4a..d2d6ef431 100644 --- a/app/models/admin_menu.rb +++ b/app/models/admin_menu.rb @@ -48,7 +48,6 @@ class AdminMenu item(name: "response templates"), item(name: "developer tools", controller: "tools", children: [ item(name: "tools"), - item(name: "vault secrets", controller: "secrets"), item(name: "data update scripts", visible: -> { FeatureFlag.enabled?(:data_update_scripts) }), item(name: "extensions", controller: "extensions"), ]), diff --git a/app/views/admin/secrets/index.html.erb b/app/views/admin/secrets/index.html.erb deleted file mode 100644 index e0b1ca26d..000000000 --- a/app/views/admin/secrets/index.html.erb +++ /dev/null @@ -1,28 +0,0 @@ -<% unless @vault_enabled %> - -<% end %> - - - - - - - - - - - <% @secrets.each do |key, partial_value| %> - - <%= form_with(url: admin_secrets_path, method: "PUT", local: true) do %> - - - - <% end %> - - <% end %> - -
Secret NameSecret ValueAction
<%= label_tag key, key %><%= text_field_tag key, partial_value %><%= submit_tag("Update", data: { confirm: "My username is @#{current_user.username} and I want to update this Vault Secret." }, disabled: !@vault_enabled) %>
diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 14c048e7c..199683aae 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -127,8 +127,6 @@ namespace :admin do scope :advanced do resources :broadcasts resources :response_templates, only: %i[index new edit create update destroy] - resources :secrets, only: %i[index] - put "secrets", to: "secrets#update" resources :tools, only: %i[index create] do collection do post "bust_cache" diff --git a/spec/requests/admin/secrets_spec.rb b/spec/requests/admin/secrets_spec.rb deleted file mode 100644 index c681e8c20..000000000 --- a/spec/requests/admin/secrets_spec.rb +++ /dev/null @@ -1,64 +0,0 @@ -require "rails_helper" - -RSpec.describe "/admin/advanced/secrets", type: :request do - before do - allow(AppSecrets).to receive(:vault_enabled?).and_return(true) - allow(AppSecrets).to receive(:[]=) - end - - context "when the user is not an admin" do - it "blocks the request" do - user = create(:user) - sign_in user - - expect do - get admin_secrets_path - end.to raise_error(Pundit::NotAuthorizedError) - end - end - - context "when the user is an admin" do - let(:admin) { create(:user, :admin) } - - before { sign_in admin } - - describe "GET /admin/advanced/secrets" do - it "renders with status 200" do - get admin_secrets_path - expect(response).to have_http_status :ok - end - - it "displays an alert when Vault is not enabled" do - allow(AppSecrets).to receive(:vault_enabled?).and_return(false) - get admin_secrets_path - expect(response.body).to include("Vault is not currently setup for your application") - end - end - - describe "PUT /admin/advanced/secrets" do - let(:valid_secret) { AppSecrets::SETTABLE_SECRETS.first } - let(:valid_params) { { valid_secret => "SECRET_VALUE" } } - - it "successfully sets a secret and shows flash message" do - allow(AppSecrets).to receive(:[]=) - put admin_secrets_path, params: valid_params - expect(response).to have_http_status :found - expect(AppSecrets).to have_received(:[]=).with(valid_secret, "SECRET_VALUE") - expect(flash[:success]).to include("Secret #{valid_secret} was successfully updated in Vault") - end - - it "returns a bad_request with invalid params" do - put admin_secrets_path, params: {} - expect(response).to have_http_status :bad_request - end - - it "creates an audit log" do - Audit::Subscribe.listen :internal - expect do - put admin_secrets_path, params: valid_params - end.to change(AuditLog, :count).by(1) - Audit::Subscribe.forget :internal - end - end - end -end