diff --git a/.github/workflows/ci-checks.yml b/.github/workflows/ci-checks.yml index 4fdc1bcba..f3b992387 100644 --- a/.github/workflows/ci-checks.yml +++ b/.github/workflows/ci-checks.yml @@ -3,6 +3,9 @@ name: Checks on: pull_request: +permissions: + contents: read + jobs: security: name: Security diff --git a/.github/workflows/ci-oss.yml b/.github/workflows/ci-oss.yml index f6f490f34..ea007b567 100644 --- a/.github/workflows/ci-oss.yml +++ b/.github/workflows/ci-oss.yml @@ -4,6 +4,9 @@ on: pull_request: types: [opened, synchronize] +permissions: + contents: read + jobs: test: if: github.event.pull_request.head.repo.full_name != github.repository diff --git a/.github/workflows/ci-saas.yml b/.github/workflows/ci-saas.yml index f1d8875b4..aa98a8e93 100644 --- a/.github/workflows/ci-saas.yml +++ b/.github/workflows/ci-saas.yml @@ -3,6 +3,9 @@ name: CI (SaaS) on: push: +permissions: + contents: read + jobs: test_oss: name: Test (OSS) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8b5925a13..e62118357 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,9 @@ on: GH_TOKEN: required: false +permissions: + contents: read + jobs: test: name: Tests (${{ matrix.mode }}) diff --git a/.mise.toml b/.mise.toml index f0f6aada7..d1671b012 100644 --- a/.mise.toml +++ b/.mise.toml @@ -1,2 +1,5 @@ +[settings] +idiomatic_version_file_enable_tools = ["ruby"] + [env] PROMETHEUS_EXPORTER_URL = "http://127.0.0.1:9306/metrics" diff --git a/AGENTS.md b/AGENTS.md index ee391cc03..428c802a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,7 @@ bin/dev # Start development server (runs on port 3006) ``` Development URL: http://fizzy.localhost:3006 -Login with: david@37signals.com (development fixtures), password will appear in the browser console +Login with: david@example.com (development fixtures), password will appear in the browser console ### Testing ```bash @@ -142,7 +142,7 @@ Key recurring tasks (via `config/recurring.yml`): ### Chrome MCP (Local Dev) URL: `http://fizzy.localhost:3006` -Login: david@37signals.com (passwordless magic link auth - check rails console for link) +Login: david@example.com (passwordless magic link auth - check rails console for link) Use Chrome MCP tools to interact with the running dev app for UI testing and debugging. diff --git a/Gemfile.lock b/Gemfile.lock index e24a38a82..b9c3489b5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,7 +6,7 @@ GIT GIT remote: https://github.com/rails/rails.git - revision: b22cb0c0cb39b103d816a66d560991acf0a57163 + revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb branch: main specs: actioncable (8.2.0.alpha) @@ -216,7 +216,7 @@ GEM actionview (>= 7.0.0) activesupport (>= 7.0.0) jmespath (1.6.2) - json (2.16.0) + json (2.17.1) jwt (3.1.2) base64 kamal (2.9.0) @@ -351,7 +351,7 @@ GEM nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rainbow (3.1.1) rake (13.3.1) - rdoc (6.16.0) + rdoc (6.16.1) erb psych (>= 4.0.0) tsort @@ -443,7 +443,7 @@ GEM ostruct stimulus-rails (1.3.4) railties (>= 6.0.0) - stringio (3.1.8) + stringio (3.1.9) thor (1.4.0) thruster (0.1.16) thruster (0.1.16-aarch64-linux) diff --git a/Gemfile.saas b/Gemfile.saas index a56a7b682..8681a31f6 100644 --- a/Gemfile.saas +++ b/Gemfile.saas @@ -3,15 +3,17 @@ eval_gemfile "Gemfile" git_source(:bc) { |repo| "https://github.com/basecamp/#{repo}" } - +# SaaS-only functionality gem "activeresource", require: "active_resource" gem "queenbee", bc: "queenbee-plugin" gem "fizzy-saas", bc: "fizzy-saas" +gem "console1984", bc: "console1984" +gem "audits1984", bc: "audits1984" + +# Telemetry gem "rails_structured_logging", bc: "rails-structured-logging" gem "sentry-ruby" gem "sentry-rails" - -# Telemetry gem "yabeda" gem "yabeda-actioncable" gem "yabeda-activejob", github: "basecamp/yabeda-activejob", branch: "bulk-and-scheduled-jobs" diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 4e243474c..5cd308155 100644 --- a/Gemfile.saas.lock +++ b/Gemfile.saas.lock @@ -1,9 +1,32 @@ +GIT + remote: https://github.com/basecamp/audits1984 + revision: c790eaec0716c8df56b3d0ab5bf5d75e3e1b0ed0 + specs: + audits1984 (0.1.7) + console1984 + importmap-rails (>= 1.2.1) + rinku + rouge + turbo-rails + +GIT + remote: https://github.com/basecamp/console1984 + revision: 02b1b9ee7fd7050174b6a98c2b43057553621dc4 + specs: + console1984 (0.2.2) + irb (~> 1.13) + parser + rails (>= 7.0) + rainbow + GIT remote: https://github.com/basecamp/fizzy-saas - revision: 6dd8bb616632edfabc3d814f88f0658d96486e17 + revision: 97d126e0a6084905aceaadf4facaa2a48b44e124 specs: fizzy-saas (0.1.0) - prometheus-client-mmap + audits1984 + console1984 + prometheus-client-mmap (~> 1.4.0) queenbee rails (>= 8.1.0.beta1) rails_structured_logging @@ -367,32 +390,32 @@ GEM prettyprint prettyprint (0.2.0) prism (1.6.0) - prometheus-client-mmap (1.3.0) + prometheus-client-mmap (1.4.0) base64 bigdecimal logger rb_sys (~> 0.9.117) - prometheus-client-mmap (1.3.0-aarch64-linux-gnu) + prometheus-client-mmap (1.4.0-aarch64-linux-gnu) base64 bigdecimal logger rb_sys (~> 0.9.117) - prometheus-client-mmap (1.3.0-aarch64-linux-musl) + prometheus-client-mmap (1.4.0-aarch64-linux-musl) base64 bigdecimal logger rb_sys (~> 0.9.117) - prometheus-client-mmap (1.3.0-arm64-darwin) + prometheus-client-mmap (1.4.0-arm64-darwin) base64 bigdecimal logger rb_sys (~> 0.9.117) - prometheus-client-mmap (1.3.0-x86_64-linux-gnu) + prometheus-client-mmap (1.4.0-x86_64-linux-gnu) base64 bigdecimal logger rb_sys (~> 0.9.117) - prometheus-client-mmap (1.3.0-x86_64-linux-musl) + prometheus-client-mmap (1.4.0-x86_64-linux-musl) base64 bigdecimal logger @@ -440,6 +463,7 @@ GEM reline (0.6.3) io-console (~> 0.5) rexml (3.4.4) + rinku (2.0.6) rouge (4.6.1) rqrcode (3.1.0) chunky_png (~> 1.0) @@ -610,6 +634,7 @@ PLATFORMS DEPENDENCIES activeresource + audits1984! autotuner aws-sdk-s3 bcrypt (~> 3.1.7) @@ -618,6 +643,7 @@ DEPENDENCIES brakeman bundler-audit capybara + console1984! debug faker fizzy-saas! diff --git a/README.md b/README.md index b668747e9..8a7877326 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ bin/dev You'll be able to access the app in development at http://fizzy.localhost:3006. -To login, enter `david@37signals.com` and grab the verification code from the browser console to sign in. +To login, enter `david@example.com` and grab the verification code from the browser console to sign in. ### Running tests diff --git a/app/assets/stylesheets/blank-slates.css b/app/assets/stylesheets/blank-slates.css new file mode 100644 index 000000000..0725dd336 --- /dev/null +++ b/app/assets/stylesheets/blank-slates.css @@ -0,0 +1,46 @@ +@layer components { + .blank-slate--drag { + box-shadow: none !important; + color: color-mix(in srgb, var(--card-color) 70%, var(--color-canvas)); + + p { + font-size: var(--text-small); + overflow: hidden; + text-align: center; + text-overflow: ellipsis; + white-space: nowrap; + } + + .cards--considering & { + background-color: var(--card-bg-color) !important; + } + + .cards--grid & { + display: none; + } + } + + .blank-slate--empty { + border: 2px dashed var(--color-ink-light); + border-radius: 1ch; + color: var(--color-ink-dark); + margin-block-start: 5dvh; + padding: 1ch 2ch; + rotate: -3deg; + font-weight: 500; + + .cards:not(.cards--grid) & { + display: none; + } + } + + .blank-slate--filters { + .cards--grid:has(.card) & { + display: none; + } + } + + .cards__list:has(> :not(.blank-slate)) .card--hide-unless-empty { + display: none; + } +} diff --git a/app/assets/stylesheets/web/card-columns.css b/app/assets/stylesheets/web/card-columns.css index 3b2faca60..ac690ac30 100644 --- a/app/assets/stylesheets/web/card-columns.css +++ b/app/assets/stylesheets/web/card-columns.css @@ -402,30 +402,6 @@ } } - .blank-slate--card { - box-shadow: none !important; - color: color-mix(in srgb, var(--card-color) 70%, var(--color-canvas)); - - .cards--considering & { - background-color: var(--card-bg-color) !important; - } - - p { - font-size: var(--text-small); - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - - &.blank-slate-for-grid { - display: none; - - .cards--grid & { - display: initial; - } - } - } - } - /* TODO: I think this is legacy now? */ .cards__heading { display: flex; diff --git a/app/assets/stylesheets/web/comments.css b/app/assets/stylesheets/web/comments.css index b58156f36..80968184e 100644 --- a/app/assets/stylesheets/web/comments.css +++ b/app/assets/stylesheets/web/comments.css @@ -51,12 +51,14 @@ padding-inline-end: var(--reaction-size); text-align: start; - :first-child { - margin-block-start: 0; - } + .action-text-content { + > action-text-attachment:first-child figure { + margin-block-start: 0.5ch; + } - :last-child { - margin-block-end: 0; + > :last-child { + margin-block-end: 0; + } } } diff --git a/app/assets/stylesheets/web/filters.css b/app/assets/stylesheets/web/filters.css index bc0ebdf98..b283fa447 100644 --- a/app/assets/stylesheets/web/filters.css +++ b/app/assets/stylesheets/web/filters.css @@ -18,7 +18,7 @@ --input-background: var(--color-canvas); } - &:has(.filter-toggle:hover) { + &:has([data-controller~="tooltip"]:hover) { z-index: calc(var(--z-nav) + 1); } } @@ -150,9 +150,6 @@ display: none !important; } - @media (max-width: 80ch) { - display: none; - } } .filters:not(.filters--expanded) { diff --git a/app/assets/stylesheets/web/nav.css b/app/assets/stylesheets/web/nav.css index bcbe83088..8d9b2e5c1 100644 --- a/app/assets/stylesheets/web/nav.css +++ b/app/assets/stylesheets/web/nav.css @@ -178,6 +178,7 @@ aspect-ratio: 5/3; background-color: var(--color-ink-lightest); border-radius: 0.5em; + container-type: inline-size; flex-basis: calc((100% - var(--gap) * 2) / 3); flex-direction: column; font-size: var(--text-small); @@ -212,6 +213,7 @@ @media (max-width: 639px) { font-size: var(--text-x-small); + font-size: clamp(var(--text-xx-small), 3.15cqi, var(--text-small)); font-weight: 500; } } diff --git a/app/assets/stylesheets/web/popup.css b/app/assets/stylesheets/web/popup.css index 06add9439..0213d04cb 100644 --- a/app/assets/stylesheets/web/popup.css +++ b/app/assets/stylesheets/web/popup.css @@ -10,8 +10,8 @@ inset: 0 auto auto 50%; max-block-size: 70dvh; - max-inline-size: min(55ch, calc(100vw - (var(--panel-padding) * 2))); - min-inline-size: min(25ch, calc(100vw - (var(--panel-padding) * 2))); + max-inline-size: min(55ch, calc(100vw - (var(--panel-padding)))); + min-inline-size: min(25ch, calc(100vw - (var(--panel-padding)))); overflow: auto; position: absolute; transform: translateX(-50%); diff --git a/app/assets/stylesheets/web/rich-text-content.css b/app/assets/stylesheets/web/rich-text-content.css index 38c7722e2..2d465c935 100644 --- a/app/assets/stylesheets/web/rich-text-content.css +++ b/app/assets/stylesheets/web/rich-text-content.css @@ -62,7 +62,7 @@ /* Links should hug media contained within */ a:has(img), a:has(video) { - display: inline-block; + inline-size: fit-content; } /* Avoid extra space due to empty paragraphs */ @@ -263,7 +263,7 @@ display: flex; flex-wrap: wrap; justify-content: center; - margin-block-start: var(--block-space-half); + margin-block-start: 0.5ch; } } diff --git a/app/controllers/boards/columns/not_nows_controller.rb b/app/controllers/boards/columns/not_nows_controller.rb index 6cb765979..0ff41e9ae 100644 --- a/app/controllers/boards/columns/not_nows_controller.rb +++ b/app/controllers/boards/columns/not_nows_controller.rb @@ -2,7 +2,7 @@ class Boards::Columns::NotNowsController < ApplicationController include BoardScoped def show - set_page_and_extract_portion_from @board.cards.postponed.reverse_chronologically.with_golden_first.preloaded + set_page_and_extract_portion_from @board.cards.postponed.latest.preloaded fresh_when etag: @page.records end end diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb index 1e4233696..ff7c35ec0 100644 --- a/app/controllers/boards_controller.rb +++ b/app/controllers/boards_controller.rb @@ -2,7 +2,7 @@ class BoardsController < ApplicationController include FilterScoped before_action :set_board, except: %i[ new create ] - before_action :ensure_permission_to_admin_board, only: %i[ update ] + before_action :ensure_permission_to_admin_board, only: %i[ update destroy ] def show if @filter.used?(ignore_boards: true) diff --git a/app/controllers/cards/comments/reactions_controller.rb b/app/controllers/cards/comments/reactions_controller.rb index 54f63dda7..c65d34291 100644 --- a/app/controllers/cards/comments/reactions_controller.rb +++ b/app/controllers/cards/comments/reactions_controller.rb @@ -2,6 +2,8 @@ class Cards::Comments::ReactionsController < ApplicationController include CardScoped before_action :set_comment + before_action :set_reaction, only: %i[ destroy ] + before_action :ensure_permision_to_administer_reaction, only: %i[ destroy ] def index end @@ -14,7 +16,6 @@ class Cards::Comments::ReactionsController < ApplicationController end def destroy - @reaction = @comment.reactions.find(params[:id]) @reaction.destroy end @@ -22,4 +23,12 @@ class Cards::Comments::ReactionsController < ApplicationController def set_comment @comment = @card.comments.find(params[:comment_id]) end + + def set_reaction + @reaction = @comment.reactions.find(params[:id]) + end + + def ensure_permision_to_administer_reaction + head :forbidden if Current.user != @reaction.reacter + end end diff --git a/app/controllers/concerns/column_scoped.rb b/app/controllers/concerns/column_scoped.rb index 92d0715fa..ca0585547 100644 --- a/app/controllers/concerns/column_scoped.rb +++ b/app/controllers/concerns/column_scoped.rb @@ -7,6 +7,6 @@ module ColumnScoped private def set_column - @column = Current.account.columns.find(params[:column_id]) + @column = Current.user.accessible_columns.find(params[:column_id]) end end diff --git a/app/controllers/join_codes_controller.rb b/app/controllers/join_codes_controller.rb index 5b0977cd5..971b3c6f8 100644 --- a/app/controllers/join_codes_controller.rb +++ b/app/controllers/join_codes_controller.rb @@ -12,7 +12,7 @@ class JoinCodesController < ApplicationController def create identity = Identity.find_or_create_by!(email_address: params.expect(:email_address)) - @join_code.redeem { |account| identity.join(account) } unless identity.member_of?(@join_code.account) + @join_code.redeem_if { |account| identity.join(account) } user = User.active.find_by!(account: @join_code.account, identity: identity) if identity == Current.identity && user.setup? diff --git a/app/controllers/public/boards/columns/not_nows_controller.rb b/app/controllers/public/boards/columns/not_nows_controller.rb index d518fb8e0..5a54df171 100644 --- a/app/controllers/public/boards/columns/not_nows_controller.rb +++ b/app/controllers/public/boards/columns/not_nows_controller.rb @@ -1,5 +1,5 @@ class Public::Boards::Columns::NotNowsController < Public::BaseController def show - set_page_and_extract_portion_from @board.cards.postponed.reverse_chronologically.with_golden_first + set_page_and_extract_portion_from @board.cards.postponed.latest end end diff --git a/app/controllers/sessions/magic_links_controller.rb b/app/controllers/sessions/magic_links_controller.rb index dc55782af..71fa1b9ac 100644 --- a/app/controllers/sessions/magic_links_controller.rb +++ b/app/controllers/sessions/magic_links_controller.rb @@ -1,7 +1,7 @@ class Sessions::MagicLinksController < ApplicationController disallow_account_scope require_unauthenticated_access - rate_limit to: 10, within: 15.minutes, only: :create, with: -> { redirect_to session_magic_link_path, alert: "Try again in 15 minutes." } + rate_limit to: 10, within: 15.minutes, only: :create, with: -> { redirect_to session_magic_link_path, alert: "Wait 15 minutes, then try again" } layout "public" @@ -13,7 +13,7 @@ class Sessions::MagicLinksController < ApplicationController start_new_session_for magic_link.identity redirect_to after_sign_in_url(magic_link) else - redirect_to session_magic_link_path, alert: "Try another code." + redirect_to session_magic_link_path, flash: { shake: true } end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 31ca0006f..5335f58f3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -9,11 +9,16 @@ class SessionsController < ApplicationController end def create - if identity = Identity.find_by_email_address(email_address) - magic_link = identity.send_magic_link - serve_development_magic_link(magic_link) + identity = Identity.find_by_email_address(email_address) + + magic_link = if identity + identity.send_magic_link + else + Signup.new(email_address: email_address).create_identity end + serve_development_magic_link(magic_link) + redirect_to session_magic_link_path end diff --git a/app/controllers/users/avatars_controller.rb b/app/controllers/users/avatars_controller.rb index 61ce75f9f..d85e5f869 100644 --- a/app/controllers/users/avatars_controller.rb +++ b/app/controllers/users/avatars_controller.rb @@ -10,7 +10,7 @@ class Users::AvatarsController < ApplicationController if @user.system? redirect_to view_context.image_path("system_user.png") elsif @user.avatar.attached? - redirect_to rails_blob_url(@user.avatar.variant(:thumb), disposition: "inline") + redirect_to rails_blob_url(@user.avatar_thumbnail, disposition: "inline") elsif stale? @user, cache_control: cache_control render_initials end diff --git a/app/controllers/users/email_addresses/confirmations_controller.rb b/app/controllers/users/email_addresses/confirmations_controller.rb index dad998671..b5fd334ea 100644 --- a/app/controllers/users/email_addresses/confirmations_controller.rb +++ b/app/controllers/users/email_addresses/confirmations_controller.rb @@ -8,12 +8,14 @@ class Users::EmailAddresses::ConfirmationsController < ApplicationController end def create - user = @user.change_email_address_using_token(token) + if @user.change_email_address_using_token(token) + terminate_session if Current.session + start_new_session_for @user.identity - terminate_session if Current.session - start_new_session_for user.identity - - redirect_to edit_user_url(script_name: user.account.slug, id: user) + redirect_to edit_user_url(script_name: @user.account.slug, id: @user) + else + render :invalid_token, status: :unprocessable_entity + end end private diff --git a/app/helpers/accesses_helper.rb b/app/helpers/accesses_helper.rb index e52aa8ede..6ba7b4d3c 100644 --- a/app/helpers/accesses_helper.rb +++ b/app/helpers/accesses_helper.rb @@ -1,6 +1,4 @@ module AccessesHelper - MAX_DISPLAYED_WATCHERS = 8 - def access_menu_tag(board, **options, &) tag.menu class: [ options[:class], { "toggler--toggled": board.all_access? } ], data: { controller: "filter toggle-class navigable-list", @@ -27,45 +25,28 @@ module AccessesHelper end def board_watchers_list(board) - watchers = board.watchers.with_avatars + watchers = board.watchers.with_avatars.load - displayed_watchers = watchers.limit(MAX_DISPLAYED_WATCHERS) - overflow_count = watchers.count - MAX_DISPLAYED_WATCHERS + displayed_watchers = watchers.first(8) + overflow_count = watchers.size - 8 - safe_join([ - tag.strong(displayed_watchers.any? ? "Watching for new cards" : "No one is watching for new cards", class: "txt-uppercase"), - tag.div(class: "board-tools__watching") do - safe_join([ - safe_join(displayed_watchers.map { |watcher| avatar_tag(watcher) }), - (tag.div(data: { controller: "dialog", action: "keydown.esc->dialog#close click@document->dialog#closeOnClickOutside" }) do - concat tag.button("+#{overflow_count}", class: "overflow-count btn btn--circle borderless", data: { action: "dialog#open" }, aria: { label: "Show #{overflow_count} more watchers" }) - concat(tag.dialog(class: "board-tools__watching-dialog dialog panel", data: { dialog_target: "dialog" }, aria: { hidden: "true" }) do - safe_join(watchers.map { |watcher| avatar_tag(watcher) }) - end) - end if overflow_count > 0) - ].compact) - end - ]) + tag.strong(watchers.any? ? "Watching for new cards" : "No one is watching for new cards", class: "txt-uppercase") + + tag.div(avatar_tags(displayed_watchers), class: "board-tools__watching") do + tag.div(data: { controller: "dialog", action: "keydown.esc->dialog#close click@document->dialog#closeOnClickOutside" }) do + tag.button("+#{overflow_count}", class: "overflow-count btn btn--circle borderless", data: { action: "dialog#open" }, aria: { label: "Show #{overflow_count} more watchers" }) + + tag.dialog(avatar_tags(watchers), class: "board-tools__watching-dialog dialog panel", data: { dialog_target: "dialog" }, aria: { hidden: "true" }) + end if overflow_count > 0 + end end def involvement_button(board, access, show_watchers, icon_only) - involvement_label_id = dom_id(board, :involvement_label) - button_to( - board_involvement_path(board), - method: :put, - aria: { labelledby: involvement_label_id }, - class: class_names("btn", { "btn--reversed": access.watching? && icon_only }), - params: { show_watchers: show_watchers, involvement: next_involvement(access.involvement), icon_only: icon_only } - ) do - safe_join([ - icon_tag("notification-bell-#{icon_only ? 'reverse-' : nil}#{access.involvement.dasherize}"), - tag.span( - involvement_access_label(access), - class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), - id: involvement_label_id - ) - ]) + board_involvement_path(board), method: :put, + params: { show_watchers: show_watchers, involvement: next_involvement(access.involvement), icon_only: icon_only }, + aria: { labelledby: dom_id(board, :involvement_label) }, title: access.access_only? ? "Watch this" : "Stop watching", + class: class_names("btn", { "btn--reversed": access.watching? && icon_only })) do + icon_tag("notification-bell-#{icon_only ? 'reverse-' : nil}#{access.involvement.dasherize}") + + tag.span(class: class_names("txt-nowrap txt-uppercase", "for-screen-reader": icon_only), id: dom_id(board, :involvement_label)) end end @@ -74,12 +55,4 @@ module AccessesHelper order = %w[ watching access_only ] order[(order.index(involvement.to_s) + 1) % order.size] end - - def involvement_access_label(access) - if access.access_only? - "Watch this" - else - "Stop watching" - end - end end diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 8fb42448a..7c064a650 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -19,6 +19,10 @@ module AvatarsHelper end end + def avatar_tags(users, **options) + users.collect { avatar_tag(it, **options) }.join.html_safe + end + def mail_avatar_tag(user, size: 48, **options) if user.avatar.attached? image_tag user_avatar_url(user), alt: user.name, class: "avatar", size: size, **options diff --git a/app/helpers/cards_helper.rb b/app/helpers/cards_helper.rb index 0dd93c520..581adc108 100644 --- a/app/helpers/cards_helper.rb +++ b/app/helpers/cards_helper.rb @@ -1,5 +1,5 @@ module CardsHelper - def card_article_tag(card, id: dom_id(card, :article), **options, &block) + def card_article_tag(card, id: dom_id(card, :article), data: {}, **options, &block) classes = [ options.delete(:class), ("golden-effect" if card.golden?), @@ -7,10 +7,13 @@ module CardsHelper ("card--active" if card.active?) ].compact.join(" ") + data[:drag_and_drop_top] = true if card.golden? && !card.closed? && !card.postponed? + tag.article \ id: id, style: "--card-color: #{card.color}; view-transition-name: #{id}", class: classes, + data: data, **options, &block end diff --git a/app/helpers/columns_helper.rb b/app/helpers/columns_helper.rb index 8a2e82e17..222d41357 100644 --- a/app/helpers/columns_helper.rb +++ b/app/helpers/columns_helper.rb @@ -10,14 +10,16 @@ module ColumnsHelper data: { turbo_frame: "_top" } end - def column_tag(id:, name:, drop_url:, collapsed: true, selected: nil, data: {}, **properties, &block) + def column_tag(id:, name:, drop_url:, collapsed: true, selected: nil, card_color: "var(--color-card-default)", data: {}, **properties, &block) classes = token_list("cards", properties.delete(:class), "is-collapsed": collapsed) data = { drag_and_drop_target: "container", navigable_list_target: "item", column_name: name, - drag_and_drop_url: drop_url + drag_and_drop_url: drop_url, + drag_and_drop_css_variable_name: "--card-color", + drag_and_drop_css_variable_value: card_color }.merge(data) data[:action] = token_list( @@ -28,7 +30,8 @@ module ColumnsHelper tag.section(id: id, class: classes, tabindex: "0", "aria-selected": selected, data: data, **properties) do tag.div(class: "cards__transition-container", data: { - controller: "navigable-list", + controller: "navigable-list css-variable-counter", + css_variable_counter_property_name_value: "--card-count", navigable_list_supports_horizontal_navigation_value: "false", navigable_list_prevent_handled_keys_value: "true", navigable_list_auto_select_value: "false", diff --git a/app/helpers/html_helper.rb b/app/helpers/html_helper.rb index 82a18722d..8998ffcb9 100644 --- a/app/helpers/html_helper.rb +++ b/app/helpers/html_helper.rb @@ -9,7 +9,7 @@ module HtmlHelper private EXCLUDED_ELEMENTS = %w[ a figcaption pre code ] - EMAIL_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/ + EMAIL_AUTOLINK_REGEXP = /\b[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*\b/ URL_REGEXP = URI::DEFAULT_PARSER.make_regexp(%w[http https]) def auto_link(fragment) @@ -48,7 +48,7 @@ module HtmlHelper end def auto_link_emails(text) - text.gsub!(EMAIL_REGEXP) do |match| + text.gsub!(EMAIL_AUTOLINK_REGEXP) do |match| %(#{match}) end end diff --git a/app/helpers/login_helper.rb b/app/helpers/login_helper.rb index 56627d968..f5e89c8d1 100644 --- a/app/helpers/login_helper.rb +++ b/app/helpers/login_helper.rb @@ -1,10 +1,12 @@ module LoginHelper def login_url - new_session_path(script_name: nil) + # Use main_app because this helper may be invoked from an engine controller + # that inherits from AdminController. + main_app.new_session_path(script_name: nil) end def logout_url - new_session_path + main_app.new_session_path end def redirect_to_login_url diff --git a/app/javascript/controllers/bubble_controller.js b/app/javascript/controllers/bubble_controller.js index 66fd469cf..714977c66 100644 --- a/app/javascript/controllers/bubble_controller.js +++ b/app/javascript/controllers/bubble_controller.js @@ -33,8 +33,7 @@ export default class extends Controller { } get #entropyCleanupInDays() { - this.entropyCleanupInDays ??= signedDifferenceInDays(new Date(), new Date(this.entropyValue.closesAt)) - return this.entropyCleanupInDays + return signedDifferenceInDays(new Date(), new Date(this.entropyValue.closesAt)) } #showEntropy() { diff --git a/app/javascript/controllers/css_variable_counter_controller.js b/app/javascript/controllers/css_variable_counter_controller.js new file mode 100644 index 000000000..1fa2cd5b9 --- /dev/null +++ b/app/javascript/controllers/css_variable_counter_controller.js @@ -0,0 +1,33 @@ +import { Controller } from "@hotwired/stimulus" +import { debounce } from "helpers/timing_helpers" + +export default class extends Controller { + static targets = [ "item", "counter" ] + static values = { + propertyName: String, + maxValue: { type: Number, default: 20 } + } + + initialize() { + this.#updateCounter = debounce(this.#updateCounter.bind(this), 50) + } + + connect() { + this.#updateCounter() + } + + itemTargetConnected() { + this.#updateCounter() + } + + itemTargetDisconnected() { + this.#updateCounter() + } + + #updateCounter = () => { + if (!this.hasCounterTarget) return + + const count = Math.min(this.itemTargets.length, this.maxValueValue) + this.counterTarget.style.setProperty(this.propertyNameValue, count) + } +} diff --git a/app/javascript/controllers/dialog_controller.js b/app/javascript/controllers/dialog_controller.js index 3df135c12..cdc8ea7cd 100644 --- a/app/javascript/controllers/dialog_controller.js +++ b/app/javascript/controllers/dialog_controller.js @@ -57,4 +57,8 @@ export default class extends Controller { loadLazyFrames() { Array.from(this.dialogTarget.querySelectorAll("turbo-frame")).forEach(frame => { frame.loading = "eager" }) } + + captureKey(event) { + if (event.key !== "Escape") { event.stopPropagation() } + } } diff --git a/app/javascript/controllers/drag_and_drop_controller.js b/app/javascript/controllers/drag_and_drop_controller.js index 5c03853be..5fc532ca1 100644 --- a/app/javascript/controllers/drag_and_drop_controller.js +++ b/app/javascript/controllers/drag_and_drop_controller.js @@ -16,11 +16,14 @@ export default class extends Controller { await nextFrame() this.dragItem = this.#itemContaining(event.target) this.sourceContainer = this.#containerContaining(this.dragItem) + this.originalDraggedItemCssVariable = this.#containerCssVariableFor(this.sourceContainer) this.dragItem.classList.add(this.draggedItemClass) } dragOver(event) { event.preventDefault() + if (!this.dragItem) { return } + const container = this.#containerContaining(event.target) this.#clearContainerHoverClasses() @@ -28,32 +31,39 @@ export default class extends Controller { if (container !== this.sourceContainer) { container.classList.add(this.hoverContainerClass) + this.#applyContainerCssVariableToDraggedItem(container) + } else { + this.#restoreOriginalDraggedItemCssVariable() } } async drop(event) { - const container = this.#containerContaining(event.target) + const targetContainer = this.#containerContaining(event.target) - if (!container || container === this.sourceContainer) { return } + if (!targetContainer || targetContainer === this.sourceContainer) { return } this.wasDropped = true + this.#increaseCounter(targetContainer) this.#decreaseCounter(this.sourceContainer) + const sourceContainer = this.sourceContainer - await this.#submitDropRequest(this.dragItem, container) - this.#reloadSourceFrame(sourceContainer); + this.#insertDraggedItem(targetContainer, this.dragItem) + await this.#submitDropRequest(this.dragItem, targetContainer) + this.#reloadSourceFrame(sourceContainer) } dragEnd() { this.dragItem.classList.remove(this.draggedItemClass) this.#clearContainerHoverClasses() - if (this.wasDropped) { - this.dragItem.remove() + if (!this.wasDropped) { + this.#restoreOriginalDraggedItemCssVariable() } this.sourceContainer = null this.dragItem = null this.wasDropped = false + this.originalDraggedItemCssVariable = null } #itemContaining(element) { @@ -68,6 +78,63 @@ export default class extends Controller { this.containerTargets.forEach(container => container.classList.remove(this.hoverContainerClass)) } + #applyContainerCssVariableToDraggedItem(container) { + const cssVariable = this.#containerCssVariableFor(container) + if (cssVariable) { + this.dragItem.style.setProperty(cssVariable.name, cssVariable.value) + } + } + + #restoreOriginalDraggedItemCssVariable() { + if (this.originalDraggedItemCssVariable) { + const { name, value } = this.originalDraggedItemCssVariable + this.dragItem.style.setProperty(name, value) + } + } + + #containerCssVariableFor(container) { + const { dragAndDropCssVariableName, dragAndDropCssVariableValue } = container.dataset + if (dragAndDropCssVariableName && dragAndDropCssVariableValue) { + return { name: dragAndDropCssVariableName, value: dragAndDropCssVariableValue } + } + return null + } + + #increaseCounter(container) { + this.#modifyCounter(container, count => count + 1) + } + + #decreaseCounter(container) { + this.#modifyCounter(container, count => Math.max(0, count - 1)) + } + + #modifyCounter(container, fn) { + const counterElement = container.querySelector("[data-drag-and-drop-counter]") + if (counterElement) { + const currentValue = counterElement.textContent.trim() + + if (!/^\d+$/.test(currentValue)) return + + counterElement.textContent = fn(parseInt(currentValue)) + } + } + + #insertDraggedItem(container, item) { + const itemContainer = container.querySelector("[data-drag-drop-item-container]") + const topItems = itemContainer.querySelectorAll("[data-drag-and-drop-top]") + const firstTopItem = topItems[0] + const lastTopItem = topItems[topItems.length - 1] + + const isTopItem = item.hasAttribute("data-drag-and-drop-top") + const referenceItem = isTopItem ? firstTopItem : lastTopItem + + if (referenceItem) { + referenceItem[isTopItem ? "before" : "after"](item) + } else { + itemContainer.prepend(item) + } + } + async #submitDropRequest(item, container) { const body = new FormData() const id = item.dataset.id @@ -80,18 +147,4 @@ export default class extends Controller { const frame = sourceContainer.querySelector("[data-drag-and-drop-refresh]") if (frame) frame.reload() } - - #decreaseCounter(sourceContainer) { - const counterElement = sourceContainer.querySelector("[data-drag-and-drop-counter]") - if (counterElement) { - const currentValue = counterElement.textContent.trim() - - if (!/^\d+$/.test(currentValue)) return - - const count = parseInt(currentValue) - if (count > 0) { - counterElement.textContent = count - 1 - } - } - } } diff --git a/app/javascript/controllers/form_controller.js b/app/javascript/controllers/form_controller.js index e910d6622..f917bd092 100644 --- a/app/javascript/controllers/form_controller.js +++ b/app/javascript/controllers/form_controller.js @@ -21,8 +21,13 @@ export default class extends Controller { if (input) { const value = (input.value || "").trim() - if (value.length === 0) { + const isEmpty = value.length === 0 + + if (isEmpty) { event.preventDefault() + input.setCustomValidity(input.dataset.validationMessage || "Please fill out this field") + input.reportValidity() + input.addEventListener("input", () => input.setCustomValidity(""), { once: true }) } } } diff --git a/app/jobs/concerns/smtp_delivery_error_handling.rb b/app/jobs/concerns/smtp_delivery_error_handling.rb new file mode 100644 index 000000000..988fbf9d0 --- /dev/null +++ b/app/jobs/concerns/smtp_delivery_error_handling.rb @@ -0,0 +1,36 @@ +module SmtpDeliveryErrorHandling + extend ActiveSupport::Concern + + included do + # Retry delivery to possibly-unavailable remote mailservers. + retry_on Net::OpenTimeout, Net::ReadTimeout, Socket::ResolutionError, wait: :polynomially_longer + + # Net::SMTPServerBusy is SMTP error code 4xx, a temporary error. + # Common one we've seen is 452 4.3.1 Insufficient system storage. + # Patiently retry. + retry_on Net::SMTPServerBusy, wait: :polynomially_longer + + # SMTP error 50x. + rescue_from Net::SMTPSyntaxError do |error| + case error.message + when /\A501 5\.1\.3/ + # Ignore undeliverable email addresses. + Sentry.capture_exception error, level: :info if Fizzy.saas? + else + raise + end + end + + # SMTP error 5xx except 50x and 53x. + # * 550 5.1.1: Unknown users + # * 552 5.6.0: Message/headers too large + rescue_from Net::SMTPFatalError do |error| + case error.message + when /\A550 5\.1\.1/, /\A552 5\.6\.0/, /\A555 5\.5\.4/ + Sentry.capture_exception error, level: :info if Fizzy.saas? + else + raise + end + end + end +end diff --git a/app/jobs/notification/bundle/deliver_job.rb b/app/jobs/notification/bundle/deliver_job.rb index 40e1df479..11f6bcda5 100644 --- a/app/jobs/notification/bundle/deliver_job.rb +++ b/app/jobs/notification/bundle/deliver_job.rb @@ -1,4 +1,6 @@ class Notification::Bundle::DeliverJob < ApplicationJob + include SmtpDeliveryErrorHandling + queue_as :backend def perform(bundle) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 7fd915dba..9f0d72359 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,5 +1,5 @@ class ApplicationMailer < ActionMailer::Base - default from: "Fizzy " + default from: ENV.fetch("MAILER_FROM_ADDRESS", "Fizzy ") layout "mailer" append_view_path Rails.root.join("app/views/mailers") diff --git a/app/models/account.rb b/app/models/account.rb index f74660cbb..50f3bf3ee 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -35,7 +35,7 @@ class Account < ApplicationRecord end def system_user - users.where(role: :system).first! + users.find_by!(role: :system) end private diff --git a/app/models/account/join_code.rb b/app/models/account/join_code.rb index 49fe9af22..82283e4f6 100644 --- a/app/models/account/join_code.rb +++ b/app/models/account/join_code.rb @@ -7,10 +7,9 @@ class Account::JoinCode < ApplicationRecord before_create :generate_code, if: -> { code.blank? } - def redeem + def redeem_if(&block) transaction do - increment!(:usage_count) - yield account if block_given? + increment!(:usage_count) if block.call(account) end end diff --git a/app/models/account/seeder.rb b/app/models/account/seeder.rb index 354674ddc..4a349b939 100644 --- a/app/models/account/seeder.rb +++ b/app/models/account/seeder.rb @@ -29,12 +29,12 @@ class Account::Seeder # Cards playground.cards.create! creator: creator, title: "Finally, watch this Fizzy orientation video", status: "published", description: <<~HTML -

There’s a whole lot more you can do in Fizzy. In the video below 37signals founder and CEO, Jason Fried, will walk you through the basics in just 8 minutes.

+

There’s a whole lot more you can do in Fizzy. In the video below, 37signals founder and CEO, Jason Fried, will walk you through the basics in just 17 minutes.

HTML playground.cards.create! creator: creator, title: "Now, grab the invite link to invite someone else", status: "published", description: <<~HTML -

Open Fizzy menu, select “+ Add people”, then copy the invite link. You can give this link to someone else so they can make an login for themselves in your account.

+

Open the Fizzy menu, select “+ Add people”, then copy the invite link. You can give this link to someone else so they can make a login for themselves in your account.

HTML @@ -43,7 +43,7 @@ class Account::Seeder HTML - playground.cards.create! creator: creator, title: "Now,check out all cards assigned to you", status: "published", description: <<~HTML + playground.cards.create! creator: creator, title: "Now, check out all cards assigned to you", status: "published", description: <<~HTML

Pull down the Fizzy menu at the top of the screen, and select “Assigned to me” or just hit “2” on your keyboard any time.

HTML @@ -54,7 +54,7 @@ class Account::Seeder HTML playground.cards.create! creator: creator, title: "Next, assign this card to yourself", status: "published", description: <<~HTML -

Click the little head with the + next to it, pick yourself.

+

Click the little head with the + next to it, then pick yourself.

HTML diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 45e9c2f21..4ed46f2c3 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -2,6 +2,4 @@ class ApplicationRecord < ActiveRecord::Base primary_abstract_class configure_replica_connections - - attribute :id, :uuid, default: -> { ActiveRecord::Type::Uuid.generate } end diff --git a/app/models/card/broadcastable.rb b/app/models/card/broadcastable.rb index aa24b776c..9cf112200 100644 --- a/app/models/card/broadcastable.rb +++ b/app/models/card/broadcastable.rb @@ -3,5 +3,16 @@ module Card::Broadcastable included do broadcasts_refreshes + + before_update :remember_if_preview_changed end + + private + def remember_if_preview_changed + @preview_changed ||= title_changed? || column_id_changed? || board_id_changed? + end + + def preview_changed? + @preview_changed + end end diff --git a/app/models/card/eventable.rb b/app/models/card/eventable.rb index c52fb7a29..520fd95d4 100644 --- a/app/models/card/eventable.rb +++ b/app/models/card/eventable.rb @@ -19,7 +19,6 @@ module Card::Eventable def touch_last_active_at # Not using touch so that we can detect attribute change on callbacks update!(last_active_at: Time.current) - broadcast_activity end private @@ -36,8 +35,4 @@ module Card::Eventable def create_system_comment_for(event) SystemCommenter.new(self, event).comment end - - def broadcast_activity - broadcast_render_later_to self, :activity, partial: "card/display/refresh_activity", locals: { card: self } - end end diff --git a/app/models/card/pinnable.rb b/app/models/card/pinnable.rb index ff3407ea0..9929a785d 100644 --- a/app/models/card/pinnable.rb +++ b/app/models/card/pinnable.rb @@ -3,6 +3,8 @@ module Card::Pinnable included do has_many :pins, dependent: :destroy + + after_update_commit :broadcast_pin_updates, if: :preview_changed? end def pinned_by?(user) @@ -20,4 +22,11 @@ module Card::Pinnable def unpin_by(user) pins.find_by(user: user).tap { it.destroy } end + + private + def broadcast_pin_updates + pins.find_each do |pin| + pin.broadcast_replace_later_to [ pin.user, :pins_tray ], partial: "my/pins/pin" + end + end end diff --git a/app/models/card/readable.rb b/app/models/card/readable.rb index 903db2d71..9c272dd75 100644 --- a/app/models/card/readable.rb +++ b/app/models/card/readable.rb @@ -14,8 +14,9 @@ module Card::Readable end def remove_inaccessible_notifications - User.find_each do |user| - all_notifications_for(user).destroy_all unless accessible_to?(user) + accessible_user_ids = board.accesses.pluck(:user_id) + notification_sources.each do |sources| + inaccessible_notifications_from(sources, accessible_user_ids).in_batches.destroy_all end end @@ -44,6 +45,14 @@ module Card::Readable Event.where(eventable: comments) end + def inaccessible_notifications_from(sources, accessible_user_ids) + Notification.where(source: sources).where.not(user_id: accessible_user_ids) + end + + def notification_sources + [ events, comment_creation_events, mentions, comment_mentions ] + end + def mention_notification_sources mentions.or(comment_mentions) end diff --git a/app/models/card/taggable.rb b/app/models/card/taggable.rb index 5ad75a593..73c89d0b6 100644 --- a/app/models/card/taggable.rb +++ b/app/models/card/taggable.rb @@ -9,7 +9,7 @@ module Card::Taggable end def toggle_tag_with(title) - tag = Tag.find_or_create_by!(title: title) + tag = account.tags.find_or_create_by!(title: title) transaction do if tagged_with?(tag) diff --git a/app/models/eventable.rb b/app/models/concerns/eventable.rb similarity index 100% rename from app/models/eventable.rb rename to app/models/concerns/eventable.rb diff --git a/app/models/current.rb b/app/models/current.rb index e40fc3156..47f2b6c21 100644 --- a/app/models/current.rb +++ b/app/models/current.rb @@ -12,11 +12,11 @@ class Current < ActiveSupport::CurrentAttributes end end - def with_account(value, &block) - with(account: value, &block) + def with_account(value, &) + with(account: value, &) end - def without_account(&block) - with(account: nil, &block) + def without_account(&) + with(account: nil, &) end end diff --git a/app/models/filter/resources.rb b/app/models/filter/resources.rb index 3521b43e3..655746f49 100644 --- a/app/models/filter/resources.rb +++ b/app/models/filter/resources.rb @@ -24,7 +24,7 @@ module Filter::Resources def board_titles if boards.none? - Board.one? ? [ Board.first.name ] : [ "all boards" ] + creator.boards.one? ? [ creator.boards.first.name ] : [ "all boards" ] else boards.map(&:name) end diff --git a/app/models/identity.rb b/app/models/identity.rb index 135fdaae7..ee4a323b2 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -10,6 +10,7 @@ class Identity < ApplicationRecord before_destroy :deactivate_users + validates :email_address, format: { with: URI::MailTo::EMAIL_REGEXP } normalizes :email_address, with: ->(value) { value.strip.downcase.presence } def send_magic_link(**attributes) diff --git a/app/models/identity/joinable.rb b/app/models/identity/joinable.rb index 4a6eabcd4..52afb339a 100644 --- a/app/models/identity/joinable.rb +++ b/app/models/identity/joinable.rb @@ -5,11 +5,9 @@ module Identity::Joinable attributes[:name] ||= email_address transaction do - account.users.create!(**attributes, identity: self) + account.users.find_or_create_by!(identity: self) do |user| + user.assign_attributes(attributes) + end.previously_new_record? end end - - def member_of?(account) - account.users.exists?(identity: self) - end end diff --git a/app/models/magic_link/code.rb b/app/models/magic_link/code.rb index 6af1619d8..2bcc0c4e6 100644 --- a/app/models/magic_link/code.rb +++ b/app/models/magic_link/code.rb @@ -4,7 +4,7 @@ module MagicLink::Code class << self def generate(length) - length.times.map { CODE_ALPHABET.sample }.join + Array.new(length) { CODE_ALPHABET[SecureRandom.random_number(CODE_ALPHABET.length)] }.join end def sanitize(code) diff --git a/app/models/search/record/sqlite.rb b/app/models/search/record/sqlite.rb index c3da9131a..ae0d34281 100644 --- a/app/models/search/record/sqlite.rb +++ b/app/models/search/record/sqlite.rb @@ -2,8 +2,6 @@ module Search::Record::SQLite extend ActiveSupport::Concern included do - # Override default UUID id attribute, as FTS5 uses rowid integer primary key - attribute :id, :integer, default: nil attribute :result_title, :string attribute :result_content, :string diff --git a/app/models/ssrf_protection.rb b/app/models/ssrf_protection.rb new file mode 100644 index 000000000..f3df2511b --- /dev/null +++ b/app/models/ssrf_protection.rb @@ -0,0 +1,37 @@ +module SsrfProtection + extend self + + DNS_RESOLUTION_TIMEOUT = 2 + + DISALLOWED_IP_RANGES = [ + IPAddr.new("0.0.0.0/8") # Broadcasts + ].freeze + + def resolve_public_ip(hostname) + ip_addresses = resolve_dns(hostname) + public_ips = ip_addresses.reject { |ip| private_address?(ip) } + public_ips.first&.to_s + end + + def private_address?(ip) + ip = IPAddr.new(ip.to_s) unless ip.is_a?(IPAddr) + ip.private? || ip.loopback? || ip.link_local? || ip.ipv4_mapped? || in_disallowed_range?(ip) + end + + private + def resolve_dns(hostname) + ip_addresses = [] + + Resolv::DNS.open(timeouts: DNS_RESOLUTION_TIMEOUT) do |dns| + dns.each_address(hostname) do |ip_address| + ip_addresses << IPAddr.new(ip_address.to_s) + end + end + + ip_addresses + end + + def in_disallowed_range?(ip) + DISALLOWED_IP_RANGES.any? { |range| range.include?(ip) } + end +end diff --git a/app/models/user.rb b/app/models/user.rb index d70a260ea..8c0ac42a5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,12 +1,8 @@ class User < ApplicationRecord - include Accessor, Assignee, Attachable, Configurable, EmailAddressChangeable, + include Accessor, Assignee, Attachable, Avatar, Configurable, EmailAddressChangeable, Mentionable, Named, Notifiable, Role, Searcher, Watcher include Timelined # Depends on Accessor - has_one_attached :avatar do |attachable| - attachable.variant :thumb, resize_to_fill: [ 256, 256 ] - end - belongs_to :account belongs_to :identity, optional: true diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index 352a9ecb2..a9cc5e459 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -4,6 +4,7 @@ module User::Accessor included do has_many :accesses, dependent: :destroy has_many :boards, through: :accesses + has_many :accessible_columns, through: :boards, source: :columns has_many :accessible_cards, through: :boards, source: :cards has_many :accessible_comments, through: :accessible_cards, source: :comments diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb new file mode 100644 index 000000000..a4b2ae631 --- /dev/null +++ b/app/models/user/avatar.rb @@ -0,0 +1,24 @@ +module User::Avatar + extend ActiveSupport::Concern + + ALLOWED_AVATAR_CONTENT_TYPES = %w[ image/jpeg image/png image/gif image/webp ].freeze + + included do + has_one_attached :avatar do |attachable| + attachable.variant :thumb, resize_to_fill: [ 256, 256 ] + end + + validate :avatar_content_type_allowed + end + + def avatar_thumbnail + avatar.variable? ? avatar.variant(:thumb) : avatar + end + + private + def avatar_content_type_allowed + if avatar.attached? && !ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type) + errors.add(:avatar, "must be a JPEG, PNG, GIF, or WebP image") + end + end +end diff --git a/app/models/user/email_address_changeable.rb b/app/models/user/email_address_changeable.rb index ce18d46d0..2c8de7217 100644 --- a/app/models/user/email_address_changeable.rb +++ b/app/models/user/email_address_changeable.rb @@ -7,14 +7,12 @@ module User::EmailAddressChangeable def change_email_address_using_token(token) parsed_token = SignedGlobalID.parse(token, for: EMAIL_CHANGE_TOKEN_PURPOSE) - if parsed_token.nil? - raise ArgumentError, "The token is invalid" - elsif parsed_token.find != self - raise ArgumentError, "The token was generated for a different user" - elsif identity.email_address != parsed_token.params.fetch("old_email_address") - raise ArgumentError, "The token was generated for a different email address" + old_email_address = parsed_token&.params&.fetch("old_email_address") + new_email_address = parsed_token&.params&.fetch("new_email_address") + + if parsed_token.nil? || parsed_token.find != self || identity.email_address != old_email_address + false else - new_email_address = parsed_token.params.fetch("new_email_address") change_email_address(new_email_address) end end @@ -37,8 +35,6 @@ module User::EmailAddressChangeable new_identity = Identity.find_or_create_by!(email_address: new_email_address) update!(identity: new_identity) end - - self end private diff --git a/app/models/webhook/delivery.rb b/app/models/webhook/delivery.rb index ce69c01ad..b635baac5 100644 --- a/app/models/webhook/delivery.rb +++ b/app/models/webhook/delivery.rb @@ -2,11 +2,6 @@ class Webhook::Delivery < ApplicationRecord STALE_TRESHOLD = 7.days USER_AGENT = "fizzy/1.0.0 Webhook" ENDPOINT_TIMEOUT = 7.seconds - DNS_RESOLUTION_TIMEOUT = 2 - DISALLOWED_IP_RANGES = [ - # Broadcasts - IPAddr.new("0.0.0.0/8") - ].freeze belongs_to :account, default: -> { webhook.account } belongs_to :webhook @@ -54,14 +49,14 @@ class Webhook::Delivery < ApplicationRecord private def perform_request - if private_uri? + if resolved_ip.nil? { error: :private_uri } else response = http.request( Net::HTTP::Post.new(uri, headers).tap { |request| request.body = payload } ) - { code: response.code.to_i } + { code: response.code.to_i } end rescue Resolv::ResolvTimeout, Resolv::ResolvError, SocketError { error: :dns_lookup_failed } @@ -73,18 +68,9 @@ class Webhook::Delivery < ApplicationRecord { error: :failed_tls } end - def private_uri? - ip_addresses = [] - - Resolv::DNS.open(timeouts: DNS_RESOLUTION_TIMEOUT) do |dns| - dns.each_address(uri.host) do |ip_address| - ip_addresses << IPAddr.new(ip_address) - end - end - - ip_addresses.any? do |ip| - ip.private? || ip.loopback? || ip.link_local? || ip.ipv4_mapped? || DISALLOWED_IP_RANGES.any? { |range| range.include?(ip) } - end + def resolved_ip + return @resolved_ip if defined?(@resolved_ip) + @resolved_ip = SsrfProtection.resolve_public_ip(uri.host) end def uri @@ -92,7 +78,7 @@ class Webhook::Delivery < ApplicationRecord end def http - Net::HTTP.new(uri.host, uri.port).tap do |http| + Net::HTTP.new(uri.host, uri.port, ipaddr: resolved_ip).tap do |http| http.use_ssl = (uri.scheme == "https") http.open_timeout = ENDPOINT_TIMEOUT http.read_timeout = ENDPOINT_TIMEOUT diff --git a/app/views/action_text/attachables/_remote_image.html.erb b/app/views/action_text/attachables/_remote_image.html.erb index 174a95c5e..f71f5dcaa 100644 --- a/app/views/action_text/attachables/_remote_image.html.erb +++ b/app/views/action_text/attachables/_remote_image.html.erb @@ -1,5 +1,5 @@
- <%= image_tag remote_image.url, width: remote_image.width, height: remote_image.height %> + <%= image_tag remote_image.url, skip_pipeline: true, width: remote_image.width, height: remote_image.height %> <% if caption = remote_image.try(:caption) %>
<%= caption %> diff --git a/app/views/boards/columns/_empty_placeholder.html.erb b/app/views/boards/columns/_empty_placeholder.html.erb new file mode 100644 index 000000000..36507c1ff --- /dev/null +++ b/app/views/boards/columns/_empty_placeholder.html.erb @@ -0,0 +1,6 @@ +
+
+

Drag cards here

+
+
No cards here
+
diff --git a/app/views/boards/columns/_list.html.erb b/app/views/boards/columns/_list.html.erb index a3a65ae74..7adedf322 100644 --- a/app/views/boards/columns/_list.html.erb +++ b/app/views/boards/columns/_list.html.erb @@ -1,3 +1,3 @@ -
+
<%= render "cards/display/previews", cards: cards, draggable: draggable %>
diff --git a/app/views/boards/columns/closeds/show.html.erb b/app/views/boards/columns/closeds/show.html.erb index 412d15183..8a9d1f56c 100644 --- a/app/views/boards/columns/closeds/show.html.erb +++ b/app/views/boards/columns/closeds/show.html.erb @@ -17,12 +17,7 @@ <%= render "boards/columns/list", cards: @page.records, draggable: true %> <% end %> <% else %> -
-
-

Drag cards here

-

Nothing here

-
-
+ <%= render "boards/columns/empty_placeholder" %> <% end %> <% end %> diff --git a/app/views/boards/columns/not_nows/show.html.erb b/app/views/boards/columns/not_nows/show.html.erb index 04ef7dd2e..88ea33868 100644 --- a/app/views/boards/columns/not_nows/show.html.erb +++ b/app/views/boards/columns/not_nows/show.html.erb @@ -17,12 +17,7 @@ <%= render "boards/columns/list", cards: @page.records, draggable: true %> <% end %> <% else %> -
-
-

Drag cards here

-

Nothing here

-
-
+ <%= render "boards/columns/empty_placeholder" %> <% end %> <% end %> diff --git a/app/views/boards/columns/show.html.erb b/app/views/boards/columns/show.html.erb index 83931aaff..4fef4eec7 100644 --- a/app/views/boards/columns/show.html.erb +++ b/app/views/boards/columns/show.html.erb @@ -17,12 +17,7 @@ <%= render "boards/columns/list", cards: @page.records, draggable: true %> <% end %> <% else %> -
-
-

Drag cards here

-

Nothing here

-
-
+ <%= render "boards/columns/empty_placeholder" %> <% end %> <% end %> diff --git a/app/views/boards/columns/streams/show.html.erb b/app/views/boards/columns/streams/show.html.erb index 141584283..9f0477b39 100644 --- a/app/views/boards/columns/streams/show.html.erb +++ b/app/views/boards/columns/streams/show.html.erb @@ -17,9 +17,7 @@ <%= render "boards/columns/list", cards: @page.records, draggable: true %> <% end %> <% else %> -
-

Nothing here

-
+ <%= render "boards/columns/empty_placeholder" %> <% end %> <% end %> diff --git a/app/views/boards/edit/_delete.html.erb b/app/views/boards/edit/_delete.html.erb index 7da3ee6c8..94a85d63f 100644 --- a/app/views/boards/edit/_delete.html.erb +++ b/app/views/boards/edit/_delete.html.erb @@ -1,5 +1,5 @@ <%= form_with model: board, class: "txt-align-center margin-block-start-auto", method: :delete do |form| %> - <%= form.button class: "btn txt-negative borderless txt-small", data: { turbo_confirm: "Are you sure you want to permanently delete this board?" } do %> + <%= form.button class: "btn txt-negative borderless txt-small", data: { turbo_confirm: "Are you sure you want to permanently delete this board and all the cards on it? This can't be undone." } do %> <%= icon_tag "trash" %> Delete this board <% end %> diff --git a/app/views/boards/new.html.erb b/app/views/boards/new.html.erb index 4536af7fc..748028b8d 100644 --- a/app/views/boards/new.html.erb +++ b/app/views/boards/new.html.erb @@ -1,9 +1,9 @@ <% @page_title = "Create a new board" %>
- <%= form_with model: @board, class: "flex flex-column gap", data: { controller: "form" } do |form| %> + <%= form_with model: @board, class: "flex flex-column gap", data: { controller: "form", action: "submit->form#preventEmptySubmit" } do |form| %>

<%= @page_title %>

- <%= form.text_field :name, required: true, class: "input full-width", autofocus: true, autocomplete: "off",placeholder: "Name it…", data: { action: "keydown.esc@document->form#cancel" } %> + <%= form.text_field :name, required: true, class: "input full-width", autofocus: true, autocomplete: "off", placeholder: "Name it…", data: { form_target: "input", action: "keydown.esc@document->form#cancel", validation_message: "Board names can‘t be blank" } %>