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

Show gem owners that are not using MFA when logged in as owner #2129

Merged
merged 9 commits into from
Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/assets/javascripts/pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,13 @@ $('.stats__graph__gem__meter').each(function() {
bar_width = $(this).data("bar_width");
$(this).animate({ width: bar_width + '%' }, 700).removeClass('t-item--hidden');
});

//gem page
$(document).ready(function() {
$('.gem__users__mfa-text.mfa-warn').on('click', function() {
$('.gem__users__mfa-text.mfa-warn').toggleClass('t-item--hidden');

$owners = $('.gem__users__mfa-disabled');
$owners.toggleClass('t-item--hidden');
});
});
4 changes: 4 additions & 0 deletions app/assets/stylesheets/modules/gem.css
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
background-color: #e9573f;
opacity: .5; }

.gem__users__mfa-text {
font-size: 9px;
color: #e9573f;
}
.gem__downloads-wrap {
margin-bottom: 30px; }

Expand Down
4 changes: 4 additions & 0 deletions app/helpers/rubygems_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ def links_to_owners(rubygem)
rubygem.owners.sort_by(&:id).inject("") { |link, owner| link << link_to_user(owner) }.html_safe
end

def links_to_owners_without_mfa(rubygem)
rubygem.owners.without_mfa.sort_by(&:id).inject("") { |link, owner| link << link_to_user(owner) }.html_safe
end

def link_to_user(user)
link_to gravatar(48, "gravatar-#{user.id}", user), profile_path(user.display_id),
alt: user.display_handle, title: user.display_handle
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ def self.notifiable_owners
where(ownerships: { notifier: true })
end

def self.without_mfa
where(mfa_level: "disabled")
end

def name
handle || email
end
Expand Down
15 changes: 15 additions & 0 deletions app/views/rubygems/_gem_members.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@
<div class="gem__users">
<%= links_to_owners(rubygem) %>
</div>

<% if rubygem.owned_by?(current_user) %>
<% if rubygem.owners.without_mfa.present? %>
<span class="gem__users__mfa-text mfa-warn"><%= t '.not_using_mfa_warning_show' %></span>

<div class="gem__users__mfa-disabled t-item--hidden">
<span class="gem__users__mfa-text mfa-warn t-item--hidden"><%= t '.not_using_mfa_warning_hide' %></span>
<div class="gem__users">
<%= links_to_owners_without_mfa(rubygem) %>
</div>
</div>
<% else %>
<span class="gem__users__mfa-text mfa-info"><%= t '.using_mfa_info' %></span>
<% end %>
<% end %>
lmansur marked this conversation as resolved.
Show resolved Hide resolved
<% end %>

<% if latest_version.pusher.present? %>
Expand Down
3 changes: 3 additions & 0 deletions config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ de:
header: "%{title} Abhängigkeiten"
gem_members:
authors_header: Autoren
not_using_mfa_warning_show:
not_using_mfa_warning_hide:
owners_header: Besitzer
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: SHA 256-Prüfsumme
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,11 @@ en:
header: "%{title} Dependencies"
gem_members:
authors_header: Authors
not_using_mfa_warning_show: "* Some owners are not using MFA. Click for the complete list."
not_using_mfa_warning_hide: "* The following owners are not using MFA. Click to hide."
owners_header: Owners
pushed_by: Pushed by
using_mfa_info: "* All owners are using MFA."
yanked_by: Yanked by
sha_256_checksum: SHA 256 checksum
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,11 @@ es:
header: "dependencias de %{title}"
gem_members:
authors_header: Autores
not_using_mfa_warning_show: "* Algunos propietarios no están usando MFA. Haga click para ver la lista completa."
not_using_mfa_warning_hide: "* Los siguientes propietarios no están usando MFA. Haga click para ocultar."
owners_header: Propietarios
pushed_by: Subida por
using_mfa_info: "* Todos los propietarios están usando MFA."
yanked_by: Borrada por
sha_256_checksum: SHA 256 checksum
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ fr:
header: Dépendances de %{title}
gem_members:
authors_header: Auteurs
not_using_mfa_warning_show:
bronzdoc marked this conversation as resolved.
Show resolved Hide resolved
not_using_mfa_warning_hide:
owners_header: Propriétaires
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: Total de contrôle SHA 256
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/ja.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ ja:
header: "%{title}依存性"
gem_members:
authors_header: 作者
not_using_mfa_warning_show:
not_using_mfa_warning_hide:
owners_header: オーナー
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: SHA 256チェックサム
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ nl:
gem_members:
authors_header:
owners_header: Eigenaren
not_using_mfa_warning_show:
not_using_mfa_warning_hide:
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: SHA 256 checksum
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/pt-BR.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ pt-BR:
header:
gem_members:
authors_header: Autores
not_using_mfa_warning_show: "* Alguns donos não estão usando MFA. Clique para a lista completa."
not_using_mfa_warning_hide: "* Os seguintes donos não estão usando MFA. Clique para esconder."
owners_header: Donos
pushed_by:
using_mfa_info: "* Todos os donos estão usando MFA."
yanked_by:
sha_256_checksum: SHA 256 checksum
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/zh-CN.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ zh-CN:
header: "%{title} 依赖关系"
gem_members:
authors_header: 作者
not_using_mfa_warning_show:
not_using_mfa_warning_hide:
owners_header: 所有者
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: SHA 256 checksum
index:
Expand Down
3 changes: 3 additions & 0 deletions config/locales/zh-TW.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,11 @@ zh-TW:
header: "%{title} 相依性套件"
gem_members:
authors_header: 作者
not_using_mfa_warning_show:
not_using_mfa_warning_hide:
owners_header: 擁有者
pushed_by:
using_mfa_info:
yanked_by:
sha_256_checksum: SHA 256 checksum
index:
Expand Down
37 changes: 37 additions & 0 deletions test/integration/gems_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,41 @@ class GemsSystemTest < SystemTest
assert page.has_content? "Subscribe"
assert_empty @user.subscribed_gems
end

test "shows owners without mfa when logged in as owner" do
@user.enable_mfa!("some-seed", "ui_and_api")
user_without_mfa = create(:user, mfa_level: "disabled")

@rubygem.owners << [@user, user_without_mfa]

visit rubygem_path(@rubygem, as: @user.id)

assert page.has_selector?(".gem__users__mfa-disabled .gem__users a")
assert page.has_selector?(".gem__users__mfa-text.mfa-warn")
end

test "show mfa enabled when logged in as owner but everyone has mfa enabled" do
@user.enable_mfa!("some-seed", "ui_and_api")
user_with_mfa = create(:user, mfa_level: "ui_only")

@rubygem.owners << [@user, user_with_mfa]

visit rubygem_path(@rubygem, as: @user.id)

assert page.has_no_selector?(".gem__users__mfa-text.mfa-warn")
assert page.has_selector?(".gem__users__mfa-text.mfa-info")
end

test "does not show owners without mfa when not logged in as owner" do
@user.enable_mfa!("some-seed", "ui_and_api")
user_without_mfa = create(:user, mfa_level: "disabled")

@rubygem.owners << [@user, user_without_mfa]

visit rubygem_path(@rubygem)

assert page.has_no_selector?(".gem__users__mfa-disabled .gem__users a")
assert page.has_no_selector?(".gem__users__mfa-text.mfa-warn")
assert page.has_no_selector?(".gem__users__mfa-text.mfa-info")
end
end
2 changes: 1 addition & 1 deletion test/integration/yank_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class YankTest < SystemTest
assert page.has_content?("Yanked by")

css = %(div.gem__users a[alt=#{@user.handle}])
assert page.has_css?(css, count: 2)
assert page.has_css?(css, count: 3)
bronzdoc marked this conversation as resolved.
Show resolved Hide resolved
end

test "yanked gem entirely then someone else pushes a new version" do
Expand Down
15 changes: 15 additions & 0 deletions test/unit/helpers/rubygems_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ class RubygemsHelperTest < ActionView::TestCase
assert_equal expected_links, links_to_owners(@rubygem)
assert links_to_owners(@rubygem).html_safe?
end

should "create links to gem owners without mfa" do
with_mfa = create(:user, mfa_level: "ui_and_api")
without_mfa = create_list(:user, 2, mfa_level: "disabled")
rubygem = create(:rubygem, owners: [*without_mfa, with_mfa])

expected_links = without_mfa.sort_by(&:id).map do |u|
link_to gravatar(48, "gravatar-#{u.id}", u),
profile_path(u.display_id),
alt: u.display_handle,
title: u.display_handle
end.join
assert_equal expected_links, links_to_owners_without_mfa(rubygem)
assert links_to_owners_without_mfa(rubygem).html_safe?
end
end

context "simple_markup" do
Expand Down
14 changes: 14 additions & 0 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,20 @@ def assert_resetting_email_changes(attr_name)
end
end

context ".without_mfa" do
setup do
create(:user, handle: "has_mfa", mfa_level: "ui_and_api")
create(:user, handle: "no_mfa", mfa_level: "disabled")
end

should "return only users without mfa" do
users_without_mfa = User.without_mfa

assert_equal 1, users_without_mfa.size
assert_equal "no_mfa", users_without_mfa.first.handle
end
end

context "rubygems" do
setup do
@user = create(:user)
Expand Down