From 7adad1a3d75c4677113063e70fc5a9c53ccb6528 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:48:43 -0600 Subject: [PATCH 01/21] successfully adds homepage url --- app/controllers/profiles_controller.rb | 2 +- app/controllers/users_controller.rb | 1 + app/models/user.rb | 7 ++++++- app/views/profiles/edit.html.erb | 9 +++++++++ db/migrate/20241114211431_add_homepage_url_to_users.rb | 5 +++++ db/schema.rb | 3 ++- 6 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20241114211431_add_homepage_url_to_users.rb diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 1adbf233817..f66aabfb1aa 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -65,7 +65,7 @@ def params_user end end - PERMITTED_PROFILE_PARAMS = %i[handle twitter_username unconfirmed_email public_email full_name].freeze + PERMITTED_PROFILE_PARAMS = %i[handle twitter_username unconfirmed_email homepage_url public_email full_name].freeze def verify_password password = params.permit(user: :password).require(:user)[:password] diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 243e4a6e231..17045ad2f02 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -27,6 +27,7 @@ def create password website twitter_username + homepage_url full_name ].freeze diff --git a/app/models/user.rb b/app/models/user.rb index 8b25cd75531..3ce93219b29 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,6 +82,11 @@ class User < ApplicationRecord message: "can only contain letters, numbers, and underscores" }, allow_nil: true, length: { within: 0..20 } + validates :homepage_url, format: { + with: /\A[a-zA-Z0-9_]*\z/, + message: "can only contain letters, numbers, and underscores" + }, allow_nil: true, length: { within: 0..20 } + validates :password, length: { minimum: 10 }, unpwn: true, @@ -351,7 +356,7 @@ def clear_personal_attributes handle: nil, email_confirmed: false, unconfirmed_email: nil, blocked_email: nil, api_key: nil, confirmation_token: nil, remember_token: nil, - twitter_username: nil, webauthn_id: nil, full_name: nil, + twitter_username: nil, webauthn_id: nil, full_name: nil, homepage_url: nil, totp_seed: nil, mfa_hashed_recovery_codes: nil, mfa_level: :disabled, password: SecureRandom.hex(20).encode("UTF-8") diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index d56e7bdf80b..eb89a4b5398 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -20,6 +20,15 @@ <%= form.text_field :handle, :class => 'form__input' %> +
+ <%= form.label :homepage_url, class: 'form__label form__label__icon-container' do %> + + <%= t('.homepage_url') %> + <% end %> + + <%= form.text_field(:homepage_url, class: 'form__input') %> +
+
<%= form.label :twitter_username, class: 'form__label form__label__icon-container' do %> <%= diff --git a/db/migrate/20241114211431_add_homepage_url_to_users.rb b/db/migrate/20241114211431_add_homepage_url_to_users.rb new file mode 100644 index 00000000000..167d4fad68b --- /dev/null +++ b/db/migrate/20241114211431_add_homepage_url_to_users.rb @@ -0,0 +1,5 @@ +class AddHomepageUrlToUsers < ActiveRecord::Migration[7.2] + def change + add_column :users, :homepage_url, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 99235afcc75..471a0478c03 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_04_065953) do +ActiveRecord::Schema[7.2].define(version: 2024_11_14_211431) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" enable_extension "pgcrypto" @@ -556,6 +556,7 @@ t.string "mfa_hashed_recovery_codes", default: [], array: true t.boolean "public_email", default: false, null: false t.datetime "deleted_at" + t.string "homepage_url" t.index "lower((email)::text) varchar_pattern_ops", name: "index_users_on_lower_email" t.index ["email"], name: "index_users_on_email" t.index ["handle"], name: "index_users_on_handle" From 29e05754235ab1555d31e6bd7a56cd381bca93f1 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:18:00 -0600 Subject: [PATCH 02/21] adds test --- app/views/dashboards/_subject.html.erb | 11 +++++++++++ app/views/profiles/edit.html.erb | 2 +- test/integration/profile_test.rb | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/views/dashboards/_subject.html.erb b/app/views/dashboards/_subject.html.erb index 64981449be9..b0c5f9ff5ff 100644 --- a/app/views/dashboards/_subject.html.erb +++ b/app/views/dashboards/_subject.html.erb @@ -23,6 +23,17 @@
<% end %> +<% if user.homepage_url.present? %> +
+ <%= icon_tag("link", color: :primary, class: "w-6 text-orange mr-3") %> +

<%= + link_to( + user.homepage_url + ) + %>

+
+<% end %> + <% if user.twitter_username.present? %>
<%= icon_tag("x-twitter", color: :primary, class: "w-6 text-orange mr-3") %> diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index eb89a4b5398..02bd72bbc8f 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -22,7 +22,7 @@
<%= form.label :homepage_url, class: 'form__label form__label__icon-container' do %> - + <%= t('.homepage_url') %> <% end %> diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index 8585cfed4ad..9ca834c44bf 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -133,6 +133,21 @@ def sign_out assert page.has_link?("@nick1", href: "https://twitter.com/nick1") end + test "adding homepage url" do + sign_in + visit profile_path("nick1") + + click_link "Edit Profile" + fill_in "user_homepage_url", with: "https://nickisawesome.com" + fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD + click_button "Update" + + click_link "Sign out" + visit profile_path("nick1") + + assert page.has_link?("https://nickisawesome.com") + end + test "deleting profile" do sign_in visit profile_path("nick1") From f76e318fe46277c75ef2cc1865eab9c8ca1a120e Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:26:35 -0600 Subject: [PATCH 03/21] prevent user from updating profile without adding password --- app/views/profiles/edit.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index 02bd72bbc8f..b1d4be708fc 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -75,7 +75,7 @@

<%= t('.enter_password') %>

- <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input' %> + <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input', required: true %>
From 9b52c4d1a9ebc40a4c22f622d48a35a821f2c340 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:28:53 -0600 Subject: [PATCH 04/21] fix homepage styling url --- app/views/profiles/edit.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index b1d4be708fc..8b37c6f261e 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -21,7 +21,7 @@
- <%= form.label :homepage_url, class: 'form__label form__label__icon-container' do %> + <%= form.label :homepage_url, class: 'form__label' do %> <%= t('.homepage_url') %> <% end %> From 5dfe4c726cbda13f3817f101afa2c643e9bf35ff Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 16:47:23 -0600 Subject: [PATCH 05/21] adds profile to show page --- app/views/profiles/edit.html.erb | 10 +++------- app/views/profiles/show.html.erb | 8 ++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index 8b37c6f261e..e1cc2ba27e8 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -21,12 +21,8 @@
- <%= form.label :homepage_url, class: 'form__label' do %> - - <%= t('.homepage_url') %> - <% end %> - - <%= form.text_field(:homepage_url, class: 'form__input') %> + <%= form.label :homepage_url, class: 'form__label' %> + <%= form.text_field(:homepage_url, class: 'form__input') %>
@@ -75,7 +71,7 @@

<%= t('.enter_password') %>

- <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input', required: true %> + <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input' %>
diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index a109e68018b..aa34402aa62 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -94,6 +94,14 @@ ) %> <% end %> + <% if @user.homepage_url.present? %> + <%= + link_to( + @user.homepage_url, + class: "profile__header__attribute t-link--black" + ) + %> + <% end %> <% end %> From 215c55e655f48b8eb1eb1f7b05f914a05efc8680 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:01:02 -0600 Subject: [PATCH 06/21] fixes validation of url --- app/models/user.rb | 5 +---- app/views/profiles/edit.html.erb | 2 +- test/integration/profile_test.rb | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 3ce93219b29..7b43776b638 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,10 +82,7 @@ class User < ApplicationRecord message: "can only contain letters, numbers, and underscores" }, allow_nil: true, length: { within: 0..20 } - validates :homepage_url, format: { - with: /\A[a-zA-Z0-9_]*\z/, - message: "can only contain letters, numbers, and underscores" - }, allow_nil: true, length: { within: 0..20 } + validates_formatting_of :homepage_url, using: :url, message: "does not appear to be a valid URL", allow_nil: true validates :password, length: { minimum: 10 }, diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index e1cc2ba27e8..38251ff3d9d 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -71,7 +71,7 @@

<%= t('.enter_password') %>

- <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input' %> + <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input', required: true %> diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index 9ca834c44bf..2c1505c5138 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -133,19 +133,19 @@ def sign_out assert page.has_link?("@nick1", href: "https://twitter.com/nick1") end - test "adding homepage url" do + test "adding homepage url" do sign_in visit profile_path("nick1") click_link "Edit Profile" - fill_in "user_homepage_url", with: "https://nickisawesome.com" + fill_in "user_homepage_url", with: "www.nickisawesome.com" fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD click_button "Update" click_link "Sign out" visit profile_path("nick1") - assert page.has_link?("https://nickisawesome.com") + assert page.has_link?("www.nickisawesome.com") end test "deleting profile" do From 6d7a95422f2d4acc44421ee098c34f9965d5e407 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:17:23 -0600 Subject: [PATCH 07/21] passing test --- app/models/user.rb | 2 +- test/integration/profile_test.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 7b43776b638..b5b5813a5ce 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,7 +82,7 @@ class User < ApplicationRecord message: "can only contain letters, numbers, and underscores" }, allow_nil: true, length: { within: 0..20 } - validates_formatting_of :homepage_url, using: :url, message: "does not appear to be a valid URL", allow_nil: true + validates_formatting_of :homepage_url, using: :url, allow_nil: true validates :password, length: { minimum: 10 }, diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index 2c1505c5138..b0f228c78b2 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -31,7 +31,7 @@ def sign_out fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD click_button "Update" - assert page.has_content? "nick2" + assert_content "nick2" end test "changing to an existing handle" do @@ -138,14 +138,14 @@ def sign_out visit profile_path("nick1") click_link "Edit Profile" - fill_in "user_homepage_url", with: "www.nickisawesome.com" + fill_in "user_homepage_url", with: "https://nickisawesome.com" fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD click_button "Update" click_link "Sign out" visit profile_path("nick1") - assert page.has_link?("www.nickisawesome.com") + assert page.has_link?("https://nickisawesome.com") end test "deleting profile" do From 62b6f6dca707294917b9fec983e6457650298700 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:30:37 -0600 Subject: [PATCH 08/21] tests are failing --- app/views/profiles/edit.html.erb | 2 +- test/integration/profile_test.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index 38251ff3d9d..e178abe7320 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -71,7 +71,7 @@

<%= t('.enter_password') %>

- <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input', required: true %> + <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input'%> diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index b0f228c78b2..cef7c5d96bf 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -31,7 +31,7 @@ def sign_out fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD click_button "Update" - assert_content "nick2" + assert page.has_content? "nick2" end test "changing to an existing handle" do @@ -130,6 +130,7 @@ def sign_out click_link "Sign out" visit profile_path("nick1") + assert_content("test") assert page.has_link?("@nick1", href: "https://twitter.com/nick1") end From e76b7bbfac61e40555aeedefd6ae61c2f8bd23c5 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 17:44:01 -0600 Subject: [PATCH 09/21] undo change --- app/views/profiles/edit.html.erb | 2 +- test/integration/profile_test.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index e178abe7320..e1cc2ba27e8 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -71,7 +71,7 @@

<%= t('.enter_password') %>

- <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input'%> + <%= form.password_field :password, autocomplete: 'current-password', class: 'form__input' %> diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index cef7c5d96bf..681fa101499 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -130,7 +130,6 @@ def sign_out click_link "Sign out" visit profile_path("nick1") - assert_content("test") assert page.has_link?("@nick1", href: "https://twitter.com/nick1") end From 94f31e536b54ed3a42f866b54b891e1944e25ecb Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:00:19 -0600 Subject: [PATCH 10/21] passing tests --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index b5b5813a5ce..16331833303 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -82,7 +82,7 @@ class User < ApplicationRecord message: "can only contain letters, numbers, and underscores" }, allow_nil: true, length: { within: 0..20 } - validates_formatting_of :homepage_url, using: :url, allow_nil: true + validates_formatting_of :homepage_url, using: :url, allow_blank: true validates :password, length: { minimum: 10 }, From 922a6d08f26ec6329ac8aaba336f4b4c59eabcb8 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Thu, 14 Nov 2024 18:15:08 -0600 Subject: [PATCH 11/21] add no follow --- app/views/profiles/show.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index aa34402aa62..de27dfdd431 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -98,6 +98,7 @@ <%= link_to( @user.homepage_url, + rel: "nofollow", class: "profile__header__attribute t-link--black" ) %> From 5bd20d82de805938b0e676538604dcc5bf3759a5 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:27:29 -0600 Subject: [PATCH 12/21] fix styling of link --- app/views/profiles/show.html.erb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index de27dfdd431..7980c1cbd7a 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -95,13 +95,15 @@ %> <% end %> <% if @user.homepage_url.present? %> - <%= - link_to( - @user.homepage_url, - rel: "nofollow", - class: "profile__header__attribute t-link--black" - ) - %> +

+ <%= + link_to( + @user.homepage_url, + rel: "nofollow", + class: "profile__header__attribute t-link--black" + ) + %> +

<% end %> <% end %> From 16e829f47e29ce1270fde4c030a25a98be465026 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:31:47 -0600 Subject: [PATCH 13/21] update user test --- test/models/user_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index add6d52b9a5..a2ab06d750a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -172,6 +172,12 @@ class UserTest < ActiveSupport::TestCase should_not allow_value("012345678901234567890").for(:twitter_username) end + context "homepage url" do + should allow_value("https://www.mywebsite.com").for(:homepage_url) + should_not allow_value("http://non-secure-site.com").for(:homepage_url) + should_not allow_value("hi").for(:homepage_url) + end + context "password" do should "be between 10 characters and 72 bytes" do user = build(:user, password: "%5a&12ed/") From 9b2bf910f3c5075c3bd2e061bbc813b52cf9f62a Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Fri, 15 Nov 2024 11:51:45 -0600 Subject: [PATCH 14/21] udpatetest --- test/models/user_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index a2ab06d750a..dd09ab9d77d 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -174,7 +174,6 @@ class UserTest < ActiveSupport::TestCase context "homepage url" do should allow_value("https://www.mywebsite.com").for(:homepage_url) - should_not allow_value("http://non-secure-site.com").for(:homepage_url) should_not allow_value("hi").for(:homepage_url) end From 9cb7e45515eb314ac8336ba7d5d14b89b46eaf6a Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 14:32:30 -0800 Subject: [PATCH 15/21] Add rel=nofollow --- app/views/dashboards/_subject.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/dashboards/_subject.html.erb b/app/views/dashboards/_subject.html.erb index b0c5f9ff5ff..5e59b43f286 100644 --- a/app/views/dashboards/_subject.html.erb +++ b/app/views/dashboards/_subject.html.erb @@ -28,7 +28,9 @@ <%= icon_tag("link", color: :primary, class: "w-6 text-orange mr-3") %>

<%= link_to( - user.homepage_url + user.homepage_url, + user.homepage_url, + rel: "nofollow" ) %>

From 922c27418cb7683b710f6404d5384c88f1443b90 Mon Sep 17 00:00:00 2001 From: Martin Emde Date: Wed, 20 Nov 2024 14:34:41 -0800 Subject: [PATCH 16/21] fix rel=nofollow on public profile --- app/views/profiles/show.html.erb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index 7980c1cbd7a..69eba0041ed 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -98,6 +98,7 @@

<%= link_to( + @user.homepage_url, @user.homepage_url, rel: "nofollow", class: "profile__header__attribute t-link--black" From 93901663c1dba1c63623637057e5ce5a61b65310 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Sat, 23 Nov 2024 20:19:44 -0500 Subject: [PATCH 17/21] new XXS tests for homepage url --- test/models/user_test.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index dd09ab9d77d..673104c6148 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -175,6 +175,18 @@ class UserTest < ActiveSupport::TestCase context "homepage url" do should allow_value("https://www.mywebsite.com").for(:homepage_url) should_not allow_value("hi").for(:homepage_url) + should_not allow_value("javascript:alert('hello');").for(:homepage_url) + should_not allow_value("file:///etc/passwd").for(:homepage_url) + should_not allow_value("file://C:/Windows/System32/cmd.exe").for(:homepage_url) + should_not allow_value("data:text/html,").for(:homepage_url) + should_not allow_value("data:text/html;base64,SGVsbG8sIFdvcmxkIQ==").for(:homepage_url) + should_not allow_value("data:text/html,").for(:homepage_url) + should_not allow_value("data:text/html,").for(:homepage_url) + # should_not allow_value("http://www.site.site?#redirect=www.fake-target.site").for(:homepage_url) + # should_not allow_value("http://www.site.com/redirect?url=http://www.malicious-site.com").for(:homepage_url) + # should_not allow_value("http://www.site.com/?next=http://www.malicious-site.com").for(:homepage_url) + # should_not allow_value("http://www.site.com/?url=http://www.malicious-site.com").for(:homepage_url) + # should_not allow_value("http://www.site.com/?redirect=http://www.malicious-site.com").for(:homepage_url) end context "password" do From 1d00a50bc7352f712ad257e00533315e6f59f8ff Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Sat, 23 Nov 2024 21:38:22 -0500 Subject: [PATCH 18/21] extract httpURL validator into a lib file and update the tests --- app/models/user.rb | 2 +- app/views/profiles/show.html.erb | 2 +- lib/http_url_validator.rb | 10 ++++++++++ test/integration/profile_test.rb | 15 +++++++++++++++ test/models/user_test.rb | 1 + 5 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 lib/http_url_validator.rb diff --git a/app/models/user.rb b/app/models/user.rb index f4549ebfe5a..38cdd54e9f7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -83,7 +83,7 @@ class User < ApplicationRecord message: "can only contain letters, numbers, and underscores" }, allow_nil: true, length: { within: 0..20 } - validates_formatting_of :homepage_url, using: :url, allow_blank: true + validates :homepage_url, http_url: true, allow_blank: true validates :password, length: { minimum: 10 }, diff --git a/app/views/profiles/show.html.erb b/app/views/profiles/show.html.erb index 69eba0041ed..db4fce982ab 100644 --- a/app/views/profiles/show.html.erb +++ b/app/views/profiles/show.html.erb @@ -97,7 +97,7 @@ <% if @user.homepage_url.present? %>

<%= - link_to( + link_to( @user.homepage_url, @user.homepage_url, rel: "nofollow", diff --git a/lib/http_url_validator.rb b/lib/http_url_validator.rb new file mode 100644 index 00000000000..bf514f457e5 --- /dev/null +++ b/lib/http_url_validator.rb @@ -0,0 +1,10 @@ +class HttpUrlValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + validUriPattern = %r{\Ahttps?://([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z} # :nodoc: + + uri = URI::DEFAULT_PARSER.parse(value) + record.errors.add attribute, "is not a valid URL" unless [URI::HTTP, URI::HTTPS].member?(uri.class) && validUriPattern.match?(value) + rescue URI::InvalidURIError + record.errors.add attribute, ("is not a valid URL") + end +end diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index 681fa101499..68d2a6009c1 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -148,6 +148,21 @@ def sign_out assert page.has_link?("https://nickisawesome.com") end + test "adding malicious homepage url" do + sign_in + visit profile_path("nick1") + + click_link "Edit Profile" + fill_in "user_homepage_url", with: "http://www.site.com/redirect?url=http://www.malicious-site.com" + fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD + click_button "Update" + + click_link "Sign out" + visit profile_path("nick1") + + assert page.has_link?("http://www.site.com/redirect?url=http://www.malicious") + end + test "deleting profile" do sign_in visit profile_path("nick1") diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 673104c6148..9ad455e367f 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -174,6 +174,7 @@ class UserTest < ActiveSupport::TestCase context "homepage url" do should allow_value("https://www.mywebsite.com").for(:homepage_url) + should allow_value("http://www.mywebsite.com").for(:homepage_url) should_not allow_value("hi").for(:homepage_url) should_not allow_value("javascript:alert('hello');").for(:homepage_url) should_not allow_value("file:///etc/passwd").for(:homepage_url) From a0170522e90a1b385fb13526833f153c348bca19 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Sat, 23 Nov 2024 22:19:54 -0500 Subject: [PATCH 19/21] remove duplication --- lib/http_url_validator.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/http_url_validator.rb b/lib/http_url_validator.rb index bf514f457e5..7ff97e56d5f 100644 --- a/lib/http_url_validator.rb +++ b/lib/http_url_validator.rb @@ -1,10 +1,7 @@ class HttpUrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - validUriPattern = %r{\Ahttps?://([^\s:@]+:[^\s:@]*@)?[A-Za-z\d\-]+(\.[A-Za-z\d\-]+)+\.?(:\d{1,5})?([\/?]\S*)?\z} # :nodoc: - uri = URI::DEFAULT_PARSER.parse(value) - record.errors.add attribute, "is not a valid URL" unless [URI::HTTP, URI::HTTPS].member?(uri.class) && validUriPattern.match?(value) - rescue URI::InvalidURIError - record.errors.add attribute, ("is not a valid URL") + record.errors.add attribute, "is not a valid URL" unless [URI::HTTP, URI::HTTPS].member?(uri.class) + record.errors.add attribute, "is not a valid URL" end end From a65e417b8da27f769509de9017e0d560f0e1dfe6 Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Sat, 23 Nov 2024 22:21:31 -0500 Subject: [PATCH 20/21] remove unneeded tests --- test/integration/profile_test.rb | 15 --------------- test/models/user_test.rb | 5 ----- 2 files changed, 20 deletions(-) diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index 68d2a6009c1..681fa101499 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -148,21 +148,6 @@ def sign_out assert page.has_link?("https://nickisawesome.com") end - test "adding malicious homepage url" do - sign_in - visit profile_path("nick1") - - click_link "Edit Profile" - fill_in "user_homepage_url", with: "http://www.site.com/redirect?url=http://www.malicious-site.com" - fill_in "Password", with: PasswordHelpers::SECURE_TEST_PASSWORD - click_button "Update" - - click_link "Sign out" - visit profile_path("nick1") - - assert page.has_link?("http://www.site.com/redirect?url=http://www.malicious") - end - test "deleting profile" do sign_in visit profile_path("nick1") diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 9ad455e367f..317782c38aa 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -183,11 +183,6 @@ class UserTest < ActiveSupport::TestCase should_not allow_value("data:text/html;base64,SGVsbG8sIFdvcmxkIQ==").for(:homepage_url) should_not allow_value("data:text/html,").for(:homepage_url) should_not allow_value("data:text/html,").for(:homepage_url) - # should_not allow_value("http://www.site.site?#redirect=www.fake-target.site").for(:homepage_url) - # should_not allow_value("http://www.site.com/redirect?url=http://www.malicious-site.com").for(:homepage_url) - # should_not allow_value("http://www.site.com/?next=http://www.malicious-site.com").for(:homepage_url) - # should_not allow_value("http://www.site.com/?url=http://www.malicious-site.com").for(:homepage_url) - # should_not allow_value("http://www.site.com/?redirect=http://www.malicious-site.com").for(:homepage_url) end context "password" do From 25fe7d3221dbec0b76c5c5ac44f273af82b883bb Mon Sep 17 00:00:00 2001 From: Jacklyn Ma <29336370+jacklynhma@users.noreply.github.com> Date: Sat, 23 Nov 2024 22:27:37 -0500 Subject: [PATCH 21/21] add rescue --- lib/http_url_validator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/http_url_validator.rb b/lib/http_url_validator.rb index 7ff97e56d5f..c122927a168 100644 --- a/lib/http_url_validator.rb +++ b/lib/http_url_validator.rb @@ -2,6 +2,7 @@ class HttpUrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) uri = URI::DEFAULT_PARSER.parse(value) record.errors.add attribute, "is not a valid URL" unless [URI::HTTP, URI::HTTPS].member?(uri.class) - record.errors.add attribute, "is not a valid URL" + rescue URI::InvalidURIError + record.errors.add attribute, "is not a valid URL" end end