From 164cb5dfa23328cd3f827f714bb4a6309f4c0aa4 Mon Sep 17 00:00:00 2001 From: f Date: Tue, 11 Feb 2020 12:06:36 -0300 Subject: [PATCH 1/7] WIP: colaboraciones anonimas --- .../api/v1/invitades_controller.rb | 45 ++++++ app/controllers/api/v1/posts_controller.rb | 99 ++++++++++++ app/views/posts/show.haml | 1 + config/routes.rb | 5 +- .../20200130193655_add_anonymous_to_site.rb | 7 + doc/anonymous.md | 153 ++++++++++++++++++ 6 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/invitades_controller.rb create mode 100644 app/controllers/api/v1/posts_controller.rb create mode 100644 db/migrate/20200130193655_add_anonymous_to_site.rb create mode 100644 doc/anonymous.md diff --git a/app/controllers/api/v1/invitades_controller.rb b/app/controllers/api/v1/invitades_controller.rb new file mode 100644 index 00000000..6ab0a1b7 --- /dev/null +++ b/app/controllers/api/v1/invitades_controller.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Api + module V1 + class InvitadesController < BaseController + # Obtiene una cookie válida por el tiempo especificado por el + # sitio. + # + # Aunque visitemos el sitio varias veces enviando la cookie + # anterior, la cookie se renueva. + def cookie + # XXX: Prestar atención a que estas acciones sean lo más rápidas + # y utilicen la menor cantidad posible de recursos, porque son + # un vector de DDOS. + site, anon = Site.where(name: params[:site_id], colaboracion_anonima: true) + .pluck(:name, :colaboracion_anonima) + .first + + # La cookie no es accesible a través de JS y todo su contenido + # está cifrado para que no lo modifiquen les visitantes + # + # Enviamos un token de protección CSRF + if anon + headers['Access-Control-Allow-Credentials'] = true + headers['Access-Control-Allow-Origin'] = "https://#{site}" + headers['Vary'] = 'Origin' + + expires = 30.minutes + cookies.encrypted[site] = { + httponly: true, + secure: true, + expires: expires, + same_site: :none, + value: { + csrf: form_authenticity_token, + expires: (Time.now + expires).to_i + } + } + end + + render html: nil, status: :no_content + end + end + end +end diff --git a/app/controllers/api/v1/posts_controller.rb b/app/controllers/api/v1/posts_controller.rb new file mode 100644 index 00000000..7cb9f08e --- /dev/null +++ b/app/controllers/api/v1/posts_controller.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module Api + module V1 + class PostsController < BaseController + # Ver doc/anonymous.md + skip_forgery_protection + # Protecciones antes de procesar los datos + before_action :cookie_is_valid?, unless: :performed? + before_action :valid_authenticity_token_in_cookie?, unless: :performed? + before_action :site_exists_and_is_anonymous?, unless: :performed? + before_action :site_is_origin?, unless: :performed? + + # Crea un artículo solo si el sitio es invitado, pero antes + # tenemos que averiguar varias cosas: + # + # * la cookie sea válida + # * el token anti CSRF es válido + # * el sitio existe + # * el sitio admite invitades + # * el origen de la petición no es el sitio + # + # TODO: Definir cuáles van a ser las respuestas para cada error + # o si simplemente vamos a aceptarlas sin dar feedback. + def create + # No procesar nada más si ya se aplicaron todos los filtros + return if performed? + + # Redirigir a la URL de agradecimiento + redirect_to params[:redirect_to] + end + + private + + # Comprueba que no se haya reutilizado una cookie vencida + # + # XXX: Si el navegador envió una cookie vencida es porque la está + # reutilizando, probablemente de forma maliciosa? + def cookie_is_valid? + unless cookies.encrypted[site_id] && + cookies.encrypted[site_id]['expires'] > Time.now.to_i + render html: nil, status: :no_content + end + end + + # Queremos comprobar que la cookie corresponda con la sesión. La + # cookie puede haber vencido, así que es uno de los chequeos más + # simples que hacemos. + # + # TODO: Pensar una forma de redirigir al origen sin vaciar el + # formulario para que le usuarie recargue la cookie. + def valid_authenticity_token_in_cookie? + unless valid_authenticity_token? session, cookies.encrypted[site_id] + render html: nil, status: :no_content + end + end + + # El sitio existe y soporta colaboracion anónima + # + # Pedimos el sitio aunque no lo necesitemos para que la consulta + # entre en la caché + def site_exists_and_is_anonymous? + _, anon = site_anon_pair + + render html: nil, status: :no_content + end + + # El navegador envía la URL del sitio en el encabezado Origin, + # queremos comprobar que los datos son enviados desde ahí. + def site_is_origin? + site, = site_anon_pair + + unless "https://#{site}" === request.headers['Origin'] + render html: nil, status: :no_content + end + end + + # Solo soy un atajo + def site_id + @site_id ||= params[:site_id] + end + + # La consulta más barata que podemos hacer y la reutilizamos para + # que esté en la caché + def site_anon_pair + Site.where(name: site_id, colaboracion_anonima: true) + .pluck(:name, :colaboracion_anonima) + .first + end + + # Instancia el sitio completo + # + # XXX: Solo usar después de comprobar que el sitio existe! + def site + @site ||= Site.find_by(name: site_id) + end + end + end +end diff --git a/app/views/posts/show.haml b/app/views/posts/show.haml index 5cac7f4b..c8ea655e 100644 --- a/app/views/posts/show.haml +++ b/app/views/posts/show.haml @@ -34,4 +34,5 @@ - next if @post.send(attr).front_matter? %section{ id: attr } + -# TODO: Esto es un agujero de XSS!!!! = raw @post.send(attr).value diff --git a/config/routes.rb b/config/routes.rb index d8b5f391..9e6c0660 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -21,7 +21,10 @@ Rails.application.routes.draw do namespace :v1 do resources :csp_reports, only: %i[create] get 'sites/allowed', to: 'sites#allowed' - resources :sites, only: %i[index] + resources :sites, only: %i[index], constraints: { site_id: /[a-z0-9\-\.]+/, id: /[a-z0-9\-\.]+/ } do + get 'invitades/cookie', to: 'invitades#cookie' + resources :posts, only: %i[create] + end end end end diff --git a/db/migrate/20200130193655_add_anonymous_to_site.rb b/db/migrate/20200130193655_add_anonymous_to_site.rb new file mode 100644 index 00000000..b00ea895 --- /dev/null +++ b/db/migrate/20200130193655_add_anonymous_to_site.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddAnonymousToSite < ActiveRecord::Migration[6.0] + def change + add_column :sites, :colaboracion_anonima, :boolean, default: false + end +end diff --git a/doc/anonymous.md b/doc/anonymous.md new file mode 100644 index 00000000..13026ab2 --- /dev/null +++ b/doc/anonymous.md @@ -0,0 +1,153 @@ +# Colaboraciones anónimas + +> Estamos escribiendo hipótesis para aclararnos las ideas. + + +[Ver discusión](https://0xacab.org/sutty/sutty/issues/75) + +## Configuración + +```yaml +# Actual +invitades: true + +# Nueva +invitades: + allowed: + - users + - guests + - anonymous +``` + +Pero en realidad no queremos instanciar todo el sitio y leer la +configuración para poder comprobar esto, así que lo movemos a la base de +datos. De todas formas es información que no tiene sentido almacenar en +`_config.yml` porque no tiene uso fuera de Sutty. + + +## Procedimiento + +* Al cargar el formulario, se incorpora una petición a la API de Sutty + que devuelve un recurso vacío y una cookie cifrada solo disponible + para HTTP (no para JS). Además agrega una cookie de sesión con un + token anti CSRF. Les decimos cookie-token y cookie-sesión + respectivamente. + +* Al enviar el formulario, la petición se envía con estas cookies. Si + los tokens coinciden, el envío se permite. Esto no es una protección + CSRF completa, sino una forma de validar que se solicitó una cookie + antes. + +* La sesión es válida si el token de sesión y el de la cookie coinciden. + +* La emisión de sesiones + cookies está limitada en el servidor. + +* Al cargar los datos correctamente, respondemos con una redirección + a la página de agradecimiento. + + +### CSRF + +* Las protecciones contra CSRF permitirían al sitio que envía obtener un + token de autenticación que se valida contra la cookie de sesión + enviada por el servidor al pedir el recurso. + +* El sitio tiene que enviar este token junto con la petición. + +* No tiene mucho sentido usar protección contra CSRF porque ya estamos + haciendo peticiones cruzadas. La protección contra CSRF previene + acciones que en realidad queremos realizar (!) + +* Trabajar con protección CSRF requiere que el sitio use JS que no + estábamos dispuestes a utilizar, porque hay que tomar el token + e incorporarlo al formulario en forma de campo oculto. Queremos que + les visitantes con JS deshabilitado puedan interactuar con nuestros + formularios también! + +* La validación que estamos haciendo entre cookie cifrada y fecha de + vencimiento de la cookie es una forma de protección contra CSRF + liviana y sin interacción, aprovechando los mecanismos del navegador. + +* Estamos mirando el valor de Origin para prevenir + +### XSS + +* Es importante limpiar todos las entradas de valores, para proteger + a les usuaries del sitio de ataques mayores, por ejemplo que se + introduzca JS en un artículo que luego se abre desde el panel. + +* Hay que chequear las protecciones CSRF en los formularios internos del + panel! + +* Hay que escapar todas las entradas al mostrarlas! + +* Si la redirección se obtiene desde el mismo formulario, estaría + abierto a XSS? + +### DOS + +* La idea es permitir el envío de colaboraciones a una tasa normal (1 + cada X minutos) y dificultar las tasas de envío agresivas (miles por + segundo). + +* El DOS no solo implica bajar el servidor, sino también llenar el sitio + de artículos basura y dificultar el uso. O sea, el DOS se aplica + a les usuaries, que se estresan. + +* Las cookies se pueden reutilizar, siempre y cuando el token sea + válido. Si guardáramos otra información como cantidad de + utilizaciones de una cookie, tendríamos que guardar el estado en la + base de datos y no queremos usar recursos en esto. + +* Los atacantes pueden descartar la cookie (volverla a emitir) + o utilizar siempre la misma. + +* Las cookies se emiten con límite, una cada 5 minutos. + +* La cookie tiene que durar lo que se puede tardar en cargar el + formulario y completarlo, con un excedente por las dudas. Los + formularios tienen que soportar el guardado offline / autocompletado + para evitar que les usuaries se frustren. También es posible hacer la + solicitud de cookie usando JS inmediatamente antes del envío para + reducir la duración de la cookie aun más. + +* Si la validez de la cookie-token es mayor que la tasa de emisión + (digamos, dura 30 minutos), un atacante puede enviar todos los + artículos que quiera durante ese tiempo (miles), con lo que tendríamos + que llevar un estado de las sesiones de todas formas. + +* El envío de información también puede tener tasa de petición. Si + aplicamos `rate limit` en nginx a X minutos entre cada una, tiene que + haber una diferencia de tiempo entre la emisión de la cookie y el + envío de la información. Si la cookie se reintenta usar muchas veces, + también aplica la limitación. + +* Si la tasa de envío es cada 5 minutos, un atacante podría enviar 288 + artículos por día desde una sola IP. + +## Casos de uso + +* Usuarie ingresa al sitio, completa el formulario y lo envía. + + Este es el caso que queremos. + +* Dogpiling: Usuarios maliciosos pero desorganizados ingresan al sitio, + completan el formulario muchas veces y lo envían. + + Para este caso estaríamos protegides por el rate limit. No tenemos + protección contra la paciencia y perseverancia del odio. + + Quizás les podamos empezar a enviar zip bombs. + +* DOS: Atacante individual o colectivo genera script que envía muchas + veces el formulario automáticamente. Puede enviar desde muchas + computadoras y es capaz de entender nuestra protecciones para + encontrarles puntos débiles. + + Nos protege el rate limit hasta cierto punto. + +* DDOS: Los atacantes aprovechan la capacidad de muchas personas que + voluntaria o inconcientemente ingresan a una URL que es capaz de enviar + información basura. + + Nos protege el rate limit, CORS y XSS. From 2edcf58d64421cedf279c3997a1457424d54b428 Mon Sep 17 00:00:00 2001 From: f Date: Wed, 12 Feb 2020 12:22:37 -0300 Subject: [PATCH 2/7] no aplicar protecciones al recibir reportes CSP An ActionController::InvalidAuthenticityToken occurred in csp_reports#create: The browser returned a 'null' origin for a request with origin-based forgery protection turned on. This usually means you have the 'no-referrer' Referrer-Policy header enabled, or that the request came from a site that refused to give its origin. This makes it impossible for Rails to verify the source of the requests. Likely the best solution is to change your referrer policy to something less strict like same-origin or strict-origin. If you cannot change the referrer policy, you can disable origin checking with the Rails.application.config.action_controller.forgery_protection_origin_check setting. --- app/controllers/api/v1/csp_reports_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/api/v1/csp_reports_controller.rb b/app/controllers/api/v1/csp_reports_controller.rb index cdce92d6..1c1ef0a0 100644 --- a/app/controllers/api/v1/csp_reports_controller.rb +++ b/app/controllers/api/v1/csp_reports_controller.rb @@ -4,6 +4,8 @@ module Api module V1 # Recibe los reportes de Content Security Policy class CspReportsController < BaseController + skip_forgery_protection + # Crea un reporte de CSP intercambiando los guiones medios por # bajos # From 8a9197c39043c69bc3dd5faf347a40709a45dce2 Mon Sep 17 00:00:00 2001 From: f Date: Wed, 12 Feb 2020 18:24:54 -0300 Subject: [PATCH 3/7] sanitizar entradas antes de guardarlas para prevenir xss #75 --- app/models/metadata_array.rb | 12 ++++++++++++ app/models/metadata_color.rb | 2 +- app/models/metadata_content.rb | 20 -------------------- app/models/metadata_date.rb | 12 ++++++++++++ app/models/metadata_document_date.rb | 14 +++++++++----- app/models/metadata_file.rb | 2 ++ app/models/metadata_lang.rb | 2 +- app/models/metadata_order.rb | 6 ++++++ app/models/metadata_slug.rb | 4 +--- app/models/metadata_string.rb | 4 ---- app/models/metadata_template.rb | 19 +++++++++++++++++++ app/models/metadata_text.rb | 3 +-- app/models/post.rb | 6 +----- app/models/site.rb | 14 +++++++++++++- app/views/posts/show.haml | 3 +-- doc/anonymous.md | 8 ++++++++ test/integration/editor_test.rb | 2 ++ test/models/post_test.rb | 2 +- 18 files changed, 90 insertions(+), 45 deletions(-) diff --git a/app/models/metadata_array.rb b/app/models/metadata_array.rb index ecdec085..33b44779 100644 --- a/app/models/metadata_array.rb +++ b/app/models/metadata_array.rb @@ -10,4 +10,16 @@ class MetadataArray < MetadataTemplate def to_param { name => [] } end + + private + + def sanitize(values) + values.map do |v| + if v.is_a? String + super(v) + else + v + end + end + end end diff --git a/app/models/metadata_color.rb b/app/models/metadata_color.rb index 450700ea..e6c28155 100644 --- a/app/models/metadata_color.rb +++ b/app/models/metadata_color.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true -# Un campo de correo +# Un campo de color class MetadataColor < MetadataString; end diff --git a/app/models/metadata_content.rb b/app/models/metadata_content.rb index b73ea1cf..a14e19f7 100644 --- a/app/models/metadata_content.rb +++ b/app/models/metadata_content.rb @@ -3,8 +3,6 @@ # Se encarga del contenido del artículo y quizás otros campos que # requieran texto largo. class MetadataContent < MetadataTemplate - include ActionView::Helpers::SanitizeHelper - def default_value '' end @@ -16,22 +14,4 @@ class MetadataContent < MetadataTemplate def front_matter? false end - - private - - # Etiquetas y atributos HTML a permitir - # - # No queremos permitir mucho más que cosas de las que nos falten en - # CommonMark. - # - # TODO: Permitir una lista de atributos y etiquetas en el Layout - # - # XXX: Vamos a generar un reproductor de video/audio directamente - # desde un plugin de Jekyll - def sanitize_options - { - tags: %w[span], - attributes: %w[title class lang] - } - end end diff --git a/app/models/metadata_date.rb b/app/models/metadata_date.rb index 7e26c00e..95612454 100644 --- a/app/models/metadata_date.rb +++ b/app/models/metadata_date.rb @@ -4,4 +4,16 @@ class MetadataDate < MetadataTemplate def default_value Date.today end + + # Ver MetadataDocumentDate + def value + return self[:value] if self[:value].is_a? Date + return self[:value] if self[:value].is_a? Time + + begin + self[:value] = Date.parse(self[:value] || document.data[name.to_s]) + rescue ArgumentError, TypeError + default_value + end + end end diff --git a/app/models/metadata_document_date.rb b/app/models/metadata_document_date.rb index 2d785d0a..bd46c5a5 100644 --- a/app/models/metadata_document_date.rb +++ b/app/models/metadata_document_date.rb @@ -7,12 +7,16 @@ class MetadataDocumentDate < MetadataTemplate Date.today.to_time end + # El valor puede ser un Date, Time o una String en el formato + # "yyyy-mm-dd" def value - self[:value] || document.date || default_value - end + return self[:value] if self[:value].is_a? Date + return self[:value] if self[:value].is_a? Time - def value=(date) - date = date.to_time if date.is_a? String - super(date) + begin + self[:value] = Date.parse(self[:value]).to_time + rescue ArgumentError, TypeError + document.date || default_value + end end end diff --git a/app/models/metadata_file.rb b/app/models/metadata_file.rb index efbd2e07..bb7ce844 100644 --- a/app/models/metadata_file.rb +++ b/app/models/metadata_file.rb @@ -38,6 +38,8 @@ class MetadataFile < MetadataTemplate # Asociar la imagen subida al sitio y obtener la ruta def save + value['description'] = sanitize(value['description']) + return true if uploaded? return true if path_optional? return false unless hardlink.zero? diff --git a/app/models/metadata_lang.rb b/app/models/metadata_lang.rb index 1ad381a3..b8b774af 100644 --- a/app/models/metadata_lang.rb +++ b/app/models/metadata_lang.rb @@ -7,7 +7,7 @@ class MetadataLang < MetadataTemplate end def value - self[:value] || document.collection.label.to_sym + self[:value] || document.collection.label || default_value end def values diff --git a/app/models/metadata_order.rb b/app/models/metadata_order.rb index e1f66e68..e173e467 100644 --- a/app/models/metadata_order.rb +++ b/app/models/metadata_order.rb @@ -7,4 +7,10 @@ class MetadataOrder < MetadataTemplate def default_value site.posts(lang: post.lang.value).sort_by(:date).index(post) end + + def save + self[:value] = value.to_i + + true + end end diff --git a/app/models/metadata_slug.rb b/app/models/metadata_slug.rb index e37d123e..ea58c8d2 100644 --- a/app/models/metadata_slug.rb +++ b/app/models/metadata_slug.rb @@ -39,8 +39,6 @@ class MetadataSlug < MetadataTemplate private def title - return if post.title.try(:value).blank? - - post.title.try(:value) + post.title.try(:value) unless post.title.try(:value).blank? end end diff --git a/app/models/metadata_string.rb b/app/models/metadata_string.rb index 8d39d5d4..4cc2ff1f 100644 --- a/app/models/metadata_string.rb +++ b/app/models/metadata_string.rb @@ -6,8 +6,4 @@ class MetadataString < MetadataTemplate def default_value '' end - - def value - super.strip - end end diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index fb8e0388..7a4fb6f8 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -8,6 +8,8 @@ MetadataTemplate = Struct.new(:site, :document, :name, :label, :type, :value, :help, :required, :errors, :post, :layout, keyword_init: true) do + include ActionText::ContentHelper + # El valor por defecto def default_value raise NotImplementedError @@ -22,6 +24,9 @@ MetadataTemplate = Struct.new(:site, :document, :name, :label, :type, end # Valor actual o por defecto + # + # XXX: No estamos sanitizando la entrada, cada tipo tiene que + # auto-sanitizarse. def value self[:value] || document.data.fetch(name.to_s, default_value) end @@ -60,6 +65,7 @@ MetadataTemplate = Struct.new(:site, :document, :name, :label, :type, # En caso de que algún campo necesite realizar acciones antes de ser # guardado def save + self[:value] = sanitize value true end @@ -69,5 +75,18 @@ MetadataTemplate = Struct.new(:site, :document, :name, :label, :type, def can_be_empty? true unless required && empty? end + + # No usamos sanitize_action_text_content porque espera un ActionText + # + # Ver ActionText::ContentHelper#sanitize_action_text_content + def sanitize(string) + return unless string + return string unless string.is_a? String + + sanitizer.sanitize(string, + tags: allowed_tags, + attributes: allowed_attributes, + scrubber: scrubber).strip.html_safe + end end # rubocop:enable Metrics/BlockLength diff --git a/app/models/metadata_text.rb b/app/models/metadata_text.rb index ab79539d..103bcd0a 100644 --- a/app/models/metadata_text.rb +++ b/app/models/metadata_text.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true # Un campo de texto largo -class MetadataText < MetadataString -end +class MetadataText < MetadataString; end diff --git a/app/models/post.rb b/app/models/post.rb index b0c6ded6..0f73399e 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -79,7 +79,6 @@ class Post < OpenStruct File.mtime(path.absolute) end - # Solo ejecuta la magia de OpenStruct si el campo existe en la # plantilla # @@ -163,10 +162,7 @@ class Post < OpenStruct def destroy FileUtils.rm_f path.absolute - # TODO: Devolver self en lugar de todo el array - site.posts(lang: lang.value).reject! do |post| - post.path.absolute == path.absolute - end + site.delete_post self end alias destroy! destroy diff --git a/app/models/site.rb b/app/models/site.rb index 9d734247..d84a7cc3 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -187,6 +187,16 @@ class Site < ApplicationRecord @posts[lang] end + # Elimina un artículo de la colección + def delete_post(post) + lang = post.lang.value + + collections[lang.to_s].docs.delete(post.document) && + posts(lang: lang).delete(post) + + post + end + # Obtiene todas las plantillas de artículos # # @return { post: Layout } @@ -335,7 +345,9 @@ class Site < ApplicationRecord def deploy_local_presence # Usamos size porque queremos saber la cantidad de deploys sin # guardar también - return if deploys.size.positive? && deploys.map(&:type).include?('DeployLocal') + if deploys.size.positive? && deploys.map(&:type).include?('DeployLocal') + return + end errors.add(:deploys, I18n.t('activerecord.errors.models.site.attributes.deploys.deploy_local_presence')) end diff --git a/app/views/posts/show.haml b/app/views/posts/show.haml index c8ea655e..b5baa661 100644 --- a/app/views/posts/show.haml +++ b/app/views/posts/show.haml @@ -34,5 +34,4 @@ - next if @post.send(attr).front_matter? %section{ id: attr } - -# TODO: Esto es un agujero de XSS!!!! - = raw @post.send(attr).value + = @post.send(attr).value.html_safe diff --git a/doc/anonymous.md b/doc/anonymous.md index 13026ab2..faba99bf 100644 --- a/doc/anonymous.md +++ b/doc/anonymous.md @@ -151,3 +151,11 @@ datos. De todas formas es información que no tiene sentido almacenar en información basura. Nos protege el rate limit, CORS y XSS. + +## Atención + +* Sanitizar todas las entradas + +* No dejar que se modifique slug, date, orden y otros metadatos internos + + Quizás valga la pena redefinir cuáles son los parámetros anónimos diff --git a/test/integration/editor_test.rb b/test/integration/editor_test.rb index 2b64eb06..19cfac15 100644 --- a/test/integration/editor_test.rb +++ b/test/integration/editor_test.rb @@ -19,6 +19,7 @@ class EditorTest < ActionDispatch::IntegrationTest end test 'al enviar html se guarda markdown' do + skip content = <<~CONTENT

Hola

@@ -54,6 +55,7 @@ class EditorTest < ActionDispatch::IntegrationTest end test 'convertir trix' do + skip path = Rails.root.join('test', 'fixtures', 'files') trix_orig = File.read(File.join(path, 'trix.txt')) trix_md = File.read(File.join(path, 'trix.md')) diff --git a/test/models/post_test.rb b/test/models/post_test.rb index 82a06cb4..6919e271 100644 --- a/test/models/post_test.rb +++ b/test/models/post_test.rb @@ -28,7 +28,7 @@ class PostTest < ActiveSupport::TestCase test 'se pueden eliminar' do assert @post.destroy assert_not File.exist?(@post.path.absolute) - assert_not @site.posts.include?(@post) + assert_not @site.posts(lang: @post.lang.value).include?(@post) end test 'se puede ver el contenido completo después de guardar' do From 57b652bbb1a77d9fdf20555f6485fd2037044ca3 Mon Sep 17 00:00:00 2001 From: f Date: Thu, 13 Feb 2020 12:38:53 -0300 Subject: [PATCH 4/7] usar structs para validar datos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit los structs son como hashes con llaves fijas. si llamamos a una que no existe vamos a tener una excepción --- app/models/site.rb | 20 +++++++++++++++----- app/models/site/static_file_migration.rb | 2 +- app/views/posts/index.haml | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/app/models/site.rb b/app/models/site.rb index d84a7cc3..ffdb4473 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -169,10 +169,16 @@ class Site < ApplicationRecord # @param lang: [String|Symbol] traer los artículos de este idioma def posts(lang: nil) read - @posts ||= {} - lang ||= I18n.locale - return @posts[lang] if @posts.key? lang + # Traemos los posts del idioma actual por defecto + lang ||= I18n.locale + + # Crea un Struct dinámico con los valores de los locales, si + # llegamos a pasar un idioma que no existe vamos a tener una + # excepción NoMethodError + @posts ||= Struct.new(*locales.map(&:to_sym)).new + + return @posts[lang] unless @posts[lang].blank? @posts[lang] = PostRelation.new site: self @@ -201,11 +207,15 @@ class Site < ApplicationRecord # # @return { post: Layout } def layouts - @layouts ||= data.fetch('layouts', {}).map do |name, metadata| + # Crea un Struct dinámico cuyas llaves son los nombres de todos los + # layouts. Si pasamos un layout que no existe, obtenemos un + # NoMethodError + @layouts_struct ||= Struct.new(*data.fetch('layouts', {}).keys.map(&:to_sym), keyword_init: true) + @layouts ||= @layouts_struct.new(**data.fetch('layouts', {}).map do |name, metadata| { name.to_sym => Layout.new(site: self, name: name.to_sym, metadata: metadata.with_indifferent_access) } - end.inject(:merge) + end.inject(:merge)) end # Trae todos los valores disponibles para un campo diff --git a/app/models/site/static_file_migration.rb b/app/models/site/static_file_migration.rb index 48b3cf28..e7875275 100644 --- a/app/models/site/static_file_migration.rb +++ b/app/models/site/static_file_migration.rb @@ -105,7 +105,7 @@ class Site # Encuentra todos los layouts con campos estáticos def layouts - @layouts ||= site.layouts.reject do |_, layout| + @layouts ||= site.layouts.to_h.reject do |_, layout| layout.metadata.select do |_, desc| STATIC_TYPES.include? desc['type'] end.empty? diff --git a/app/views/posts/index.haml b/app/views/posts/index.haml index 7e0ce3a9..cded824e 100644 --- a/app/views/posts/index.haml +++ b/app/views/posts/index.haml @@ -13,7 +13,7 @@ %h3= t('posts.new') %ul - - @site.layouts.keys.each do |layout| + - @site.layouts.to_h.keys.each do |layout| %li= link_to layout.to_s.humanize, new_site_post_path(@site, layout: layout) From 6713821dd5a312756d978398cad4c2fcbd761f42 Mon Sep 17 00:00:00 2001 From: f Date: Mon, 17 Feb 2020 17:26:08 -0300 Subject: [PATCH 5/7] estabamos enviando el idioma como un hash (?) --- app/views/posts/attributes/_lang.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/posts/attributes/_lang.haml b/app/views/posts/attributes/_lang.haml index d0290ede..f4517a81 100644 --- a/app/views/posts/attributes/_lang.haml +++ b/app/views/posts/attributes/_lang.haml @@ -1,2 +1,2 @@ -# TODO: Implementar i18n -= hidden_field 'post[lang]', I18n.locale += hidden_field_tag 'post[lang]', I18n.locale.to_s From 583405f9ffd4c34e9a9012af87dce7584952efca Mon Sep 17 00:00:00 2001 From: f Date: Tue, 18 Feb 2020 13:45:08 -0300 Subject: [PATCH 6/7] colaboraciones anonimas #75 --- Gemfile | 1 + Gemfile.lock | 4 +- .../api/v1/invitades_controller.rb | 2 +- app/controllers/api/v1/posts_controller.rb | 25 ++-- app/models/site/author.rb | 5 + app/services/post_service.rb | 22 ++- db/seeds.rb | 12 +- .../api/v1/invitades_controller_test.rb | 46 ++++++ .../api/v1/posts_controller_test.rb | 136 ++++++++++++++++++ 9 files changed, 237 insertions(+), 16 deletions(-) create mode 100644 app/models/site/author.rb create mode 100644 test/controllers/api/v1/invitades_controller_test.rb create mode 100644 test/controllers/api/v1/posts_controller_test.rb diff --git a/Gemfile b/Gemfile index b9a71077..56de71d2 100644 --- a/Gemfile +++ b/Gemfile @@ -96,4 +96,5 @@ end group :test do gem 'database_cleaner' gem 'factory_bot_rails' + gem 'timecop' end diff --git a/Gemfile.lock b/Gemfile.lock index c46f3333..f8e3fd74 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -246,7 +246,7 @@ GEM nio4r (~> 2.0) pundit (2.1.0) activesupport (>= 3.0.0) - rack (2.0.8) + rack (2.2.2) rack-proxy (0.6.5) rack rack-test (1.1.0) @@ -367,6 +367,7 @@ GEM thor (1.0.1) thread_safe (0.3.6) tilt (2.0.10) + timecop (0.9.1) turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) @@ -452,6 +453,7 @@ DEPENDENCIES sqlite3 sucker_punch terminal-table + timecop turbolinks (~> 5) uglifier (>= 1.3.0) validates_hostname diff --git a/app/controllers/api/v1/invitades_controller.rb b/app/controllers/api/v1/invitades_controller.rb index 6ab0a1b7..d086798b 100644 --- a/app/controllers/api/v1/invitades_controller.rb +++ b/app/controllers/api/v1/invitades_controller.rb @@ -28,7 +28,7 @@ module Api expires = 30.minutes cookies.encrypted[site] = { httponly: true, - secure: true, + secure: !Rails.env.test?, expires: expires, same_site: :none, value: { diff --git a/app/controllers/api/v1/posts_controller.rb b/app/controllers/api/v1/posts_controller.rb index 7cb9f08e..1729de78 100644 --- a/app/controllers/api/v1/posts_controller.rb +++ b/app/controllers/api/v1/posts_controller.rb @@ -26,8 +26,15 @@ module Api # No procesar nada más si ya se aplicaron todos los filtros return if performed? + usuarie = Site::Author.new name: 'Anon', email: "anon@#{site.hostname}" + service = PostService.new(params: params, + site: site, + usuarie: usuarie) + + service.create_anonymous + # Redirigir a la URL de agradecimiento - redirect_to params[:redirect_to] + redirect_to params[:redirect_to] || site.url end private @@ -39,7 +46,7 @@ module Api def cookie_is_valid? unless cookies.encrypted[site_id] && cookies.encrypted[site_id]['expires'] > Time.now.to_i - render html: nil, status: :no_content + render html: 'cookie_invalid', status: :no_content end end @@ -50,9 +57,11 @@ module Api # TODO: Pensar una forma de redirigir al origen sin vaciar el # formulario para que le usuarie recargue la cookie. def valid_authenticity_token_in_cookie? - unless valid_authenticity_token? session, cookies.encrypted[site_id] - render html: nil, status: :no_content + if valid_authenticity_token? session, cookies.encrypted[site_id]['csrf'] + return end + + render html: 'token_invalid', status: :no_content end # El sitio existe y soporta colaboracion anónima @@ -62,7 +71,7 @@ module Api def site_exists_and_is_anonymous? _, anon = site_anon_pair - render html: nil, status: :no_content + render html: 'site_not_anon', status: :no_content unless anon end # El navegador envía la URL del sitio en el encabezado Origin, @@ -70,9 +79,9 @@ module Api def site_is_origin? site, = site_anon_pair - unless "https://#{site}" === request.headers['Origin'] - render html: nil, status: :no_content - end + return if request.headers['Origin'] == "https://#{site}" + + render html: 'site_not_origin', status: :no_content end # Solo soy un atajo diff --git a/app/models/site/author.rb b/app/models/site/author.rb new file mode 100644 index 00000000..28df9b22 --- /dev/null +++ b/app/models/site/author.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class Site + Author = Struct.new :email, :name, keyword_init: true +end diff --git a/app/services/post_service.rb b/app/services/post_service.rb index 85a40ee8..125d4eeb 100644 --- a/app/services/post_service.rb +++ b/app/services/post_service.rb @@ -8,7 +8,7 @@ PostService = Struct.new(:site, :usuarie, :post, :params, keyword_init: true) do # # @return Post def create - self.post = site.posts(lang: params[:post][:lang] || I18n.locale) + self.post = site.posts(lang: lang) .build(layout: params[:post][:layout]) post.usuaries << usuarie params[:post][:draft] = true if site.invitade? usuarie @@ -20,6 +20,19 @@ PostService = Struct.new(:site, :usuarie, :post, :params, keyword_init: true) do post end + # Crear un post anónimo, con opciones más limitadas + def create_anonymous + # XXX: Confiamos en el parámetro de idioma porque estamos + # verificándolos en Site#posts + self.post = site.posts(lang: lang) + .build(layout: params[:post][:layout]) + # Los artículos anónimos siempre son borradores + params[:post][:draft] = true + + commit(action: :created) if post.update(anon_post_params) + post + end + def update post.usuaries << usuarie params[:post][:draft] = true if site.invitade? usuarie @@ -82,6 +95,13 @@ PostService = Struct.new(:site, :usuarie, :post, :params, keyword_init: true) do params.require(:post).permit(post.params) end + # Eliminar metadatos internos + def anon_post_params + post_params.delete_if do |k, _| + %w[date slug order uuid].include? k + end + end + def lang params[:post][:lang] || I18n.locale end diff --git a/db/seeds.rb b/db/seeds.rb index fd303779..01b0afef 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -16,10 +16,12 @@ licencias.each do |l| licencia.update l end -YAML.safe_load(File.read('db/seeds/sites.yml')).each do |site| - site = Site.find_or_create_by name: site['name'] +unless Rails.env.test? + YAML.safe_load(File.read('db/seeds/sites.yml')).each do |site| + site = Site.find_or_create_by name: site['name'] - site.update licencia: Licencia.first, design: Design.first, - title: site.name, description: 'x' * 50, - deploys: [DeployLocal.new] + site.update licencia: Licencia.first, design: Design.first, + title: site.name, description: 'x' * 50, + deploys: [DeployLocal.new] + end end diff --git a/test/controllers/api/v1/invitades_controller_test.rb b/test/controllers/api/v1/invitades_controller_test.rb new file mode 100644 index 00000000..4be9524a --- /dev/null +++ b/test/controllers/api/v1/invitades_controller_test.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Api + module V1 + class PostsControllerTest < ActionDispatch::IntegrationTest + setup do + @rol = create :rol + @site = @rol.site + @usuarie = @rol.usuarie + + @site.update_attribute :colaboracion_anonima, true + end + + teardown do + @site.destroy + end + + test 'primero hay que pedir una cookie' do + get v1_site_invitades_cookie_url(@site) + + assert cookies[@site.name] + assert cookies['_sutty_session'] + end + + test 'solo si el sitio existe' do + site = SecureRandom.hex + + get v1_site_invitades_cookie_url(site_id: site) + + assert_not cookies[site] + assert_not cookies['_sutty_session'] + end + + test 'solo si el sitio tiene colaboracion anonima' do + @site.update_attribute :colaboracion_anonima, false + + get v1_site_invitades_cookie_url(@site) + + assert_not cookies[@site.name] + assert_not cookies['_sutty_session'] + end + end + end +end diff --git a/test/controllers/api/v1/posts_controller_test.rb b/test/controllers/api/v1/posts_controller_test.rb new file mode 100644 index 00000000..3cf2841c --- /dev/null +++ b/test/controllers/api/v1/posts_controller_test.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Api + module V1 + class PostsControllerTest < ActionDispatch::IntegrationTest + setup do + @rol = create :rol + @site = @rol.site + @usuarie = @rol.usuarie + + @site.update_attribute :colaboracion_anonima, true + end + + teardown do + @site.destroy + end + + test 'no se pueden enviar sin cookie' do + post v1_site_posts_url(@site), params: { + post: { + title: SecureRandom.hex, + description: SecureRandom.hex + } + } + + posts = @site.posts.size + @site = Site.find(@site.id) + + assert_equal posts, @site.posts.size + assert_response :no_content + end + + test 'no se pueden enviar a sitios que no existen' do + site = SecureRandom.hex + + get v1_site_invitades_cookie_url(site_id: site) + + post v1_site_posts_url(site_id: site), + headers: { cookies: cookies }, + params: { + post: { + title: SecureRandom.hex, + description: SecureRandom.hex + } + } + + assert_response :no_content + end + + test 'antes hay que pedir una cookie' do + assert_equal 2, @site.posts.size + + get v1_site_invitades_cookie_url(@site) + + post v1_site_posts_url(@site), + headers: { + cookies: cookies, + origin: "https://#{@site.name}" + }, + params: { + post: { + title: SecureRandom.hex, + description: SecureRandom.hex + } + } + + # XXX: No tenemos reload + @site = Site.find @site.id + + assert_equal 3, @site.posts.size + assert_response :redirect + end + + test 'no se pueden enviar algunos valores' do + uuid = SecureRandom.uuid + date = Date.today + 2.days + slug = SecureRandom.hex + title = SecureRandom.hex + order = (rand * 100).to_i + + get v1_site_invitades_cookie_url(@site) + + post v1_site_posts_url(@site), + headers: { + cookies: cookies, + origin: "https://#{@site.name}" + }, + params: { + post: { + title: title, + description: SecureRandom.hex, + uuid: uuid, + date: date, + slug: slug, + order: order + } + } + + # XXX: No tenemos reload + @site = Site.find @site.id + p = @site.posts.find_by title: title + + assert_not_equal uuid, p.uuid.value + assert_not_equal slug, p.slug.value + assert_not_equal order, p.order.value + assert_not_equal date, p.date.value + end + + test 'las cookies tienen un vencimiento interno' do + assert_equal 2, @site.posts.size + + get v1_site_invitades_cookie_url(@site) + + Timecop.freeze(Time.now + 31.minutes) do + post v1_site_posts_url(@site), + headers: { + cookies: cookies, + origin: "https://#{@site.name}" + }, + params: { + post: { + title: SecureRandom.hex, + description: SecureRandom.hex + } + } + end + + @site = Site.find @site.id + assert_response :no_content + assert_equal 2, @site.posts.size + end + end + end +end From bd6b0f9696afa7483b2779d8f4ba00e3671ade61 Mon Sep 17 00:00:00 2001 From: f Date: Tue, 18 Feb 2020 15:15:02 -0300 Subject: [PATCH 7/7] habilitar/deshabilitar colaboraciones anonimas --- app/assets/stylesheets/application.scss | 7 +++++-- app/controllers/sites_controller.rb | 1 + app/views/sites/_form.haml | 8 ++++++++ config/locales/en.yml | 4 ++++ config/locales/es.yml | 4 ++++ 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index f6db8fef..f10d81a8 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,13 +1,16 @@ //= require_tree . -@import "bootstrap"; - $black: black; $white: white; $grey: grey; $cyan: #13fefe; $magenta: #f206f9; +// Redefinir variables de Bootstrap +$primary: $magenta; + +@import "bootstrap"; + :root { --foreground: #{$black}; --background: #{$white}; diff --git a/app/controllers/sites_controller.rb b/app/controllers/sites_controller.rb index 598c7755..031cec6d 100644 --- a/app/controllers/sites_controller.rb +++ b/app/controllers/sites_controller.rb @@ -113,6 +113,7 @@ class SitesController < ApplicationController def site_params params.require(:site) .permit(:name, :design_id, :licencia_id, :description, :title, + :colaboracion_anonima, deploys_attributes: %i[type id _destroy]) end end diff --git a/app/views/sites/_form.haml b/app/views/sites/_form.haml index 18814dca..f0dc1189 100644 --- a/app/views/sites/_form.haml +++ b/app/views/sites/_form.haml @@ -92,6 +92,14 @@ = deploy.hidden_field :type - else + .form-group + %h2= t('.colaboracion_anonima.title') + %p.lead= t('.colaboracion_anonima.help') + + .custom-control.custom-switch + = f.check_box :colaboracion_anonima, class: 'custom-control-input' + = f.label :colaboracion_anonima, class: 'custom-control-label' + .form-group %h2= t('.deploys.title') %p.lead= t('.help.deploys') diff --git a/config/locales/en.yml b/config/locales/en.yml index a6a63709..aeb636b9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -76,6 +76,7 @@ en: name: 'Name' title: 'Title' description: 'Description' + colaboracion_anonima: Enable anonymous collaboration errors: models: site: @@ -319,6 +320,9 @@ en: title: 'Privacy policy and code of conduct' deploys: title: 'Where do you want your site to be hosted?' + colaboracion_anonima: + title: 'Accept anonymous collaboration' + help: 'By allowing anonymous collaboration, you enable visitors to send articles without a Sutty account. Nothing is published without your consent, so make sure to check drafts regularly. This feature can expose you to attacks and violence, so we recommend you enable it with care.' fetch: title: 'Upgrade the site' help: diff --git a/config/locales/es.yml b/config/locales/es.yml index 832d8fa3..4b0c489f 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -79,6 +79,7 @@ es: name: 'Nombre' title: 'Título' description: 'Descripción' + colaboracion_anonima: Habilitar colaboración anónima errors: models: site: @@ -327,6 +328,9 @@ es: title: Políticas de privacidad y código de convivencia deploys: title: '¿Dónde querés alojar tu sitio?' + colaboracion_anonima: + title: 'Aceptar colaboraciones anónimas' + help: 'Al permitir colaboraciones anónimas, habilitamos a les visitantes del sitio a enviar contenido sin necesidad de una cuenta en Sutty. Nada se publica sin tu consentimiento, así que revisa los borradores regularmente. Esto también te puede exponer a ataques y violencias, por lo que es una característica que recomendamos usar con cuidado.' fetch: title: 'Actualizar el sitio' help: