Commit d69d2901 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Nick Thomas

Mark unverified pages domains for removal

Set pages_domain.remove_at when disabling it
Add specs for marking pages domain for removal
Notify user that domain is being removed
Add documentation
parent 61ab1f73
......@@ -8,6 +8,7 @@ class VerifyPagesDomainService < BaseService
# How long verification lasts for
VERIFICATION_PERIOD = 7.days
REMOVAL_DELAY = 1.week.freeze
attr_reader :domain
......@@ -36,7 +37,7 @@ class VerifyPagesDomainService < BaseService
# Prevent any pre-existing grace period from being truncated
reverify = [domain.enabled_until, VERIFICATION_PERIOD.from_now].compact.max
domain.assign_attributes(verified_at: Time.now, enabled_until: reverify)
domain.assign_attributes(verified_at: Time.now, enabled_until: reverify, remove_at: nil)
domain.save!(validate: false)
if was_disabled
......@@ -49,18 +50,20 @@ class VerifyPagesDomainService < BaseService
end
def unverify_domain!
if domain.verified?
was_verified = domain.verified?
domain.assign_attributes(verified_at: nil)
domain.remove_at ||= REMOVAL_DELAY.from_now unless domain.enabled?
domain.save!(validate: false)
notify(:verification_failed)
end
notify(:verification_failed) if was_verified
error("Couldn't verify #{domain.domain}")
end
def disable_domain!
domain.assign_attributes(verified_at: nil, enabled_until: nil)
domain.remove_at ||= REMOVAL_DELAY.from_now
domain.save!(validate: false)
notify(:disabled)
......
- if @domain.remove_at
%p
Unless you verify your domain by
%strong= @domain.remove_at.strftime('%F %T,')
it will be removed from your GitLab project.
- else
%p
If you no longer wish to use this domain with GitLab Pages, please remove it
from your GitLab project and delete any related DNS records.
......@@ -10,6 +10,4 @@
If this domain has been disabled in error, please follow
= link_to 'these instructions', help_page_url('user/project/pages/getting_started_part_three.md', anchor: 'dns-txt-record')
to verify and re-enable your domain.
%p
If you no longer wish to use this domain with GitLab Pages, please remove it
from your GitLab project and delete any related DNS records.
= render 'removal_notification'
......@@ -12,6 +12,4 @@
Please visit
= link_to 'these instructions', help_page_url('user/project/pages/getting_started_part_three.md', anchor: 'dns-txt-record')
for more information about custom domain verification.
%p
If you no longer wish to use this domain with GitLab Pages, please remove it
from your GitLab project and delete any related DNS records.
= render 'removal_notification'
---
title: Mark disabled pages domains for removal, but don't remove them yet
merge_request: 26212
author:
type: added
# frozen_string_literal: true
class AddRemoveAtToPagesDomains < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :pages_domains, :remove_at, :datetime_with_timezone
end
end
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddRemoveAtIndexToPagesDomains < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index :pages_domains, :remove_at
end
def down
remove_concurrent_index :pages_domains, :remove_at
end
end
......@@ -1547,9 +1547,11 @@ ActiveRecord::Schema.define(version: 20190326164045) do
t.datetime_with_timezone "verified_at"
t.string "verification_code", null: false
t.datetime_with_timezone "enabled_until"
t.datetime_with_timezone "remove_at"
t.index ["domain"], name: "index_pages_domains_on_domain", unique: true, using: :btree
t.index ["project_id", "enabled_until"], name: "index_pages_domains_on_project_id_and_enabled_until", using: :btree
t.index ["project_id"], name: "index_pages_domains_on_project_id", using: :btree
t.index ["remove_at"], name: "index_pages_domains_on_remove_at", using: :btree
t.index ["verified_at", "enabled_until"], name: "index_pages_domains_on_verified_at_and_enabled_until", using: :btree
t.index ["verified_at"], name: "index_pages_domains_on_verified_at", using: :btree
end
......
......@@ -115,6 +115,8 @@ If using a [DNS A record](#dns-a-record), you can place the TXT record directly
under the domain. If using a [DNS CNAME record](#dns-cname-record), the two record types won't
co-exist, so you need to place the TXT record in a special subdomain of its own.
If the domain cannot be verified for 7 days, it will be removed from the GitLab project.
#### TL;DR
For root domains (`domain.com`), set a DNS `A` record and verify your
......
......@@ -41,6 +41,10 @@ nNp/xedE1YxutQ==
enabled_until nil
end
trait :scheduled_for_removal do
remove_at { 1.day.from_now }
end
trait :unverified do
verified_at nil
end
......
......@@ -26,6 +26,26 @@ describe Emails::PagesDomains do
end
end
shared_examples 'notification about upcoming domain removal' do
context 'when domain is not scheduled for removal' do
it 'asks user to remove it' do
is_expected.to have_body_text 'please remove it'
end
end
context 'when domain is scheduled for removal' do
before do
domain.update!(remove_at: 1.week.from_now)
end
it 'notifies user that domain will be removed automatically' do
aggregate_failures do
is_expected.to have_body_text domain.remove_at.strftime('%F %T')
is_expected.to have_body_text "it will be removed from your GitLab project"
end
end
end
end
describe '#pages_domain_enabled_email' do
let(:email_subject) { "#{project.path} | GitLab Pages domain '#{domain.domain}' has been enabled" }
......@@ -43,6 +63,8 @@ describe Emails::PagesDomains do
it_behaves_like 'a pages domain email'
it_behaves_like 'notification about upcoming domain removal'
it { is_expected.to have_body_text 'has been disabled' }
end
......@@ -63,6 +85,8 @@ describe Emails::PagesDomains do
it_behaves_like 'a pages domain email'
it_behaves_like 'notification about upcoming domain removal'
it 'says verification has failed and when the domain is enabled until' do
is_expected.to have_body_text 'Verification has failed'
is_expected.to have_body_text domain.enabled_until.strftime('%F %T')
......
......@@ -27,6 +27,7 @@ describe VerifyPagesDomainService do
expect(domain).to be_verified
expect(domain).to be_enabled
expect(domain.remove_at).to be_nil
end
end
......@@ -48,18 +49,32 @@ describe VerifyPagesDomainService do
end
end
shared_examples 'unverifies and disables domain' do
it 'unverifies domain' do
expect(service.execute).to eq(error_status)
expect(domain).not_to be_verified
end
it 'disables domain and shedules it for removal' do
Timecop.freeze do
service.execute
expect(domain).not_to be_enabled
expect(domain.remove_at).to be_within(1.second).of(1.week.from_now)
end
end
end
context 'when domain is disabled(or new)' do
let(:domain) { create(:pages_domain, :disabled) }
include_examples 'successful enablement and verification'
shared_examples 'unverifies and disables domain' do
it 'unverifies and disables domain' do
expect(service.execute).to eq(error_status)
expect(domain).not_to be_verified
expect(domain).not_to be_enabled
context 'when txt record does not contain verification code' do
before do
stub_resolver(domain_name => 'something else')
end
include_examples 'unverifies and disables domain'
end
context 'when txt record does not contain verification code' do
......@@ -84,16 +99,25 @@ describe VerifyPagesDomainService do
include_examples 'successful enablement and verification'
context 'when txt record does not contain verification code' do
before do
stub_resolver(domain_name => 'something else')
end
shared_examples 'unverifing domain' do
it 'unverifies but does not disable domain' do
expect(service.execute).to eq(error_status)
expect(domain).not_to be_verified
expect(domain).to be_enabled
end
it 'does not schedule domain for removal' do
service.execute
expect(domain.remove_at).to be_nil
end
end
context 'when txt record does not contain verification code' do
before do
stub_resolver(domain_name => 'something else')
end
include_examples 'unverifing domain'
end
context 'when no txt records are present' do
......@@ -101,11 +125,7 @@ describe VerifyPagesDomainService do
stub_resolver
end
it 'unverifies but does not disable domain' do
expect(service.execute).to eq(error_status)
expect(domain).not_to be_verified
expect(domain).to be_enabled
end
include_examples 'unverifing domain'
end
end
......@@ -125,13 +145,40 @@ describe VerifyPagesDomainService do
stub_resolver
end
it 'disables domain' do
error_status[:message] += '. It is now disabled.'
let(:error_status) { { status: :error, message: "Couldn't verify #{domain.domain}. It is now disabled." } }
expect(service.execute).to eq(error_status)
include_examples 'unverifies and disables domain'
end
end
expect(domain).not_to be_verified
expect(domain).not_to be_enabled
context 'when domain is disabled and scheduled for removal' do
let(:domain) { create(:pages_domain, :disabled, :scheduled_for_removal) }
context 'when the right code is present' do
before do
stub_resolver(domain.domain => domain.keyed_verification_code)
end
it 'verifies and enables domain' do
expect(service.execute).to eq(status: :success)
expect(domain).to be_verified
expect(domain).to be_enabled
end
it 'prevent domain from being removed' do
expect { service.execute }.to change { domain.remove_at }.to(nil)
end
end
context 'when the right code is not present' do
before do
stub_resolver
end
it 'keeps domain scheduled for removal but does not change removal time' do
expect { service.execute }.not_to change { domain.remove_at }
expect(domain.remove_at).to be_present
end
end
end
......
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