diff --git a/app/controllers/admin/feedback_messages_controller.rb b/app/controllers/admin/feedback_messages_controller.rb index 3b9a5ec26..3d2ac6ca2 100644 --- a/app/controllers/admin/feedback_messages_controller.rb +++ b/app/controllers/admin/feedback_messages_controller.rb @@ -107,7 +107,7 @@ module Admin <<~HEREDOC *New note from #{params['author_name']}:* *Report status: #{params['feedback_message_status']}* - Report page: https://#{ApplicationConfig['APP_DOMAIN']}/admin/reports/#{params['noteable_id']} + Report page: https://#{SiteConfig.app_domain}/admin/reports/#{params['noteable_id']} -------- Message: #{params['content']} HEREDOC diff --git a/app/controllers/admin/tools_controller.rb b/app/controllers/admin/tools_controller.rb index 7c2cee38b..db9de65b7 100644 --- a/app/controllers/admin/tools_controller.rb +++ b/app/controllers/admin/tools_controller.rb @@ -40,9 +40,8 @@ module Admin end def bust_link(link) - if link.starts_with?("https://#{ApplicationConfig['APP_DOMAIN']}") - link.sub!("https://#{ApplicationConfig['APP_DOMAIN']}", - "") + if link.starts_with?(URL.url) + link.sub!(URL.url, "") end CacheBuster.bust(link) CacheBuster.bust("#{link}/") diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 63bb54fb2..d4462b91a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base skip_before_action :track_ahoy_visit before_action :verify_private_forem protect_from_forgery with: :exception, prepend: true + before_action :remember_cookie_sync include SessionCurrentUser include ValidRequest @@ -10,6 +11,7 @@ class ApplicationController < ActionController::Base include CachingHeaders include ImageUploads include VerifySetupCompleted + include Devise::Controllers::Rememberable rescue_from ActionView::MissingTemplate, with: :routing_error @@ -144,6 +146,16 @@ class ApplicationController < ActionController::Base Stripe.log_level = Stripe::LEVEL_INFO end + def remember_cookie_sync + # Set remember cookie token in case not properly set. + if user_signed_in? && + cookies[:remember_user_token].blank? + current_user.remember_me = true + current_user.remember_me! + remember_me(current_user) + end + end + protected def configure_permitted_parameters diff --git a/app/controllers/async_info_controller.rb b/app/controllers/async_info_controller.rb index f6ff9c9d0..2e89bfd51 100644 --- a/app/controllers/async_info_controller.rb +++ b/app/controllers/async_info_controller.rb @@ -1,5 +1,4 @@ class AsyncInfoController < ApplicationController - include Devise::Controllers::Rememberable # No pundit policy. All actions are unrestricted. before_action :set_cache_control_headers, only: %i[shell_version] @@ -13,11 +12,6 @@ class AsyncInfoController < ApplicationController } return end - if remember_user_token.blank? - current_user.remember_me = true - current_user.remember_me! - remember_me(current_user) - end @user = current_user.decorate respond_to do |format| format.json do @@ -87,11 +81,6 @@ class AsyncInfoController < ApplicationController #{current_user&.checked_code_of_conduct}__ #{current_user&.articles_count}__ #{current_user&.pro?}__ - #{current_user&.blocking_others_count}__ - #{remember_user_token}" - end - - def remember_user_token - cookies[:remember_user_token] + #{current_user&.blocking_others_count}__" end end diff --git a/app/controllers/buffered_articles_controller.rb b/app/controllers/buffered_articles_controller.rb index f1faac952..e3503d99f 100644 --- a/app/controllers/buffered_articles_controller.rb +++ b/app/controllers/buffered_articles_controller.rb @@ -16,6 +16,6 @@ class BufferedArticlesController < ApplicationController end paths = relation.pluck(:path) - paths.map { |path| "https://#{ApplicationConfig['APP_DOMAIN']}#{path}" } + paths.map { |path| URL.url(path) } end end diff --git a/app/decorators/article_decorator.rb b/app/decorators/article_decorator.rb index 8ddd60cca..f32e9bf98 100644 --- a/app/decorators/article_decorator.rb +++ b/app/decorators/article_decorator.rb @@ -22,7 +22,7 @@ class ArticleDecorator < ApplicationDecorator end def url - "https://#{ApplicationConfig['APP_DOMAIN']}#{path}" + URL.url(path) end def title_length_classification diff --git a/app/labor/cache_buster.rb b/app/labor/cache_buster.rb index 6cba24fdc..8d877f773 100644 --- a/app/labor/cache_buster.rb +++ b/app/labor/cache_buster.rb @@ -37,14 +37,15 @@ module CacheBuster end def self.bust_fastly_cache(path) + # @forem/systems Fastly-enabled forems don't need "flexible" domains. HTTParty.post( - "https://api.fastly.com/purge/https://#{ApplicationConfig['APP_DOMAIN']}#{path}", + "https://api.fastly.com/purge/https://#{URL.domain}#{path}", headers: { "Fastly-Key" => ApplicationConfig["FASTLY_API_KEY"] }, ) HTTParty.post( - "https://api.fastly.com/purge/https://#{ApplicationConfig['APP_DOMAIN']}#{path}?i=i", + "https://api.fastly.com/purge/https://#{URL.domain}#{path}?i=i", headers: { "Fastly-Key" => ApplicationConfig["FASTLY_API_KEY"] }, diff --git a/app/labor/markdown_parser.rb b/app/labor/markdown_parser.rb index 3fb28028d..444fba295 100644 --- a/app/labor/markdown_parser.rb +++ b/app/labor/markdown_parser.rb @@ -252,7 +252,7 @@ class MarkdownParser username = mention.delete("@").downcase if User.find_by(username: username) <<~HTML - @#{username} + @#{username} HTML else mention diff --git a/app/lib/redcarpet/render/html_rouge.rb b/app/lib/redcarpet/render/html_rouge.rb index ffca9f3ac..54244191d 100644 --- a/app/lib/redcarpet/render/html_rouge.rb +++ b/app/lib/redcarpet/render/html_rouge.rb @@ -50,7 +50,7 @@ module Redcarpet end def app_domain - ApplicationConfig["APP_DOMAIN"] + SiteConfig.app_domain end def slugify(string) diff --git a/app/lib/url.rb b/app/lib/url.rb index 1e402bdea..986ea10c2 100644 --- a/app/lib/url.rb +++ b/app/lib/url.rb @@ -7,7 +7,7 @@ module URL end def self.domain - ApplicationConfig["APP_DOMAIN"] + Rails.application&.initialized? ? SiteConfig.app_domain : ApplicationConfig["APP_DOMAIN"] end def self.url(uri = nil) diff --git a/app/middlewares/set_cookie_domain.rb b/app/middlewares/set_cookie_domain.rb new file mode 100644 index 000000000..4ce8b3962 --- /dev/null +++ b/app/middlewares/set_cookie_domain.rb @@ -0,0 +1,15 @@ +# Since we must explicitly set the cookie domain in session_store before SiteConfig is available, +# this ensures we properly set the cookie to SiteConfig.app_domain at runtime. + +class SetCookieDomain + def initialize(app) + @app = app + end + + def call(env) + if Rails.env.production? + env["rack.session.options"][:domain] = ".#{SiteConfig.app_domain}" + end + @app.call(env) + end +end diff --git a/app/models/message.rb b/app/models/message.rb index 6989c9e1c..0359ca3a5 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -196,19 +196,19 @@ class Message < ApplicationRecord end def rich_link_article(link) - return unless link["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/") && link["href"].split("/")[4] + return unless link["href"].include?("//#{SiteConfig.app_domain}/") && link["href"].split("/")[4] Article.find_by(slug: link["href"].split("/")[4].split("?")[0]) end def rich_link_tag(link) - return unless link["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/t/") + return unless link["href"].include?("//#{SiteConfig.app_domain}/t/") Tag.find_by(name: link["href"].split("/t/")[1].split("/")[0]) end def rich_user_link(link) - return unless link["href"].include?("//#{ApplicationConfig['APP_DOMAIN']}/") + return unless link["href"].include?("//#{SiteConfig.app_domain}/") User.find_by(username: link["href"].split("/")[3].split("/")[0]) end diff --git a/app/models/site_config.rb b/app/models/site_config.rb index 069409ca7..a2d564bb6 100644 --- a/app/models/site_config.rb +++ b/app/models/site_config.rb @@ -11,7 +11,9 @@ class SiteConfig < RailsSettings::Base STACK_ICON = File.read(Rails.root.join("app/assets/images/stack.svg")).freeze LIGHTNING_ICON = File.read(Rails.root.join("app/assets/images/lightning.svg")).freeze + # Core setup field :waiting_on_first_user, type: :boolean, default: !User.exists? + field :app_domain, type: :string, default: ApplicationConfig["APP_DOMAIN"] # API Tokens field :health_check_token, type: :string diff --git a/app/services/streams/twitch_webhook/register.rb b/app/services/streams/twitch_webhook/register.rb index 03c4a4b1d..1f7787d29 100644 --- a/app/services/streams/twitch_webhook/register.rb +++ b/app/services/streams/twitch_webhook/register.rb @@ -47,7 +47,7 @@ module Streams def twitch_stream_updates_url_for_user(user) Rails.application.routes.url_helpers.user_twitch_stream_updates_url(user_id: user.id, - host: ApplicationConfig["APP_DOMAIN"]) + host: SiteConfig.app_domain) end end end diff --git a/app/view_objects/articles/social_image.rb b/app/view_objects/articles/social_image.rb index ea29f6b60..2421c5b32 100644 --- a/app/view_objects/articles/social_image.rb +++ b/app/view_objects/articles/social_image.rb @@ -18,7 +18,7 @@ module Articles end return legacy_article_social_image unless use_new_social_url? - article_social_preview_url(article, format: :png) + article_social_preview_url(article, format: :png, host: SiteConfig.app_domain) end private diff --git a/app/views/service_worker/manifest.json.erb b/app/views/service_worker/manifest.json.erb index ef3e321b4..369243ae4 100644 --- a/app/views/service_worker/manifest.json.erb +++ b/app/views/service_worker/manifest.json.erb @@ -1,6 +1,6 @@ { "name": "<%= community_qualified_name %>", - "short_name": "<%= ApplicationConfig["APP_DOMAIN"] %>", + "short_name": "<%= SiteConfig.app_domain %>", "description": "<%= SiteConfig.community_description %>", "start_url": "/", "display": "standalone", diff --git a/app/views/shell/_top.html.erb b/app/views/shell/_top.html.erb index 361975b2b..420c97512 100644 --- a/app/views/shell/_top.html.erb +++ b/app/views/shell/_top.html.erb @@ -40,8 +40,8 @@ "> " rel="icon" sizes="192x192" /> " rel="icon" sizes="128x128" /> - "> - "> + + diff --git a/app/views/users/_profile.html.erb b/app/views/users/_profile.html.erb index e9d0d554a..9fef2f8ec 100644 --- a/app/views/users/_profile.html.erb +++ b/app/views/users/_profile.html.erb @@ -2,7 +2,7 @@ <%= render "users/additional_authentication" %> -<% if ApplicationConfig["APP_DOMAIN"] == "dev.to" %> +<% if SiteConfig.app_domain == "dev.to" %> <%# @forem/oss Temporary disabling of overly DEV-specialized link. %> <%# We should create a generalized version of the badge page, including a permanent proxied path for the badge image for hotlinking. %>
diff --git a/config/application.rb b/config/application.rb index 19bfdd8ae..b64b08237 100644 --- a/config/application.rb +++ b/config/application.rb @@ -44,6 +44,11 @@ module PracticalDeveloper config.autoload_paths += Dir["#{config.root}/lib"] config.eager_load_paths += Dir["#{config.root}/lib"] + # Middlewares folder is not otherwise autorequired. + Dir["./app/middlewares/*.rb"].sort.each do |file| + require file + end + config.active_job.queue_adapter = :sidekiq config.middleware.use Rack::Deflater @@ -56,6 +61,8 @@ module PracticalDeveloper # Therefore we disable "per_form_csrf_tokens" for the time being. config.action_controller.per_form_csrf_tokens = false + config.middleware.use SetCookieDomain + # NOTE: [Rails 6] # To improve security, Rails embeds the purpose and expiry metadata inside encrypted or signed cookies value. config.action_dispatch.use_cookies_with_metadata = false diff --git a/config/environments/production.rb b/config/environments/production.rb index e459a7fc7..ceed3db19 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -110,13 +110,12 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false - config.app_domain = ENV["APP_DOMAIN"] || "localhost:3000" protocol = ENV["APP_PROTOCOL"] || "http://" config.action_mailer.delivery_method = :smtp config.action_mailer.perform_deliveries = true sendgrid_api_key_present = ENV["SENDGRID_API_KEY"].present? - config.action_mailer.default_url_options = { host: protocol + config.app_domain } + config.action_mailer.default_url_options = { host: protocol + ENV["APP_DOMAIN"] } ActionMailer::Base.smtp_settings = { address: "smtp.sendgrid.net", port: "587", @@ -127,14 +126,13 @@ Rails.application.configure do enable_starttls_auto: true } - if ENV["HEROKU_APP_URL"].present? && ENV["HEROKU_APP_URL"] != config.app_domain + if ENV["HEROKU_APP_URL"].present? && ENV["HEROKU_APP_URL"] != ENV["APP_DOMAIN"] config.middleware.use Rack::HostRedirect, - ENV["HEROKU_APP_URL"] => config.app_domain + ENV["HEROKU_APP_URL"] => ENV["APP_DOMAIN"] end end # rubocop:enable Metrics/BlockLength Rails.application.routes.default_url_options = { - host: Rails.application.config.app_domain, protocol: (ENV["APP_PROTOCOL"] || "http://").delete_suffix("://") } diff --git a/config/initializers/carrierwave_monkeypatch.rb b/config/initializers/carrierwave_monkeypatch.rb index e3d680153..d6f571ea8 100644 --- a/config/initializers/carrierwave_monkeypatch.rb +++ b/config/initializers/carrierwave_monkeypatch.rb @@ -1,14 +1,16 @@ # @forem/systems Force "public_url" even when fog_public is false via this monkeypatch # Because we still want the "public" version path in all current scenarios. +# We also force this to use the SiteConfig instead of APP_DOMAIN because the value +# could change after initial boot. -if Rails.env.production? && ENV["HEROKU_APP_ID"].blank? +if Rails.env.production? module CarrierWave module Storage class Fog < Abstract class File include CarrierWave::Utilities::Uri def url - public_url + public_url.gsub(ApplicationConfig["APP_DOMAIN"], SiteConfig.app_domain) end end end diff --git a/config/initializers/devise_monkeypatch.rb b/config/initializers/devise_monkeypatch.rb new file mode 100644 index 000000000..7214dcf90 --- /dev/null +++ b/config/initializers/devise_monkeypatch.rb @@ -0,0 +1,24 @@ +# Changing the value for "domain" in each context instead of using the one set on boot. +# This allows changing domain settings without restarting app. +module Devise + module Controllers + module Rememberable + # We need to use SiteConfig.app_domain instead of default Rails config on boot + def remember_cookie_values(resource) + options = { httponly: true } + options.merge!(forget_cookie_values(resource)) + options.merge!( + value: resource.class.serialize_into_cookie(resource), + expires: resource.remember_expires_at, + domain: ".#{SiteConfig.app_domain}", + ) + end + + def self.cookie_values + # Default: Rails.configuration.session_options.slice(:path, :domain, :secure) + # We need to use SiteConfig.app_domain instead of default Rails config on boot + { domain: ".#{SiteConfig.app_domain}", secure: ApplicationConfig["FORCE_SSL_IN_RAILS"] == "true" } + end + end + end +end diff --git a/config/sitemap.rb b/config/sitemap.rb index 618d4dc6b..79cb6390b 100644 --- a/config/sitemap.rb +++ b/config/sitemap.rb @@ -1,5 +1,7 @@ require "sitemap_generator/s3_adapter" +# @forem/systems: It's fine if this doesn't 100% work correctly right now, as long as it doesn't break stuff at runtime. + if Rails.env.production? region = ApplicationConfig["AWS_UPLOAD_REGION"].presence || ApplicationConfig["AWS_DEFAULT_REGION"] config_hash = if ENV["FOREM_CONTEXT"] == "forem_cloud" # @forem/systems jdoss's special sauce. diff --git a/spec/decorators/article_decorator_spec.rb b/spec/decorators/article_decorator_spec.rb index daaa4f2c5..d75be2cb9 100644 --- a/spec/decorators/article_decorator_spec.rb +++ b/spec/decorators/article_decorator_spec.rb @@ -46,7 +46,7 @@ RSpec.describe ArticleDecorator, type: :decorator do it "returns the article url without a canonical_url" do article.canonical_url = "" - expected_url = "https://#{ApplicationConfig['APP_DOMAIN']}#{article.path}" + expected_url = "#{ApplicationConfig['APP_PROTOCOL']}#{ApplicationConfig['APP_DOMAIN']}#{article.path}" expect(article.decorate.processed_canonical_url).to eq(expected_url) end end @@ -77,7 +77,7 @@ RSpec.describe ArticleDecorator, type: :decorator do describe "#url" do it "returns the article url" do - expected_url = "https://#{ApplicationConfig['APP_DOMAIN']}#{article.path}" + expected_url = "#{ApplicationConfig['APP_PROTOCOL']}#{ApplicationConfig['APP_DOMAIN']}#{article.path}" expect(article.decorate.url).to eq(expected_url) end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index caddc6399..5a853208a 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -82,6 +82,7 @@ RSpec.describe ApplicationHelper, type: :helper do before do allow(ApplicationConfig).to receive(:[]).with("APP_PROTOCOL").and_return("https://") allow(ApplicationConfig).to receive(:[]).with("APP_DOMAIN").and_return("dev.to") + allow(SiteConfig).to receive(:app_domain).and_return("dev.to") end it "creates the correct base app URL" do diff --git a/spec/helpers/social_image_helper_spec.rb b/spec/helpers/social_image_helper_spec.rb index fddcdbb1f..b92f08b56 100644 --- a/spec/helpers/social_image_helper_spec.rb +++ b/spec/helpers/social_image_helper_spec.rb @@ -36,9 +36,10 @@ describe SocialImageHelper do describe ".article_social_image_url" do it "returns social preview path for newer articles" do + SiteConfig.app_domain = "hello.com" url = helper.article_social_image_url(article) - expect(url).to eq article_social_preview_url(article, format: :png) + expect(url).to eq article_social_preview_url(article, format: :png, host: "hello.com") end it "returns the main image if set" do @@ -59,9 +60,10 @@ describe SocialImageHelper do end it "returns social preview path for newer decorated articles" do + SiteConfig.app_domain = "hello.com" url = helper.article_social_image_url(article.decorate) - expect(url).to eq article_social_preview_url(article, format: :png) + expect(url).to eq article_social_preview_url(article, format: :png, host: "hello.com") end it "returns correct manipulation of cloudinary links" do diff --git a/spec/lib/url_spec.rb b/spec/lib/url_spec.rb index c13c89f23..b10fc6e9a 100644 --- a/spec/lib/url_spec.rb +++ b/spec/lib/url_spec.rb @@ -3,7 +3,8 @@ require "rails_helper" RSpec.describe URL, type: :lib do before do allow(ApplicationConfig).to receive(:[]).with("APP_PROTOCOL").and_return("https://") - allow(ApplicationConfig).to receive(:[]).with("APP_DOMAIN").and_return("dev.to") + allow(ApplicationConfig).to receive(:[]).with("APP_DOMAIN").and_return("test.forem.cloud") + SiteConfig.app_domain = "dev.to" end describe ".protocol" do @@ -13,8 +14,8 @@ RSpec.describe URL, type: :lib do end describe ".domain" do - it "returns the value of APP_DOMAIN env variable" do - expect(described_class.domain).to eq(ApplicationConfig["APP_DOMAIN"]) + it "returns the value of SiteConfig" do + expect(described_class.domain).to eq(SiteConfig.app_domain) end end diff --git a/spec/requests/async_info_spec.rb b/spec/requests/async_info_spec.rb index 4772afc4a..d31552286 100644 --- a/spec/requests/async_info_spec.rb +++ b/spec/requests/async_info_spec.rb @@ -23,7 +23,6 @@ RSpec.describe "AsyncInfo", type: :request do context "when logged in" do it "returns token and user" do - allow(controller_instance).to receive(:remember_user_token).and_return(nil) sign_in create(:user) get "/async_info/base_data" @@ -38,15 +37,4 @@ RSpec.describe "AsyncInfo", type: :request do expect(response.body).to include("version") end end - - describe "#remember_user_token" do - # We require the remember_user_token key bc we also use it for caching in Fastly - # If this key changes, Fastly needs to be updated - it "requires remember_user_token cookie to be present" do - get "/async_info/base_data" - token = "a_token" - controller.__send__("cookies")[:remember_user_token] = "a_token" - expect(controller.remember_user_token).to eq(token) - end - end end diff --git a/spec/requests/stories_index_spec.rb b/spec/requests/stories_index_spec.rb index 266e1a941..658f91ec2 100644 --- a/spec/requests/stories_index_spec.rb +++ b/spec/requests/stories_index_spec.rb @@ -385,6 +385,11 @@ RSpec.describe "StoriesIndex", type: :request do get "/t/#{tag.name}" expect(response.body).not_to include('') end + + it "sets remember_user_token" do + get "/t/#{tag.name}" + expect(response.cookies["remember_user_token"]).not_to be nil + end end context "without user signed in" do @@ -447,6 +452,11 @@ RSpec.describe "StoriesIndex", type: :request do expected_tag = "" expect(response.body).to include(expected_tag) end + + it "sets does not set remember_user_token" do + get "/t/#{tag.name}" + expect(response.cookies["remember_user_token"]).to be nil + end end end