From 0f3b033f790636c0209a1def1fb45665289a8084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Wed, 16 Sep 2020 15:59:02 +0300 Subject: [PATCH 01/35] Create bounced_mail_addresses table --- ...916125326_create_bounced_mail_addresses.rb | 12 ++++ db/structure.sql | 65 +++++++++++++++++-- 2 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20200916125326_create_bounced_mail_addresses.rb diff --git a/db/migrate/20200916125326_create_bounced_mail_addresses.rb b/db/migrate/20200916125326_create_bounced_mail_addresses.rb new file mode 100644 index 000000000..c6600afea --- /dev/null +++ b/db/migrate/20200916125326_create_bounced_mail_addresses.rb @@ -0,0 +1,12 @@ +class CreateBouncedMailAddresses < ActiveRecord::Migration[6.0] + def change + create_table :bounced_mail_addresses do |t| + t.string :email, null: false + t.string :bounce_reason, null: false + t.integer :incidents, null: false, default: 1 + t.jsonb :response_json + + t.timestamps + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 6a30fbc84..3ec055386 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -475,6 +475,40 @@ CREATE SEQUENCE public.blocked_domains_id_seq ALTER SEQUENCE public.blocked_domains_id_seq OWNED BY public.blocked_domains.id; +-- +-- Name: bounced_mail_addresses; Type: TABLE; Schema: public; Owner: -; Tablespace: +-- + +CREATE TABLE public.bounced_mail_addresses ( + id bigint NOT NULL, + email character varying NOT NULL, + bounce_reason character varying NOT NULL, + incidents integer DEFAULT 1 NOT NULL, + response_json jsonb, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: bounced_mail_addresses_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.bounced_mail_addresses_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: bounced_mail_addresses_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.bounced_mail_addresses_id_seq OWNED BY public.bounced_mail_addresses.id; + + -- -- Name: certificates; Type: TABLE; Schema: public; Owner: -; Tablespace: -- @@ -1706,7 +1740,7 @@ ALTER SEQUENCE public.log_payment_orders_id_seq OWNED BY public.log_payment_orde -- --- Name: log_prices; Type: TABLE; Schema: public; Owner: - +-- Name: log_prices; Type: TABLE; Schema: public; Owner: -; Tablespace: -- CREATE TABLE public.log_prices ( @@ -1744,7 +1778,7 @@ ALTER SEQUENCE public.log_prices_id_seq OWNED BY public.log_prices.id; -- --- Name: log_registrant_verifications; Type: TABLE; Schema: public; Owner: - +-- Name: log_registrant_verifications; Type: TABLE; Schema: public; Owner: -; Tablespace: -- CREATE TABLE public.log_registrant_verifications ( @@ -2658,6 +2692,13 @@ ALTER TABLE ONLY public.bank_transactions ALTER COLUMN id SET DEFAULT nextval('p ALTER TABLE ONLY public.blocked_domains ALTER COLUMN id SET DEFAULT nextval('public.blocked_domains_id_seq'::regclass); +-- +-- Name: id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.bounced_mail_addresses ALTER COLUMN id SET DEFAULT nextval('public.bounced_mail_addresses_id_seq'::regclass); + + -- -- Name: id; Type: DEFAULT; Schema: public; Owner: - -- @@ -2876,14 +2917,14 @@ ALTER TABLE ONLY public.log_payment_orders ALTER COLUMN id SET DEFAULT nextval(' -- --- Name: log_prices id; Type: DEFAULT; Schema: public; Owner: - +-- Name: id; Type: DEFAULT; Schema: public; Owner: - -- ALTER TABLE ONLY public.log_prices ALTER COLUMN id SET DEFAULT nextval('public.log_prices_id_seq'::regclass); -- --- Name: log_registrant_verifications id; Type: DEFAULT; Schema: public; Owner: - +-- Name: id; Type: DEFAULT; Schema: public; Owner: - -- ALTER TABLE ONLY public.log_registrant_verifications ALTER COLUMN id SET DEFAULT nextval('public.log_registrant_verifications_id_seq'::regclass); @@ -3100,6 +3141,14 @@ ALTER TABLE ONLY public.blocked_domains ADD CONSTRAINT blocked_domains_pkey PRIMARY KEY (id); +-- +-- Name: bounced_mail_addresses_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: +-- + +ALTER TABLE ONLY public.bounced_mail_addresses + ADD CONSTRAINT bounced_mail_addresses_pkey PRIMARY KEY (id); + + -- -- Name: certificates_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- @@ -3349,7 +3398,7 @@ ALTER TABLE ONLY public.log_payment_orders -- --- Name: log_prices log_prices_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- Name: log_prices_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- ALTER TABLE ONLY public.log_prices @@ -3357,7 +3406,7 @@ ALTER TABLE ONLY public.log_prices -- --- Name: log_registrant_verifications log_registrant_verifications_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- Name: log_registrant_verifications_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- ALTER TABLE ONLY public.log_registrant_verifications @@ -4906,5 +4955,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200902131603'), ('20200908131554'), ('20200910085157'), -('20200910102028'); +('20200910102028'), +('20200916125326'); + From 188cca0c063325b91d641e808fbd3592a38f66eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 11:12:01 +0300 Subject: [PATCH 02/35] Create views for bounced emails --- .../bounced_mail_addresses_controller.rb | 60 +++++++++++++++++++ app/models/ability.rb | 1 + app/models/bounced_mail_address.rb | 2 + .../bounced_mail_addresses/_form.html.erb | 37 ++++++++++++ .../bounced_mail_addresses/edit.html.erb | 6 ++ .../bounced_mail_addresses/index.html.erb | 33 ++++++++++ .../admin/bounced_mail_addresses/new.html.erb | 5 ++ .../bounced_mail_addresses/show.html.erb | 24 ++++++++ config/routes.rb | 1 + 9 files changed, 169 insertions(+) create mode 100644 app/controllers/admin/bounced_mail_addresses_controller.rb create mode 100644 app/models/bounced_mail_address.rb create mode 100644 app/views/admin/bounced_mail_addresses/_form.html.erb create mode 100644 app/views/admin/bounced_mail_addresses/edit.html.erb create mode 100644 app/views/admin/bounced_mail_addresses/index.html.erb create mode 100644 app/views/admin/bounced_mail_addresses/new.html.erb create mode 100644 app/views/admin/bounced_mail_addresses/show.html.erb diff --git a/app/controllers/admin/bounced_mail_addresses_controller.rb b/app/controllers/admin/bounced_mail_addresses_controller.rb new file mode 100644 index 000000000..a33f90ab3 --- /dev/null +++ b/app/controllers/admin/bounced_mail_addresses_controller.rb @@ -0,0 +1,60 @@ +module Admin + class BouncedMailAddressesController < BaseController + before_action :set_bounced_mail_address, only: %i[show edit update destroy] + load_and_authorize_resource + + # GET /bounced_mail_addresses + def index + @bounced_mail_addresses = BouncedMailAddress.all + end + + # GET /bounced_mail_addresses/1 + def show; end + + # GET /bounced_mail_addresses/new + def new + @bounced_mail_address = BouncedMailAddress.new + end + + # GET /bounced_mail_addresses/1/edit + def edit; end + + # POST /bounced_mail_addresses + def create + @bounced_mail_address = BouncedMailAddress.new(bounced_mail_address_params) + + if @bounced_mail_address.save + redirect_to(admin_bounced_mail_addresses_url, notice: 'Bounced mail address was successfully created.') + else + render(:new) + end + end + + # PATCH/PUT /bounced_mail_addresses/1 + def update + if @bounced_mail_address.update(bounced_mail_address_params) + redirect_to(@bounced_mail_address, notice: 'Bounced mail address was successfully updated.') + else + render(:edit) + end + end + + # DELETE /bounced_mail_addresses/1 + def destroy + @bounced_mail_address.destroy + redirect_to(admin_bounced_mail_addresses_url, notice: 'Bounced mail address was successfully destroyed.') + end + + private + + # Use callbacks to share common setup or constraints between actions. + def set_bounced_mail_address + @bounced_mail_address = BouncedMailAddress.find(params[:id]) + end + + # Only allow a trusted parameter "white list" through. + def bounced_mail_address_params + params.require(:bounced_mail_address).permit(:email, :bounce_reason, :incidents, :response_json) + end + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index dce8a515b..31637b8ea 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -109,6 +109,7 @@ class Ability can :destroy, :pending can :create, :zonefile can :access, :settings_menu + can :manage, BouncedMailAddress end def static_registrant diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb new file mode 100644 index 000000000..d8f6a037b --- /dev/null +++ b/app/models/bounced_mail_address.rb @@ -0,0 +1,2 @@ +class BouncedMailAddress < ApplicationRecord +end diff --git a/app/views/admin/bounced_mail_addresses/_form.html.erb b/app/views/admin/bounced_mail_addresses/_form.html.erb new file mode 100644 index 000000000..7a384bd3f --- /dev/null +++ b/app/views/admin/bounced_mail_addresses/_form.html.erb @@ -0,0 +1,37 @@ +<%= form_for([:admin, @bounced_mail_address], html: { class: 'form-horizontal' }) do |form| %> + <% if @bounced_mail_address.errors.any? %> +
+

<%= pluralize(bounced_mail_address.errors.count, "error") %> prohibited this bounced_mail_address from being saved:

+ +
    + <% @bounced_mail_address.errors.full_messages.each do |message| %> +
  • <%= message %>
  • + <% end %> +
+
+ <% end %> + +
+ <%= form.label :email %> + <%= form.text_field :email %> +
+ +
+ <%= form.label :bounce_reason %> + <%= form.text_field :bounce_reason %> +
+ +
+ <%= form.label :incidents %> + <%= form.number_field :incidents %> +
+ +
+ <%= form.label :response_json %> + <%= form.text_field :response_json %> +
+ +
+ <%= form.submit %> +
+<% end %> diff --git a/app/views/admin/bounced_mail_addresses/edit.html.erb b/app/views/admin/bounced_mail_addresses/edit.html.erb new file mode 100644 index 000000000..a3dfe2d84 --- /dev/null +++ b/app/views/admin/bounced_mail_addresses/edit.html.erb @@ -0,0 +1,6 @@ +

Editing Bounced Mail Address

+ +<%= render 'form', bounced_mail_address: @bounced_mail_address %> + +<%= link_to 'Show', admin_bounced_mail_address_path(@bounced_mail_address)%> | +<%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/app/views/admin/bounced_mail_addresses/index.html.erb b/app/views/admin/bounced_mail_addresses/index.html.erb new file mode 100644 index 000000000..c2f95ca68 --- /dev/null +++ b/app/views/admin/bounced_mail_addresses/index.html.erb @@ -0,0 +1,33 @@ +

<%= notice %>

+ +

Bounced Mail Addresses

+ + + + + + + + + + + + + + <% @bounced_mail_addresses.each do |mail_addr| %> + + + + + + + + + + <% end %> + +
EmailBounce reasonIncidentsResponse json
<%= mail_addr.email %><%= mail_addr.bounce_reason %><%= mail_addr.incidents %><%= mail_addr.response_json %><%= link_to 'Show', admin_bounced_mail_address_path(mail_addr) %><%= link_to 'Edit', edit_admin_bounced_mail_address_path(mail_addr) %><%= link_to 'Destroy', admin_bounced_mail_address_path(mail_addr), method: :delete, data: { confirm: 'Are you sure?' } %>
+ +
+ +<%= link_to 'New Bounced Mail Address', new_admin_bounced_mail_address_path %> diff --git a/app/views/admin/bounced_mail_addresses/new.html.erb b/app/views/admin/bounced_mail_addresses/new.html.erb new file mode 100644 index 000000000..010ac79dc --- /dev/null +++ b/app/views/admin/bounced_mail_addresses/new.html.erb @@ -0,0 +1,5 @@ +

New Bounced Mail Address

+ +<%= render 'form', bounced_mail_address: @bounced_mail_address %> + +<%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/app/views/admin/bounced_mail_addresses/show.html.erb b/app/views/admin/bounced_mail_addresses/show.html.erb new file mode 100644 index 000000000..1f48ad5cf --- /dev/null +++ b/app/views/admin/bounced_mail_addresses/show.html.erb @@ -0,0 +1,24 @@ +

<%= notice %>

+ +

+ Email: + <%= @bounced_mail_address.email %> +

+ +

+ Bounce reason: + <%= @bounced_mail_address.bounce_reason %> +

+ +

+ Incidents: + <%= @bounced_mail_address.incidents %> +

+ +

+ Response json: + <%= @bounced_mail_address.response_json %> +

+ +<%= link_to 'Edit', edit_admin_bounced_mail_address_path(@bounced_mail_address) %> | +<%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/config/routes.rb b/config/routes.rb index 223cf3171..cdbd63f31 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -298,6 +298,7 @@ Rails.application.routes.draw do resources :delayed_jobs resources :epp_logs resources :repp_logs + resources :bounced_mail_addresses authenticate :admin_user do mount Que::Web, at: 'que' From 22f9c2058d137a021034030c66ac33f8049f17a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 11:55:52 +0300 Subject: [PATCH 03/35] Create api/v1/bounces endpoint --- app/controllers/api/v1/bounces_controller.rb | 12 ++++++++++++ config/routes.rb | 1 + 2 files changed, 13 insertions(+) create mode 100644 app/controllers/api/v1/bounces_controller.rb diff --git a/app/controllers/api/v1/bounces_controller.rb b/app/controllers/api/v1/bounces_controller.rb new file mode 100644 index 000000000..cff8c3efe --- /dev/null +++ b/app/controllers/api/v1/bounces_controller.rb @@ -0,0 +1,12 @@ +module Api + module V1 + class BouncesController < BaseController + before_action :authenticate + + def create + bounced_mail_address = BouncedMailAddress.record(json) + bounced_mail_address ? render(head: :ok) : render(head: :failed) + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index cdbd63f31..89f8f0cd6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -64,6 +64,7 @@ Rails.application.routes.draw do end resources :auctions, only: %i[index show update], param: :uuid + resources :bounces, only: %i[create] end match '*all', controller: 'cors', action: 'cors_preflight_check', via: [:options], From 140df0acf78a9e0139301649b244247c66b612e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 13:26:00 +0300 Subject: [PATCH 04/35] Fix CC issues --- .../admin/bounced_mail_addresses_controller.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/bounced_mail_addresses_controller.rb b/app/controllers/admin/bounced_mail_addresses_controller.rb index a33f90ab3..551413e2c 100644 --- a/app/controllers/admin/bounced_mail_addresses_controller.rb +++ b/app/controllers/admin/bounced_mail_addresses_controller.rb @@ -24,7 +24,10 @@ module Admin @bounced_mail_address = BouncedMailAddress.new(bounced_mail_address_params) if @bounced_mail_address.save - redirect_to(admin_bounced_mail_addresses_url, notice: 'Bounced mail address was successfully created.') + redirect_to( + admin_bounced_mail_addresses_url, + notice: 'Bounced mail address was successfully created.' + ) else render(:new) end @@ -42,7 +45,10 @@ module Admin # DELETE /bounced_mail_addresses/1 def destroy @bounced_mail_address.destroy - redirect_to(admin_bounced_mail_addresses_url, notice: 'Bounced mail address was successfully destroyed.') + redirect_to( + admin_bounced_mail_addresses_url, + notice: 'Bounced mail address was successfully destroyed.' + ) end private @@ -54,7 +60,12 @@ module Admin # Only allow a trusted parameter "white list" through. def bounced_mail_address_params - params.require(:bounced_mail_address).permit(:email, :bounce_reason, :incidents, :response_json) + params.require(:bounced_mail_address).permit( + :email, + :bounce_reason, + :incidents, + :response_json + ) end end end From 6af37a787dc56ec7b4d89079d6bf11d5ddf58651 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 13:26:48 +0300 Subject: [PATCH 05/35] Restructure bounced mails view --- app/models/bounced_mail_address.rb | 22 +++++++ .../bounced_mail_addresses/index.html.erb | 61 ++++++++++--------- 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index d8f6a037b..6609829da 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -1,2 +1,24 @@ class BouncedMailAddress < ApplicationRecord + validates :email, presence: true + validates :bounce_reason, presence: true + + def diagnostic + response_json['diagnosticCode'] + end + + def action + response_json['action'] + end + + def status + response_json['status'] + end + + def self.record(json) + bounced_records = json['bounce']['bouncedRecipients'] + bounced_records.each do |record| + bounce_record = BouncedMailAddress.new(email: record['emailAddress'], response_json: record) + bounce_record.save + end + end end diff --git a/app/views/admin/bounced_mail_addresses/index.html.erb b/app/views/admin/bounced_mail_addresses/index.html.erb index c2f95ca68..b6a019282 100644 --- a/app/views/admin/bounced_mail_addresses/index.html.erb +++ b/app/views/admin/bounced_mail_addresses/index.html.erb @@ -2,32 +2,37 @@

Bounced Mail Addresses

- - - - - - - - - - +
+
+
+
+
EmailBounce reasonIncidentsResponse json
+ + + + + + + + + + - - <% @bounced_mail_addresses.each do |mail_addr| %> - - - - - - - - - - <% end %> - -
EmailActionStatusDiagnosticTrackedActions
<%= mail_addr.email %><%= mail_addr.bounce_reason %><%= mail_addr.incidents %><%= mail_addr.response_json %><%= link_to 'Show', admin_bounced_mail_address_path(mail_addr) %><%= link_to 'Edit', edit_admin_bounced_mail_address_path(mail_addr) %><%= link_to 'Destroy', admin_bounced_mail_address_path(mail_addr), method: :delete, data: { confirm: 'Are you sure?' } %>
- -
- -<%= link_to 'New Bounced Mail Address', new_admin_bounced_mail_address_path %> + + <% @bounced_mail_addresses.each do |mail_addr| %> + + <%= mail_addr.email %> + <%= mail_addr.action %> + <%= mail_addr.status %> + <%= mail_addr.diagnostic %> + <%= mail_addr.created_at %> + <%= link_to 'Detailed', admin_bounced_mail_address_path(mail_addr) %> + <%= link_to 'Destroy', admin_bounced_mail_address_path(mail_addr), method: :delete, data: { confirm: 'Are you sure?' } %> + + <% end %> + + + + + + From 834b2c95bc7e56c2c4ba15a60d64f7cd9b89cbfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 14:21:25 +0300 Subject: [PATCH 06/35] Add full request JSON to bounced mail, remove unused views --- .../bounced_mail_addresses_controller.rb | 35 +----------------- app/models/bounced_mail_address.rb | 15 ++++++-- app/views/admin/base/_menu.haml | 1 + .../bounced_mail_addresses/_form.html.erb | 37 ------------------- .../bounced_mail_addresses/edit.html.erb | 6 --- .../admin/bounced_mail_addresses/new.html.erb | 5 --- .../bounced_mail_addresses/show.html.erb | 10 +++-- config/locales/admin/menu.en.yml | 1 + ..._recipient_json_to_bounced_mail_address.rb | 5 +++ db/structure.sql | 6 ++- 10 files changed, 31 insertions(+), 90 deletions(-) delete mode 100644 app/views/admin/bounced_mail_addresses/_form.html.erb delete mode 100644 app/views/admin/bounced_mail_addresses/edit.html.erb delete mode 100644 app/views/admin/bounced_mail_addresses/new.html.erb create mode 100644 db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb diff --git a/app/controllers/admin/bounced_mail_addresses_controller.rb b/app/controllers/admin/bounced_mail_addresses_controller.rb index 551413e2c..da9421450 100644 --- a/app/controllers/admin/bounced_mail_addresses_controller.rb +++ b/app/controllers/admin/bounced_mail_addresses_controller.rb @@ -1,47 +1,16 @@ module Admin class BouncedMailAddressesController < BaseController - before_action :set_bounced_mail_address, only: %i[show edit update destroy] + before_action :set_bounced_mail_address, only: %i[show destroy] load_and_authorize_resource # GET /bounced_mail_addresses def index - @bounced_mail_addresses = BouncedMailAddress.all + @bounced_mail_addresses = BouncedMailAddress.all.order(created_at: :desc) end # GET /bounced_mail_addresses/1 def show; end - # GET /bounced_mail_addresses/new - def new - @bounced_mail_address = BouncedMailAddress.new - end - - # GET /bounced_mail_addresses/1/edit - def edit; end - - # POST /bounced_mail_addresses - def create - @bounced_mail_address = BouncedMailAddress.new(bounced_mail_address_params) - - if @bounced_mail_address.save - redirect_to( - admin_bounced_mail_addresses_url, - notice: 'Bounced mail address was successfully created.' - ) - else - render(:new) - end - end - - # PATCH/PUT /bounced_mail_addresses/1 - def update - if @bounced_mail_address.update(bounced_mail_address_params) - redirect_to(@bounced_mail_address, notice: 'Bounced mail address was successfully updated.') - else - render(:edit) - end - end - # DELETE /bounced_mail_addresses/1 def destroy @bounced_mail_address.destroy diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index 6609829da..78f37c374 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -1,23 +1,30 @@ class BouncedMailAddress < ApplicationRecord validates :email, presence: true validates :bounce_reason, presence: true + before_validation :assign_bounce_reason + + def assign_bounce_reason + self.bounce_reason = "#{action} (#{status} #{diagnostic})" + end def diagnostic - response_json['diagnosticCode'] + recipient_json['diagnosticCode'] end def action - response_json['action'] + recipient_json['action'] end def status - response_json['status'] + recipient_json['status'] end def self.record(json) bounced_records = json['bounce']['bouncedRecipients'] bounced_records.each do |record| - bounce_record = BouncedMailAddress.new(email: record['emailAddress'], response_json: record) + bounce_record = BouncedMailAddress.new(email: record['emailAddress'], recipient_json: record, + response_json: json) + bounce_record.save end end diff --git a/app/views/admin/base/_menu.haml b/app/views/admin/base/_menu.haml index a327419fd..5853bd3e6 100644 --- a/app/views/admin/base/_menu.haml +++ b/app/views/admin/base/_menu.haml @@ -33,6 +33,7 @@ %li= link_to t('.blocked_domains'), admin_blocked_domains_path %li= link_to t('.reserved_domains'), admin_reserved_domains_path %li= link_to t('.disputed_domains'), admin_disputes_path + %li= link_to t('.bounced_email_addresses'), admin_bounced_mail_addresses_path %li= link_to t('.epp_log'), admin_epp_logs_path(created_after: 'today') %li= link_to t('.repp_log'), admin_repp_logs_path(created_after: 'today') %li= link_to t('.que'), '/admin/que' diff --git a/app/views/admin/bounced_mail_addresses/_form.html.erb b/app/views/admin/bounced_mail_addresses/_form.html.erb deleted file mode 100644 index 7a384bd3f..000000000 --- a/app/views/admin/bounced_mail_addresses/_form.html.erb +++ /dev/null @@ -1,37 +0,0 @@ -<%= form_for([:admin, @bounced_mail_address], html: { class: 'form-horizontal' }) do |form| %> - <% if @bounced_mail_address.errors.any? %> -
-

<%= pluralize(bounced_mail_address.errors.count, "error") %> prohibited this bounced_mail_address from being saved:

- -
    - <% @bounced_mail_address.errors.full_messages.each do |message| %> -
  • <%= message %>
  • - <% end %> -
-
- <% end %> - -
- <%= form.label :email %> - <%= form.text_field :email %> -
- -
- <%= form.label :bounce_reason %> - <%= form.text_field :bounce_reason %> -
- -
- <%= form.label :incidents %> - <%= form.number_field :incidents %> -
- -
- <%= form.label :response_json %> - <%= form.text_field :response_json %> -
- -
- <%= form.submit %> -
-<% end %> diff --git a/app/views/admin/bounced_mail_addresses/edit.html.erb b/app/views/admin/bounced_mail_addresses/edit.html.erb deleted file mode 100644 index a3dfe2d84..000000000 --- a/app/views/admin/bounced_mail_addresses/edit.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -

Editing Bounced Mail Address

- -<%= render 'form', bounced_mail_address: @bounced_mail_address %> - -<%= link_to 'Show', admin_bounced_mail_address_path(@bounced_mail_address)%> | -<%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/app/views/admin/bounced_mail_addresses/new.html.erb b/app/views/admin/bounced_mail_addresses/new.html.erb deleted file mode 100644 index 010ac79dc..000000000 --- a/app/views/admin/bounced_mail_addresses/new.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -

New Bounced Mail Address

- -<%= render 'form', bounced_mail_address: @bounced_mail_address %> - -<%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/app/views/admin/bounced_mail_addresses/show.html.erb b/app/views/admin/bounced_mail_addresses/show.html.erb index 1f48ad5cf..752bd7b0a 100644 --- a/app/views/admin/bounced_mail_addresses/show.html.erb +++ b/app/views/admin/bounced_mail_addresses/show.html.erb @@ -16,9 +16,13 @@

- Response json: - <%= @bounced_mail_address.response_json %> + Bounced recipient JSON: +

<%= JSON.pretty_generate(@bounced_mail_address.recipient_json) %>
+

+ +

+ Bounce payload: +

<%= JSON.pretty_generate(@bounced_mail_address.response_json) %>

-<%= link_to 'Edit', edit_admin_bounced_mail_address_path(@bounced_mail_address) %> | <%= link_to 'Back', admin_bounced_mail_addresses_path %> diff --git a/config/locales/admin/menu.en.yml b/config/locales/admin/menu.en.yml index 617341c6a..cb1060e6f 100644 --- a/config/locales/admin/menu.en.yml +++ b/config/locales/admin/menu.en.yml @@ -14,6 +14,7 @@ en: blocked_domains: Blocked domains reserved_domains: Reserved domains disputed_domains: Disputed domains + bounced_email_addresses: Bounced emails epp_log: EPP log repp_log: REPP log que: Que diff --git a/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb b/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb new file mode 100644 index 000000000..bad3d846e --- /dev/null +++ b/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb @@ -0,0 +1,5 @@ +class AddRecipientJsonToBouncedMailAddress < ActiveRecord::Migration[6.0] + def change + add_column :bounced_mail_addresses, :recipient_json, :jsonb, null: false + end +end diff --git a/db/structure.sql b/db/structure.sql index 3ec055386..23669c665 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -486,7 +486,8 @@ CREATE TABLE public.bounced_mail_addresses ( incidents integer DEFAULT 1 NOT NULL, response_json jsonb, created_at timestamp(6) without time zone NOT NULL, - updated_at timestamp(6) without time zone NOT NULL + updated_at timestamp(6) without time zone NOT NULL, + recipient_json jsonb NOT NULL ); @@ -4956,6 +4957,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200908131554'), ('20200910085157'), ('20200910102028'), -('20200916125326'); +('20200916125326'), +('20200917104213'); From b2c5a9a5ec69ac8b27e7fa7ab8fef96f292f1485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 15:55:37 +0300 Subject: [PATCH 07/35] Verify param integrity for bounces --- app/controllers/api/v1/bounces_controller.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/bounces_controller.rb b/app/controllers/api/v1/bounces_controller.rb index cff8c3efe..40a3c1c91 100644 --- a/app/controllers/api/v1/bounces_controller.rb +++ b/app/controllers/api/v1/bounces_controller.rb @@ -1,11 +1,20 @@ module Api module V1 class BouncesController < BaseController - before_action :authenticate - + # POST api/v1/bounces/ def create - bounced_mail_address = BouncedMailAddress.record(json) - bounced_mail_address ? render(head: :ok) : render(head: :failed) + BouncedMailAddress.record(bounce_params) + head(:ok) + rescue ActionController::ParameterMissing + head(:bad_request) + end + + def bounce_params + params.require(:data).require(:bounce).require(:bouncedRecipients).each do |r| + r.require(:emailAddress) + end + + params.require(:data) end end end From 03182f92227acceae7c1cf6a08e192e1c44472d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 17 Sep 2020 16:26:50 +0300 Subject: [PATCH 08/35] Implement shared key authentication to bounces API --- app/controllers/api/v1/base_controller.rb | 5 +++++ app/controllers/api/v1/bounces_controller.rb | 4 +++- config/application.yml.sample | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index 54930edf9..b62b3e063 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/app/controllers/api/v1/base_controller.rb @@ -10,6 +10,11 @@ module Api head :unauthorized unless ip_allowed end + def authenticate_shared_key + api_key = "Basic #{ENV['api_shared_key']}" + head(:unauthorized) unless api_key == request.authorization + end + def not_found_error uuid = params['uuid'] json = { error: 'Not Found', uuid: uuid, message: 'Record not found' } diff --git a/app/controllers/api/v1/bounces_controller.rb b/app/controllers/api/v1/bounces_controller.rb index 40a3c1c91..296c9d9bd 100644 --- a/app/controllers/api/v1/bounces_controller.rb +++ b/app/controllers/api/v1/bounces_controller.rb @@ -1,10 +1,12 @@ module Api module V1 class BouncesController < BaseController + before_action :authenticate_shared_key + # POST api/v1/bounces/ def create BouncedMailAddress.record(bounce_params) - head(:ok) + head(:created) rescue ActionController::ParameterMissing head(:bad_request) end diff --git a/config/application.yml.sample b/config/application.yml.sample index 72b55e2ea..237617be3 100644 --- a/config/application.yml.sample +++ b/config/application.yml.sample @@ -87,6 +87,9 @@ sk_digi_doc_service_name: 'Testimine' registrant_api_base_url: registrant_api_auth_allowed_ips: '127.0.0.1, 0.0.0.0' #ips, separated with commas +# Bounces API +api_shared_key: testkey + # # MISC From 036a3a37207f4a89d01aad6361e59bd7d7297ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 18 Sep 2020 10:52:14 +0300 Subject: [PATCH 09/35] Return empty user object when authorized user not found --- app/controllers/registrar/sessions_controller.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 5bebe5619..709e66955 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -158,12 +158,15 @@ class Registrar def find_user_by_idc_and_allowed(idc) return User.new unless idc + possible_users = ApiUser.where(identity_code: idc) || User.new possible_users.each do |selected_user| - if selected_user.registrar.white_ips.registrar_area.include_ip?(request.ip) - return selected_user - end + next unless selected_user.registrar.white_ips.registrar_area.include_ip?(request.ip) + + return selected_user end + + User.new end def check_ip_restriction From 7d8c24533af3727b0817db6822b89eb35b373472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 18 Sep 2020 14:44:40 +0300 Subject: [PATCH 10/35] Create unit tests for bounced mail address --- app/models/bounced_mail_address.rb | 11 ++++- test/models/bounced_mail_address_test.rb | 61 ++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 test/models/bounced_mail_address_test.rb diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index 78f37c374..f12c7f19c 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -1,21 +1,28 @@ class BouncedMailAddress < ApplicationRecord validates :email, presence: true - validates :bounce_reason, presence: true + validates :bounce_reason, :recipient_json, :response_json, presence: true before_validation :assign_bounce_reason def assign_bounce_reason - self.bounce_reason = "#{action} (#{status} #{diagnostic})" + if recipient_json + self.bounce_reason = "#{action} (#{status} #{diagnostic})" + else + self.bounce_reason = nil + end end def diagnostic + return nil unless recipient_json recipient_json['diagnosticCode'] end def action + return nil unless recipient_json recipient_json['action'] end def status + return nil unless recipient_json recipient_json['status'] end diff --git a/test/models/bounced_mail_address_test.rb b/test/models/bounced_mail_address_test.rb new file mode 100644 index 000000000..2caea7d98 --- /dev/null +++ b/test/models/bounced_mail_address_test.rb @@ -0,0 +1,61 @@ +require 'test_helper' + +class BouncedMailAddressTest < ActiveSupport::TestCase + include ActionMailer::TestHelper + + def setup + @bounced_mail = BouncedMailAddress.new + @bounced_mail.email = 'recipient@registry.test' + @bounced_mail.bounce_reason = 'failed (5.1.1 smtp; 550 5.1.1 user unknown)' + @bounced_mail.response_json = {"mail"=>{"source"=>"noreply@internet.test", "sourceIp"=>"195.43.86.5", "messageId"=>"010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", "sourceArn"=>"arn:aws:ses:us-east-2:650268220328:identity/noreply@internet.test", "timestamp"=>"2020-09-18T10:34:44.000Z", "destination"=>["#{@bounced_mail.email}"], "sendingAccountId"=>"650268220328"}, "bounce"=>{"timestamp"=>"2020-09-18T10:34:44.911Z", "bounceType"=>"Permanent", "feedbackId"=>"010f0174a0c7d4f9-27d59756-6111-xxxx-a507-26bee0d55fa2-000000", "remoteMtaIp"=>"127.0.0.1", "reportingMTA"=>"dsn; xxx.amazonses.com", "bounceSubType"=>"General", "bouncedRecipients"=>[{"action"=>"failed", "status"=>"5.1.1", "emailAddress"=>"#{@bounced_mail.email}", "diagnosticCode"=>"smtp; 550 5.1.1 user unknown"}]}, "notificationType"=>"Bounce"} + @bounced_mail.recipient_json = {"action"=>"failed", "status"=>"5.1.1", "emailAddress"=>"#{@bounced_mail.email}", "diagnosticCode"=>"smtp; 550 5.1.1 user unknown"} + + end + + def test_bounce_reason_is_autoassigned + assert @bounced_mail.valid? + @bounced_mail.bounce_reason = nil + assert @bounced_mail.valid? + + assert_equal 'failed (5.1.1 smtp; 550 5.1.1 user unknown)', @bounced_mail.bounce_reason + end + + def test_response_json_is_required + assert @bounced_mail.valid? + @bounced_mail.response_json = nil + assert_not @bounced_mail.valid? + assert @bounced_mail.errors.full_messages.include? 'Response json is missing' + end + + def test_recipient_json_is_required + assert @bounced_mail.valid? + @bounced_mail.recipient_json = nil + assert_not @bounced_mail.valid? + + assert @bounced_mail.errors.full_messages.include? 'Recipient json is missing' + end + + def test_status_is_determined_dynamically + assert @bounced_mail.valid? + assert_equal '5.1.1', @bounced_mail.status + @bounced_mail.recipient_json['status'] = 'xxx_status' + assert_equal 'xxx_status', @bounced_mail.status + end + + def test_action_is_determined_dynamically + assert @bounced_mail.valid? + assert_equal 'failed', @bounced_mail.action + @bounced_mail.recipient_json['action'] = 'xxx_action' + assert_equal 'xxx_action', @bounced_mail.action + end + + def test_diagnostic_is_determined_dynamically + assert @bounced_mail.valid? + assert_equal 'smtp; 550 5.1.1 user unknown', @bounced_mail.diagnostic + @bounced_mail.recipient_json['diagnosticCode'] = 'xxx_diagnostic' + assert_equal 'xxx_diagnostic', @bounced_mail.diagnostic + end + + def test_creates_objects_from_sns_json + end +end From 64308e11040fcc172ce2dd30b25a7179258e58a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 18 Sep 2020 14:49:27 +0300 Subject: [PATCH 11/35] Fix CC issues --- .../registrar/sessions_controller.rb | 8 ++-- app/models/bounced_mail_address.rb | 17 +++++---- test/models/bounced_mail_address_test.rb | 38 +++++++++++++++++++ 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 709e66955..df90ea57b 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -161,12 +161,10 @@ class Registrar possible_users = ApiUser.where(identity_code: idc) || User.new possible_users.each do |selected_user| - next unless selected_user.registrar.white_ips.registrar_area.include_ip?(request.ip) - - return selected_user + if selected_user.registrar.white_ips.registrar_area.include_ip?(request.ip) + return selected_user + end end - - User.new end def check_ip_restriction diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index f12c7f19c..02bb42337 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -4,25 +4,26 @@ class BouncedMailAddress < ApplicationRecord before_validation :assign_bounce_reason def assign_bounce_reason - if recipient_json - self.bounce_reason = "#{action} (#{status} #{diagnostic})" - else - self.bounce_reason = nil - end + return self.bounce_reason = nil unless recipient_json + + self.bounce_reason = "#{action} (#{status} #{diagnostic})" end def diagnostic - return nil unless recipient_json + return unless recipient_json + recipient_json['diagnosticCode'] end def action - return nil unless recipient_json + return unless recipient_json + recipient_json['action'] end def status - return nil unless recipient_json + return unless recipient_json + recipient_json['status'] end diff --git a/test/models/bounced_mail_address_test.rb b/test/models/bounced_mail_address_test.rb index 2caea7d98..0e6a62ab8 100644 --- a/test/models/bounced_mail_address_test.rb +++ b/test/models/bounced_mail_address_test.rb @@ -57,5 +57,43 @@ class BouncedMailAddressTest < ActiveSupport::TestCase end def test_creates_objects_from_sns_json + BouncedMailAddress.record(sns_bounce_payload) + + bounced_mail = BouncedMailAddress.last + assert_equal domains(:shop).registrant.email, bounced_mail.email + assert_equal 'failed', bounced_mail.action + assert_equal '5.1.1', bounced_mail.status + assert_equal 'smtp; 550 5.1.1 user unknown', bounced_mail.diagnostic + end + + def sns_bounce_payload + { + "notificationType": "Bounce", + "mail": { + "source": "noreply@registry.test", + "sourceIp": "195.43.86.5", + "messageId": "010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", + "sourceArn": "arn:aws:ses:us-east-2:65026820000:identity/noreply@registry.test", + "timestamp": "2020-09-18T10:34:44.000Z", + "destination": [ "#{domains(:shop).registrant.email}" ], + "sendingAccountId": "650268220000" + }, + "bounce": { + "timestamp": "2020-09-18T10:34:44.911Z", + "bounceType": "Permanent", + "feedbackId": "010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000", + "remoteMtaIp": "127.0.01", + "reportingMTA": "dsn; xxx.amazonses.com", + "bounceSubType": "General", + "bouncedRecipients": [ + { + "action": "failed", + "status": "5.1.1", + "emailAddress": "#{domains(:shop).registrant.email}", + "diagnosticCode": "smtp; 550 5.1.1 user unknown" + } + ] + } + }.as_json end end From 101d5d4a7846fea4c3e242dcf3c57af5eb4b5bd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 18 Sep 2020 16:17:42 +0300 Subject: [PATCH 12/35] Add tests for admin bounced mails views --- .../bounced_mail_addresses_controller.rb | 10 ----- .../bounced_mail_addresses/index.html.erb | 2 - .../bounced_mail_addresses/show.html.erb | 4 +- test/fixtures/bounced_mail_addresses.yml | 40 ++++++++++++++++++ .../admin_area/bounced_mail_addresses_test.rb | 41 +++++++++++++++++++ 5 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 test/fixtures/bounced_mail_addresses.yml create mode 100644 test/system/admin_area/bounced_mail_addresses_test.rb diff --git a/app/controllers/admin/bounced_mail_addresses_controller.rb b/app/controllers/admin/bounced_mail_addresses_controller.rb index da9421450..1c59acaa4 100644 --- a/app/controllers/admin/bounced_mail_addresses_controller.rb +++ b/app/controllers/admin/bounced_mail_addresses_controller.rb @@ -26,15 +26,5 @@ module Admin def set_bounced_mail_address @bounced_mail_address = BouncedMailAddress.find(params[:id]) end - - # Only allow a trusted parameter "white list" through. - def bounced_mail_address_params - params.require(:bounced_mail_address).permit( - :email, - :bounce_reason, - :incidents, - :response_json - ) - end end end diff --git a/app/views/admin/bounced_mail_addresses/index.html.erb b/app/views/admin/bounced_mail_addresses/index.html.erb index b6a019282..913cbd19d 100644 --- a/app/views/admin/bounced_mail_addresses/index.html.erb +++ b/app/views/admin/bounced_mail_addresses/index.html.erb @@ -1,5 +1,3 @@ -

<%= notice %>

-

Bounced Mail Addresses

diff --git a/app/views/admin/bounced_mail_addresses/show.html.erb b/app/views/admin/bounced_mail_addresses/show.html.erb index 752bd7b0a..9b0b5919f 100644 --- a/app/views/admin/bounced_mail_addresses/show.html.erb +++ b/app/views/admin/bounced_mail_addresses/show.html.erb @@ -1,6 +1,3 @@ -

<%= notice %>

- -

Email: <%= @bounced_mail_address.email %>

@@ -26,3 +23,4 @@

<%= link_to 'Back', admin_bounced_mail_addresses_path %> +<%= link_to 'Destroy', admin_bounced_mail_address_path(@bounced_mail_address), method: :delete, data: { confirm: 'Are you sure?' } %> diff --git a/test/fixtures/bounced_mail_addresses.yml b/test/fixtures/bounced_mail_addresses.yml new file mode 100644 index 000000000..1261f1429 --- /dev/null +++ b/test/fixtures/bounced_mail_addresses.yml @@ -0,0 +1,40 @@ +one: + email: bounced@registry.test + bounce_reason: failed (5.1.1 smtp; 550 5.1.1 user unknown) + incidents: 1 + recipient_json: { + "action": "failed", + "status": "5.1.1", + "emailAddress": "bounced@registry.test", + "diagnosticCode": "smtp; 550 5.1.1 user unknown" + } + response_json: { + "notificationType": "Bounce", + "mail": { + "source": "noreply@registry.test", + "sourceIp": "195.43.86.5", + "messageId": "010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", + "sourceArn": "arn:aws:ses:us-east-2:65026820000:identity/noreply@registry.test", + "timestamp": "2020-09-18T10:34:44.000Z", + "destination": [ "bounced@registry.test" ], + "sendingAccountId": "650268220000" + }, + "bounce": { + "timestamp": "2020-09-18T10:34:44.911Z", + "bounceType": "Permanent", + "feedbackId": "010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000", + "remoteMtaIp": "127.0.01", + "reportingMTA": "dsn; xxx.amazonses.com", + "bounceSubType": "General", + "bouncedRecipients": [ + { + "action": "failed", + "status": "5.1.1", + "emailAddress": "bounced@registry.test", + "diagnosticCode": "smtp; 550 5.1.1 user unknown" + } + ] + } + } + created_at: <%= Time.zone.parse('2010-07-05').to_s(:db) %> + updated_at: <%= Time.zone.parse('2010-07-05').to_s(:db) %> diff --git a/test/system/admin_area/bounced_mail_addresses_test.rb b/test/system/admin_area/bounced_mail_addresses_test.rb new file mode 100644 index 000000000..5500f4375 --- /dev/null +++ b/test/system/admin_area/bounced_mail_addresses_test.rb @@ -0,0 +1,41 @@ +require 'application_system_test_case' + +class AdminBouncedMailAddressesTest < ApplicationSystemTestCase + include ActionView::Helpers::NumberHelper + + def setup + @bounced_mail = bounced_mail_addresses(:one) + @original_default_language = Setting.default_language + sign_in users(:admin) + end + + def teardown + Setting.default_language = @original_default_language + end + + def test_shows_bounced_emails + visit admin_bounced_mail_addresses_path + assert_text @bounced_mail.status + assert_text @bounced_mail.action + assert_text @bounced_mail.diagnostic + assert_text @bounced_mail.email + end + + def test_shows_detailed_bounced_email + visit admin_bounced_mail_address_path(@bounced_mail) + assert_text @bounced_mail.status + assert_text @bounced_mail.action + assert_text @bounced_mail.diagnostic + assert_text @bounced_mail.email + + assert_text 'bouncedRecipients' + assert_text '010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000' + end + + def test_deletes_registrar + visit admin_bounced_mail_address_path(@bounced_mail) + click_on 'Destroy' + + assert_text 'Bounced mail address was successfully destroyed.' + end +end From 6a93395fa493252f5f717799a0f5943257e1ddb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 21 Sep 2020 11:37:31 +0300 Subject: [PATCH 13/35] Add integration tests for Bounces API --- app/controllers/api/v1/bounces_controller.rb | 2 + .../integration/api/v1/bounces/create_test.rb | 75 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 test/integration/api/v1/bounces/create_test.rb diff --git a/app/controllers/api/v1/bounces_controller.rb b/app/controllers/api/v1/bounces_controller.rb index 296c9d9bd..fd10a3398 100644 --- a/app/controllers/api/v1/bounces_controller.rb +++ b/app/controllers/api/v1/bounces_controller.rb @@ -5,6 +5,8 @@ module Api # POST api/v1/bounces/ def create + return head(:bad_request) unless bounce_params[:bounce][:bouncedRecipients].any? + BouncedMailAddress.record(bounce_params) head(:created) rescue ActionController::ParameterMissing diff --git a/test/integration/api/v1/bounces/create_test.rb b/test/integration/api/v1/bounces/create_test.rb new file mode 100644 index 000000000..899b6c5c7 --- /dev/null +++ b/test/integration/api/v1/bounces/create_test.rb @@ -0,0 +1,75 @@ +require 'test_helper' + +class BouncesApiV1CreateTest < ActionDispatch::IntegrationTest + def setup + @api_key = "Basic #{ENV['api_shared_key']}" + @headers = { "Authorization": "#{@api_key}" } + @json_body = { "data": valid_bounce_request }.as_json + end + + def test_authorizes_api_request + post api_v1_bounces_path, params: @json_body, headers: @headers + assert_response :created + + invalid_headers = { "Authorization": "Basic invalid_api_key" } + post api_v1_bounces_path, params: @json_body, headers: invalid_headers + assert_response :unauthorized + end + + def test_returns_bad_request_if_invalid_payload + invalid_json_body = @json_body.dup + invalid_json_body['data']['bounce']['bouncedRecipients'] = nil + + post api_v1_bounces_path, params: invalid_json_body, headers: @headers + assert_response :bad_request + + invalid_json_body = 'aaaa' + post api_v1_bounces_path, params: invalid_json_body, headers: @headers + assert_response :bad_request + end + + def test_saves_new_bounce_object + request_body = @json_body.dup + random_mail = "#{rand(10000..99999)}@registry.test" + request_body['data']['bounce']['bouncedRecipients'][0]['emailAddress'] = random_mail + + post api_v1_bounces_path, params: request_body, headers: @headers + assert_response :created + + bounced_mail = BouncedMailAddress.last + assert bounced_mail.email = random_mail + assert '5.1.1', bounced_mail.status + assert 'failed', bounced_mail.action + end + + def valid_bounce_request + { + "notificationType": "Bounce", + "mail": { + "source": "noreply@registry.test", + "sourceIp": "195.43.86.5", + "messageId": "010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", + "sourceArn": "arn:aws:ses:us-east-2:65026820000:identity/noreply@registry.test", + "timestamp": "2020-09-18T10:34:44.000Z", + "destination": [ "bounced@registry.test" ], + "sendingAccountId": "650268220000" + }, + "bounce": { + "timestamp": "2020-09-18T10:34:44.911Z", + "bounceType": "Permanent", + "feedbackId": "010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000", + "remoteMtaIp": "127.0.01", + "reportingMTA": "dsn; xxx.amazonses.com", + "bounceSubType": "General", + "bouncedRecipients": [ + { + "action": "failed", + "status": "5.1.1", + "emailAddress": "bounced@registry.test", + "diagnosticCode": "smtp; 550 5.1.1 user unknown" + } + ] + } + }.as_json + end +end From 818869d249d4e5ebdf1945f454968a0ebcd8c78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 21 Sep 2020 11:46:42 +0300 Subject: [PATCH 14/35] Remove incidents count from bounced mails --- app/views/admin/bounced_mail_addresses/show.html.erb | 5 ----- ...84356_remove_incidents_from_bounced_mail_addresses.rb | 9 +++++++++ db/structure.sql | 4 ++-- test/fixtures/bounced_mail_addresses.yml | 1 - 4 files changed, 11 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb diff --git a/app/views/admin/bounced_mail_addresses/show.html.erb b/app/views/admin/bounced_mail_addresses/show.html.erb index 9b0b5919f..98eeabcd2 100644 --- a/app/views/admin/bounced_mail_addresses/show.html.erb +++ b/app/views/admin/bounced_mail_addresses/show.html.erb @@ -7,11 +7,6 @@ <%= @bounced_mail_address.bounce_reason %>

-

- Incidents: - <%= @bounced_mail_address.incidents %> -

-

Bounced recipient JSON:

<%= JSON.pretty_generate(@bounced_mail_address.recipient_json) %>
diff --git a/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb b/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb new file mode 100644 index 000000000..0704795df --- /dev/null +++ b/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb @@ -0,0 +1,9 @@ +class RemoveIncidentsFromBouncedMailAddresses < ActiveRecord::Migration[6.0] + def up + remove_column :bounced_mail_addresses, :incidents + end + + def down + add_column :bounced_mail_addresses, :incidents, :integer, null: false, default: 1 + end +end diff --git a/db/structure.sql b/db/structure.sql index 23669c665..74e784408 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -483,7 +483,6 @@ CREATE TABLE public.bounced_mail_addresses ( id bigint NOT NULL, email character varying NOT NULL, bounce_reason character varying NOT NULL, - incidents integer DEFAULT 1 NOT NULL, response_json jsonb, created_at timestamp(6) without time zone NOT NULL, updated_at timestamp(6) without time zone NOT NULL, @@ -4958,6 +4957,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200910085157'), ('20200910102028'), ('20200916125326'), -('20200917104213'); +('20200917104213'), +('20200921084356'); diff --git a/test/fixtures/bounced_mail_addresses.yml b/test/fixtures/bounced_mail_addresses.yml index 1261f1429..345b2ef22 100644 --- a/test/fixtures/bounced_mail_addresses.yml +++ b/test/fixtures/bounced_mail_addresses.yml @@ -1,7 +1,6 @@ one: email: bounced@registry.test bounce_reason: failed (5.1.1 smtp; 550 5.1.1 user unknown) - incidents: 1 recipient_json: { "action": "failed", "status": "5.1.1", From fae620c19d87fc4df9710be88e3608a4192b29a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 21 Sep 2020 13:12:23 +0300 Subject: [PATCH 15/35] Define routing scope for :bounced_mail_addresses --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 89f8f0cd6..9938403e7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -299,7 +299,7 @@ Rails.application.routes.draw do resources :delayed_jobs resources :epp_logs resources :repp_logs - resources :bounced_mail_addresses + resources :bounced_mail_addresses, only: %i[index show destroy] authenticate :admin_user do mount Que::Web, at: 'que' From 659cb7f4e67ee51545b6ea0ce5b20e030e881591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 21 Sep 2020 13:34:34 +0300 Subject: [PATCH 16/35] Combine migrations, remove json objects from bounced mails --- ...0200916125326_create_bounced_mail_addresses.rb | 9 ++++++--- ..._add_recipient_json_to_bounced_mail_address.rb | 5 ----- ...emove_incidents_from_bounced_mail_addresses.rb | 9 --------- db/structure.sql | 15 ++++++++------- 4 files changed, 14 insertions(+), 24 deletions(-) delete mode 100644 db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb delete mode 100644 db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb diff --git a/db/migrate/20200916125326_create_bounced_mail_addresses.rb b/db/migrate/20200916125326_create_bounced_mail_addresses.rb index c6600afea..e1744cc9a 100644 --- a/db/migrate/20200916125326_create_bounced_mail_addresses.rb +++ b/db/migrate/20200916125326_create_bounced_mail_addresses.rb @@ -2,9 +2,12 @@ class CreateBouncedMailAddresses < ActiveRecord::Migration[6.0] def change create_table :bounced_mail_addresses do |t| t.string :email, null: false - t.string :bounce_reason, null: false - t.integer :incidents, null: false, default: 1 - t.jsonb :response_json + t.string :message_id, null: false + t.string :bounce_type, null: false + t.string :bounce_subtype, null: false + t.string :action, null: false + t.string :status, null: false + t.string :diagnostic, null: true t.timestamps end diff --git a/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb b/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb deleted file mode 100644 index bad3d846e..000000000 --- a/db/migrate/20200917104213_add_recipient_json_to_bounced_mail_address.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddRecipientJsonToBouncedMailAddress < ActiveRecord::Migration[6.0] - def change - add_column :bounced_mail_addresses, :recipient_json, :jsonb, null: false - end -end diff --git a/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb b/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb deleted file mode 100644 index 0704795df..000000000 --- a/db/migrate/20200921084356_remove_incidents_from_bounced_mail_addresses.rb +++ /dev/null @@ -1,9 +0,0 @@ -class RemoveIncidentsFromBouncedMailAddresses < ActiveRecord::Migration[6.0] - def up - remove_column :bounced_mail_addresses, :incidents - end - - def down - add_column :bounced_mail_addresses, :incidents, :integer, null: false, default: 1 - end -end diff --git a/db/structure.sql b/db/structure.sql index 74e784408..e064d968f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -482,11 +482,14 @@ ALTER SEQUENCE public.blocked_domains_id_seq OWNED BY public.blocked_domains.id; CREATE TABLE public.bounced_mail_addresses ( id bigint NOT NULL, email character varying NOT NULL, - bounce_reason character varying NOT NULL, - response_json jsonb, + message_id character varying NOT NULL, + bounce_type character varying NOT NULL, + bounce_subtype character varying NOT NULL, + action character varying NOT NULL, + status character varying NOT NULL, + diagnostic character varying, created_at timestamp(6) without time zone NOT NULL, - updated_at timestamp(6) without time zone NOT NULL, - recipient_json jsonb NOT NULL + updated_at timestamp(6) without time zone NOT NULL ); @@ -4956,8 +4959,6 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200908131554'), ('20200910085157'), ('20200910102028'), -('20200916125326'), -('20200917104213'), -('20200921084356'); +('20200916125326'); From 98674ab3817148e6c5e15acd7b48a956e3ef58d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 21 Sep 2020 13:47:57 +0300 Subject: [PATCH 17/35] Reflect new bounced mail structure --- app/models/bounced_mail_address.rb | 36 ++++++++----------- .../bounced_mail_addresses/show.html.erb | 18 ++++++---- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index 02bb42337..61679f543 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -1,6 +1,5 @@ class BouncedMailAddress < ApplicationRecord - validates :email, presence: true - validates :bounce_reason, :recipient_json, :response_json, presence: true + validates :email, :message_id, :bounce_type, :bounce_subtype, :action, :status, presence: true before_validation :assign_bounce_reason def assign_bounce_reason @@ -9,31 +8,24 @@ class BouncedMailAddress < ApplicationRecord self.bounce_reason = "#{action} (#{status} #{diagnostic})" end - def diagnostic - return unless recipient_json - - recipient_json['diagnosticCode'] - end - - def action - return unless recipient_json - - recipient_json['action'] - end - - def status - return unless recipient_json - - recipient_json['status'] - end - def self.record(json) bounced_records = json['bounce']['bouncedRecipients'] bounced_records.each do |record| - bounce_record = BouncedMailAddress.new(email: record['emailAddress'], recipient_json: record, - response_json: json) + bounce_record = BouncedMailAddress.new(params_from_json(json, record)) bounce_record.save end end + + def params_from_json(json, bounced_record) + { + email: bounced_record['emailAddress'], + message_id: json['mail']['messageId'], + bounce_type: json['bounce']['bounceType'], + bounce_subtype: json['bounce']['bounceSubType'], + action: bounced_record['action'], + status: bounced_record['status'], + diagnostic: bounced_record['diagnosticCode'], + } + end end diff --git a/app/views/admin/bounced_mail_addresses/show.html.erb b/app/views/admin/bounced_mail_addresses/show.html.erb index 98eeabcd2..5183ae5a1 100644 --- a/app/views/admin/bounced_mail_addresses/show.html.erb +++ b/app/views/admin/bounced_mail_addresses/show.html.erb @@ -1,20 +1,26 @@ +

Email: <%= @bounced_mail_address.email %>

- Bounce reason: - <%= @bounced_mail_address.bounce_reason %> + Bounced message ID: + <%= @bounced_mail_address.message_id %>

- Bounced recipient JSON: -

<%= JSON.pretty_generate(@bounced_mail_address.recipient_json) %>
+ Overall bounce type: + <%= @bounced_mail_address.bounce_type %> (<%= @bounced_mail_address.bounce_subtype %> )

- Bounce payload: -

<%= JSON.pretty_generate(@bounced_mail_address.response_json) %>
+ Bounced recipient status: + <%= @bounced_mail_address.action %> (<%= @bounced_mail_address.status %>) +

+ +

+ Bounced recipient diagnostic: +

<%= @bounced_mail_address.diagnostic %>

<%= link_to 'Back', admin_bounced_mail_addresses_path %> From 3222a8b9a79b80444a41b8b348c98d8d260aeb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Wed, 30 Sep 2020 12:48:21 +0300 Subject: [PATCH 18/35] Bounces: Fix tests --- app/models/bounced_mail_address.rb | 9 +- test/fixtures/bounced_mail_addresses.yml | 41 ++------- test/models/bounced_mail_address_test.rb | 91 ++++++++++--------- .../admin_area/bounced_mail_addresses_test.rb | 3 +- 4 files changed, 58 insertions(+), 86 deletions(-) diff --git a/app/models/bounced_mail_address.rb b/app/models/bounced_mail_address.rb index 61679f543..73c6a0941 100644 --- a/app/models/bounced_mail_address.rb +++ b/app/models/bounced_mail_address.rb @@ -1,11 +1,8 @@ class BouncedMailAddress < ApplicationRecord validates :email, :message_id, :bounce_type, :bounce_subtype, :action, :status, presence: true - before_validation :assign_bounce_reason - def assign_bounce_reason - return self.bounce_reason = nil unless recipient_json - - self.bounce_reason = "#{action} (#{status} #{diagnostic})" + def bounce_reason + "#{action} (#{status} #{diagnostic})" end def self.record(json) @@ -17,7 +14,7 @@ class BouncedMailAddress < ApplicationRecord end end - def params_from_json(json, bounced_record) + def self.params_from_json(json, bounced_record) { email: bounced_record['emailAddress'], message_id: json['mail']['messageId'], diff --git a/test/fixtures/bounced_mail_addresses.yml b/test/fixtures/bounced_mail_addresses.yml index 345b2ef22..cbfdff680 100644 --- a/test/fixtures/bounced_mail_addresses.yml +++ b/test/fixtures/bounced_mail_addresses.yml @@ -1,39 +1,10 @@ one: email: bounced@registry.test - bounce_reason: failed (5.1.1 smtp; 550 5.1.1 user unknown) - recipient_json: { - "action": "failed", - "status": "5.1.1", - "emailAddress": "bounced@registry.test", - "diagnosticCode": "smtp; 550 5.1.1 user unknown" - } - response_json: { - "notificationType": "Bounce", - "mail": { - "source": "noreply@registry.test", - "sourceIp": "195.43.86.5", - "messageId": "010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", - "sourceArn": "arn:aws:ses:us-east-2:65026820000:identity/noreply@registry.test", - "timestamp": "2020-09-18T10:34:44.000Z", - "destination": [ "bounced@registry.test" ], - "sendingAccountId": "650268220000" - }, - "bounce": { - "timestamp": "2020-09-18T10:34:44.911Z", - "bounceType": "Permanent", - "feedbackId": "010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000", - "remoteMtaIp": "127.0.01", - "reportingMTA": "dsn; xxx.amazonses.com", - "bounceSubType": "General", - "bouncedRecipients": [ - { - "action": "failed", - "status": "5.1.1", - "emailAddress": "bounced@registry.test", - "diagnosticCode": "smtp; 550 5.1.1 user unknown" - } - ] - } - } + message_id: 010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000 + bounce_type: Permanent + bounce_subtype: General + action: failed + status: '5.1.1' + diagnostic: 'smtp; 550 5.1.1 user unknown' created_at: <%= Time.zone.parse('2010-07-05').to_s(:db) %> updated_at: <%= Time.zone.parse('2010-07-05').to_s(:db) %> diff --git a/test/models/bounced_mail_address_test.rb b/test/models/bounced_mail_address_test.rb index 0e6a62ab8..4af401711 100644 --- a/test/models/bounced_mail_address_test.rb +++ b/test/models/bounced_mail_address_test.rb @@ -6,56 +6,61 @@ class BouncedMailAddressTest < ActiveSupport::TestCase def setup @bounced_mail = BouncedMailAddress.new @bounced_mail.email = 'recipient@registry.test' - @bounced_mail.bounce_reason = 'failed (5.1.1 smtp; 550 5.1.1 user unknown)' - @bounced_mail.response_json = {"mail"=>{"source"=>"noreply@internet.test", "sourceIp"=>"195.43.86.5", "messageId"=>"010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000", "sourceArn"=>"arn:aws:ses:us-east-2:650268220328:identity/noreply@internet.test", "timestamp"=>"2020-09-18T10:34:44.000Z", "destination"=>["#{@bounced_mail.email}"], "sendingAccountId"=>"650268220328"}, "bounce"=>{"timestamp"=>"2020-09-18T10:34:44.911Z", "bounceType"=>"Permanent", "feedbackId"=>"010f0174a0c7d4f9-27d59756-6111-xxxx-a507-26bee0d55fa2-000000", "remoteMtaIp"=>"127.0.0.1", "reportingMTA"=>"dsn; xxx.amazonses.com", "bounceSubType"=>"General", "bouncedRecipients"=>[{"action"=>"failed", "status"=>"5.1.1", "emailAddress"=>"#{@bounced_mail.email}", "diagnosticCode"=>"smtp; 550 5.1.1 user unknown"}]}, "notificationType"=>"Bounce"} - @bounced_mail.recipient_json = {"action"=>"failed", "status"=>"5.1.1", "emailAddress"=>"#{@bounced_mail.email}", "diagnosticCode"=>"smtp; 550 5.1.1 user unknown"} - + @bounced_mail.message_id = '010f0174a0c7d348-ea6e2fc1-0854-4073-b71f-5cecf9b0d0b2-000000' + @bounced_mail.bounce_type = 'Permanent' + @bounced_mail.bounce_subtype = 'General' + @bounced_mail.action = 'failed' + @bounced_mail.status = '5.1.1' + @bounced_mail.diagnostic = 'smtp; 550 5.1.1 user unknown' end - def test_bounce_reason_is_autoassigned - assert @bounced_mail.valid? - @bounced_mail.bounce_reason = nil + def test_email_is_required assert @bounced_mail.valid? + @bounced_mail.email = nil + assert @bounced_mail.invalid? + end + def test_message_id_is_required + assert @bounced_mail.valid? + @bounced_mail.message_id = nil + assert @bounced_mail.invalid? + end + + def test_bounce_type_is_required + assert @bounced_mail.valid? + @bounced_mail.bounce_type = nil + assert @bounced_mail.invalid? + end + + def test_bounce_subtype_is_required + assert @bounced_mail.valid? + @bounced_mail.bounce_subtype = nil + assert @bounced_mail.invalid? + end + + def test_action_is_required + assert @bounced_mail.valid? + @bounced_mail.action = nil + assert @bounced_mail.invalid? + end + + def test_status_is_required + assert @bounced_mail.valid? + @bounced_mail.status = nil + assert @bounced_mail.invalid? + end + + def test_diagnostic_is_not_required + assert @bounced_mail.valid? + @bounced_mail.diagnostic = nil + assert @bounced_mail.valid? + end + + def test_bounce_reason_is_determined_dynamically + assert @bounced_mail.valid? assert_equal 'failed (5.1.1 smtp; 550 5.1.1 user unknown)', @bounced_mail.bounce_reason end - def test_response_json_is_required - assert @bounced_mail.valid? - @bounced_mail.response_json = nil - assert_not @bounced_mail.valid? - assert @bounced_mail.errors.full_messages.include? 'Response json is missing' - end - - def test_recipient_json_is_required - assert @bounced_mail.valid? - @bounced_mail.recipient_json = nil - assert_not @bounced_mail.valid? - - assert @bounced_mail.errors.full_messages.include? 'Recipient json is missing' - end - - def test_status_is_determined_dynamically - assert @bounced_mail.valid? - assert_equal '5.1.1', @bounced_mail.status - @bounced_mail.recipient_json['status'] = 'xxx_status' - assert_equal 'xxx_status', @bounced_mail.status - end - - def test_action_is_determined_dynamically - assert @bounced_mail.valid? - assert_equal 'failed', @bounced_mail.action - @bounced_mail.recipient_json['action'] = 'xxx_action' - assert_equal 'xxx_action', @bounced_mail.action - end - - def test_diagnostic_is_determined_dynamically - assert @bounced_mail.valid? - assert_equal 'smtp; 550 5.1.1 user unknown', @bounced_mail.diagnostic - @bounced_mail.recipient_json['diagnosticCode'] = 'xxx_diagnostic' - assert_equal 'xxx_diagnostic', @bounced_mail.diagnostic - end - def test_creates_objects_from_sns_json BouncedMailAddress.record(sns_bounce_payload) diff --git a/test/system/admin_area/bounced_mail_addresses_test.rb b/test/system/admin_area/bounced_mail_addresses_test.rb index 5500f4375..36b81f0f8 100644 --- a/test/system/admin_area/bounced_mail_addresses_test.rb +++ b/test/system/admin_area/bounced_mail_addresses_test.rb @@ -28,8 +28,7 @@ class AdminBouncedMailAddressesTest < ApplicationSystemTestCase assert_text @bounced_mail.diagnostic assert_text @bounced_mail.email - assert_text 'bouncedRecipients' - assert_text '010f0174a0c7d4f9-27d59756-6111-4d5f-xxxx-26bee0d55fa2-000000' + assert_text @bounced_mail.message_id end def test_deletes_registrar From 574846c626e23b2368c30c3088de9046f36e946b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 2 Oct 2020 16:02:44 +0300 Subject: [PATCH 19/35] Remove unneeded modifications in structure.sql --- db/structure.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index e064d968f..acf134a55 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1743,7 +1743,7 @@ ALTER SEQUENCE public.log_payment_orders_id_seq OWNED BY public.log_payment_orde -- --- Name: log_prices; Type: TABLE; Schema: public; Owner: -; Tablespace: +-- Name: log_prices; Type: TABLE; Schema: public; Owner: - -- CREATE TABLE public.log_prices ( @@ -1781,7 +1781,7 @@ ALTER SEQUENCE public.log_prices_id_seq OWNED BY public.log_prices.id; -- --- Name: log_registrant_verifications; Type: TABLE; Schema: public; Owner: -; Tablespace: +-- Name: log_registrant_verifications; Type: TABLE; Schema: public; Owner: - -- CREATE TABLE public.log_registrant_verifications ( @@ -3401,7 +3401,7 @@ ALTER TABLE ONLY public.log_payment_orders -- --- Name: log_prices_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: +-- Name: log_prices_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- ALTER TABLE ONLY public.log_prices @@ -3409,7 +3409,7 @@ ALTER TABLE ONLY public.log_prices -- --- Name: log_registrant_verifications_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: +-- Name: log_registrant_verifications_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- ALTER TABLE ONLY public.log_registrant_verifications From ef5c4bd54ebcdcab6f6544cf5b490494126c4110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 19 Nov 2020 16:15:23 +0200 Subject: [PATCH 20/35] Return total domain count in domains query --- app/controllers/api/v1/registrant/domains_controller.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/registrant/domains_controller.rb b/app/controllers/api/v1/registrant/domains_controller.rb index b985ef390..48cb95cf2 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -18,14 +18,13 @@ module Api status: :bad_request) && return end - @domains = current_user_domains.limit(limit).offset(offset) - - serialized_domains = @domains.map do |item| + domains = current_user_domains + serialized_domains = domains.limit(limit).offset(offset).map do |item| serializer = Serializers::RegistrantApi::Domain.new(item) serializer.to_json end - render json: serialized_domains + render json: {count: domains.count, domains: serialized_domains} end def show From 556008debee12987dfc31edabc4be62a4566cb4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 19 Nov 2020 16:18:25 +0200 Subject: [PATCH 21/35] Fix CC issues --- app/controllers/api/v1/registrant/domains_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/v1/registrant/domains_controller.rb b/app/controllers/api/v1/registrant/domains_controller.rb index 48cb95cf2..26f9f813b 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -24,7 +24,7 @@ module Api serializer.to_json end - render json: {count: domains.count, domains: serialized_domains} + render json: { count: domains.count, domains: serialized_domains } end def show From 7b3361394168656ce305cbbc0e885555fa5e712f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 30 Nov 2020 10:31:46 +0200 Subject: [PATCH 22/35] Reduce domains list API call --- .../api/v1/registrant/domains_controller.rb | 4 ++-- lib/serializers/registrant_api/domain.rb | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/registrant/domains_controller.rb b/app/controllers/api/v1/registrant/domains_controller.rb index 26f9f813b..390148488 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -20,7 +20,7 @@ module Api domains = current_user_domains serialized_domains = domains.limit(limit).offset(offset).map do |item| - serializer = Serializers::RegistrantApi::Domain.new(item) + serializer = Serializers::RegistrantApi::Domain.new(item, simplify: true) serializer.to_json end @@ -31,7 +31,7 @@ module Api @domain = current_user_domains.find_by(uuid: params[:uuid]) if @domain - serializer = Serializers::RegistrantApi::Domain.new(@domain) + serializer = Serializers::RegistrantApi::Domain.new(@domain, simplify: false) render json: serializer.to_json else render json: { errors: [{ base: ['Domain not found'] }] }, status: :not_found diff --git a/lib/serializers/registrant_api/domain.rb b/lib/serializers/registrant_api/domain.rb index 542f2d0de..f0c9c1876 100644 --- a/lib/serializers/registrant_api/domain.rb +++ b/lib/serializers/registrant_api/domain.rb @@ -3,11 +3,27 @@ module Serializers class Domain attr_reader :domain - def initialize(domain) + def initialize(domain, simplify: false) @domain = domain + @simplify = simplify end def to_json + if simplify + return { + id: domain.uuid, + name: domain.name, + registered_at: domain.registered_at, + valid_to: domain.valid_to, + registrant_verification_asked_at: domain.registrant_verification_asked_at, + statuses: domain.statuses, + registrar: { + name: domain.registrar.name, + website: domain.registrar.website + } + } + end + { id: domain.uuid, name: domain.name, From c9d4aaded98c98e0b9dc38ed5d64861493ea0ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Mon, 30 Nov 2020 16:50:14 +0200 Subject: [PATCH 23/35] Fix registrant domain serializer --- .../api/v1/registrant/domains_controller.rb | 3 ++- lib/serializers/registrant_api/domain.rb | 11 ++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v1/registrant/domains_controller.rb b/app/controllers/api/v1/registrant/domains_controller.rb index 390148488..73b534598 100644 --- a/app/controllers/api/v1/registrant/domains_controller.rb +++ b/app/controllers/api/v1/registrant/domains_controller.rb @@ -7,6 +7,7 @@ module Api def index limit = params[:limit] || 200 offset = params[:offset] || 0 + simple = params[:simple] == 'true' || false if limit.to_i > 200 || limit.to_i < 1 render(json: { errors: [{ limit: ['parameter is out of range'] }] }, @@ -20,7 +21,7 @@ module Api domains = current_user_domains serialized_domains = domains.limit(limit).offset(offset).map do |item| - serializer = Serializers::RegistrantApi::Domain.new(item, simplify: true) + serializer = Serializers::RegistrantApi::Domain.new(item, simplify: simple) serializer.to_json end diff --git a/lib/serializers/registrant_api/domain.rb b/lib/serializers/registrant_api/domain.rb index f0c9c1876..29362ad5c 100644 --- a/lib/serializers/registrant_api/domain.rb +++ b/lib/serializers/registrant_api/domain.rb @@ -9,18 +9,23 @@ module Serializers end def to_json - if simplify + if @simplify return { id: domain.uuid, name: domain.name, registered_at: domain.registered_at, valid_to: domain.valid_to, + outzone_at: domain.outzone_at, registrant_verification_asked_at: domain.registrant_verification_asked_at, statuses: domain.statuses, registrar: { name: domain.registrar.name, - website: domain.registrar.website - } + website: domain.registrar.website, + }, + registrant: { + name: domain.registrant.name, + id: domain.registrant.uuid, + }, } end From 15e090c3b1710f13ab63ecf25c2bfa5023e54043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 1 Dec 2020 12:53:30 +0200 Subject: [PATCH 24/35] Fix CC issue --- lib/serializers/registrant_api/domain.rb | 32 +++++++++--------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/lib/serializers/registrant_api/domain.rb b/lib/serializers/registrant_api/domain.rb index 29362ad5c..f8d5bb830 100644 --- a/lib/serializers/registrant_api/domain.rb +++ b/lib/serializers/registrant_api/domain.rb @@ -8,26 +8,8 @@ module Serializers @simplify = simplify end - def to_json - if @simplify - return { - id: domain.uuid, - name: domain.name, - registered_at: domain.registered_at, - valid_to: domain.valid_to, - outzone_at: domain.outzone_at, - registrant_verification_asked_at: domain.registrant_verification_asked_at, - statuses: domain.statuses, - registrar: { - name: domain.registrar.name, - website: domain.registrar.website, - }, - registrant: { - name: domain.registrant.name, - id: domain.registrant.uuid, - }, - } - end + def to_json(_obj = nil) + return simple_object if @simplify { id: domain.uuid, @@ -70,6 +52,16 @@ module Serializers private + def simple_object + { + id: domain.uuid, name: domain.name, registered_at: domain.registered_at, + valid_to: domain.valid_to, outzone_at: domain.outzone_at, statuses: domain.statuses, + registrant_verification_asked_at: domain.registrant_verification_asked_at, + registrar: { name: domain.registrar.name, website: domain.registrar.website }, + registrant: { name: domain.registrant.name, id: domain.registrant.uuid } + } + end + def dnssec_keys domain.dnskeys.map do |key| "#{key.flags} #{key.protocol} #{key.alg} #{key.public_key}" From 15d2ffe200837ddfde536bfb22474ce3fb1099ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 3 Dec 2020 16:12:36 +0200 Subject: [PATCH 25/35] Registrant API: Return associated domains for contact query --- .../api/v1/registrant/contacts_controller.rb | 11 ++++++----- app/models/contact.rb | 13 +++++++++---- lib/serializers/registrant_api/contact.rb | 13 +++++++++---- lib/serializers/registrant_api/domain.rb | 3 ++- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/app/controllers/api/v1/registrant/contacts_controller.rb b/app/controllers/api/v1/registrant/contacts_controller.rb index 10f9abacf..e11458ff8 100644 --- a/app/controllers/api/v1/registrant/contacts_controller.rb +++ b/app/controllers/api/v1/registrant/contacts_controller.rb @@ -19,15 +19,16 @@ module Api end contacts = current_user_contacts.limit(limit).offset(offset) - serialized_contacts = contacts.collect { |contact| serialize_contact(contact) } + serialized_contacts = contacts.collect { |contact| serialize_contact(contact, false) } render json: serialized_contacts end def show contact = current_user_contacts.find_by(uuid: params[:uuid]) + links = params[:links] == 'true' if contact - render json: serialize_contact(contact) + render json: serialize_contact(contact, links) else render json: { errors: [{ base: ['Contact not found'] }] }, status: :not_found end @@ -85,7 +86,7 @@ module Api contact.registrar.notify(action) end - render json: serialize_contact(contact) + render json: serialize_contact(contact, false) end private @@ -96,8 +97,8 @@ module Api current_registrant_user.direct_contacts end - def serialize_contact(contact) - Serializers::RegistrantApi::Contact.new(contact).to_json + def serialize_contact(contact, links) + Serializers::RegistrantApi::Contact.new(contact, links).to_json end end end diff --git a/app/models/contact.rb b/app/models/contact.rb index 8a154c50c..e30312b4a 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -347,19 +347,24 @@ class Contact < ApplicationRecord @desc = {} registrant_domains.each do |dom| - @desc[dom.name] ||= [] - @desc[dom.name] << :registrant + @desc[dom.name] ||= { id: dom.uuid, roles: [] } + @desc[dom.name][:roles] << :registrant end domain_contacts.each do |dc| - @desc[dc.domain.name] ||= [] - @desc[dc.domain.name] << dc.name.downcase.to_sym + @desc[dc.domain.name] ||= { id: dc.domain.uuid, roles: [] } + @desc[dc.domain.name][:roles] << dc.name.downcase.to_sym @desc[dc.domain.name] = @desc[dc.domain.name].compact end @desc end + def related_domains + a = related_domain_descriptions + a.keys.map { |d| { name: d, id: a[d][:id], roles: a[d][:roles] } } + end + def status_notes_array=(notes) self.status_notes = {} notes ||= [] diff --git a/lib/serializers/registrant_api/contact.rb b/lib/serializers/registrant_api/contact.rb index dd36b4400..023544174 100644 --- a/lib/serializers/registrant_api/contact.rb +++ b/lib/serializers/registrant_api/contact.rb @@ -1,14 +1,15 @@ module Serializers module RegistrantApi class Contact - attr_reader :contact + attr_reader :contact, :links - def initialize(contact) + def initialize(contact, links) @contact = contact + @links = links end - def to_json - { + def to_json(_obj = nil) + obj = { id: contact.uuid, name: contact.name, code: contact.code, @@ -31,6 +32,10 @@ module Serializers statuses: contact.statuses, disclosed_attributes: contact.disclosed_attributes, } + + obj[:links] = contact.related_domains if @links + + obj end end end diff --git a/lib/serializers/registrant_api/domain.rb b/lib/serializers/registrant_api/domain.rb index f8d5bb830..2359d77c9 100644 --- a/lib/serializers/registrant_api/domain.rb +++ b/lib/serializers/registrant_api/domain.rb @@ -58,7 +58,8 @@ module Serializers valid_to: domain.valid_to, outzone_at: domain.outzone_at, statuses: domain.statuses, registrant_verification_asked_at: domain.registrant_verification_asked_at, registrar: { name: domain.registrar.name, website: domain.registrar.website }, - registrant: { name: domain.registrant.name, id: domain.registrant.uuid } + registrant: { name: domain.registrant.name, id: domain.registrant.uuid, + phone: domain.registrant.phone, email: domain.registrant.email } } end From 92b294e81b10e69109e20f2e3d1d54e0d81b6c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 4 Dec 2020 15:24:14 +0200 Subject: [PATCH 26/35] Reflect new behaviour of registrant API in tests --- .../api/registrant/registrant_api_domains_test.rb | 8 ++++---- test/lib/serializers/registrant_api/contact_test.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/api/registrant/registrant_api_domains_test.rb b/test/integration/api/registrant/registrant_api_domains_test.rb index 22516fecc..61d635e5f 100644 --- a/test/integration/api/registrant/registrant_api_domains_test.rb +++ b/test/integration/api/registrant/registrant_api_domains_test.rb @@ -50,10 +50,10 @@ class RegistrantApiDomainsTest < ApplicationIntegrationTest assert_equal(200, response.status) response_json = JSON.parse(response.body, symbolize_names: true) - array_of_domain_names = response_json.map { |x| x[:name] } + array_of_domain_names = response_json[:domains].map { |x| x[:name] } assert(array_of_domain_names.include?('hospital.test')) - array_of_domain_registrars = response_json.map { |x| x[:registrar] } + array_of_domain_registrars = response_json[:domains].map { |x| x[:registrar] } assert(array_of_domain_registrars.include?({name: 'Good Names', website: nil})) end @@ -63,12 +63,12 @@ class RegistrantApiDomainsTest < ApplicationIntegrationTest response_json = JSON.parse(response.body, symbolize_names: true) assert_equal(200, response.status) - assert_equal(2, response_json.count) + assert_equal(2, response_json[:domains].count) get '/api/v1/registrant/domains', headers: @auth_headers response_json = JSON.parse(response.body, symbolize_names: true) - assert_equal(4, response_json.count) + assert_equal(4, response_json[:domains].count) end def test_root_does_not_accept_limit_higher_than_200 diff --git a/test/lib/serializers/registrant_api/contact_test.rb b/test/lib/serializers/registrant_api/contact_test.rb index 165c91e00..8b84abd38 100644 --- a/test/lib/serializers/registrant_api/contact_test.rb +++ b/test/lib/serializers/registrant_api/contact_test.rb @@ -4,7 +4,7 @@ require 'serializers/registrant_api/contact' class SerializersRegistrantApiContactTest < ActiveSupport::TestCase def setup @contact = contacts(:william) - @serializer = Serializers::RegistrantApi::Contact.new(@contact) + @serializer = Serializers::RegistrantApi::Contact.new(@contact, false) @json = @serializer.to_json end From ccf91d306abdadced826e6afb09d6b60dbfa9e5f Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 4 Dec 2020 17:48:42 +0500 Subject: [PATCH 27/35] Rename email sending interactor --- .../{delete_confirm => delete_confirm_email}/send_request.rb | 2 +- app/models/domain.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/interactions/domains/{delete_confirm => delete_confirm_email}/send_request.rb (96%) diff --git a/app/interactions/domains/delete_confirm/send_request.rb b/app/interactions/domains/delete_confirm_email/send_request.rb similarity index 96% rename from app/interactions/domains/delete_confirm/send_request.rb rename to app/interactions/domains/delete_confirm_email/send_request.rb index 91afaefb8..7070423a6 100644 --- a/app/interactions/domains/delete_confirm/send_request.rb +++ b/app/interactions/domains/delete_confirm_email/send_request.rb @@ -1,5 +1,5 @@ module Domains - module DeleteConfirm + module DeleteConfirmEmail class SendRequest < ActiveInteraction::Base object :domain, class: Domain, diff --git a/app/models/domain.rb b/app/models/domain.rb index dc7d86da8..ee3481bb2 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -418,7 +418,7 @@ class Domain < ApplicationRecord pending_delete_confirmation! save(validate: false) # should check if this did succeed - Domains::DeleteConfirm::SendRequest.run(domain: self) + Domains::DeleteConfirmEmail::SendRequest.run(domain: self) end def cancel_pending_delete From f6a7a08b2433b20b1c131f7bc307b52a9e98b580 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 4 Dec 2020 18:09:11 +0500 Subject: [PATCH 28/35] Move job to interactor --- .../domains/delete_confirm/base.rb | 23 ++++++++++ .../domains/delete_confirm/process_action.rb | 17 ++++++++ .../process_delete_confirmed.rb | 11 +++++ .../delete_confirm/process_delete_rejected.rb | 34 +++++++++++++++ app/jobs/application_job.rb | 2 + app/jobs/domain_delete_confirm_job.rb | 42 ++++--------------- app/models/registrant_verification.rb | 4 +- test/jobs/domain_delete_confirm_job_test.rb | 8 ++-- .../domains/domain_delete_confirms_test.rb | 9 +++- 9 files changed, 107 insertions(+), 43 deletions(-) create mode 100644 app/interactions/domains/delete_confirm/base.rb create mode 100644 app/interactions/domains/delete_confirm/process_action.rb create mode 100644 app/interactions/domains/delete_confirm/process_delete_confirmed.rb create mode 100644 app/interactions/domains/delete_confirm/process_delete_rejected.rb create mode 100644 app/jobs/application_job.rb diff --git a/app/interactions/domains/delete_confirm/base.rb b/app/interactions/domains/delete_confirm/base.rb new file mode 100644 index 000000000..d1d45f006 --- /dev/null +++ b/app/interactions/domains/delete_confirm/base.rb @@ -0,0 +1,23 @@ +module Domains + module DeleteConfirm + class Base < ActiveInteraction::Base + object :domain, + class: Domain, + description: 'Domain to confirm release' + string :action + string :initiator, + default: nil + + validates :domain, :action, presence: true + validates :action, inclusion: { in: [RegistrantVerification::CONFIRMED, + RegistrantVerification::REJECTED] } + + def raise_errors!(domain) + return unless domain.errors.any? + + message = "domain #{domain.name} failed with errors #{domain.errors.full_messages}" + throw message + end + end + end +end diff --git a/app/interactions/domains/delete_confirm/process_action.rb b/app/interactions/domains/delete_confirm/process_action.rb new file mode 100644 index 000000000..59f23de67 --- /dev/null +++ b/app/interactions/domains/delete_confirm/process_action.rb @@ -0,0 +1,17 @@ +module Domains + module DeleteConfirm + class ProcessAction < Base + def execute + ::PaperTrail.request.whodunnit = "interaction - #{self.class.name} - #{action} by"\ + " #{initiator}" + + case action + when RegistrantVerification::CONFIRMED + compose(ProcessDeleteConfirmed, inputs) + when RegistrantVerification::REJECTED + compose(ProcessDeleteRejected, inputs) + end + end + end + end +end diff --git a/app/interactions/domains/delete_confirm/process_delete_confirmed.rb b/app/interactions/domains/delete_confirm/process_delete_confirmed.rb new file mode 100644 index 000000000..553809666 --- /dev/null +++ b/app/interactions/domains/delete_confirm/process_delete_confirmed.rb @@ -0,0 +1,11 @@ +module Domains + module DeleteConfirm + class ProcessDeleteConfirmed < Base + def execute + domain.notify_registrar(:poll_pending_delete_confirmed_by_registrant) + domain.apply_pending_delete! + raise_errors!(domain) + end + end + end +end diff --git a/app/interactions/domains/delete_confirm/process_delete_rejected.rb b/app/interactions/domains/delete_confirm/process_delete_rejected.rb new file mode 100644 index 000000000..5038ba150 --- /dev/null +++ b/app/interactions/domains/delete_confirm/process_delete_rejected.rb @@ -0,0 +1,34 @@ +module Domains + module DeleteConfirm + class ProcessDeleteRejected < Base + def execute + domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION) + domain.notify_registrar(:poll_pending_delete_rejected_by_registrant) + + domain.cancel_pending_delete + domain.save(validate: false) + raise_errors!(domain) + + send_domain_deleted_email + end + + def send_domain_deleted_email + if domain.registrant_verification_token.blank? + warn "EMAIL NOT DELIVERED: registrant_verification_token is missing for #{domain.name}" + elsif domain.registrant_verification_asked_at.blank? + warn "EMAIL NOT DELIVERED: registrant_verification_asked_at is missing for #{domain.name}" + else + send_email + end + end + + def warn(message) + Rails.logger.warn(message) + end + + def send_email + DomainDeleteMailer.rejected(domain).deliver_now + end + end + end +end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb new file mode 100644 index 000000000..a009ace51 --- /dev/null +++ b/app/jobs/application_job.rb @@ -0,0 +1,2 @@ +class ApplicationJob < ActiveJob::Base +end diff --git a/app/jobs/domain_delete_confirm_job.rb b/app/jobs/domain_delete_confirm_job.rb index e94f2432c..afea3da59 100644 --- a/app/jobs/domain_delete_confirm_job.rb +++ b/app/jobs/domain_delete_confirm_job.rb @@ -1,39 +1,11 @@ -class DomainDeleteConfirmJob < Que::Job - def run(domain_id, action, initiator = nil) - ::PaperTrail.request.whodunnit = "job - #{self.class.name} - #{action} by #{initiator}" - # it's recommended to keep transaction against job table as short as possible. - ActiveRecord::Base.transaction do - domain = Epp::Domain.find(domain_id) +class DomainDeleteConfirmJob < ApplicationJob + queue_as :default - case action - when RegistrantVerification::CONFIRMED - domain.notify_registrar(:poll_pending_delete_confirmed_by_registrant) - domain.apply_pending_delete! - raise_errors!(domain) + def perform(domain_id, action, initiator = nil) + domain = Epp::Domain.find(domain_id) - when RegistrantVerification::REJECTED - domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION) - domain.notify_registrar(:poll_pending_delete_rejected_by_registrant) - - domain.cancel_pending_delete - domain.save(validate: false) - raise_errors!(domain) - - if domain.registrant_verification_token.blank? - Rails.logger.warn "EMAIL NOT DELIVERED: registrant_verification_token is missing for #{domain.name}" - elsif domain.registrant_verification_asked_at.blank? - Rails.logger.warn "EMAIL NOT DELIVERED: registrant_verification_asked_at is missing for #{domain.name}" - else - DomainDeleteMailer.rejected(domain).deliver_now - end - end - - destroy # it's best to destroy the job in the same transaction - end - end - - - def raise_errors!(domain) - throw "domain #{domain.name} failed with errors #{domain.errors.full_messages}" if domain.errors.any? + Domains::DeleteConfirm::ProcessAction.run(domain: domain, + action: action, + initiator: initiator) end end diff --git a/app/models/registrant_verification.rb b/app/models/registrant_verification.rb index 097f0cfa9..f0c9f3b97 100644 --- a/app/models/registrant_verification.rb +++ b/app/models/registrant_verification.rb @@ -30,12 +30,12 @@ class RegistrantVerification < ApplicationRecord def domain_registrant_delete_confirm!(initiator) self.action_type = DOMAIN_DELETE self.action = CONFIRMED - DomainDeleteConfirmJob.enqueue domain.id, CONFIRMED, initiator if save + DomainDeleteConfirmJob.perform_later domain.id, CONFIRMED, initiator if save end def domain_registrant_delete_reject!(initiator) self.action_type = DOMAIN_DELETE self.action = REJECTED - DomainDeleteConfirmJob.enqueue domain.id, REJECTED, initiator if save + DomainDeleteConfirmJob.perform_later domain.id, REJECTED, initiator if save end end diff --git a/test/jobs/domain_delete_confirm_job_test.rb b/test/jobs/domain_delete_confirm_job_test.rb index b999bd3c7..cbe4b87f1 100644 --- a/test/jobs/domain_delete_confirm_job_test.rb +++ b/test/jobs/domain_delete_confirm_job_test.rb @@ -18,7 +18,7 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase new_registrant_email: @new_registrant.email, current_user_id: @user.id }) - DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED) + DomainDeleteConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED) last_registrar_notification = @domain.registrar.notifications.last assert_equal(last_registrar_notification.attached_obj_id, @domain.id) @@ -31,7 +31,7 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase new_registrant_email: @new_registrant.email, current_user_id: @user.id }) - DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED) + DomainDeleteConfirmJob.perform_now(@domain.id, RegistrantVerification::CONFIRMED) last_registrar_notification = @domain.registrar.notifications.last assert_equal(last_registrar_notification.attached_obj_id, @domain.id) @@ -51,7 +51,7 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase assert @domain.registrant_delete_confirmable?(@domain.registrant_verification_token) assert_equal @user.id, @domain.pending_json['current_user_id'] - DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::CONFIRMED) + DomainDeleteConfirmJob.perform_now(@domain.id, RegistrantVerification::CONFIRMED) @domain.reload assert @domain.statuses.include? DomainStatus::PENDING_DELETE @@ -72,7 +72,7 @@ class DomainDeleteConfirmJobTest < ActiveSupport::TestCase assert @domain.registrant_delete_confirmable?(@domain.registrant_verification_token) assert_equal @user.id, @domain.pending_json['current_user_id'] - DomainDeleteConfirmJob.enqueue(@domain.id, RegistrantVerification::REJECTED) + DomainDeleteConfirmJob.perform_now(@domain.id, RegistrantVerification::REJECTED) @domain.reload assert_equal ['ok'], @domain.statuses diff --git a/test/system/registrant_area/domains/domain_delete_confirms_test.rb b/test/system/registrant_area/domains/domain_delete_confirms_test.rb index 0eb61ada8..765cd0149 100644 --- a/test/system/registrant_area/domains/domain_delete_confirms_test.rb +++ b/test/system/registrant_area/domains/domain_delete_confirms_test.rb @@ -1,6 +1,7 @@ require 'application_system_test_case' class DomainDeleteConfirmsTest < ApplicationSystemTestCase + include ActionMailer::TestHelper setup do @user = users(:registrant) sign_in @user @@ -13,7 +14,9 @@ class DomainDeleteConfirmsTest < ApplicationSystemTestCase def test_enqueues_approve_job_after_verification visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) - click_on 'Confirm domain delete' + perform_enqueued_jobs do + click_on 'Confirm domain delete' + end assert_text 'Domain registrant change has successfully received.' @domain.reload @@ -23,7 +26,9 @@ class DomainDeleteConfirmsTest < ApplicationSystemTestCase def test_enqueues_reject_job_after_verification visit registrant_domain_delete_confirm_url(@domain.id, token: @domain.registrant_verification_token) - click_on 'Reject domain delete' + perform_enqueued_jobs do + click_on 'Reject domain delete' + end assert_text 'Domain registrant change has been rejected successfully.' @domain.reload From 5a059e86bb9bf380e610350adf705062276d726a Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 7 Dec 2020 17:08:54 +0500 Subject: [PATCH 29/35] Move confirmed delete procedures to interactor --- .../domains/delete_confirm/base.rb | 30 ++++++++++++++ .../process_delete_confirmed.rb | 41 ++++++++++++++++++- .../delete_confirm/process_delete_rejected.rb | 4 +- 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/app/interactions/domains/delete_confirm/base.rb b/app/interactions/domains/delete_confirm/base.rb index d1d45f006..5bd07f40f 100644 --- a/app/interactions/domains/delete_confirm/base.rb +++ b/app/interactions/domains/delete_confirm/base.rb @@ -18,6 +18,36 @@ module Domains message = "domain #{domain.name} failed with errors #{domain.errors.full_messages}" throw message end + + def notify_registrar(message_key) + domain.registrar.notifications.create!( + text: "#{I18n.t(message_key)}: #{domain.name}", + attached_obj_id: domain.id, + attached_obj_type: domain.class.to_s + ) + end + + def preclean_pendings + domain.registrant_verification_token = nil + domain.registrant_verification_asked_at = nil + end + + def clean_pendings! + domain.is_admin = true + domain.registrant_verification_token = nil + domain.registrant_verification_asked_at = nil + domain.pending_json = {} + clear_statuses + domain.save + end + + def clear_statuses + domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION) + domain.statuses.delete(DomainStatus::PENDING_UPDATE) + domain.statuses.delete(DomainStatus::PENDING_DELETE) + domain.status_notes[DomainStatus::PENDING_UPDATE] = '' + domain.status_notes[DomainStatus::PENDING_DELETE] = '' + end end end end diff --git a/app/interactions/domains/delete_confirm/process_delete_confirmed.rb b/app/interactions/domains/delete_confirm/process_delete_confirmed.rb index 553809666..1f3068107 100644 --- a/app/interactions/domains/delete_confirm/process_delete_confirmed.rb +++ b/app/interactions/domains/delete_confirm/process_delete_confirmed.rb @@ -2,10 +2,49 @@ module Domains module DeleteConfirm class ProcessDeleteConfirmed < Base def execute - domain.notify_registrar(:poll_pending_delete_confirmed_by_registrant) + notify_registrar(:poll_pending_delete_confirmed_by_registrant) domain.apply_pending_delete! raise_errors!(domain) end + + def apply_pending_delete! + preclean_pendings + clean_pendings! + DomainDeleteMailer.accepted(domain).deliver_now + domain.set_pending_delete! + end + + def set_pending_delete! + unless domain.pending_deletable? + add_epp_error + return + end + + domain.delete_date = delete_date + domain.statuses << DomainStatus::PENDING_DELETE + set_server_hold if server_holdable? + domain.save(validate: false) + end + + def set_server_hold + domain.statuses << DomainStatus::SERVER_HOLD + domain.outzone_at = Time.current + end + + def server_holdable? + return false if domain.statuses.include?(DomainStatus::SERVER_HOLD) + return false if domain.statuses.include?(DomainStatus::SERVER_MANUAL_INZONE) + + true + end + + def delete_date + Time.zone.today + Setting.redemption_grace_period.days + 1.day + end + + def add_epp_error + domain.add_epp_error('2304', nil, nil, I18n.t(:object_status_prohibits_operation)) + end end end end diff --git a/app/interactions/domains/delete_confirm/process_delete_rejected.rb b/app/interactions/domains/delete_confirm/process_delete_rejected.rb index 5038ba150..1844801ac 100644 --- a/app/interactions/domains/delete_confirm/process_delete_rejected.rb +++ b/app/interactions/domains/delete_confirm/process_delete_rejected.rb @@ -9,10 +9,10 @@ module Domains domain.save(validate: false) raise_errors!(domain) - send_domain_deleted_email + send_domain_delete_rejected_email end - def send_domain_deleted_email + def send_domain_delete_rejected_email if domain.registrant_verification_token.blank? warn "EMAIL NOT DELIVERED: registrant_verification_token is missing for #{domain.name}" elsif domain.registrant_verification_asked_at.blank? From ce07c5eae83c27f0043d98ad58a99bdb9bbc5dd4 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 7 Dec 2020 17:37:44 +0500 Subject: [PATCH 30/35] Move rejected delete procedures to interactor --- .../domains/delete_confirm/process_delete_rejected.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/interactions/domains/delete_confirm/process_delete_rejected.rb b/app/interactions/domains/delete_confirm/process_delete_rejected.rb index 1844801ac..25b491660 100644 --- a/app/interactions/domains/delete_confirm/process_delete_rejected.rb +++ b/app/interactions/domains/delete_confirm/process_delete_rejected.rb @@ -2,10 +2,8 @@ module Domains module DeleteConfirm class ProcessDeleteRejected < Base def execute - domain.statuses.delete(DomainStatus::PENDING_DELETE_CONFIRMATION) - domain.notify_registrar(:poll_pending_delete_rejected_by_registrant) - domain.cancel_pending_delete + notify_registrar(:poll_pending_delete_rejected_by_registrant) domain.save(validate: false) raise_errors!(domain) From a54249f6ff7ecaddab9d9b6c7e7e174c59db5f25 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 8 Dec 2020 16:08:33 +0500 Subject: [PATCH 31/35] Move WhoisUpdate job to interactor --- app/interactions/whois/delete_record.rb | 48 ++++++++++++++ app/interactions/whois/update.rb | 39 ++++++++++++ app/interactions/whois/update_record.rb | 32 ++++++++++ app/jobs/update_whois_record_job.rb | 84 +------------------------ 4 files changed, 120 insertions(+), 83 deletions(-) create mode 100644 app/interactions/whois/delete_record.rb create mode 100644 app/interactions/whois/update.rb create mode 100644 app/interactions/whois/update_record.rb diff --git a/app/interactions/whois/delete_record.rb b/app/interactions/whois/delete_record.rb new file mode 100644 index 000000000..62e416d2f --- /dev/null +++ b/app/interactions/whois/delete_record.rb @@ -0,0 +1,48 @@ +module Whois + class DeleteRecord < ActiveInteraction::Base + string :name + string :type + + validates :type, inclusion: { in: %w[reserved blocked domain disputed zone] } + + def execute + send "delete_#{type}", name + end + + # 1. deleting own + # 2. trying to regenerate reserved in order domain is still in the list + def delete_domain(name) + WhoisRecord.where(name: name).destroy_all + + BlockedDomain.find_by(name: name).try(:generate_data) + ReservedDomain.find_by(name: name).try(:generate_data) + Dispute.active.find_by(domain_name: name).try(:generate_data) + end + + def delete_reserved(name) + remove_status_from_whois(domain_name: name, domain_status: 'Reserved') + end + + def delete_blocked(name) + delete_reserved(name) + end + + def delete_disputed(name) + return if Dispute.active.find_by(domain_name: name).present? + + remove_status_from_whois(domain_name: name, domain_status: 'disputed') + end + + def delete_zone(name) + WhoisRecord.where(name: name).destroy_all + Whois::Record.where(name: name).destroy_all + end + + def remove_status_from_whois(domain_name:, domain_status:) + Whois::Record.where(name: domain_name).each do |r| + r.json['status'] = r.json['status'].delete_if { |status| status == domain_status } + r.json['status'].blank? ? r.destroy : r.save + end + end + end +end diff --git a/app/interactions/whois/update.rb b/app/interactions/whois/update.rb new file mode 100644 index 000000000..bd37b4576 --- /dev/null +++ b/app/interactions/whois/update.rb @@ -0,0 +1,39 @@ +module Whois + class Update < ActiveInteraction::Base + array :names + string :type + + validates :type, inclusion: { in: %w[reserved blocked domain disputed zone] } + + def execute + ::PaperTrail.request.whodunnit = "job - #{self.class.name} - #{type}" + + klass = determine_class + + Array(names).each do |name| + record = find_record(klass, name) + if record + Whois::UpdateRecord.run(record: record, type: type) + else + Whois::DeleteRecord.run(name: name, type: type) + end + end + end + + private + + def determine_class + case type + when 'reserved' then ReservedDomain + when 'blocked' then BlockedDomain + when 'domain' then Domain + when 'disputed' then Dispute.active + else DNS::Zone + end + end + + def find_record(klass, name) + klass == DNS::Zone ? klass.find_by(origin: name) : klass.find_by(name: name) + end + end +end diff --git a/app/interactions/whois/update_record.rb b/app/interactions/whois/update_record.rb new file mode 100644 index 000000000..6fc838968 --- /dev/null +++ b/app/interactions/whois/update_record.rb @@ -0,0 +1,32 @@ +module Whois + class UpdateRecord < ActiveInteraction::Base + interface :record + string :type + + validates :type, inclusion: { in: %w[reserved blocked domain disputed zone] } + + def execute + send "update_#{type}", record + end + + def update_domain(domain) + domain.whois_record ? domain.whois_record.save : domain.create_whois_record + end + + def update_reserved(record) + record.generate_data + end + + def update_blocked(record) + update_reserved(record) + end + + def update_disputed(record) + update_reserved(record) + end + + def update_zone(record) + update_reserved(record) + end + end +end diff --git a/app/jobs/update_whois_record_job.rb b/app/jobs/update_whois_record_job.rb index 2973d5a6b..d067807a0 100644 --- a/app/jobs/update_whois_record_job.rb +++ b/app/jobs/update_whois_record_job.rb @@ -1,87 +1,5 @@ class UpdateWhoisRecordJob < Que::Job - def run(names, type) - ::PaperTrail.request.whodunnit = "job - #{self.class.name} - #{type}" - - klass = determine_class(type) - - Array(names).each do |name| - record = find_record(klass, name) - if record - send "update_#{type}", record - else - send "delete_#{type}", name - end - end - end - - def find_record(klass, name) - klass == DNS::Zone ? klass.find_by(origin: name) : klass.find_by(name: name) - end - - def determine_class(type) - case type - when 'reserved' then ReservedDomain - when 'blocked' then BlockedDomain - when 'domain' then Domain - when 'disputed' then Dispute.active - when 'zone' then DNS::Zone - end - end - - def update_domain(domain) - domain.whois_record ? domain.whois_record.save : domain.create_whois_record - end - - def update_reserved(record) - record.generate_data - end - - def update_blocked(record) - update_reserved(record) - end - - def update_disputed(record) - update_reserved(record) - end - - def update_zone(record) - update_reserved(record) - end - - # 1. deleting own - # 2. trying to regenerate reserved in order domain is still in the list - def delete_domain(name) - WhoisRecord.where(name: name).destroy_all - - BlockedDomain.find_by(name: name).try(:generate_data) - ReservedDomain.find_by(name: name).try(:generate_data) - Dispute.active.find_by(domain_name: name).try(:generate_data) - end - - def delete_reserved(name) - remove_status_from_whois(domain_name: name, domain_status: 'Reserved') - end - - def delete_blocked(name) - delete_reserved(name) - end - - def delete_disputed(name) - return if Dispute.active.find_by(domain_name: name).present? - - remove_status_from_whois(domain_name: name, domain_status: 'disputed') - end - - def delete_zone(name) - WhoisRecord.where(name: name).destroy_all - Whois::Record.where(name: name).destroy_all - end - - def remove_status_from_whois(domain_name:, domain_status:) - Whois::Record.where(name: domain_name).each do |r| - r.json['status'] = r.json['status'].delete_if { |status| status == domain_status } - r.json['status'].blank? ? r.destroy : r.save - end + Whois::Update.run(names: [names].flatten, type: type) end end From fafb02e9a04e463f5c1f39c2c215f55ac2838792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Tue, 15 Dec 2020 11:32:55 +0200 Subject: [PATCH 32/35] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d7c1e24..59ec26c4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +15.12.2020 +* improved logic for domain list request in registrant API [#1750](https://github.com/internetee/registry/pull/1750) + 14.12.2020 * Refactored domain cron jobs for interactors [#1767](https://github.com/internetee/registry/issues/1767) From 06a59c9aebd9c8e303bfa2d6329db1711db4a6e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Tue, 15 Dec 2020 18:57:43 +0200 Subject: [PATCH 33/35] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59ec26c4c..d0cd1822c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ 15.12.2020 -* improved logic for domain list request in registrant API [#1750](https://github.com/internetee/registry/pull/1750) +* Improved logic for domain list request in registrant API [#1750](https://github.com/internetee/registry/pull/1750) +* Refactored Whois update job for interactors [#1771](https://github.com/internetee/registry/issues/1771) 14.12.2020 * Refactored domain cron jobs for interactors [#1767](https://github.com/internetee/registry/issues/1767) From f1f8dba8254868d964de7bbb907854f2d5eed8ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Wed, 16 Dec 2020 16:47:52 +0200 Subject: [PATCH 34/35] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0cd1822c..28d3fd67d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +16.12.2020 +* Refactored domain delete confirmation for interactors [#1769](https://github.com/internetee/registry/issues/1769) + 15.12.2020 * Improved logic for domain list request in registrant API [#1750](https://github.com/internetee/registry/pull/1750) * Refactored Whois update job for interactors [#1771](https://github.com/internetee/registry/issues/1771) From b833110ddd7294044692169718baafd381cec657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Thu, 17 Dec 2020 18:31:45 +0200 Subject: [PATCH 35/35] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28d3fd67d..0907d33e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +17.12.2020 +* New API for registering bounced emails [#1687](https://github.com/internetee/registry/pull/1687) + 16.12.2020 * Refactored domain delete confirmation for interactors [#1769](https://github.com/internetee/registry/issues/1769)