diff options
author | Nick Thomas <nick@gitlab.com> | 2018-02-28 18:19:19 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-02-28 18:19:19 +0300 |
commit | 01b8d4bdda9568d458e3c0c94c5fcbc2c49b7b71 (patch) | |
tree | 019504dbb100bbb8d41fbb1a4ecb6f37f9182bbd | |
parent | 2e48bf116c4e307f125b3bbaf0c754574c4eb22c (diff) | |
parent | e9ece462b9dbe5c403f6d3d7043781ce83df3fdc (diff) |
Merge branch 'feature/edit_pages_domain' into 'master'
PagesDomain: allow update of key/certificate
Closes #29720
See merge request gitlab-org/gitlab-ce!17376
-rw-r--r-- | app/controllers/projects/pages_domains_controller.rb | 29 | ||||
-rw-r--r-- | app/views/projects/pages_domains/_form.html.haml | 56 | ||||
-rw-r--r-- | app/views/projects/pages_domains/edit.html.haml | 11 | ||||
-rw-r--r-- | app/views/projects/pages_domains/new.html.haml | 6 | ||||
-rw-r--r-- | app/views/projects/pages_domains/show.html.haml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/feature-edit_pages_domain.yml | 5 | ||||
-rw-r--r-- | config/routes/project.rb | 2 | ||||
-rw-r--r-- | spec/controllers/projects/pages_domains_controller_spec.rb | 60 | ||||
-rw-r--r-- | spec/features/projects/pages_spec.rb | 31 |
9 files changed, 164 insertions, 40 deletions
diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index b71f1e5fef4..4856be61e88 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -3,7 +3,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController before_action :require_pages_enabled! before_action :authorize_update_pages!, except: [:show] - before_action :domain, only: [:show, :destroy, :verify] + before_action :domain, except: [:new, :create] def show end @@ -24,8 +24,11 @@ class Projects::PagesDomainsController < Projects::ApplicationController redirect_to project_pages_domain_path(@project, @domain) end + def edit + end + def create - @domain = @project.pages_domains.create(pages_domain_params) + @domain = @project.pages_domains.create(create_params) if @domain.valid? redirect_to project_pages_domain_path(@project, @domain) @@ -34,6 +37,16 @@ class Projects::PagesDomainsController < Projects::ApplicationController end end + def update + if @domain.update(update_params) + redirect_to project_pages_path(@project), + status: 302, + notice: 'Domain was updated' + else + render 'edit' + end + end + def destroy @domain.destroy @@ -49,12 +62,12 @@ class Projects::PagesDomainsController < Projects::ApplicationController private - def pages_domain_params - params.require(:pages_domain).permit( - :certificate, - :key, - :domain - ) + def create_params + params.require(:pages_domain).permit(:key, :certificate, :domain) + end + + def update_params + params.require(:pages_domain).permit(:key, :certificate) end def domain diff --git a/app/views/projects/pages_domains/_form.html.haml b/app/views/projects/pages_domains/_form.html.haml index ca1b41b140a..d81b07832bb 100644 --- a/app/views/projects/pages_domains/_form.html.haml +++ b/app/views/projects/pages_domains/_form.html.haml @@ -1,34 +1,30 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| - - if @domain.errors.any? - #error_explanation - .alert.alert-danger - - @domain.errors.full_messages.each do |msg| - %p= msg +- if @domain.errors.any? + #error_explanation + .alert.alert-danger + - @domain.errors.full_messages.each do |msg| + %p= msg +.form-group + = f.label :domain, class: 'control-label' do + Domain + .col-sm-10 + = f.text_field :domain, required: true, autocomplete: 'off', class: 'form-control', disabled: @domain.persisted? + +- if Gitlab.config.pages.external_https .form-group - = f.label :domain, class: 'control-label' do - Domain + = f.label :certificate, class: 'control-label' do + Certificate (PEM) .col-sm-10 - = f.text_field :domain, required: true, autocomplete: 'off', class: 'form-control' - - - if Gitlab.config.pages.external_https - .form-group - = f.label :certificate, class: 'control-label' do - Certificate (PEM) - .col-sm-10 - = f.text_area :certificate, rows: 5, class: 'form-control' - %span.help-inline Upload a certificate for your domain with all intermediates - - .form-group - = f.label :key, class: 'control-label' do - Key (PEM) - .col-sm-10 - = f.text_area :key, rows: 5, class: 'form-control' - %span.help-inline Upload a private key for your certificate - - else - .nothing-here-block - Support for custom certificates is disabled. - Ask your system's administrator to enable it. + = f.text_area :certificate, rows: 5, class: 'form-control' + %span.help-inline Upload a certificate for your domain with all intermediates - .form-actions - = f.submit 'Create New Domain', class: "btn btn-save" + .form-group + = f.label :key, class: 'control-label' do + Key (PEM) + .col-sm-10 + = f.text_area :key, rows: 5, class: 'form-control' + %span.help-inline Upload a private key for your certificate +- else + .nothing-here-block + Support for custom certificates is disabled. + Ask your system's administrator to enable it. diff --git a/app/views/projects/pages_domains/edit.html.haml b/app/views/projects/pages_domains/edit.html.haml new file mode 100644 index 00000000000..5645a4604bf --- /dev/null +++ b/app/views/projects/pages_domains/edit.html.haml @@ -0,0 +1,11 @@ +- add_to_breadcrumbs "Pages", project_pages_path(@project) +- breadcrumb_title @domain.domain +- page_title @domain.domain +%h3.page_title + = @domain.domain +%hr.clearfix +%div + = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| + = render 'form', { f: f } + .form-actions + = f.submit 'Save Changes', class: "btn btn-save" diff --git a/app/views/projects/pages_domains/new.html.haml b/app/views/projects/pages_domains/new.html.haml index e1477c71d06..5a397c9d3c7 100644 --- a/app/views/projects/pages_domains/new.html.haml +++ b/app/views/projects/pages_domains/new.html.haml @@ -1,6 +1,10 @@ +- add_to_breadcrumbs "Pages", project_pages_path(@project) - page_title 'New Pages Domain' %h3.page_title New Pages Domain %hr.clearfix %div - = render 'form' + = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'form-horizontal fieldset-form' } do |f| + = render 'form', { f: f } + .form-actions + = f.submit 'Create New Domain', class: "btn btn-save" diff --git a/app/views/projects/pages_domains/show.html.haml b/app/views/projects/pages_domains/show.html.haml index 72e9203bdb0..ba0713daee9 100644 --- a/app/views/projects/pages_domains/show.html.haml +++ b/app/views/projects/pages_domains/show.html.haml @@ -1,4 +1,7 @@ +- add_to_breadcrumbs "Pages", project_pages_path(@project) +- breadcrumb_title @domain.domain - page_title "#{@domain.domain}", 'Pages Domains' + - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled? - if verification_enabled && @domain.unverified? %p.alert.alert-warning @@ -8,6 +11,7 @@ %h3.page-title Pages Domain + = link_to 'Edit', edit_project_pages_domain_path(@project, @domain), class: 'btn btn-success pull-right' .table-holder %table.table diff --git a/changelogs/unreleased/feature-edit_pages_domain.yml b/changelogs/unreleased/feature-edit_pages_domain.yml new file mode 100644 index 00000000000..bd0af53296c --- /dev/null +++ b/changelogs/unreleased/feature-edit_pages_domain.yml @@ -0,0 +1,5 @@ +--- +title: 'Pages custom domain: allow update of key/certificate' +merge_request: 17376 +author: rfwatson +type: changed diff --git a/config/routes/project.rb b/config/routes/project.rb index dd1a9c3be3b..34636285c51 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -55,7 +55,7 @@ constraints(ProjectUrlConstrainer.new) do end resource :pages, only: [:show, :destroy] do - resources :domains, only: [:show, :new, :create, :destroy], controller: 'pages_domains', constraints: { id: %r{[^/]+} } do + resources :domains, except: :index, controller: 'pages_domains', constraints: { id: %r{[^/]+} } do member do post :verify end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 2192fd5cae2..83a3799e883 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -53,6 +53,66 @@ describe Projects::PagesDomainsController do end end + describe 'GET edit' do + it "displays the 'edit' page" do + get(:edit, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template('edit') + end + end + + describe 'PATCH update' do + before do + controller.instance_variable_set(:@domain, pages_domain) + end + + let(:pages_domain_params) do + attributes_for(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate) + end + + let(:params) do + request_params.merge(id: pages_domain.domain, pages_domain: pages_domain_params) + end + + it 'updates the domain' do + expect(pages_domain) + .to receive(:update) + .with(pages_domain_params) + .and_return(true) + + patch(:update, params) + end + + it 'redirects to the project page' do + patch(:update, params) + + expect(flash[:notice]).to eq 'Domain was updated' + expect(response).to redirect_to(project_pages_path(project)) + end + + context 'the domain is invalid' do + it 'renders the edit action' do + allow(pages_domain).to receive(:update).and_return(false) + + patch(:update, params) + + expect(response).to render_template('edit') + end + end + + context 'the parameters include the domain' do + it 'renders 400 Bad Request' do + expect(pages_domain) + .to receive(:update) + .with(hash_not_including(:domain)) + .and_return(true) + + patch(:update, params.deep_merge(pages_domain: { domain: 'abc' })) + end + end + end + describe 'POST verify' do let(:params) { request_params.merge(id: pages_domain.domain) } diff --git a/spec/features/projects/pages_spec.rb b/spec/features/projects/pages_spec.rb index a96f2c186a4..2a0d235ef04 100644 --- a/spec/features/projects/pages_spec.rb +++ b/spec/features/projects/pages_spec.rb @@ -160,6 +160,37 @@ feature 'Pages' do expect(page).to have_content('my.test.domain.com') end + + describe 'updating the certificate for an existing domain' do + let!(:domain) do + create(:pages_domain, :with_key, :with_certificate, project: project) + end + + it 'allows the certificate to be updated' do + visit project_pages_path(project) + + within('#content-body') { click_link 'Details' } + click_link 'Edit' + click_button 'Save Changes' + + expect(page).to have_content('Domain was updated') + end + + context 'when the certificate is invalid' do + it 'tells the user what the problem is' do + visit project_pages_path(project) + + within('#content-body') { click_link 'Details' } + click_link 'Edit' + fill_in 'Certificate (PEM)', with: 'invalid data' + click_button 'Save Changes' + + expect(page).to have_content('Certificate must be a valid PEM certificate') + expect(page).to have_content('Certificate misses intermediates') + expect(page).to have_content("Key doesn't match the certificate") + end + end + end end end |