Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add homepage url to user profile #5240

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
2 changes: 1 addition & 1 deletion app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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]
Expand Down
1 change: 1 addition & 0 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def create
password
website
twitter_username
homepage_url
full_name
].freeze

Expand Down
4 changes: 3 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ 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
martinemde marked this conversation as resolved.
Show resolved Hide resolved

validates :password,
length: { minimum: 10 },
unpwn: true,
Expand Down Expand Up @@ -352,7 +354,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")
Expand Down
13 changes: 13 additions & 0 deletions app/views/dashboards/_subject.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@
</div>
<% end %>

<% if user.homepage_url.present? %>
<div class="flex items-center mb-4 text-b3 lg:text-b2">
<%= icon_tag("link", color: :primary, class: "w-6 text-orange mr-3") %>
<p class="text-neutral-800 dark:text-white"><%=
link_to(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: suggest the appropriate rel=none; noreferrer options

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like rel="nofollow" is what github uses on their profile links. I'm agnostic about "noreferrer" in addition.

user.homepage_url,
user.homepage_url,
rel: "nofollow"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opps sorry I didn't realize there was a final decision on the rel="nofollow". Thank you for adding these changes :) @martinemde

)
%></p>
</div>
<% end %>

<% if user.twitter_username.present? %>
<div class="flex items-center mb-4 text-b3 lg:text-b2">
<%= icon_tag("x-twitter", color: :primary, class: "w-6 text-orange mr-3") %>
Expand Down
5 changes: 5 additions & 0 deletions app/views/profiles/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
<%= form.text_field :handle, :class => 'form__input' %>
</div>

<div class="text_field">
<%= form.label :homepage_url, class: 'form__label' %>
<%= form.text_field(:homepage_url, class: 'form__input') %>
</div>

<div class="text_field">
<%= form.label :twitter_username, class: 'form__label form__label__icon-container' do %>
<%=
Expand Down
12 changes: 12 additions & 0 deletions app/views/profiles/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@
)
%>
<% end %>
<% if @user.homepage_url.present? %>
<p id="homepage-url">
<%=
link_to(
@user.homepage_url,
@user.homepage_url,

Check warning

Code scanning / CodeQL

Stored cross-site scripting Medium

Stored cross-site scripting vulnerability due to
stored value
.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should consider something like HackerOne's "you're about to leave this site for ...". Github appends http:// to urls that don't have either http / https in the front, and they are probably doing more.

rel: "nofollow",
class: "profile__header__attribute t-link--black"
)
%>
</p>
<% end %>
<% end %>
</div>

Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20241114211431_add_homepage_url_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddHomepageUrlToUsers < ActiveRecord::Migration[7.2]
def change
add_column :users, :homepage_url, :string
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
15 changes: 15 additions & 0 deletions test/integration/profile_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
5 changes: 5 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ 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("<html><body>hi</body><html>").for(:homepage_url)
end

context "password" do
should "be between 10 characters and 72 bytes" do
user = build(:user, password: "%5a&12ed/")
Expand Down
Loading