From 5966ef90ce7ea91c84fc0fa1d34125dea3a0e1df Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Mon, 16 Sep 2019 10:14:56 -0700 Subject: [PATCH 1/7] Warn if queue size is greater than 1000. Closes #1969 (#2080) * Warn if queue size is greater than 1000. Closes #1969 Co-Authored-By: Cory McDonald --- config/routes.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 755fcaac0f..748ab8d35e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -275,4 +275,9 @@ end end mount Sidekiq::Web, at: "/magic" + require 'sidekiq/api' + match "mailer-queue-status" => proc { [200, {"Content-Type" => "text/plain"}, [Sidekiq::Queue.new('mailer').size < 100 ? "OK" : "UHOH" ]] }, via: :get + match "default-queue-status" => proc { [200, {"Content-Type" => "text/plain"}, [Sidekiq::Queue.new('default').size < 5000 ? "OK" : "UHOH" ]] }, via: :get + match "scheduler-queue-status" => proc { [200, {"Content-Type" => "text/plain"}, [Sidekiq::Queue.new('scheduler').size < 5000 ? "OK" : "UHOH" ]] }, via: :get + match "transactional-queue-status" => proc { [200, {"Content-Type" => "text/plain"}, [Sidekiq::Queue.new('transactional').size < 5000 ? "OK" : "UHOH" ]] }, via: :get end From abba7f715cfaeddd6dade47cdb2f69ef4c9c0ab3 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Mon, 16 Sep 2019 14:26:27 -0700 Subject: [PATCH 2/7] Bugfix/performance dyno caching (#2218) * Allow dyno caching. --- Gemfile.lock | 2 +- .../api/v1/public/channels_controller.rb | 17 ++++---- .../api/v2/public/channels_controller.rb | 17 ++++---- .../api/v3/public/channels_controller.rb | 16 ++++---- .../concerns/browser_channels_dyno_caching.rb | 41 +++++++++++++++++++ app/jobs/cache_browser_channels_json_job.rb | 2 +- .../cache_browser_channels_json_job_v2.rb | 4 +- .../cache_browser_channels_json_job_v3.rb | 3 +- 8 files changed, 73 insertions(+), 29 deletions(-) create mode 100644 app/controllers/concerns/browser_channels_dyno_caching.rb diff --git a/Gemfile.lock b/Gemfile.lock index 0ca9d9b56a..e9c33e4a93 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -608,4 +608,4 @@ RUBY VERSION ruby 2.5.5p157 BUNDLED WITH - 2.0.1 + 2.0.2 diff --git a/app/controllers/api/v1/public/channels_controller.rb b/app/controllers/api/v1/public/channels_controller.rb index 9215f2fa0d..80eee24235 100644 --- a/app/controllers/api/v1/public/channels_controller.rb +++ b/app/controllers/api/v1/public/channels_controller.rb @@ -1,14 +1,15 @@ class Api::V1::Public::ChannelsController < Api::V1::Public::BaseController - def channels - channels_json = Rails.cache.fetch('browser_channels_json', race_condition_ttl: 30) do - require 'sentry-raven' - Raven.capture_message("Failed to use redis cache for /api/public/channels, using DB instead.") - channels_json = JsonBuilders::ChannelsJsonBuilder.new.build - end - render(json: channels_json, status: 200) - end + include BrowserChannelsDynoCaching + @@cached_payload ||= nil + REDIS_KEY = 'browser_channels_json'.freeze def totals render(json: Channel.statistical_totals, status: 200) end + + private + + def dyno_expiration_key + "browser_v1_channels_expiration:#{ENV['DYNO']}" + end end diff --git a/app/controllers/api/v2/public/channels_controller.rb b/app/controllers/api/v2/public/channels_controller.rb index ad914795e5..751fced122 100644 --- a/app/controllers/api/v2/public/channels_controller.rb +++ b/app/controllers/api/v2/public/channels_controller.rb @@ -1,14 +1,15 @@ class Api::V2::Public::ChannelsController < Api::V2::Public::BaseController - def channels - channels_json = Rails.cache.fetch('browser_channels_json_v2', race_condition_ttl: 30) do - require 'sentry-raven' - Raven.capture_message("Failed to use redis cache for /api/public/channels V2, using DB instead.") - channels_json = JsonBuilders::ChannelsJsonBuilderV2.new.build - end - render(json: channels_json, status: 200) - end + include BrowserChannelsDynoCaching + @@cached_payload = nil + REDIS_KEY = 'browser_channels_json_v2'.freeze def totals render(json: Channel.statistical_totals, status: 200) end + + private + + def dyno_expiration_key + "browser_v3_channels_expiration:#{ENV['DYNO']}" + end end diff --git a/app/controllers/api/v3/public/channels_controller.rb b/app/controllers/api/v3/public/channels_controller.rb index 86f392150f..8fd68f4266 100644 --- a/app/controllers/api/v3/public/channels_controller.rb +++ b/app/controllers/api/v3/public/channels_controller.rb @@ -1,16 +1,18 @@ require 'sentry-raven' class Api::V3::Public::ChannelsController < Api::V3::Public::BaseController - def channels - channels_json = Rails.cache.fetch('browser_channels_json_v3', race_condition_ttl: 30) do - Raven.capture_message("Failed to use redis cache for /api/public/channels V3, using DB instead.") - channels_json = JsonBuilders::ChannelsJsonBuilderV3.new.build - end - render(json: channels_json, status: 200) - end + include BrowserChannelsDynoCaching + @@cached_payload = nil + REDIS_KEY = 'browser_channels_json_v3'.freeze def totals statistical_totals_json = Rails.cache.fetch(CacheBrowserChannelsJsonJobV3::TOTALS_CACHE_KEY, race_condition_ttl: 30) render(json: statistical_totals_json, status: 200) end + + private + + def dyno_expiration_key + "browser_v3_channels_expiration:#{ENV['DYNO']}" + end end diff --git a/app/controllers/concerns/browser_channels_dyno_caching.rb b/app/controllers/concerns/browser_channels_dyno_caching.rb new file mode 100644 index 0000000000..054f9f0afe --- /dev/null +++ b/app/controllers/concerns/browser_channels_dyno_caching.rb @@ -0,0 +1,41 @@ +module BrowserChannelsDynoCaching + extend ActiveSupport::Concern + require 'sentry-raven' + + def channels + if dyno_cache_expired? || invalid_dyno_cache? + update_dyno_cache + end + render(json: self.class.class_variable_get(klass_dyno_cache), status: 200) + end + + private + + def dyno_cache_expired? + expiration_time = Rails.cache.fetch(dyno_expiration_key) + return expiration_time.nil? || Time.parse(expiration_time) <= Time.now + end + + def invalid_dyno_cache? + cached_dyno_value = self.class.class_variable_get(klass_dyno_cache) + cached_dyno_value.nil? || !cached_dyno_value.is_a?(String) + end + + def update_dyno_cache + redis_value = Rails.cache.fetch(self.class::REDIS_KEY, race_condition_ttl: 30) do + Raven.capture_message("Failed to use redis cache for Dyno cache: #{klass_dyno_cache}, continuing to read from cache instead") + end + if redis_value.present? + self.class.class_variable_set(klass_dyno_cache, redis_value) + Rails.cache.write(dyno_expiration_key, 1.hour.from_now.to_s, expires_in: 1.hour.from_now ) + end + end + + def dyno_expiration_key + raise "Define me for dyno_expiration_name!" + end + + def klass_dyno_cache + :@@cached_payload + end +end diff --git a/app/jobs/cache_browser_channels_json_job.rb b/app/jobs/cache_browser_channels_json_job.rb index d1801d843c..44dfd484ee 100644 --- a/app/jobs/cache_browser_channels_json_job.rb +++ b/app/jobs/cache_browser_channels_json_job.rb @@ -9,7 +9,7 @@ def perform result = nil loop do - result = Rails.cache.write('browser_channels_json', channels_json) + result = Rails.cache.write(Api::V1::Public::ChannelsController::REDIS_KEY, channels_json) break if result || retry_count > MAX_RETRY retry_count += 1 diff --git a/app/jobs/cache_browser_channels_json_job_v2.rb b/app/jobs/cache_browser_channels_json_job_v2.rb index 2a8984e820..ce2cd572eb 100644 --- a/app/jobs/cache_browser_channels_json_job_v2.rb +++ b/app/jobs/cache_browser_channels_json_job_v2.rb @@ -9,7 +9,7 @@ def perform result = nil loop do - result = Rails.cache.write('browser_channels_json_v2', channels_json) + result = Rails.cache.write(Api::V2::Public::ChannelsController::REDIS_KEY, channels_json) break if result || retry_count > MAX_RETRY retry_count += 1 @@ -23,4 +23,4 @@ def perform Rails.logger.info("CacheBrowserChannelsJsonJob V2 could not update the channels JSON.") end end - end +end diff --git a/app/jobs/cache_browser_channels_json_job_v3.rb b/app/jobs/cache_browser_channels_json_job_v3.rb index ef87d7b215..8117439de9 100644 --- a/app/jobs/cache_browser_channels_json_job_v3.rb +++ b/app/jobs/cache_browser_channels_json_job_v3.rb @@ -2,7 +2,6 @@ class CacheBrowserChannelsJsonJobV3 < ApplicationJob queue_as :heavy MAX_RETRY = 10 - CACHE_KEY = 'browser_channels_json_v3' TOTALS_CACHE_KEY = 'browser_channels_json_v3_totals' def perform @@ -11,7 +10,7 @@ def perform result = nil loop do - result = Rails.cache.write(CACHE_KEY, @channels_json) + result = Rails.cache.write(Api::V3::Public::ChannelsController::REDIS_KEY, @channels_json) break if result || retry_count > MAX_RETRY retry_count += 1 From eb6eb6e3b09e7b4403e188e4a1c49e2ed7b53f27 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Mon, 16 Sep 2019 15:58:48 -0700 Subject: [PATCH 3/7] Remove legacy versions (#2226) --- .../20190913151449_drop_legacy_versions.rb | 5 ++++ db/schema.rb | 24 ++++++------------- 2 files changed, 12 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20190913151449_drop_legacy_versions.rb diff --git a/db/migrate/20190913151449_drop_legacy_versions.rb b/db/migrate/20190913151449_drop_legacy_versions.rb new file mode 100644 index 0000000000..9998253bb3 --- /dev/null +++ b/db/migrate/20190913151449_drop_legacy_versions.rb @@ -0,0 +1,5 @@ +class DropLegacyVersions < ActiveRecord::Migration[6.0] + def change + drop_table :legacy_versions + end +end diff --git a/db/schema.rb b/db/schema.rb index 179943e0c7..5364e43e45 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2,17 +2,18 @@ # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. # -# Note that this schema.rb definition is the authoritative source for your -# database schema. If you need to create the application database on another -# system, you should be using db:schema:load, not running all the migrations -# from scratch. The latter is a flawed and unsustainable approach (the more migrations -# you'll amass, the slower it'll run and the greater likelihood for issues). +# This file is the source Rails uses to define your schema when running `rails +# db:schema:load`. When creating a new database, `rails db:schema:load` tends to +# be faster and is potentially less error prone than running all of your +# migrations from scratch. Old migrations may fail to apply correctly if those +# migrations use external dependencies or application code. # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_08_29_141054) do +ActiveRecord::Schema.define(version: 2019_09_13_151449) do # These are extensions that must be enabled in order to support this database + enable_extension "pg_stat_statements" enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -221,17 +222,6 @@ t.index ["publisher_id"], name: "index_legacy_totp_registrations_on_publisher_id" end - create_table "legacy_versions", id: :serial, force: :cascade do |t| - t.string "item_type", null: false - t.integer "item_id", null: false - t.string "event", null: false - t.string "whodunnit" - t.text "object" - t.datetime "created_at" - t.text "object_changes" - t.index ["item_type", "item_id"], name: "index_legacy_versions_on_item_type_and_item_id" - end - create_table "legacy_youtube_channels", id: :string, force: :cascade do |t| t.string "title" t.string "description" From 631c389fd1c7446f94b3eed2799645532b5a2fbe Mon Sep 17 00:00:00 2001 From: Cory McDonald Date: Tue, 17 Sep 2019 11:27:15 -0500 Subject: [PATCH 4/7] 2-FA Removal Improvements (#2227) * 2FA refactor * Extend Linter * Fix tests --- .rubocop.yml | 2 +- .../promo_registrations_controller.rb | 9 ++- .../publishers/site_banners_controller.rb | 8 +-- ...tor_authentications_removals_controller.rb | 26 ++++++++ app/controllers/publishers_controller.rb | 59 +------------------ .../two_factor_authentications_controller.rb | 2 + app/mailers/publisher_mailer.rb | 10 ---- .../two_factor_authentication_removal.rb | 2 +- ...wo_factor_authentication_removal.html.slim | 34 ----------- .../new.html.slim | 17 ++++++ app/views/static/no_js.html.slim | 2 - .../_removal.html.slim | 11 ++++ .../index.html.slim | 11 ++-- config/locales/en.yml | 16 ++++- config/routes.rb | 7 +-- .../two_factor_authentication_removals.yml | 8 ++- ..._factor_authentication_removal_job_test.rb | 8 +-- .../two_factor_authentication_removal_test.rb | 6 +- 18 files changed, 106 insertions(+), 132 deletions(-) create mode 100644 app/controllers/publishers/two_factor_authentications_removals_controller.rb delete mode 100644 app/views/publishers/two_factor_authentication_removal.html.slim create mode 100644 app/views/publishers/two_factor_authentications_removals/new.html.slim create mode 100644 app/views/two_factor_authentications/_removal.html.slim diff --git a/.rubocop.yml b/.rubocop.yml index d670c8c8ab..260a1094a7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,7 +17,7 @@ AllCops: - app/controllers/admin/publishers_controller.rb - app/controllers/admin/publishers/publisher_status_updates_controller.rb - app/controllers/publishers/cases_controller.rb - - app/controllers/publishers/uphold_controller.rb + - app/controllers/publishers/** - app/services/uphold/**/* - app/jobs/create_uphold_cards_job.rb - app/jobs/create_uphold_channel_card_job.rb diff --git a/app/controllers/publishers/promo_registrations_controller.rb b/app/controllers/publishers/promo_registrations_controller.rb index 0b6f9f2759..e726aba5b6 100644 --- a/app/controllers/publishers/promo_registrations_controller.rb +++ b/app/controllers/publishers/promo_registrations_controller.rb @@ -1,8 +1,11 @@ class Publishers::PromoRegistrationsController < PublishersController def for_referral_code - promo_registration = current_publisher.admin? ? - PromoRegistration.find_by(referral_code: params[:referral_code]) : - current_publisher.promo_registrations.find_by(referral_code: params[:referral_code]) + promo_registration = + if current_publisher.admin? + PromoRegistration.find_by(referral_code: params[:referral_code]) + else + current_publisher.promo_registrations.find_by(referral_code: params[:referral_code]) + end render :unauthorized and return if promo_registration.nil? render json: promo_registration.stats_by_date.to_json end diff --git a/app/controllers/publishers/site_banners_controller.rb b/app/controllers/publishers/site_banners_controller.rb index d80dcf987c..d5f0e733a4 100644 --- a/app/controllers/publishers/site_banners_controller.rb +++ b/app/controllers/publishers/site_banners_controller.rb @@ -6,7 +6,7 @@ class Publishers::SiteBannersController < ApplicationController def show if site_banner - render json:site_banner.read_only_react_property + render json: site_banner.read_only_react_property else render(json: nil.to_json) end @@ -86,8 +86,8 @@ def image_properties(attachment_type:) ) rescue OutsidePaddingRangeError => e logger.error "Outside padding range #{e.message}" - LogException.perform(StandardError.new("File size too big for #{attachment_type}"), params: {publisher_id: current_publisher.id}) - raise StandardError.new("File size too big for #{attachment_type}") + LogException.perform(StandardError.new("File size too big for #{attachment_type}"), params: { publisher_id: current_publisher.id }) + raise StandardError.new("File size too big for #{attachment_type}") # rubocop:disable Style/RaiseArgs end new_filename = generate_filename(source_image_path: padded_resized_jpg_path) @@ -95,7 +95,7 @@ def image_properties(attachment_type:) { io: open(padded_resized_jpg_path), filename: new_filename + ".jpg", - content_type: "image/jpg" + content_type: "image/jpg", } end end diff --git a/app/controllers/publishers/two_factor_authentications_removals_controller.rb b/app/controllers/publishers/two_factor_authentications_removals_controller.rb new file mode 100644 index 0000000000..0504f4b613 --- /dev/null +++ b/app/controllers/publishers/two_factor_authentications_removals_controller.rb @@ -0,0 +1,26 @@ +module Publishers + class TwoFactorAuthenticationsRemovalsController < ApplicationController + include TwoFactorAuth + + def new + end + + def create + publisher = pending_2fa_current_publisher + + publisher.register_for_2fa_removal if publisher.two_factor_authentication_removal.blank? + publisher.reload + + MailerServices::TwoFactorAuthenticationRemovalReminderEmailer.new(publisher: publisher).perform + redirect_to two_factor_authentications_path, flash: { notice: t("publishers.two_factor_authentication_removal.request_success") } + end + + def destroy + publisher = pending_2fa_current_publisher + + publisher.two_factor_authentication_removal.destroy if publisher.two_factor_authentication_removal.present? + + redirect_to two_factor_authentications_path, flash: { notice: t("publishers.two_factor_authentication_removal.confirm_cancel_flash") } + end + end +end diff --git a/app/controllers/publishers_controller.rb b/app/controllers/publishers_controller.rb index 613fd334a8..cc71a57bcf 100644 --- a/app/controllers/publishers_controller.rb +++ b/app/controllers/publishers_controller.rb @@ -1,6 +1,4 @@ class PublishersController < ApplicationController - # Number of requests to #create before we present a captcha. - include PublishersHelper include PromosHelper @@ -13,17 +11,11 @@ class PublishersController < ApplicationController :statement, :statements, :update, - :uphold_status, - :uphold_verified, ].freeze before_action :authenticate_via_token, only: %i(show) - before_action :authenticate_publisher!, except: %i( - two_factor_authentication_removal - request_two_factor_authentication_removal - confirm_two_factor_authentication_removal - cancel_two_factor_authentication_removal - ) + before_action :authenticate_publisher! + before_action :require_publisher_email_not_verified_through_youtube_auth, except: %i(update_email change_email) @@ -104,53 +96,6 @@ def update end end - def request_two_factor_authentication_removal - publisher = Publisher.by_email_case_insensitive(params[:email]).first - flash[:notice] = t("publishers.two_factor_authentication_removal.request_success") - if publisher - if publisher.two_factor_authentication_removal.blank? - MailerServices::TwoFactorAuthenticationRemovalRequestEmailer.new(publisher: publisher).perform - elsif !publisher.two_factor_authentication_removal.removal_completed - MailerServices::TwoFactorAuthenticationRemovalCancellationEmailer.new(publisher: publisher).perform - end - end - redirect_to two_factor_authentication_removal_publishers_path - end - - def cancel_two_factor_authentication_removal - sign_out(current_publisher) if current_publisher - - publisher = Publisher.find(params[:id]) - token = params[:token] - - if PublisherTokenAuthenticator.new(publisher: publisher, token: token, confirm_email: publisher.email).perform - publisher.two_factor_authentication_removal.destroy if publisher.two_factor_authentication_removal.present? - flash[:notice] = t("publishers.two_factor_authentication_removal.confirm_cancel_flash") - redirect_to(root_path) - else - flash[:notice] = t("publishers.shared.error") - redirect_to(root_path) - end - end - - def confirm_two_factor_authentication_removal - sign_out(current_publisher) if current_publisher - - publisher = Publisher.find(params[:id]) - token = params[:token] - - if PublisherTokenAuthenticator.new(publisher: publisher, token: token, confirm_email: publisher.email).perform - publisher.register_for_2fa_removal if publisher.two_factor_authentication_removal.blank? - publisher.reload - MailerServices::TwoFactorAuthenticationRemovalReminderEmailer.new(publisher: publisher).perform - flash[:notice] = t("publishers.two_factor_authentication_removal.confirm_login_flash") - redirect_to(root_path) - else - flash[:notice] = t("publishers.shared.error") - redirect_to(root_path) - end - end - def protect if current_publisher.nil? redirect_to root_url and return diff --git a/app/controllers/two_factor_authentications_controller.rb b/app/controllers/two_factor_authentications_controller.rb index 5d059b3a3b..7e1bec9374 100644 --- a/app/controllers/two_factor_authentications_controller.rb +++ b/app/controllers/two_factor_authentications_controller.rb @@ -6,6 +6,8 @@ class TwoFactorAuthenticationsController < ApplicationController def index @u2f_enabled = u2f_enabled?(pending_2fa_current_publisher) + @removal = pending_2fa_current_publisher.two_factor_authentication_removal + if !params[:request_totp] && @u2f_enabled challenge = u2f.challenge session[:challenge] = challenge diff --git a/app/mailers/publisher_mailer.rb b/app/mailers/publisher_mailer.rb index 21d42ba6b7..e0e4214887 100644 --- a/app/mailers/publisher_mailer.rb +++ b/app/mailers/publisher_mailer.rb @@ -145,16 +145,6 @@ def suspend_publisher(publisher) ) end - def two_factor_authentication_removal_request(publisher) - @publisher = publisher - @publisher_private_two_factor_removal_url = publisher_private_two_factor_removal_url(publisher: @publisher) - mail( - to: @publisher.email, - subject: default_i18n_subject, - template_name: "two_factor_authentication_removal_request" - ) - end - def two_factor_authentication_removal_cancellation(publisher) @publisher = publisher @publisher_private_two_factor_cancellation_url = publisher_private_two_factor_cancellation_url(publisher: @publisher) diff --git a/app/models/two_factor_authentication_removal.rb b/app/models/two_factor_authentication_removal.rb index 1f98f18902..318020c810 100644 --- a/app/models/two_factor_authentication_removal.rb +++ b/app/models/two_factor_authentication_removal.rb @@ -1,7 +1,7 @@ class TwoFactorAuthenticationRemoval < ApplicationRecord include ActionView::Helpers::DateHelper belongs_to :publisher - TWO_FACTOR_AUTHENTICATION_REMOVAL_WAITING_PERIOD = 2.weeks.seconds + TWO_FACTOR_AUTHENTICATION_REMOVAL_WAITING_PERIOD = 2.days.seconds LOCKED_STATUS_WAITING_PERIOD = 4.weeks.seconds # 6 Weeks represented in seconds TOTAL_WAITING_PERIOD = TWO_FACTOR_AUTHENTICATION_REMOVAL_WAITING_PERIOD + LOCKED_STATUS_WAITING_PERIOD diff --git a/app/views/publishers/two_factor_authentication_removal.html.slim b/app/views/publishers/two_factor_authentication_removal.html.slim deleted file mode 100644 index 3362723f77..0000000000 --- a/app/views/publishers/two_factor_authentication_removal.html.slim +++ /dev/null @@ -1,34 +0,0 @@ -css: - .leadin { - width: 85%; - margin: auto; - padding-bottom: 60px; - } - - .note { - width: 85%; - margin: auto; - padding-top: 60px; - } - -.single-panel--wrapper - = render "panel_flash_messages" - .single-panel--content - .single-panel--padded-content - - h4.single-panel--headline= t("publishers.two_factor_authentication_removal.heading") - p.leadin= t("publishers.two_factor_authentication_removal.leadin") - - .col-small-centered - = form_tag request_two_factor_authentication_removal_publishers_path, method: :post do |f| - .form-group - = email_field_tag :email, nil, autofocus: true, class: "form-control", placeholder: t("publishers.shared.enter_email"), required: true - - if params[:captcha] - = hidden_field_tag :captcha - - if @should_throttle - .form-group - = recaptcha_tags - = submit_tag t("publishers.two_factor_authentication_removal.request_2fa_removal"), class: "btn btn-block btn-primary", :"data-piwik-action" => "StartFlowClicked", :"data-piwik-name" => "Clicked", :"data-piwik-value" => "Landing" - - p.note - ' #{t("publishers.two_factor_authentication_removal.note")} diff --git a/app/views/publishers/two_factor_authentications_removals/new.html.slim b/app/views/publishers/two_factor_authentications_removals/new.html.slim new file mode 100644 index 0000000000..4a42ab7716 --- /dev/null +++ b/app/views/publishers/two_factor_authentications_removals/new.html.slim @@ -0,0 +1,17 @@ + +.single-panel--wrapper + = render "panel_flash_messages" + .single-panel--content + .single-panel--padded-content + + h4.single-panel--headline= t("publishers.two_factor_authentication_removal.heading") + + p= t("publishers.two_factor_authentication_removal.leadin") + + == t("publishers.two_factor_authentication_removal.note") + .my-3.text-left + == t("publishers.two_factor_authentication_removal.removals") + + = link_to(t("publishers.two_factor_authentication_removal.request_2fa_removal"), two_factor_authentications_removal_path, method: :post, class: "btn btn-block btn-primary") + + diff --git a/app/views/static/no_js.html.slim b/app/views/static/no_js.html.slim index 19c29e76cd..d46bddff44 100644 --- a/app/views/static/no_js.html.slim +++ b/app/views/static/no_js.html.slim @@ -28,7 +28,5 @@ span= link_to(t("shared.terms_of_service"), "https://basicattentiontoken.org/publisher-terms-of-service/") span.link-separator= " | " = link_to(t("shared.help_center"), "https://support.brave.com/hc/en-us/articles/360022724391-What-do-I-do-if-I-get-locked-out-of-my-account-") - span.link-separator= " | " - = link_to(t("shared.lost_2fa"), two_factor_authentication_removal_publishers_path) diff --git a/app/views/two_factor_authentications/_removal.html.slim b/app/views/two_factor_authentications/_removal.html.slim new file mode 100644 index 0000000000..065a75a5a9 --- /dev/null +++ b/app/views/two_factor_authentications/_removal.html.slim @@ -0,0 +1,11 @@ + +h3.single-panel--headline = t(".heading") + + +p == t(".body", time: removal.two_factor_authentication_removal_days_remaining) +.my-3.text-left + == t("publishers.two_factor_authentication_removal.removals") + += link_to(t('.cancel'), two_factor_authentications_removal_path, method: :delete, class: 'btn btn-danger') + + diff --git a/app/views/two_factor_authentications/index.html.slim b/app/views/two_factor_authentications/index.html.slim index e5ae9ad856..079cc774f5 100644 --- a/app/views/two_factor_authentications/index.html.slim +++ b/app/views/two_factor_authentications/index.html.slim @@ -56,14 +56,15 @@ .single-panel--content .single-panel--padded-content - - if @u2f_authentication_attempt + - if @removal.present? + = render partial: 'removal', locals: { removal: @removal } + - elsif @u2f_authentication_attempt = render partial: "u2f", locals: @u2f_authentication_attempt - - elsif @totp_enabled = render "totp" - .single-panel--footer - span = t(".lost_account.lost_2fa") - span = link_to(" " + t(".lost_account.lost_2fa_link"), two_factor_authentication_removal_publishers_path) + .single-panel--footer.mt-0 + strong = t(".lost_account.lost_2fa") + strong.ml-1 = link_to(t(".lost_account.lost_2fa_link"), new_two_factor_authentications_removal_path) br br p = t(".lost_account.lost_2fa_note_html") diff --git a/config/locales/en.yml b/config/locales/en.yml index 9677a91db4..29960fec76 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -652,7 +652,14 @@ en: two_factor_authentication_removal: heading: Having trouble with 2-factor authentication? leadin: If you've lost your authenticator app or device, access can be restored by removing your 2-factor security settings. - note: Please note, for your security, we require a 2 week waiting period for the 2-factor authentication removal to complete. Additionally, once it is completed you will need to reverify your channels and uphold wallet. + note: Please note, for your security, we require a 2 day waiting period for the 2-factor authentication removal to complete + removals: > +
    +
  • The account channels will need to be re-verified.
    • +
  • The account wallet will need to be re-verified.
    • +
  • There will be a 30-day waiting period on payments.
    • +
+ reminder_body_html: > This is a friendly reminder that the requested changes to your security settings are in progress. In %{remainder}, 2-factor authentication will be removed from your account. Please note, once your 2-factor authentication is removed:
    @@ -673,7 +680,7 @@ en:


request_note: - request_success: You have requested a 2-factor security removal. Please check your e-mail to confirm and initiate the process. + request_success: You have successfully requested a 2-factor security removal. request_not_found: Publisher not found. Please ensure you have entered the correct e-mail request_2fa_removal: Request 2-factor Security Removal confirm_2fa_removal: Confirm 2-factor Security Removal @@ -945,6 +952,11 @@ en: totp_alternative_link: Use authentication code instead. u2f_unavailable_totp_alternative_link: Use Authentication Code Instead submit_value: Try Again + removal: + heading: Two-factor Authentication + body: Your two-factor authentication is in the process of being removed.
%{time} remaining. + cancel: Cancel removal + index: u2f_unavailable_html: | Security key not supported by your browser.
diff --git a/config/routes.rb b/config/routes.rb index 748ab8d35e..e338a5bd2c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -24,6 +24,8 @@ patch :disconnect_uphold, action: :destroy patch :confirm_default_currency end + + resource :two_factor_authentications_removal end get :log_out @@ -41,13 +43,10 @@ patch :update patch :complete_signup get :choose_new_channel_type - get :two_factor_authentication_removal get :security, to: 'publishers/security#index' get :prompt_security, to: 'publishers/security#prompt' get :settings, to: 'publishers/settings#index' - post :request_two_factor_authentication_removal - get :confirm_two_factor_authentication_removal - get :cancel_two_factor_authentication_removal + resources :two_factor_authentications, only: %i(index) resources :u2f_registrations, only: %i(new create destroy) resources :u2f_authentications, only: %i(create) diff --git a/test/fixtures/two_factor_authentication_removals.yml b/test/fixtures/two_factor_authentication_removals.yml index 63557fadbb..3c17396e5f 100644 --- a/test/fixtures/two_factor_authentication_removals.yml +++ b/test/fixtures/two_factor_authentication_removals.yml @@ -1,9 +1,13 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html one: &one - publisher: verified + publisher: uphold_connected removal_completed: false two: &two - publisher: verified + publisher: uphold_connected + removal_completed: false + +three: &three + publisher: admin removal_completed: false diff --git a/test/jobs/two_factor_authentication_removal_job_test.rb b/test/jobs/two_factor_authentication_removal_job_test.rb index 3bf5a08ca5..04f8dd915c 100644 --- a/test/jobs/two_factor_authentication_removal_job_test.rb +++ b/test/jobs/two_factor_authentication_removal_job_test.rb @@ -2,13 +2,13 @@ class TwoFactorAuthenticationRemovalJobTest < ActiveJob::TestCase test "Does not remove publisher's 2fa until timeout period has passed" do - publisher = publishers(:verified) + publisher = publishers(:verified_totp_only) TwoFactorAuthenticationRemovalJob.perform_now assert_not_nil(publisher.totp_registration) end test "Removes publisher's 2fa when timeout period has passed" do - publisher = publishers(:verified) + publisher = publishers(:uphold_connected) two_factor_authentication_removal = two_factor_authentication_removals(:one) original_date = two_factor_authentication_removal.created_at advanced_date = original_date - 14.days @@ -18,7 +18,7 @@ class TwoFactorAuthenticationRemovalJobTest < ActiveJob::TestCase end test "Removes publisher's channels when timeout period has passed" do - publisher = publishers(:verified) + publisher = publishers(:uphold_connected) two_factor_authentication_removal = two_factor_authentication_removals(:one) original_date = two_factor_authentication_removal.created_at advanced_date = original_date - 14.days @@ -28,7 +28,7 @@ class TwoFactorAuthenticationRemovalJobTest < ActiveJob::TestCase end test "Does not remove publisher's channels more than once" do - publisher = publishers(:verified) + publisher = publishers(:uphold_connected) two_factor_authentication_removal = two_factor_authentication_removals(:one) channel = channels(:google_verified) channel_details = channel.details.dup diff --git a/test/models/two_factor_authentication_removal_test.rb b/test/models/two_factor_authentication_removal_test.rb index 8883b12f8c..324e85f595 100644 --- a/test/models/two_factor_authentication_removal_test.rb +++ b/test/models/two_factor_authentication_removal_test.rb @@ -1,10 +1,10 @@ require "test_helper" class TwoFactorAuthenticationRemovalTest < ActiveSupport::TestCase - test "Two factor removal takes 14 days" do + test "Two factor removal takes 2 days" do two_factor_authentication_removal = two_factor_authentication_removals(:one) remainder = two_factor_authentication_removal.two_factor_authentication_removal_days_remaining - assert_equal("14 days", remainder) + assert_equal("2 days", remainder) end test "Locked state doesn't being until 2fa removal is complete" do @@ -19,6 +19,6 @@ class TwoFactorAuthenticationRemovalTest < ActiveSupport::TestCase advanced_date = original_date - 14.days two_factor_authentication_removal.update(created_at: advanced_date) remainder = two_factor_authentication_removal.locked_status_days_remaining - assert_equal("28 days", remainder) + assert_equal("16 days", remainder) end end From a76a5b4d0306b1c80a665496faf280f4a4cfbf62 Mon Sep 17 00:00:00 2001 From: Albert Wang Date: Tue, 17 Sep 2019 12:54:59 -0700 Subject: [PATCH 5/7] Specify 5.2 for legacy versions migration (#2231) --- db/migrate/20190913151449_drop_legacy_versions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20190913151449_drop_legacy_versions.rb b/db/migrate/20190913151449_drop_legacy_versions.rb index 9998253bb3..54f9814591 100644 --- a/db/migrate/20190913151449_drop_legacy_versions.rb +++ b/db/migrate/20190913151449_drop_legacy_versions.rb @@ -1,4 +1,4 @@ -class DropLegacyVersions < ActiveRecord::Migration[6.0] +class DropLegacyVersions < ActiveRecord::Migration[5.2] def change drop_table :legacy_versions end From a95106b245857832f0310651bcdae14dc274694e Mon Sep 17 00:00:00 2001 From: Cory McDonald Date: Wed, 18 Sep 2019 17:54:33 -0500 Subject: [PATCH 6/7] Fix the manual report json (#2236) --- .../manual_payout_report_json_builder.rb | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/app/models/json_builders/manual_payout_report_json_builder.rb b/app/models/json_builders/manual_payout_report_json_builder.rb index fb52f8fe15..ff90405a87 100644 --- a/app/models/json_builders/manual_payout_report_json_builder.rb +++ b/app/models/json_builders/manual_payout_report_json_builder.rb @@ -6,19 +6,22 @@ def initialize(payout_report:) def build contents = [] @payout_report.potential_payments.to_be_paid.where(kind: PotentialPayment::MANUAL).find_each do |potential_payment| + publisher = Publisher.find(potential_payment.publisher_id).owner_identifier + contents.push( { - "name" => potential_payment.name.to_s, - "altcurrency" => "BAT", - "probi" => potential_payment.amount.to_s, - "fees" => potential_payment.fees.to_s, - "authority" => Publisher.find(potential_payment.finalized_by_id).email, - "transactionId" => potential_payment.payout_report_id.to_s, - "owner" => Publisher.find(potential_payment.publisher_id).owner_identifier.to_s, - "type" => PotentialPayment::MANUAL, - "address" => potential_payment.address.to_s, - "upholdId" => potential_payment.uphold_id.to_s, - "documentId" => potential_payment.invoice_id.to_s, + name: potential_payment.name, + altcurrency: "BAT", + probi: potential_payment.amount.to_s, + fees: potential_payment.fees.to_s, + authority: Publisher.find(potential_payment.finalized_by_id).email, + transactionId: potential_payment.payout_report_id, + owner: publisher, + publisher: publisher, + type: PotentialPayment::MANUAL, + address: potential_payment.address, + upholdId: potential_payment.uphold_id, + documentId: potential_payment.invoice_id, } ) end From f3627dfa96e7a27ce706fa1dcc49a7122e133084 Mon Sep 17 00:00:00 2001 From: Cory McDonald Date: Thu, 19 Sep 2019 16:32:59 -0500 Subject: [PATCH 7/7] Create Uphold Report (#2219) * Create Uphold Report * UpholdReport -> UpholdStatusReport * Tests pass * Apply suggestions from code review Co-Authored-By: Albert Wang * Apply suggestions from code review Co-Authored-By: Albert Wang * Apply suggestions from code review Co-Authored-By: Albert Wang * Rename relevant files from uphold_reports to uphold_status_reports (#2240) * KYC report route fix (#2241) * Fix route to use uphold_status_report --- app/assets/stylesheets/admin/style.scss | 3 +- .../admin/uphold_status_reports_controller.rb | 30 +++++++++++++++++++ .../publishers/uphold_controller.rb | 15 ++++++++++ app/models/uphold_connection.rb | 5 ++-- app/models/uphold_status_report.rb | 3 ++ app/views/admin/publishers/_uphold.html.slim | 8 ++++- app/views/admin/shared/_sidebar.html.slim | 1 + .../uphold_status_reports/index.html.slim | 22 ++++++++++++++ config/routes.rb | 2 ++ ...90910163930_create_uphold_status_report.rb | 13 ++++++++ db/schema.rb | 11 +++++++ 11 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 app/controllers/admin/uphold_status_reports_controller.rb create mode 100644 app/models/uphold_status_report.rb create mode 100644 app/views/admin/uphold_status_reports/index.html.slim create mode 100644 db/migrate/20190910163930_create_uphold_status_report.rb diff --git a/app/assets/stylesheets/admin/style.scss b/app/assets/stylesheets/admin/style.scss index fa726d4738..3f70c00f2e 100644 --- a/app/assets/stylesheets/admin/style.scss +++ b/app/assets/stylesheets/admin/style.scss @@ -14,7 +14,7 @@ grid-gap: 32px; align-items: center; } - + // Bootstrap overrides. Could be refined .btn-default { color: $braveGray-2 !important; @@ -58,7 +58,6 @@ bottom: 0; background: $braveGray-2; margin: 60px 0 0 0; - padding: 24px 0; overflow: auto; .sidebar-menu { diff --git a/app/controllers/admin/uphold_status_reports_controller.rb b/app/controllers/admin/uphold_status_reports_controller.rb new file mode 100644 index 0000000000..c46e86ee70 --- /dev/null +++ b/app/controllers/admin/uphold_status_reports_controller.rb @@ -0,0 +1,30 @@ +require 'csv' + +module Admin + class UpholdStatusReportsController < AdminController + def index + @uphold_status_reports = UpholdStatusReport. + group('(EXTRACT(YEAR FROM created_at))::integer'). + group('(EXTRACT(MONTH FROM created_at))::integer'). + order('2 DESC, 3 DESC').count + end + + def show + date = DateTime.strptime(params[:id], "%Y-%m") + start_date = date.at_beginning_of_month + end_date = date.at_end_of_month + + uphold_status_reports = UpholdStatusReport.where("created_at >= :start AND created_at <= :finish", start: start_date, finish: end_date) + + generated = [] + generated << ["publisher id", "publisher created at", "uphold id", "uphold connected time"].to_csv + + uphold_status_reports.each do |report| + generated << [report.publisher_id, report.publisher.created_at, report.uphold_id, report.created_at].to_csv + end + + send_data generated.join(''), filename: "uphold-#{params[:id]}.csv" + end + end +end + diff --git a/app/controllers/publishers/uphold_controller.rb b/app/controllers/publishers/uphold_controller.rb index 63d4bb7373..f2c96274fb 100644 --- a/app/controllers/publishers/uphold_controller.rb +++ b/app/controllers/publishers/uphold_controller.rb @@ -76,6 +76,9 @@ def create ExchangeUpholdCodeForAccessTokenJob.perform_now(publisher_id: current_publisher.id) + connection.reload + create_uphold_report!(connection) + redirect_to(home_publishers_path) rescue UpholdError, Faraday::Error => e Rails.logger.info("Uphold Error: #{e.message}") @@ -95,6 +98,18 @@ def destroy class UpholdError < StandardError; end + def create_uphold_report!(connection) + uphold_id = connection.uphold_details&.id + return if uphold_id.blank? + # Return if we've already created a report for this id + return if UpholdStatusReport.find_by(uphold_id: uphold_id).present? + + UpholdStatusReport.create( + publisher: current_publisher, + uphold_id: uphold_id + ) + end + def validate_uphold!(connection) # Ensure the uphold_state_token has been set. If not send back to try again raise UpholdError.new, t('.missing_state') if connection&.uphold_state_token.blank? && !connection.uphold_verified? diff --git a/app/models/uphold_connection.rb b/app/models/uphold_connection.rb index 264c7b2f35..e11a3d0677 100644 --- a/app/models/uphold_connection.rb +++ b/app/models/uphold_connection.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class UpholdConnection < ActiveRecord::Base - has_paper_trail only: [:is_member, :uphold_id, :address, :status, :default_currency] + has_paper_trail only: [:is_member, :member_at, :uphold_id, :address, :status, :default_currency] UPHOLD_CODE_TIMEOUT = 5.minutes UPHOLD_ACCESS_PARAMS_TIMEOUT = 2.hours @@ -100,7 +100,7 @@ def uphold_client def uphold_details @user ||= uphold_client.user.find(self) rescue Faraday::ClientError => e - if e.response[:status] == 401 + if e.response&.dig(:status) == 401 Rails.logger.info("#{e.response[:body]} for uphold connection #{id}") update(uphold_access_parameters: nil) nil @@ -169,6 +169,7 @@ def sync_from_uphold! update( is_member: uphold_information.memberAt.present?, + member_at: uphold_information.memberAt, status: uphold_information.status, uphold_id: uphold_information.id, country: uphold_information.country diff --git a/app/models/uphold_status_report.rb b/app/models/uphold_status_report.rb new file mode 100644 index 0000000000..b4813d34e3 --- /dev/null +++ b/app/models/uphold_status_report.rb @@ -0,0 +1,3 @@ +class UpholdStatusReport < ApplicationRecord + belongs_to :publisher +end diff --git a/app/views/admin/publishers/_uphold.html.slim b/app/views/admin/publishers/_uphold.html.slim index 9765ba01c7..8ed41a43fc 100644 --- a/app/views/admin/publishers/_uphold.html.slim +++ b/app/views/admin/publishers/_uphold.html.slim @@ -26,7 +26,13 @@ h3.text-dark.d-flex.align-items-center td User has completed KYC td span class=(@publisher.uphold_connection.is_member? ? 'text-success' : 'text-danger') - = @publisher.uphold_connection.is_member? ? fa_icon("check", text: "Yes") : fa_icon("times", text: "No") + - if @publisher.uphold_connection.is_member? + = fa_icon("check", text: "Yes") + span.text-muted + span.mx-2= " – " + = @publisher.uphold_connection.member_at.strftime("%B %d, %Y %k:%M %Z") + - else + = fa_icon("times", text: "No") tr td Uphold ID td= link_to_if @publisher.uphold_connection.uphold_id, @publisher.uphold_connection.uphold_id, admin_publishers_path(q: @publisher.uphold_connection.uphold_id) diff --git a/app/views/admin/shared/_sidebar.html.slim b/app/views/admin/shared/_sidebar.html.slim index ae15a27e41..30f7dbc48a 100644 --- a/app/views/admin/shared/_sidebar.html.slim +++ b/app/views/admin/shared/_sidebar.html.slim @@ -48,3 +48,4 @@ aside = nav_link "FAQs", admin_faq_categories_path = nav_link "Payout Reports", admin_payout_reports_path = nav_link "Referral Promo", admin_unattached_promo_registrations_path + = nav_link "Uphold Reports", admin_uphold_status_reports_path diff --git a/app/views/admin/uphold_status_reports/index.html.slim b/app/views/admin/uphold_status_reports/index.html.slim new file mode 100644 index 0000000000..869300ec2c --- /dev/null +++ b/app/views/admin/uphold_status_reports/index.html.slim @@ -0,0 +1,22 @@ +.panel-heading + h4 Uphold Reports + +div + .font-weight-bold How this works: + p Anytime a publisher connects their uphold account we create an entry in our database that logs this. If an uphold id has already been reported we do not create an entry. This data is then aggregated and grouped by month and displayed here. + +table.table + thead + tr + td Period + td Number of users + td + tbody + - @uphold_status_reports.each do |report| + tr + td + - date = report.as_json.first + = Date::MONTHNAMES[date.second] + = " #{date.first}" + td= report.as_json.second + td= link_to("Download", admin_uphold_status_report_path(date.join('-')), class:'btn btn-default') diff --git a/config/routes.rb b/config/routes.rb index e338a5bd2c..539e2f0b1f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -250,6 +250,8 @@ end resources :promo_campaigns, only: %i(create) root to: "dashboard#index" # <--- Root route + + resources :uphold_status_reports, only: [:index, :show] end resources :errors, only: [], path: "/" do diff --git a/db/migrate/20190910163930_create_uphold_status_report.rb b/db/migrate/20190910163930_create_uphold_status_report.rb new file mode 100644 index 0000000000..9d5e8ca082 --- /dev/null +++ b/db/migrate/20190910163930_create_uphold_status_report.rb @@ -0,0 +1,13 @@ +class CreateUpholdStatusReport < ActiveRecord::Migration[5.2] + def change + add_column :uphold_connections, :member_at, :datetime + + create_table :uphold_status_reports, id: :uuid, default: -> { "uuid_generate_v4()"}, force: :cascade do |t| + t.belongs_to :publisher, index: true, type: :uuid + t.uuid :uphold_id, index: true + t.timestamps + end + + add_index :uphold_status_reports, :created_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 5364e43e45..a36704857c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -529,9 +529,20 @@ t.string "country" t.string "default_currency" t.datetime "default_currency_confirmed_at" + t.datetime "member_at" t.index ["publisher_id"], name: "index_uphold_connections_on_publisher_id", unique: true end + create_table "uphold_status_reports", id: :uuid, default: -> { "uuid_generate_v4()" }, force: :cascade do |t| + t.uuid "publisher_id" + t.uuid "uphold_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["created_at"], name: "index_uphold_status_reports_on_created_at" + t.index ["publisher_id"], name: "index_uphold_status_reports_on_publisher_id" + t.index ["uphold_id"], name: "index_uphold_status_reports_on_uphold_id" + end + create_table "user_authentication_tokens", id: :uuid, default: -> { "uuid_generate_v4()" }, force: :cascade do |t| t.string "encrypted_authentication_token" t.string "encrypted_authentication_token_iv"