From 1c84f551dca6d0169b3c239da1856a9cc4b3e394 Mon Sep 17 00:00:00 2001 From: Cinder Date: Sat, 14 Dec 2024 16:45:45 -0800 Subject: [PATCH 01/12] [Posts] Fix an issue with the "no results" message (#824) --- app/javascript/src/styles/common/thumbnails.scss | 4 ++++ app/views/posts/_blank.html.erb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/javascript/src/styles/common/thumbnails.scss b/app/javascript/src/styles/common/thumbnails.scss index 2cdd3cd52..8ddc5eae6 100644 --- a/app/javascript/src/styles/common/thumbnails.scss +++ b/app/javascript/src/styles/common/thumbnails.scss @@ -24,6 +24,10 @@ section.posts-container { h2.posts-container-header { grid-column: -1 / 1; } + + .no-results { + grid-column: -1 / 1; + } } diff --git a/app/views/posts/_blank.html.erb b/app/views/posts/_blank.html.erb index 70183e6ab..4c350a06a 100644 --- a/app/views/posts/_blank.html.erb +++ b/app/views/posts/_blank.html.erb @@ -1,3 +1,3 @@ -

Nobody here but us chickens!

+

Nobody here but us chickens!

<%= link_to "Go back", :back, :rel => "prev" %>

From a0b51e40bcc0248350be7b19f18e0b6ce6172954 Mon Sep 17 00:00:00 2001 From: Cinder Date: Sat, 14 Dec 2024 17:37:53 -0800 Subject: [PATCH 02/12] [Users] Rework login pages and increase password requirements (#825) --- Gemfile | 1 + Gemfile.lock | 2 + README.md | 2 +- app/javascript/src/javascripts/password.js | 70 ++++++++++++++++ app/javascript/src/styles/base.scss | 1 - .../src/styles/specific/maintenance.scss | 5 -- app/javascript/src/styles/specific/users.scss | 80 +++++++++++++++++-- app/models/user.rb | 15 +++- .../user/login_reminders/new.html.erb | 28 +++---- .../user/password_resets/edit.html.erb | 44 +++++----- .../user/password_resets/new.html.erb | 27 +++---- .../maintenance/user/passwords/edit.html.erb | 19 ++--- app/views/sessions/new.html.erb | 62 +++++++------- app/views/users/new.html.erb | 48 +++++------ db/populate.rb | 2 +- db/seeds.rb | 4 +- package.json | 3 +- test/factories/user.rb | 8 +- .../functional/application_controller_test.rb | 8 +- .../user/api_keys_controller_test.rb | 12 +-- .../user/deletions_controller_test.rb | 2 +- .../user/email_changes_controller_test.rb | 6 +- test/functional/sessions_controller_test.rb | 6 +- test/functional/users_controller_test.rb | 2 +- test/unit/user_deletion_test.rb | 6 +- test/unit/user_test.rb | 18 +++-- yarn.lock | 5 ++ 27 files changed, 323 insertions(+), 163 deletions(-) create mode 100644 app/javascript/src/javascripts/password.js delete mode 100644 app/javascript/src/styles/specific/maintenance.scss diff --git a/Gemfile b/Gemfile index 09b10c97f..b681721c0 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,7 @@ gem 'marcel' gem 'sidekiq-unique-jobs' gem 'redis' gem 'request_store' +gem "zxcvbn-ruby", require: "zxcvbn" gem "diffy" gem "rugged" diff --git a/Gemfile.lock b/Gemfile.lock index d924493e8..17e4506a4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,6 +376,7 @@ GEM websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) zeitwerk (2.6.13) + zxcvbn-ruby (1.2.0) PLATFORMS ruby @@ -426,6 +427,7 @@ DEPENDENCIES streamio-ffmpeg webmock webpacker (>= 4.0.x) + zxcvbn-ruby BUNDLED WITH 2.4.10 diff --git a/README.md b/README.md index 04d410097..8af7f592d 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ docker compose up ``` After running the commands once only `docker compose up` is needed to bring up the containers. -1. To confirm the installation worked, open the web browser of your choice and enter `http://localhost:3000` into the address bar and see if the website loads correctly. An admin account has been created automatically, the username and password are `admin` and `qwerty` respectively. +1. To confirm the installation worked, open the web browser of your choice and enter `http://localhost:3000` into the address bar and see if the website loads correctly. An admin account has been created automatically, the username and password are `admin` and `hexerade` respectively. 1. By default, the site will lack any content. For testing purposes, you can generate some using the following command: ``` docker exec -it e621ng-e621-1 /app/bin/populate diff --git a/app/javascript/src/javascripts/password.js b/app/javascript/src/javascripts/password.js new file mode 100644 index 000000000..158ee56a8 --- /dev/null +++ b/app/javascript/src/javascripts/password.js @@ -0,0 +1,70 @@ +import zxcvbn from "zxcvbn"; +import Page from "./utility/page"; + +let Password = {}; + +Password.init_validation = function () { + if (Page.matches("users", "new") || Page.matches("users", "create")) + Password.bootstrap_input($("#user_password"), [$("#user_name"), $("#user_email")]); + + if (Page.matches("maintenance-user-password-resets", "edit")) + Password.bootstrap_input($("#password")); + + if (Page.matches("maintenance-user-passwords", "edit")) + Password.bootstrap_input($("#user_password")); +}; + +Password.bootstrap_input = function ($password, $inputs = []) { + // Set up the UI + $password.parent().addClass("password-input"); + + const hint = $("
") + .addClass("password-feedback") + .insertAfter($password); + const display = $("
") + .addClass("password-strength") + .insertAfter($password); + const progress = $("
") + .addClass("password-progress") + .css("width", 0) + .appendTo(display); + + // Listen to secondary input changes + let extraData = getExtraData(); + for (const one of $inputs) + one.on("input", () => { + extraData = getExtraData(); + }); + + // Listen to main input changes + $password.on("input", () => { + const analysis = zxcvbn($password.val() + "", extraData); + + progress.css("width", ((analysis.score * 25) + 10) + "%"); + hint.html(""); + if (analysis.feedback.warning) + $("") + .text(analysis.feedback.warning) + .addClass("password-warning") + .appendTo(hint); + for (const one of analysis.feedback.suggestions) + $("") + .text(one) + .appendTo(hint); + }); + + function getExtraData () { + const output = []; + for (const one of $inputs) { + const val = one.val() + ""; + if (val) output.push(one.val() + ""); + } + return output; + } +}; + +$(() => { + Password.init_validation(); +}); + +export default Password; diff --git a/app/javascript/src/styles/base.scss b/app/javascript/src/styles/base.scss index e8b01f5e7..0b92714a2 100644 --- a/app/javascript/src/styles/base.scss +++ b/app/javascript/src/styles/base.scss @@ -54,7 +54,6 @@ @import "specific/iqdb_queries.scss"; @import "specific/keyboard_shortcuts.scss"; @import "specific/lockdown.scss"; -@import "specific/maintenance.scss"; @import "specific/meta_searches.scss"; @import "specific/moderator_dashboard.scss"; @import "specific/news_updates.scss"; diff --git a/app/javascript/src/styles/specific/maintenance.scss b/app/javascript/src/styles/specific/maintenance.scss deleted file mode 100644 index 300f58e97..000000000 --- a/app/javascript/src/styles/specific/maintenance.scss +++ /dev/null @@ -1,5 +0,0 @@ -div#c-maintenance-user-login-reminders { - div#a-new { - width: 50em; - } -} diff --git a/app/javascript/src/styles/specific/users.scss b/app/javascript/src/styles/specific/users.scss index 807ac467b..22af7125b 100644 --- a/app/javascript/src/styles/specific/users.scss +++ b/app/javascript/src/styles/specific/users.scss @@ -223,13 +223,83 @@ div#c-users { color: themed("color-link-active"); } } +} - div#a-new { - max-width: 60em; +// User signup and login +#c-users #a-new, +#c-sessions #a-new, +#c-maintenance-user-password-resets #a-new, +#c-maintenance-user-login-reminders #a-new { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(auto, 360px)); + gap: 1em; - p { - font-size: 1.2em; - line-height: 1.4em; + margin-bottom: 1em; +} + +.session_form { + box-sizing: border-box; + max-width: 360px; + margin: 0; + + h1 { + margin-bottom: 0.5em; + text-align: center; + } + + div.input { + input[type="text"], input[type="email"], input[type="password"], select { + width: 100%; + max-width: unset; + box-sizing: border-box; } } } + +.session_info { + display: flex; + flex-flow: column; + justify-content: center; + box-sizing: border-box; + max-width: 360px; + padding: 0.5rem; + border-radius: 3px; + background-color: themed("color-section"); + + h3 { margin-bottom: 1em; } +} + +// Password validation +.password-input { + input[type="password"] { + border-radius: 3px 3px 0 0; + } + + .password-strength { + width: 100%; + height: 0.25rem; + border-radius: 0 0 3px 3px; + + background: white; + overflow: hidden; + margin: 0; + + .password-progress { + background: linear-gradient(to right, palette("text-red") 0%, palette("text-yellow") 25%, palette("text-green") 100%); + background-size: 360px 100%; + + height: 100%; + transition: width 1s ease-in-out; + } + } + + .password-feedback { + display: flex; + flex-flow: column; + padding-left: 1em; + margin-top: 0.5em; + + span { display: list-item; } + .password-warning { font-weight: bold; } + } +} \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 5c0956c23..b03d0c410 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "zxcvbn" + class User < ApplicationRecord class Error < Exception ; end class PrivilegeError < Exception @@ -71,7 +73,8 @@ class User < ApplicationRecord validates :per_page, inclusion: { :in => 1..320 } validates :comment_threshold, presence: true validates :comment_threshold, numericality: { only_integer: true, less_than: 50_000, greater_than: -50_000 } - validates :password, length: { :minimum => 6, :if => ->(rec) { rec.new_record? || rec.password.present? || rec.old_password.present? } } + validates :password, length: { minimum: 8, if: ->(rec) { rec.new_record? || rec.password.present? || rec.old_password.present? } } + validate :password_is_secure, if: ->(rec) { rec.new_record? || rec.password.present? || rec.old_password.present? } validates :password, confirmation: true validates :password_confirmation, presence: { if: ->(rec) { rec.new_record? || rec.old_password.present? } } validate :validate_ip_addr_is_not_banned, :on => :create @@ -227,6 +230,16 @@ class User < ApplicationRecord def upgrade_password(pass) self.update_columns(password_hash: '', bcrypt_password_hash: User.bcrypt(pass)) end + + def password_is_secure + analysis = Zxcvbn.test(password, [name, email]) + return unless analysis.score < 2 + if analysis.feedback.warning + errors.add(:password, "is insecure: #{analysis.feedback.warning}") + else + errors.add(:password, "is insecure") + end + end end module AuthenticationMethods diff --git a/app/views/maintenance/user/login_reminders/new.html.erb b/app/views/maintenance/user/login_reminders/new.html.erb index d22249427..a5924a768 100644 --- a/app/views/maintenance/user/login_reminders/new.html.erb +++ b/app/views/maintenance/user/login_reminders/new.html.erb @@ -1,21 +1,21 @@ -
-
+
+ + <%= form_tag(maintenance_user_login_reminder_path, :class => "simple_form session_form") do %>

Login Reminder

+ + + <%= submit_tag "Submit" %> + <% end %> + +

If you supplied an email address when signing up, <%= Danbooru.config.app_name %> can email you your login information. Password details will not be provided and will not be changed.

-

If you didn't supply a valid email address, you are out of luck.

- - <%= form_tag(maintenance_user_login_reminder_path, :class => "simple_form") do %> - - - <%= submit_tag "Submit" %> - <% end %> -
-
+ +
<%= render "sessions/secondary_links" %> diff --git a/app/views/maintenance/user/password_resets/edit.html.erb b/app/views/maintenance/user/password_resets/edit.html.erb index 4b3940f58..7916dc7fa 100644 --- a/app/views/maintenance/user/password_resets/edit.html.erb +++ b/app/views/maintenance/user/password_resets/edit.html.erb @@ -1,26 +1,28 @@ -
-
-

Reset Password

+
- <% if @nonce %> - <%= form_tag(maintenance_user_password_reset_path, :method => :put) do %> - <%= hidden_field_tag :uid, params[:uid] %> - <%= hidden_field_tag :key, params[:key] %> -
- - <%= password_field_tag :password %> -
-
- - <%= password_field_tag :password_confirm %> -
- <%= submit_tag "Reset" %> - <% end %> - <% else %> -

Invalid reset

+ <% if @nonce %> + <%= form_tag(maintenance_user_password_reset_path, :method => :put, :class => "simple_form session_form") do %> +

Reset Password

+ + <%= hidden_field_tag :uid, params[:uid] %> + <%= hidden_field_tag :key, params[:key] %> + +
+ + <%= password_field_tag :password %> +
+ +
+ + <%= password_field_tag :password_confirm %> +
+ + <%= submit_tag "Reset" %> <% end %> -
-
+ <% else %> +

Invalid reset

+ <% end %> +
<%= render "sessions/secondary_links" %> diff --git a/app/views/maintenance/user/password_resets/new.html.erb b/app/views/maintenance/user/password_resets/new.html.erb index 16e754347..821e70c21 100644 --- a/app/views/maintenance/user/password_resets/new.html.erb +++ b/app/views/maintenance/user/password_resets/new.html.erb @@ -1,20 +1,19 @@ -
-
+
+ + <%= form_tag(maintenance_user_password_reset_path, :class => "simple_form session_form") do %>

Reset Password

- + + + <%= submit_tag "Submit" %> + <% end %> +

If you supplied an email address when signing up, <%= Danbooru.config.app_name %> can reset your password. You will receive an email confirming your request for a new password.

-

If you didn't supply a valid email address, there is no way to recover your account.

- - <%= form_tag(maintenance_user_password_reset_path, :class => "simple_form") do %> - - <%= submit_tag "Submit" %> - <% end %> -
-
+ +
<%= render "sessions/secondary_links" %> diff --git a/app/views/maintenance/user/passwords/edit.html.erb b/app/views/maintenance/user/passwords/edit.html.erb index 5352fe654..f212fc900 100644 --- a/app/views/maintenance/user/passwords/edit.html.erb +++ b/app/views/maintenance/user/passwords/edit.html.erb @@ -1,15 +1,12 @@ -
-
+
+ <%= custom_form_for(@user, html: { class: "session_form" }) do |f| %>

Change Password

- - <%= custom_form_for @user do |f| %> - <%= f.input :old_password, :as => :password, :input_html => {:autocomplete => "off"} %> - <%= f.input :password, :label => "New password", :input_html => {:autocomplete => "off"} %> - <%= f.input :password_confirmation %> - <%= f.button :submit, "Submit" %> - <% end %> -
-
+ <%= f.input :old_password, :as => :password, :input_html => {:autocomplete => "off"} %> + <%= f.input :password, :label => "New password", :input_html => {:autocomplete => "off"} %> + <%= f.input :password_confirmation %> + <%= f.button :submit, "Submit" %> + <% end %> +
<% content_for(:page_title) do %> Change Password diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index fa43c68a3..e7698cb13 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -1,38 +1,40 @@ -
-
-
-

Sign in

- <%= form_tag(session_path, :class => "simple_form") do %> - <%= hidden_field_tag "url", params[:url] %> +
+ <%= form_tag(session_path, :class => "simple_form session_form") do %> +

Sign in

-
- - <%= text_field_tag :name %> -
+ <%= hidden_field_tag "url", params[:url] %> -
- - <%= password_field_tag :password %> -
+
+ + <%= text_field_tag :name %> +
-
- <%= check_box_tag :remember, "1", true %> - -
+
+ + <%= password_field_tag :password %> +
-
- <%= submit_tag "Submit", :data => { :disable_with => "Signing in..." } %> -
- <% end %> -
+
+ <%= check_box_tag :remember, "1", true %> + +
-
-

<%= link_to "Don't have an account? Sign up here.", new_user_path() %>

-

<%= link_to "Reset Password", new_maintenance_user_password_reset_path %>

-

<%= link_to "Login Reminder", new_maintenance_user_login_reminder_path %>

-
-
-
+
+ <%= submit_tag "Submit", :data => { :disable_with => "Signing in..." } %> +
+ <% end %> + +
+

+ Don't have an account?
+ <%= link_to "Sign up here.", new_user_path() %> +

+
+ +

<%= link_to "Reset Password", new_maintenance_user_password_reset_path %>

+

<%= link_to "Login Reminder", new_maintenance_user_login_reminder_path %>

+
+
<% content_for(:page_title) do %> Sign in diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 3bf422fef..e2ec32e1e 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -1,34 +1,30 @@ -
-
+
+ <%= custom_form_for(@user, html: { id: "signup-form", class: "session_form" }) do |f| %>

Sign Up

-

An account is free and lets you keep favorites, upload artwork, and write comments.

+ <%= f.input :name, label: "Username", as: :string %> + <%= f.input :email, :required => true, as: :email %> + <%= f.input :password %> + <%= f.input :password_confirmation %> -
-

Make sure to read the site rules before continuing.

-

You must confirm your email address, so use something you can receive email with.

-

This site is open to web crawlers so whatever name you choose will be public!

-

This includes favorites, uploads, and comments. Almost everything is public. So don't choose a name you don't want to be associated with.

-

Accounts are prefilled with the same blacklist as guests have. You can access your blacklist in your account settings.

-
+ <%= f.input :time_zone, include_blank: false %> -
- <%= custom_form_for(@user, html: {id: "signup-form"}) do |f| %> - <%= f.input :name, :as => :string %> - <%= f.input :email, :required => true, :as => :email %> - <%= f.input :password %> - <%= f.input :password_confirmation %> - - <%= f.input :time_zone, :include_blank => false %> - - <% if Danbooru.config.enable_recaptcha? %> - <%= recaptcha_tags theme: 'dark', nonce: content_security_policy_nonce %> - <% end %> - <%= f.submit "Sign up", :data => { :disable_with => "Signing up..." } %> - <% end %> -
+ <% if Danbooru.config.enable_recaptcha? %> + <%= recaptcha_tags theme: "dark", nonce: content_security_policy_nonce %> + <% end %> + <%= f.submit "Sign up", :data => { :disable_with => "Signing up..." } %> + <% end %> +
+

+ Already have an account? <%= link_to "Sign In.", new_session_path %> +

+

Please, read the site rules before making an account.

+

You must confirm your email address, so you should only use one that you have access to.

+

This site is open to web crawlers, meaning that almost everything is public.

+

This includes your account name, favorites, uploads, and comments. Do not choose a name you don't want to be associated with.

+

Accounts have the same blacklist as guests by default. You will be able to modify your blacklist in the account settings.

-
+
<%= render "secondary_links" %> diff --git a/db/populate.rb b/db/populate.rb index bbeb5aadb..9af5e2cac 100644 --- a/db/populate.rb +++ b/db/populate.rb @@ -37,7 +37,7 @@ POSTVOTES = presets[:postvotes] POOLS = presets[:pools] DISTRIBUTION = ENV.fetch("DISTRIBUTION", 10).to_i -DEFAULT_PASSWORD = ENV.fetch("PASSWORD", "qwerty") +DEFAULT_PASSWORD = ENV.fetch("PASSWORD", "hexerade") CurrentUser.user = User.system diff --git a/db/seeds.rb b/db/seeds.rb index fa4c38a26..4587c56f9 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -9,8 +9,8 @@ require "tempfile" admin = User.find_or_create_by!(name: "admin") do |user| user.created_at = 2.weeks.ago - user.password = "qwerty" - user.password_confirmation = "qwerty" + user.password = "hexerade" + user.password_confirmation = "hexerade" user.password_hash = "" user.email = "admin@e621.local" user.can_upload_free = true diff --git a/package.json b/package.json index 02d96d044..bf3746417 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "vue": "^3.1.0", "vue-loader": "^17.4.2", "webpack": "^5.51.1", - "zingtouch": "^1.0.6" + "zingtouch": "^1.0.6", + "zxcvbn": "^4.4.2" }, "version": "0.1.0", "devDependencies": { diff --git a/test/factories/user.rb b/test/factories/user.rb index 4190f3bb3..54fb75424 100644 --- a/test/factories/user.rb +++ b/test/factories/user.rb @@ -5,14 +5,14 @@ FactoryBot.define do sequence :name do |n| "user#{n}" end - password { "password" } - password_confirmation { "password" } + password { "6cQE!wbA" } + password_confirmation { "6cQE!wbA" } sequence(:email) { |n| "user_email_#{n}@example.com" } default_image_size { "large" } base_upload_limit { 10 } level { 20 } - created_at {Time.now} - last_logged_in_at {Time.now} + created_at { Time.now } + last_logged_in_at { Time.now } factory(:banned_user) do transient { ban_duration { 3 } } diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 6606f3680..1e92e0701 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -36,7 +36,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest context "on api authentication" do setup do - @user = create(:user, password: "password") + @user = create(:user, password: "6cQE!wbA") @api_key = ApiKey.generate!(@user) ActionController::Base.allow_forgery_protection = true @@ -108,7 +108,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest token = css_select("form input[name=authenticity_token]").first["value"] # login - post session_path, params: { authenticity_token: token, name: @user.name, password: "password" } + post session_path, params: { authenticity_token: token, name: @user.name, password: "6cQE!wbA" } assert_redirected_to posts_path # try to submit a form with cookies but without the csrf token @@ -122,9 +122,9 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest context "on session cookie authentication" do should "succeed" do - user = create(:user, password: "password") + user = create(:user, password: "6cQE!wbA") - post session_path, params: { name: user.name, password: "password" } + post session_path, params: { name: user.name, password: "6cQE!wbA" } get edit_user_path(user) assert_response :success diff --git a/test/functional/maintenance/user/api_keys_controller_test.rb b/test/functional/maintenance/user/api_keys_controller_test.rb index 61d1bdf0d..22b03bb40 100644 --- a/test/functional/maintenance/user/api_keys_controller_test.rb +++ b/test/functional/maintenance/user/api_keys_controller_test.rb @@ -7,7 +7,7 @@ module Maintenance class ApiKeysControllerTest < ActionDispatch::IntegrationTest context "An api keys controller" do setup do - @user = create(:privileged_user, :password => "password") + @user = create(:privileged_user, password: "6cQE!wbA") ApiKey.generate!(@user) end @@ -21,7 +21,7 @@ module Maintenance context "#view" do context "with a correct password" do should "succeed" do - post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: {user: {password: "password"}} + post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: { user: { password: "6cQE!wbA" } } assert_response :success end @@ -37,7 +37,7 @@ module Maintenance # ApiKey.expects(:generate!) # assert_difference("ApiKey.count", 1) do - # post view_maintenance_user_api_key_path(user_id: @user.id), params: {user: {password: "password"}} + # post view_maintenance_user_api_key_path(user_id: @user.id), params: { user: { password: "6cQE!wbA" } } # end # assert_not_nil(@user.reload.api_key) @@ -46,7 +46,7 @@ module Maintenance should "not generate another API key if the user already has one" do assert_difference("ApiKey.count", 0) do - post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: {user: {password: "password"}} + post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: { user: { password: "6cQE!wbA" } } end end end @@ -55,14 +55,14 @@ module Maintenance context "#update" do should "regenerate the API key" do old_key = @user.api_key - put_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: {password: "password"}} + put_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: { password: "6cQE!wbA" } } assert_not_equal(old_key.key, @user.reload.api_key.key) end end context "#destroy" do should "delete the API key" do - delete_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: {password: "password"}} + delete_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: { password: "6cQE!wbA" } } assert_nil(@user.reload.api_key) end end diff --git a/test/functional/maintenance/user/deletions_controller_test.rb b/test/functional/maintenance/user/deletions_controller_test.rb index d9632f46f..cf9731ead 100644 --- a/test/functional/maintenance/user/deletions_controller_test.rb +++ b/test/functional/maintenance/user/deletions_controller_test.rb @@ -19,7 +19,7 @@ module Maintenance context "#destroy" do should "render" do - delete_auth maintenance_user_deletion_path, @user, params: { password: "password" } + delete_auth maintenance_user_deletion_path, @user, params: { password: "6cQE!wbA" } assert_redirected_to(posts_path) end end diff --git a/test/functional/maintenance/user/email_changes_controller_test.rb b/test/functional/maintenance/user/email_changes_controller_test.rb index be5a1ff3b..c0e48573c 100644 --- a/test/functional/maintenance/user/email_changes_controller_test.rb +++ b/test/functional/maintenance/user/email_changes_controller_test.rb @@ -21,7 +21,7 @@ module Maintenance context "#create" do context "with the correct password" do should "work" do - post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "password", email: "abc@ogres.net" } } + post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "6cQE!wbA", email: "abc@ogres.net" } } assert_redirected_to(home_users_path) @user.reload assert_equal("abc@ogres.net", @user.email) @@ -37,7 +37,7 @@ module Maintenance end should "not work with an invalid email" do - post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "password", email: "" } } + post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "6cQE!wbA", email: "" } } @user.reload assert_not_equal("", @user.email) assert_match(/Email can't be blank/, flash[:notice]) @@ -45,7 +45,7 @@ module Maintenance should "work with a valid email when the users current email is invalid" do @user = create(:user, email: "") - post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "password", email: "abc@ogres.net" } } + post_auth maintenance_user_email_change_path, @user, params: { email_change: { password: "6cQE!wbA", email: "abc@ogres.net" } } @user.reload assert_equal("abc@ogres.net", @user.email) end diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb index 249a128c9..66f016a92 100644 --- a/test/functional/sessions_controller_test.rb +++ b/test/functional/sessions_controller_test.rb @@ -15,7 +15,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest should "create a new session" do user = create(:user) - post session_path, params: { name: user.name, password: "password" } + post session_path, params: { name: user.name, password: "6cQE!wbA" } user.reload assert_redirected_to(posts_path) @@ -33,7 +33,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest end should "fail when provided an invalid password" do - user = create(:user, password: "xxxxxx", password_confirmation: "xxxxxx") + user = create(:user, password: "6cQE!wbA", password_confirmation: "6cQE!wbA") post session_path, params: { name: user.name, password: "yyy" } assert_nil(session[:user_id]) @@ -45,7 +45,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest should "clear the session" do user = create(:user) - post session_path, params: { name: user.name, password: "password" } + post session_path, params: { name: user.name, password: "6cQE!wbA" } assert_not_nil(session[:user_id]) delete_auth(session_path, user) diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index bd9a259d6..0c822f5c8 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -76,7 +76,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest context "create action" do should "create a user" do assert_difference(-> { User.count }, 1) do - post users_path, params: { user: { name: "xxx", password: "xxxxx1", password_confirmation: "xxxxx1" } } + post users_path, params: { user: { name: "xxx", password: "nePD.3L4", password_confirmation: "nePD.3L4" } } end created_user = User.find(session[:user_id]) assert_equal("xxx", created_user.name) diff --git a/test/unit/user_deletion_test.rb b/test/unit/user_deletion_test.rb index 487bd552c..6ea316dff 100644 --- a/test/unit/user_deletion_test.rb +++ b/test/unit/user_deletion_test.rb @@ -22,7 +22,7 @@ class UserDeletionTest < ActiveSupport::TestCase setup do @user = create(:admin_user) CurrentUser.user = @user - @deletion = UserDeletion.new(@user, "password") + @deletion = UserDeletion.new(@user, "6cQE!wbA") end should "fail" do @@ -43,7 +43,7 @@ class UserDeletionTest < ActiveSupport::TestCase @user.update(email: "ted@danbooru.com") - @deletion = UserDeletion.new(@user, "password") + @deletion = UserDeletion.new(@user, "6cQE!wbA") with_inline_jobs { @deletion.delete! } @user.reload end @@ -58,7 +58,7 @@ class UserDeletionTest < ActiveSupport::TestCase should "reset the password" do assert_raises(BCrypt::Errors::InvalidHash) do - User.authenticate(@user.name, "password") + User.authenticate(@user.name, "6cQE!wbA") end end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 403e4e7de..90d1a511c 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -116,8 +116,8 @@ class UserTest < ActiveSupport::TestCase end should "authenticate" do - assert(User.authenticate(@user.name, "password"), "Authentication should have succeeded") - assert(!User.authenticate(@user.name, "password2"), "Authentication should not have succeeded") + assert(User.authenticate(@user.name, "6cQE!wbA"), "Authentication should have succeeded") + assert_not(User.authenticate(@user.name, "password2"), "Authentication should not have succeeded") end should "normalize its level" do @@ -209,8 +209,8 @@ class UserTest < ActiveSupport::TestCase should "fail if the confirmation does not match" do @user = create(:user) - @user.password = "zugzug6" - @user.password_confirmation = "zugzug5" + @user.password = "6cQE!wbA" + @user.password_confirmation = "7cQE!wbA" @user.save assert_equal(["Password confirmation doesn't match Password"], @user.errors.full_messages) end @@ -220,7 +220,15 @@ class UserTest < ActiveSupport::TestCase @user.password = "x5" @user.password_confirmation = "x5" @user.save - assert_equal(["Password is too short (minimum is 6 characters)"], @user.errors.full_messages) + assert_equal(["Password is too short (minimum is 8 characters)", "Password is insecure"], @user.errors.full_messages) + end + + should "not be insecure" do + @user = create(:user) + @user.password = "qwerty123" + @user.password_confirmation = "qwerty123" + @user.save + assert_equal(["Password is insecure: This is similar to a commonly used password"], @user.errors.full_messages) end # should "not change the password if the password and old password are blank" do diff --git a/yarn.lock b/yarn.lock index 0fa0597a0..809e7f10d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3716,3 +3716,8 @@ zingtouch@^1.0.6: version "1.0.6" resolved "https://registry.yarnpkg.com/zingtouch/-/zingtouch-1.0.6.tgz#456cf2b0a69f91a5ffbd8a83b18033c671f1096d" integrity sha512-S7jcR7cSRy28VmQBO0Tq7ZJV4pzfvvrTU9FrrL0K1QPpfBal9wm0oKhoCuifc+PPCq+hQMTJr5E9XKUQDm00VA== + +zxcvbn@^4.4.2: + version "4.4.2" + resolved "https://registry.yarnpkg.com/zxcvbn/-/zxcvbn-4.4.2.tgz#28ec17cf09743edcab056ddd8b1b06262cc73c30" + integrity sha512-Bq0B+ixT/DMyG8kgX2xWcI5jUvCwqrMxSFam7m0lAf78nf04hv6lNCsyLYdyYTrCVMqNDY/206K7eExYCeSyUQ== From 32a1367cfe97a8d27c7bcbb721c475c48ff693b2 Mon Sep 17 00:00:00 2001 From: Donovan Daniels Date: Wed, 18 Dec 2024 08:02:02 -0600 Subject: [PATCH 03/12] [Users] Consolidate password confirmation into singular route (#813) --- app/controllers/application_controller.rb | 9 +++ .../maintenance/user/api_keys_controller.rb | 22 ++----- app/controllers/sessions_controller.rb | 19 ++++-- .../src/styles/specific/sessions.scss | 4 +- app/javascript/src/styles/specific/users.scss | 4 +- app/logical/session_creator.rb | 12 ++-- .../maintenance/user/api_keys/show.html.erb | 33 ++++++++-- .../maintenance/user/api_keys/view.html.erb | 38 ----------- app/views/sessions/confirm_password.html.erb | 16 +++++ app/views/sessions/new.html.erb | 36 +++------- config/routes.rb | 8 +-- .../functional/application_controller_test.rb | 4 +- .../user/api_keys_controller_test.rb | 65 +++++++------------ test/functional/sessions_controller_test.rb | 6 +- test/test_helper.rb | 2 +- 15 files changed, 122 insertions(+), 156 deletions(-) delete mode 100644 app/views/maintenance/user/api_keys/view.html.erb create mode 100644 app/views/sessions/confirm_password.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 490cafc3d..68d84c5f8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -165,6 +165,7 @@ class ApplicationController < ActionController::Base def set_current_user SessionLoader.new(request).load + session.send(:load!) unless session.send(:loaded?) end def reset_current_user @@ -173,6 +174,14 @@ class ApplicationController < ActionController::Base CurrentUser.safe_mode = Danbooru.config.safe_mode? end + def requires_reauthentication + return redirect_to(new_session_path(url: request.fullpath)) if CurrentUser.user.is_anonymous? + last_authenticated_at = session[:last_authenticated_at] + if last_authenticated_at.blank? || Time.zone.parse(last_authenticated_at) < 1.hour.ago + redirect_to(confirm_password_session_path(url: request.fullpath)) + end + end + def user_access_check(method) if !CurrentUser.user.send(method) || CurrentUser.user.is_banned? || IpBan.is_banned?(CurrentUser.ip_addr) access_denied diff --git a/app/controllers/maintenance/user/api_keys_controller.rb b/app/controllers/maintenance/user/api_keys_controller.rb index f2809e1dc..f6b0e5d02 100644 --- a/app/controllers/maintenance/user/api_keys_controller.rb +++ b/app/controllers/maintenance/user/api_keys_controller.rb @@ -3,17 +3,14 @@ module Maintenance module User class ApiKeysController < ApplicationController + before_action :requires_reauthentication before_action :member_only - before_action :authenticate!, except: [:show] - rescue_from ::SessionLoader::AuthenticationFailure, with: :authentication_failed + before_action :load_apikey respond_to :html def show end - def view - end - def update @api_key.regenerate! redirect_to(user_api_key_path(CurrentUser.user), notice: "API key regenerated") @@ -24,19 +21,10 @@ module Maintenance redirect_to(CurrentUser.user) end - protected + private - def authenticate! - if ::User.authenticate(CurrentUser.user.name, params[:user][:password]) == CurrentUser.user - @api_key = CurrentUser.user.api_key || ApiKey.generate!(CurrentUser.user) - @password = params[:user][:password] - else - raise ::SessionLoader::AuthenticationFailure - end - end - - def authentication_failed - redirect_to(user_api_key_path(CurrentUser.user), notice: "Password was incorrect.") + def load_apikey + @api_key = CurrentUser.user.api_key || ApiKey.generate!(CurrentUser.user) end end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a95243a6f..0a62a3f34 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -6,26 +6,31 @@ class SessionsController < ApplicationController end def create + sparams = params.fetch(:session, {}).slice(:url, :name, :password, :remember) if RateLimiter.check_limit("login:#{request.remote_ip}", 15, 12.hours) DanbooruLogger.add_attributes("user.login" => "rate_limited") - return redirect_to(new_session_path, :notice => "Username/Password was incorrect") + return redirect_to(new_session_path, notice: "Username/Password was incorrect") end - session_creator = SessionCreator.new(session, cookies, params[:name], params[:password], request.remote_ip, params[:remember], request.ssl?) + session_creator = SessionCreator.new(request, session, cookies, sparams[:name], sparams[:password], sparams[:remember].to_s.truthy?) if session_creator.authenticate - url = params[:url] if params[:url] && params[:url].start_with?("/") && !params[:url].start_with?("//") + url = sparams[:url] if sparams[:url] && sparams[:url].start_with?("/") && !sparams[:url].start_with?("//") DanbooruLogger.add_attributes("user.login" => "success") - redirect_to(url || posts_path, :notice => "You are now logged in") + redirect_to(url || posts_path) else RateLimiter.hit("login:#{request.remote_ip}", 6.hours) DanbooruLogger.add_attributes("user.login" => "fail") - redirect_to(new_session_path, :notice => "Username/Password was incorrect") + redirect_back(fallback_location: new_session_path, notice: "Username/Password was incorrect") end end def destroy session.delete(:user_id) - cookies.delete :remember - redirect_to(posts_path, :notice => "You are now logged out") + cookies.delete(:remember) + session.delete(:last_authenticated_at) + redirect_to(posts_path, notice: "You are now logged out") + end + + def confirm_password end end diff --git a/app/javascript/src/styles/specific/sessions.scss b/app/javascript/src/styles/specific/sessions.scss index ea217013f..21d502d51 100644 --- a/app/javascript/src/styles/specific/sessions.scss +++ b/app/javascript/src/styles/specific/sessions.scss @@ -1,8 +1,6 @@ - - div#c-sessions { div#a-new { - label#remember-label { + label[for=session_remember] { display: inline; font-weight: normal; font-style: italic; diff --git a/app/javascript/src/styles/specific/users.scss b/app/javascript/src/styles/specific/users.scss index 22af7125b..52fe1b601 100644 --- a/app/javascript/src/styles/specific/users.scss +++ b/app/javascript/src/styles/specific/users.scss @@ -237,7 +237,7 @@ div#c-users { margin-bottom: 1em; } -.session_form { +.simple_form.session { box-sizing: border-box; max-width: 360px; margin: 0; @@ -302,4 +302,4 @@ div#c-users { span { display: list-item; } .password-warning { font-weight: bold; } } -} \ No newline at end of file +} diff --git a/app/logical/session_creator.rb b/app/logical/session_creator.rb index c43ea5f41..119dd58a9 100644 --- a/app/logical/session_creator.rb +++ b/app/logical/session_creator.rb @@ -1,16 +1,15 @@ # frozen_string_literal: true class SessionCreator - attr_reader :session, :cookies, :name, :password, :ip_addr, :remember, :secure + attr_reader :request, :session, :cookies, :name, :password, :remember - def initialize(session, cookies, name, password, ip_addr, remember = false, secure = false) + def initialize(request, session, cookies, name, password, remember = false) + @request = request @session = session @cookies = cookies @name = name @password = password - @ip_addr = ip_addr @remember = remember - @secure = secure end def authenticate @@ -18,12 +17,13 @@ class SessionCreator user = User.find_by_name(name) session[:user_id] = user.id + session[:last_authenticated_at] = Time.now.utc.to_s session[:ph] = user.password_token - user.update_column(:last_ip_addr, ip_addr) unless user.is_blocked? + user.update_column(:last_ip_addr, request.remote_ip) unless user.is_blocked? if remember verifier = ActiveSupport::MessageVerifier.new(Danbooru.config.remember_key, serializer: JSON, digest: "SHA256") - cookies.encrypted[:remember] = {value: verifier.generate("#{user.id}:#{user.password_token}", purpose: "rbr", expires_in: 14.days), expires: Time.now + 14.days, httponly: true, same_site: :lax, secure: Rails.env.production?} + cookies.encrypted[:remember] = { value: verifier.generate("#{user.id}:#{user.password_token}", purpose: "rbr", expires_in: 14.days), expires: Time.now + 14.days, httponly: true, same_site: :lax, secure: Rails.env.production? } end return true else diff --git a/app/views/maintenance/user/api_keys/show.html.erb b/app/views/maintenance/user/api_keys/show.html.erb index 516321041..0fdf9dba1 100644 --- a/app/views/maintenance/user/api_keys/show.html.erb +++ b/app/views/maintenance/user/api_keys/show.html.erb @@ -1,12 +1,35 @@

API Key

-

You must re-enter your password to view or change your API key.

- <%= custom_form_for CurrentUser.user, url: view_user_api_key_path(CurrentUser.user), method: :post do |f| %> - <%= f.input :password, :as => :password, :input_html => {:autocomplete => "off"} %> - <%= f.button :submit, "Submit" %> - <% end %> +

Your API key is like your password

+

+ Anyone who has it has full access to your account. Don't give your API key + to third-party apps you don't trust, and don't post your API key in public places. +

+ + + + + + + + + + + + + + + + + + + +
API KeyCreatedUpdatedActions
<%= @api_key.key %><%= compact_time(@api_key.created_at) %><%= compact_time(@api_key.updated_at) %> + <%= button_to("Regenerate", user_api_key_path(CurrentUser.user), method: :put) %> + <%= button_to("Delete", user_api_key_path(CurrentUser.user), method: :delete) %> +
diff --git a/app/views/maintenance/user/api_keys/view.html.erb b/app/views/maintenance/user/api_keys/view.html.erb deleted file mode 100644 index b51506592..000000000 --- a/app/views/maintenance/user/api_keys/view.html.erb +++ /dev/null @@ -1,38 +0,0 @@ -
-
-

API Key

- -

-

Your API key is like your password

- Anyone who has it has full access to your account. Don't give your API key - to third-party apps you don't trust, and don't post your API key in public places. -

- - - - - - - - - - - - - - - - - - - -
API KeyCreatedUpdatedActions
<%= @api_key.key %><%= compact_time @api_key.created_at %><%= compact_time @api_key.updated_at %> - <%= button_to "Regenerate", user_api_key_path(CurrentUser.user), method: :put, params: { 'user[password]': @password } %> - <%= button_to "Delete", user_api_key_path(CurrentUser.user), method: :delete, params: { 'user[password]': @password } %> -
-
-
- -<% content_for(:page_title) do %> - API Key -<% end %> diff --git a/app/views/sessions/confirm_password.html.erb b/app/views/sessions/confirm_password.html.erb new file mode 100644 index 000000000..23a642058 --- /dev/null +++ b/app/views/sessions/confirm_password.html.erb @@ -0,0 +1,16 @@ +<% content_for(:page_title) do %> + Confirm Password +<% end %> + + +<%= render "secondary_links" %> +
+

Confirm password

+

You must re-enter your password to continue.

+ <%= simple_form_for(:session, url: session_path) do |f| %> + <%= f.input(:url, as: :hidden, input_html: { value: params[:url] }) %> + <%= f.input(:name, as: :hidden, input_html: { value: CurrentUser.user.name }) %> + <%= f.input(:password, hint: link_to("Forgot password?", new_maintenance_user_password_reset_path), input_html: { autocomplete: "current-password" }) %> + <%= f.submit("Continue") %> + <% end %> +
diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index e7698cb13..1e97827cb 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -1,38 +1,22 @@
- <%= form_tag(session_path, :class => "simple_form session_form") do %> + <%= simple_form_for(:session, url: session_path) do |f| %>

Sign in

- - <%= hidden_field_tag "url", params[:url] %> - -
- - <%= text_field_tag :name %> -
- -
- - <%= password_field_tag :password %> -
- -
- <%= check_box_tag :remember, "1", true %> - -
- -
- <%= submit_tag "Submit", :data => { :disable_with => "Signing in..." } %> -
+ <%= f.input(:url, as: :hidden, input_html: { value: params[:url] }) %> + <%= f.input(:name, label: "Username") %> + <%= f.input(:password) %> + <%= f.input(:remember, as: :boolean, input_html: { checked: "checked" }) %> + <%= f.submit("Continue") %> <% end %>

Don't have an account?
- <%= link_to "Sign up here.", new_user_path() %> + <%= link_to("Sign up here.", new_user_path) %>


- -

<%= link_to "Reset Password", new_maintenance_user_password_reset_path %>

-

<%= link_to "Login Reminder", new_maintenance_user_login_reminder_path %>

+ +

<%= link_to("Reset Password", new_maintenance_user_password_reset_path) %>

+

<%= link_to("Login Reminder", new_maintenance_user_login_reminder_path) %>

diff --git a/config/routes.rb b/config/routes.rb index 776f15029..530c3b935 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -267,7 +267,9 @@ Rails.application.routes.draw do end resource :related_tag, :only => [:show, :update] match "related_tag/bulk", to: "related_tags#bulk", via: [:get, :post] - resource :session, only: [:new, :create, :destroy] + resource :session, only: %i[new create destroy] do + get :confirm_password, on: :collection + end resources :stats, only: [:index] resources :tags, constraints: id_name_constraint do resource :correction, :only => [:new, :create, :show], :controller => "tag_corrections" @@ -291,9 +293,7 @@ Rails.application.routes.draw do resources :uploads resources :users do resource :password, :only => [:edit], :controller => "maintenance/user/passwords" - resource :api_key, :only => [:show, :view, :update, :destroy], :controller => "maintenance/user/api_keys" do - post :view - end + resource :api_key, only: %i[show update destroy], controller: "maintenance/user/api_keys" resources :staff_notes, only: [:index, :new, :create], controller: "admin/staff_notes" collection do diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 1e92e0701..87e2d73f5 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -108,7 +108,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest token = css_select("form input[name=authenticity_token]").first["value"] # login - post session_path, params: { authenticity_token: token, name: @user.name, password: "6cQE!wbA" } + post session_path, params: { authenticity_token: token, session: { name: @user.name, password: "6cQE!wbA" } } assert_redirected_to posts_path # try to submit a form with cookies but without the csrf token @@ -124,7 +124,7 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest should "succeed" do user = create(:user, password: "6cQE!wbA") - post session_path, params: { name: user.name, password: "6cQE!wbA" } + post session_path, params: { session: { name: user.name, password: "6cQE!wbA" } } get edit_user_path(user) assert_response :success diff --git a/test/functional/maintenance/user/api_keys_controller_test.rb b/test/functional/maintenance/user/api_keys_controller_test.rb index 22b03bb40..1237d917d 100644 --- a/test/functional/maintenance/user/api_keys_controller_test.rb +++ b/test/functional/maintenance/user/api_keys_controller_test.rb @@ -8,61 +8,42 @@ module Maintenance context "An api keys controller" do setup do @user = create(:privileged_user, password: "6cQE!wbA") - ApiKey.generate!(@user) + @api_key = ApiKey.generate!(@user) end - context "#show" do - should "render" do - get_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id} + context "show action" do + should "let a user see their own API keys" do + get_auth maintenance_user_api_key_path(@user.id), @user assert_response :success + assert_select "#api-key-#{@api_key.id}", count: 1 + end + + should "not let a user see API keys belonging to other users" do + get_auth maintenance_user_api_key_path(@user.id), create(:user) + assert_response :success + assert_select "#api-key-#{@api_key.id}", count: 0 + end + + should "redirect to the confirm password page if the user hasn't recently authenticated" do + post session_path, params: { session: { name: @user.name, password: @user.password } } + travel_to 2.hours.from_now do + get maintenance_user_api_key_path(@user.id) + end + assert_redirected_to confirm_password_session_path(url: maintenance_user_api_key_path(@user.id)) end end - context "#view" do - context "with a correct password" do - should "succeed" do - post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: { user: { password: "6cQE!wbA" } } - assert_response :success - end - - # hard to test this in integrationtest - # context "if the user doesn't already have an api key" do - # setup do - # ::User.any_instance.stubs(:api_key).returns(nil) - # cookies[:user_name] = @user.name - # cookies[:password_hash] = @user.bcrypt_cookie_password_hash - # end - - # should "generate one" do - # ApiKey.expects(:generate!) - - # assert_difference("ApiKey.count", 1) do - # post view_maintenance_user_api_key_path(user_id: @user.id), params: { user: { password: "6cQE!wbA" } } - # end - - # assert_not_nil(@user.reload.api_key) - # end - # end - - should "not generate another API key if the user already has one" do - assert_difference("ApiKey.count", 0) do - post_auth view_maintenance_user_api_key_path(user_id: @user.id), @user, params: { user: { password: "6cQE!wbA" } } - end - end - end - end - - context "#update" do + context "update action" do should "regenerate the API key" do old_key = @user.api_key - put_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: { password: "6cQE!wbA" } } + put_auth maintenance_user_api_key_path, @user assert_not_equal(old_key.key, @user.reload.api_key.key) end end - context "#destroy" do + context "destroy action" do should "delete the API key" do - delete_auth maintenance_user_api_key_path, @user, params: {user_id: @user.id, user: { password: "6cQE!wbA" } } + delete_auth maintenance_user_api_key_path, @user assert_nil(@user.reload.api_key) end end diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb index 66f016a92..c2f1acce4 100644 --- a/test/functional/sessions_controller_test.rb +++ b/test/functional/sessions_controller_test.rb @@ -15,7 +15,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest should "create a new session" do user = create(:user) - post session_path, params: { name: user.name, password: "6cQE!wbA" } + post session_path, params: { session: { name: user.name, password: "6cQE!wbA" } } user.reload assert_redirected_to(posts_path) @@ -34,7 +34,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest should "fail when provided an invalid password" do user = create(:user, password: "6cQE!wbA", password_confirmation: "6cQE!wbA") - post session_path, params: { name: user.name, password: "yyy" } + post session_path, params: { session: { name: user.name, password: "yyy" } } assert_nil(session[:user_id]) assert_equal("Username/Password was incorrect", flash[:notice]) @@ -45,7 +45,7 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest should "clear the session" do user = create(:user) - post session_path, params: { name: user.name, password: "6cQE!wbA" } + post session_path, params: { session: { name: user.name, password: "6cQE!wbA" } } assert_not_nil(session[:user_id]) delete_auth(session_path, user) diff --git a/test/test_helper.rb b/test/test_helper.rb index d26cd5ad0..f903b1c59 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -95,7 +95,7 @@ end class ActionDispatch::IntegrationTest def method_authenticated(method_name, url, user, options) - post session_path, params: { name: user.name, password: user.password } + post session_path, params: { session: { name: user.name, password: user.password } } self.send(method_name, url, **options) end From b44f8a7d6f1a889c358694cdd4f07c507beb1d7d Mon Sep 17 00:00:00 2001 From: Donovan Daniels Date: Wed, 18 Dec 2024 08:07:30 -0600 Subject: [PATCH 04/12] [Posts] Add contributor category (#794) --- app/indexes/post_index.rb | 60 ++++++++++--------- app/javascript/src/javascripts/tag_editor.vue | 1 + .../src/javascripts/uploader/uploader.vue.erb | 4 +- app/javascript/src/styles/base/_colors.scss | 2 + .../src/styles/themes/_theme_hexagon.scss | 3 + app/logical/tag_category.rb | 14 ++++- app/views/stats/index.html.erb | 1 + .../122_1_add_contributor_category_index.rb | 7 +++ .../122_2_add_contributor_category_data.rb | 9 +++ ...20241114055212_add_contributor_category.rb | 9 +++ db/structure.sql | 4 +- 11 files changed, 79 insertions(+), 35 deletions(-) create mode 100755 db/fixes/122_1_add_contributor_category_index.rb create mode 100755 db/fixes/122_2_add_contributor_category_data.rb create mode 100644 db/migrate/20241114055212_add_contributor_category.rb diff --git a/app/indexes/post_index.rb b/app/indexes/post_index.rb index 8ff243f1e..4a12ccc5b 100644 --- a/app/indexes/post_index.rb +++ b/app/indexes/post_index.rb @@ -28,6 +28,7 @@ module PostIndex tag_count_general: { type: "integer" }, tag_count_artist: { type: "integer" }, + tag_count_contributor: { type: "integer" }, tag_count_character: { type: "integer" }, tag_count_copyright: { type: "integer" }, tag_count_meta: { type: "integer" }, @@ -235,36 +236,37 @@ module PostIndex tag_count: tag_count, change_seq: change_seq, - tag_count_general: tag_count_general, - tag_count_artist: tag_count_artist, - tag_count_character: tag_count_character, - tag_count_copyright: tag_count_copyright, - tag_count_meta: tag_count_meta, - tag_count_species: tag_count_species, - tag_count_invalid: tag_count_invalid, - tag_count_lore: tag_count_lore, - comment_count: options[:comment_count] || comment_count, + tag_count_general: tag_count_general, + tag_count_artist: tag_count_artist, + tag_count_contributor: tag_count_contributor, + tag_count_character: tag_count_character, + tag_count_copyright: tag_count_copyright, + tag_count_meta: tag_count_meta, + tag_count_species: tag_count_species, + tag_count_invalid: tag_count_invalid, + tag_count_lore: tag_count_lore, - file_size: file_size, - parent: parent_id, - pools: options[:pools] || ::Pool.where("? = ANY(post_ids)", id).pluck(:id), - sets: options[:sets] || ::PostSet.where("? = ANY(post_ids)", id).pluck(:id), - commenters: options[:commenters] || ::Comment.undeleted.where(post_id: id).pluck(:creator_id), - noters: options[:noters] || ::Note.active.where(post_id: id).pluck(:creator_id), - faves: options[:faves] || ::Favorite.where(post_id: id).pluck(:user_id), - upvotes: options[:upvotes] || ::PostVote.where(post_id: id).where("score > 0").pluck(:user_id), - downvotes: options[:downvotes] || ::PostVote.where(post_id: id).where("score < 0").pluck(:user_id), - children: options[:children] || ::Post.where(parent_id: id).pluck(:id), - notes: options[:notes] || ::Note.active.where(post_id: id).pluck(:body), - uploader: uploader_id, - approver: approver_id, - deleter: options[:deleter] || ::PostFlag.where(post_id: id, is_resolved: false, is_deletion: true).order(id: :desc).first&.creator_id, - del_reason: options[:del_reason] || ::PostFlag.where(post_id: id, is_resolved: false, is_deletion: true).order(id: :desc).first&.reason&.downcase, - width: image_width, - height: image_height, - mpixels: image_width && image_height ? (image_width.to_f * image_height / 1_000_000).round(2) : 0.0, - aspect_ratio: image_width && image_height ? (image_width.to_f / [image_height, 1].max).round(2) : 1.0, - duration: duration, + comment_count: options[:comment_count] || comment_count, + file_size: file_size, + parent: parent_id, + pools: options[:pools] || ::Pool.where("? = ANY(post_ids)", id).pluck(:id), + sets: options[:sets] || ::PostSet.where("? = ANY(post_ids)", id).pluck(:id), + commenters: options[:commenters] || ::Comment.undeleted.where(post_id: id).pluck(:creator_id), + noters: options[:noters] || ::Note.active.where(post_id: id).pluck(:creator_id), + faves: options[:faves] || ::Favorite.where(post_id: id).pluck(:user_id), + upvotes: options[:upvotes] || ::PostVote.where(post_id: id).where("score > 0").pluck(:user_id), + downvotes: options[:downvotes] || ::PostVote.where(post_id: id).where("score < 0").pluck(:user_id), + children: options[:children] || ::Post.where(parent_id: id).pluck(:id), + notes: options[:notes] || ::Note.active.where(post_id: id).pluck(:body), + uploader: uploader_id, + approver: approver_id, + deleter: options[:deleter] || ::PostFlag.where(post_id: id, is_resolved: false, is_deletion: true).order(id: :desc).first&.creator_id, + del_reason: options[:del_reason] || ::PostFlag.where(post_id: id, is_resolved: false, is_deletion: true).order(id: :desc).first&.reason&.downcase, + width: image_width, + height: image_height, + mpixels: image_width && image_height ? (image_width.to_f * image_height / 1_000_000).round(2) : 0.0, + aspect_ratio: image_width && image_height ? (image_width.to_f / [image_height, 1].max).round(2) : 1.0, + duration: duration, tags: tag_string.split(" "), md5: md5, diff --git a/app/javascript/src/javascripts/tag_editor.vue b/app/javascript/src/javascripts/tag_editor.vue index 182efdfde..90b6f1ddf 100644 --- a/app/javascript/src/javascripts/tag_editor.vue +++ b/app/javascript/src/javascripts/tag_editor.vue @@ -11,6 +11,7 @@ Related: Tags | Artists | + Contributors | Copyrights | Characters | Species | diff --git a/app/javascript/src/javascripts/uploader/uploader.vue.erb b/app/javascript/src/javascripts/uploader/uploader.vue.erb index 3b069e351..79d15034e 100644 --- a/app/javascript/src/javascripts/uploader/uploader.vue.erb +++ b/app/javascript/src/javascripts/uploader/uploader.vue.erb @@ -28,7 +28,7 @@