Merge branch '38133-credential-inventory-for-group-managed-accounts' into 'master'

Resolve "Credentials inventory for group managed accounts"

Closes #38133

See merge request gitlab-org/gitlab!23944

app/finders/keys_finder.rb

@@ -8,16 +8,13 @@ class KeysFinder
     'md5' => 'fingerprint'
   }.freeze
 
-  def initialize(current_user, params)
-    @current_user = current_user
+  def initialize(params)
     @params = params
   end
 
   def execute
-    raise GitLabAccessDeniedError unless current_user.admin?
-
     keys = by_key_type
-    keys = by_user(keys)
+    keys = by_users(keys)
     keys = sort(keys)
 
     by_fingerprint(keys)
@@ -25,7 +22,7 @@ class KeysFinder
 
   private
 
-  attr_reader :current_user, :params
+  attr_reader :params
 
   def by_key_type
     if params[:key_type] == 'ssh'
@@ -39,10 +36,10 @@ class KeysFinder
     keys.order_last_used_at_desc
   end
 
-  def by_user(keys)
-    return keys unless params[:user]
+  def by_users(keys)
+    return keys unless params[:users]
 
-    keys.for_user(params[:user])
+    keys.for_user(params[:users])
   end
 
   def by_fingerprint(keys)

doc/user/group/saml_sso/index.md

@@ -81,6 +81,20 @@ Since use of the group-managed account requires the use of SSO, users of group-m
 - The user will be unable to access the group (their credentials will no longer work on the identity provider when prompted to SSO).
 - Contributions in the group (e.g. issues, merge requests) will remain intact.
 
+##### Credentials inventory for Group-managed accounts **(ULTIMATE)**
+
+> [Introduced in GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/issues/38133)
+
+Owners who manage user accounts in a group can view the following details of personal access tokens and SSH keys:
+
+- Owners
+- Scopes
+- Usage patterns
+
+To access the Credentials inventory of a group, navigate to **{shield}** **Security & Compliance > Credentials** in your group's sidebar.
+
+This feature is similar to the [Credentials inventory for self-managed instances](../../admin_area/credentials_inventory.md).
+
 #### Assertions
 
 When using group-managed accounts, the following user details need to be passed to GitLab as SAML

ee/app/controllers/admin/credentials_controller.rb

 # frozen_string_literal: true
 
 class Admin::CredentialsController < Admin::ApplicationController
+  extend ::Gitlab::Utils::Override
   include CredentialsInventoryActions
 
   helper_method :credentials_inventory_path, :user_detail_path
 
+  before_action :check_license_credentials_inventory_available!, only: [:index]
+
   private
 
+  def check_license_credentials_inventory_available!
+    render_404 unless credentials_inventory_feature_available?
+  end
+
+  override :credentials_inventory_path
   def credentials_inventory_path(args)
     admin_credentials_path(args)
   end
 
+  override :user_detail_path
   def user_detail_path(user)
     admin_user_path(user)
   end
 
-  def user
+  override :users
+  def users
     nil
   end
 end

ee/app/controllers/concerns/credentials_inventory_actions.rb

@@ -4,10 +4,6 @@ module CredentialsInventoryActions
   extend ActiveSupport::Concern
   include CredentialsInventoryHelper
 
-  included do
-    before_action :check_license_credentials_inventory_available!, only: [:index]
-  end
-
   def index
     @credentials = filter_credentials.page(params[:page]).preload_users.without_count # rubocop:disable Gitlab/ModuleWithInstanceVariables
 
@@ -18,17 +14,13 @@ module CredentialsInventoryActions
 
   def filter_credentials
     if show_personal_access_tokens?
-      ::PersonalAccessTokensFinder.new({ user: user, impersonation: false, state: 'active', sort: 'id_desc' }).execute
+      ::PersonalAccessTokensFinder.new({ user: users, impersonation: false, state: 'active', sort: 'id_desc' }).execute
     elsif show_ssh_keys?
-      ::KeysFinder.new(current_user, { user: user, key_type: 'ssh' }).execute
+      ::KeysFinder.new({ users: users, key_type: 'ssh' }).execute
     end
   end
 
-  def check_license_credentials_inventory_available!
-    render_404 unless credentials_inventory_feature_available?
-  end
-
-  def user
+  def users
     raise NotImplementedError, "#{self.class} does not implement #{__method__}"
   end
 end

ee/app/controllers/groups/security/compliance_dashboards_controller.rb

 # frozen_string_literal: true
 class Groups::Security::ComplianceDashboardsController < Groups::ApplicationController
+  include Groups::SecurityFeaturesHelper
+
   layout 'group'
 
   before_action :authorize_compliance_dashboard!
@@ -13,7 +15,6 @@ class Groups::Security::ComplianceDashboardsController < Groups::ApplicationCont
   private
 
   def authorize_compliance_dashboard!
-    render_404 unless group.feature_available?(:group_level_compliance_dashboard) &&
-      can?(current_user, :read_group_compliance_dashboard, group)
+    render_404 unless group_level_compliance_dashboard_available?(group)
   end
 end

ee/app/controllers/groups/security/credentials_controller.rb

+# frozen_string_literal: true
+
+class Groups::Security::CredentialsController < Groups::ApplicationController
+  layout 'group'
+
+  extend ::Gitlab::Utils::Override
+  include CredentialsInventoryActions
+  include Groups::SecurityFeaturesHelper
+
+  helper_method :credentials_inventory_path, :user_detail_path
+
+  before_action :validate_group_level_credentials_inventory_available!, only: [:index]
+
+  private
+
+  def validate_group_level_credentials_inventory_available!
+    render_404 unless group_level_credentials_inventory_available?(group)
+  end
+
+  override :credentials_inventory_path
+  def credentials_inventory_path(args)
+    group_security_credentials_path(args)
+  end
+
+  override :user_detail_path
+  def user_detail_path(user)
+    user_path(user)
+  end
+
+  override :users
+  def users
+    group.managed_users
+  end
+end

ee/app/helpers/groups/security_features_helper.rb

+# frozen_string_literal: true
+
+module Groups::SecurityFeaturesHelper
+  def group_level_security_dashboard_available?(group)
+    group.feature_available?(:security_dashboard)
+  end
+
+  def group_level_compliance_dashboard_available?(group)
+    group.feature_available?(:group_level_compliance_dashboard) &&
+    can?(current_user, :read_group_compliance_dashboard, group)
+  end
+
+  def group_level_credentials_inventory_available?(group)
+    can?(current_user, :read_group_credentials_inventory, group) &&
+    group.feature_available?(:credentials_inventory) &&
+    group.enforced_group_managed_accounts?
+  end
+
+  def primary_group_level_security_feature_path(group)
+    if group_level_security_dashboard_available?(group)
+      group_security_dashboard_path(group)
+    elsif group_level_compliance_dashboard_available?(group)
+      group_security_compliance_dashboard_path(group)
+    elsif group_level_credentials_inventory_available?(group)
+      group_security_credentials_path(group)
+    end
+  end
+end

ee/app/models/ee/group.rb

@@ -45,6 +45,7 @@ module EE
 
       has_one :deletion_schedule, class_name: 'GroupDeletionSchedule'
       delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true
+      delegate :enforced_group_managed_accounts?, to: :saml_provider, allow_nil: true
 
       belongs_to :file_template_project, class_name: "Project"
 

ee/app/policies/ee/group_policy.rb

@@ -135,6 +135,7 @@ module EE
 
       rule { admin | owner }.policy do
         enable :read_group_compliance_dashboard
+        enable :read_group_credentials_inventory
       end
 
       rule { needs_new_sso_session }.policy do

ee/app/views/layouts/nav/ee/_security_link.html.haml

-- compliance_dashboard_available = @group.feature_available?(:group_level_compliance_dashboard)
-- security_dashboard_available = @group.feature_available?(:security_dashboard)
-- if compliance_dashboard_available || security_dashboard_available
-  - main_path = compliance_dashboard_available ? group_security_compliance_dashboard_path(@group) : group_security_dashboard_path(@group)
-  = nav_link(path: %w[groups/security/compliance_dashboards#show groups/security/dashboard#show]) do
+- main_path = primary_group_level_security_feature_path(@group)
+- if main_path.present?
+  = nav_link(path: %w[dashboard#show compliance_dashboards#show credentials#index]) do
     = link_to main_path, data: { qa_selector: 'security_compliance_link' } do
       .nav-icon-container
         = sprite_icon('shield')
       %span.nav-item-name
         = _('Security & Compliance')
     %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_secure_submenu' } }
-      - if security_dashboard_available
-        = nav_link(path: 'groups/security/dashboard#show') do
+      - if group_level_security_dashboard_available?(@group)
+        = nav_link(path: 'dashboard#show') do
           = link_to group_security_dashboard_path(@group), title: _('Security'), data: { qa_selector: 'security_dashboard_link' } do
             %span= _('Security')
 
-      - if compliance_dashboard_available
-        = nav_link(path: 'groups/security/compliance_dashboards#show') do
+      - if group_level_compliance_dashboard_available?(@group)
+        = nav_link(path: 'compliance_dashboards#show') do
           = link_to group_security_compliance_dashboard_path(@group), title: _('Compliance') do
             %span= _('Compliance')
+
+      - if group_level_credentials_inventory_available?(@group)
+        = nav_link(path: 'credentials#index') do
+          = link_to group_security_credentials_path(@group), title: _('Credentials') do
+            %span= _('Credentials')
+
 - elsif show_discover_group_security?(@group)
   = nav_link(path: group_security_discover_path(@group)) do
     = link_to group_security_discover_path(@group) do

ee/changelogs/unreleased/38133-credential-inventory-for-group-managed-accounts.yml

+---
+title: Introduce Credentials Inventory for Groups that enforce Group Managed Accounts
+merge_request: 23944
+author:
+type: added

ee/config/routes/group.rb

@@ -119,6 +119,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       resource :compliance_dashboard, only: [:show]
       resources :vulnerable_projects, only: [:index]
       resource :discover, only: [:show], controller: :discover
+      resources :credentials, only: [:index]
 
       resources :vulnerability_findings, only: [:index] do
         collection do

ee/spec/controllers/groups/security/credentials_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Security::CredentialsController do
+  let_it_be(:group_with_managed_accounts) { create(:group_with_managed_accounts, :private) }
+  let_it_be(:managed_users) { create_list(:user, 2, :group_managed, managing_group: group_with_managed_accounts) }
+
+  before do
+    allow_next_instance_of(Gitlab::Auth::GroupSaml::SsoEnforcer) do |sso_enforcer|
+      allow(sso_enforcer).to receive(:active_session?).and_return(true)
+    end
+
+    owner = managed_users.first
+    group_with_managed_accounts.add_owner(owner)
+
+    sign_in(owner)
+  end
+
+  describe 'GET #index' do
+    let(:filter) {}
+    let(:group_id) { group_with_managed_accounts.to_param }
+
+    subject { get :index, params: { group_id: group_id.to_param, filter: filter } }
+
+    context 'when `credentials_inventory` feature is enabled' do
+      before do
+        stub_licensed_features(credentials_inventory: true)
+      end
+
+      context 'for a group that enforces group managed accounts' do
+        context 'for a user with access to view credentials inventory' do
+          it 'responds with 200' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
+
+          context 'filtering by type of credential' do
+            before do
+              managed_users.each do |user|
+                create(:personal_access_token, user: user)
+              end
+            end
+
+            shared_examples_for 'filtering by `personal_access_tokens`' do
+              it do
+                subject
+
+                expect(assigns(:credentials)).to match_array(PersonalAccessToken.where(user: managed_users))
+              end
+            end
+
+            context 'no credential type specified' do
+              let(:filter) { nil }
+
+              it_behaves_like 'filtering by `personal_access_tokens`'
+            end
+
+            context 'non-existent credential type specified' do
+              let(:filter) { 'non_existent_credential_type' }
+
+              it_behaves_like 'filtering by `personal_access_tokens`'
+            end
+
+            context 'credential type specified as `personal_access_tokens`' do
+              let(:filter) { 'personal_access_tokens' }
+
+              it_behaves_like 'filtering by `personal_access_tokens`'
+            end
+
+            context 'user scope' do
+              it 'does not show the credentials of a user outside the group' do
+                personal_access_token = create(:personal_access_token, user: create(:user))
+
+                subject
+
+                expect(assigns(:credentials)).not_to include(personal_access_token)
+              end
+            end
+
+            context 'credential type specified as `ssh_keys`' do
+              let(:filter) { 'ssh_keys' }
+
+              before do
+                managed_users.each do |user|
+                  create(:personal_key, user: user)
+                end
+              end
+
+              it 'filters by ssh keys' do
+                subject
+
+                expect(assigns(:credentials)).to match_array(Key.regular_keys.where(user: managed_users))
+              end
+            end
+          end
+
+          context 'for a user without access to view credentials inventory' do
+            before do
+              maintainer = managed_users.last
+
+              group_with_managed_accounts.add_maintainer(maintainer)
+
+              sign_in(maintainer)
+            end
+
+            it 'responds with 404' do
+              subject
+
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
+          end
+        end
+      end
+
+      context 'for a group that does not enforce group managed accounts' do
+        let(:group_id) { create(:group).to_param }
+
+        it 'responds with 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    context 'when `credentials_inventory` feature is disabled' do
+      before do
+        stub_licensed_features(credentials_inventory: false)
+      end
+
+      it 'returns 404' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+end

ee/spec/factories/groups.rb

@@ -52,4 +52,12 @@ FactoryBot.define do
       )
     end
   end
+
+  factory :group_with_managed_accounts, parent: :group do
+    after(:create) do |group, evaluator|
+      create(:saml_provider,
+        :enforced_group_managed_accounts,
+        group: group)
+    end
+  end
 end

ee/spec/factories/users.rb

@@ -7,7 +7,14 @@ FactoryBot.modify do
     end
 
     trait :group_managed do
-      association :managing_group, factory: :group
+      association :managing_group, factory: :group_with_managed_accounts
+
+      after(:create) do |user, evaluator|
+        create(:group_saml_identity,
+          user: user,
+          saml_provider: user.managing_group.saml_provider
+        )
+      end
     end
   end
 

ee/spec/features/groups/groups_security_credentials_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Groups::Security::Credentials' do
+  include Spec::Support::Helpers::Features::ResponsiveTableHelpers
+
+  let_it_be(:group_with_managed_accounts) { create(:group_with_managed_accounts, :private) }
+  let_it_be(:managed_user) { create(:user, :group_managed, managing_group: group_with_managed_accounts, name: 'David') }
+  let(:group_id) { group_with_managed_accounts.to_param }
+
+  before do
+    allow_next_instance_of(Gitlab::Auth::GroupSaml::SsoEnforcer) do |sso_enforcer|
+      allow(sso_enforcer).to receive(:active_session?).and_return(true)
+    end
+
+    group_with_managed_accounts.add_owner(managed_user)
+    sign_in(managed_user)
+  end
+
+  context 'licensed' do
+    before do
+      stub_licensed_features(credentials_inventory: true)
+    end
+
+    context 'links' do
+      before do
+        visit group_security_credentials_path(group_id: group_id)
+      end
+
+      it 'has Credentials Inventory link in sidebar' do
+        expect(page).to have_link('Credentials', href: group_security_credentials_path(group_id: group_id))
+      end
+
+      context 'tabs' do
+        it 'contains the relevant filter tabs' do
+          expect(page).to have_link('Personal Access Tokens', href: group_security_credentials_path(group_id: group_id, filter: 'personal_access_tokens'))
+          expect(page).to have_link('SSH Keys', href: group_security_credentials_path(group_id: group_id, filter: 'ssh_keys'))
+        end
+      end
+    end
+
+    context 'filtering' do
+      context 'by Personal Access Tokens' do
+        before do
+          create(:personal_access_token,
+            user: managed_user,
+            created_at: '2019-12-10',
+            expires_at: nil)
+
+          visit group_security_credentials_path(group_id: group_id, filter: 'personal_access_tokens')
+        end
+
+        it 'shows details of personal access tokens' do
+          expect(first_row.text).to include('David')
+          expect(first_row.text).to include('api')
+          expect(first_row.text).to include('2019-12-10')
+          expect(first_row.text).to include('Never')
+        end
+      end
+
+      context 'by SSH Keys' do
+        before do
+          create(:personal_key,
+            user: managed_user,
+            created_at: '2019-12-09',
+            last_used_at: '2019-12-10')
+
+          visit group_security_credentials_path(group_id: group_id, filter: 'ssh_keys')
+        end
+
+        it 'shows details of ssh keys' do
+          expect(first_row.text).to include('David')
+          expect(first_row.text).to include('2019-12-09')
+          expect(first_row.text).to include('2019-12-10')
+        end
+      end
+    end
+  end
+
+  context 'unlicensed' do
+    before do
+      stub_licensed_features(credentials_inventory: false)
+    end
+
+    it 'returns 400' do
+      visit group_security_credentials_path(group_id: group_id)
+
+      expect(page.status_code).to eq(404)
+    end
+  end
+end

ee/spec/helpers/groups/security_features_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::SecurityFeaturesHelper do
+  using RSpec::Parameterized::TableSyntax
+
+  let_it_be(:group, refind: true) { create(:group) }
+  let_it_be(:user, refind: true) { create(:user) }
+
+  before do
+    allow(helper).to receive(:current_user).and_return(user)
+  end
+
+  describe '#group_level_security_dashboard_available?' do
+    where(:security_dashboard_feature_enabled, :result) do
+      true  | true
+      false | false
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(security_dashboard: security_dashboard_feature_enabled)
+      end
+
+      it 'returns the expected result' do
+        expect(helper.group_level_security_dashboard_available?(group)).to eq(result)
+      end
+    end
+  end
+
+  describe '#group_level_security_dashboard_available?' do
+    where(:group_level_compliance_dashboard_enabled, :read_group_compliance_dashboard_permission, :result) do
+      false | false | false
+      true  | false | false
+      false | true  | false
+      true  | true  | true
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(group_level_compliance_dashboard: group_level_compliance_dashboard_enabled)
+        allow(helper).to receive(:can?).with(user, :read_group_compliance_dashboard, group).and_return(read_group_compliance_dashboard_permission)
+      end
+
+      it 'returns the expected result' do
+        expect(helper.group_level_compliance_dashboard_available?(group)).to eq(result)
+      end
+    end
+  end
+
+  describe '#group_level_credentials_inventory_available?' do
+    where(:credentials_inventory_feature_enabled, :enforced_group_managed_accounts, :read_group_credentials_inventory_permission, :result) do
+      true  | false | false | false
+      true  | true  | false | false
+      true  | false | true  | false
+      true  | true  | true  | true
+      false | false | false | false
+      false | false | false | false
+      false | false | true  | false
+      false | true  | true  | false
+    end
+
+    with_them do
+      before do
+        stub_licensed_features(credentials_inventory: credentials_inventory_feature_enabled)
+        allow(group).to receive(:enforced_group_managed_accounts?).and_return(enforced_group_managed_accounts)
+        allow(helper).to receive(:can?).with(user, :read_group_credentials_inventory, group).and_return(read_group_credentials_inventory_permission)
+      end
+
+      it 'returns the expected result' do
+        expect(helper.group_level_credentials_inventory_available?(group)).to eq(result)
+      end
+    end
+  end
+
+  describe '#primary_group_level_security_feature_path' do
+    subject { helper.primary_group_level_security_feature_path(group) }
+
+    context 'group_level_security_dashboard is available' do
+      before do
+        allow(helper).to receive(:group_level_security_dashboard_available?).with(group).and_return(true)
+      end
+
+      it 'returns path to security dashboard' do
+        expect(subject).to eq(group_security_dashboard_path(group))
+      end
+    end
+
+    context 'group_level_compliance_dashboard is available' do
+      before do
+        allow(helper).to receive(:group_level_compliance_dashboard_available?).with(group).and_return(true)
+      end
+
+      it 'returns path to compliance dashboard' do
+        expect(subject).to eq(group_security_compliance_dashboard_path(group))
+      end
+    end
+
+    context 'group_level_credentials_inventory is available' do
+      before do
+        allow(helper).to receive(:group_level_credentials_inventory_available?).with(group).and_return(true)
+      end
+
+      it 'returns path to credentials inventory dashboard' do
+        expect(subject).to eq(group_security_credentials_path(group))
+      end
+    end
+
+    context 'when no security features are available' do
+      before do
+        allow(helper).to receive(:group_level_security_dashboard_available?).with(group).and_return(false)
+        allow(helper).to receive(:group_level_compliance_dashboard_available?).with(group).and_return(false)
+        allow(helper).to receive(:group_level_credentials_inventory_available?).with(group).and_return(false)
+      end
+
+      it 'returns nil' do
+        expect(subject).to be_nil
+      end
+    end
+  end
+end

ee/spec/policies/group_policy_spec.rb

@@ -371,6 +371,64 @@ describe GroupPolicy do
     end
   end
 
+  describe 'read_group_credentials_inventory' do
+    context 'with admin' do
+      let(:current_user) { admin }
+
+      it { is_expected.to be_allowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with owner' do
+      let(:current_user) { owner }
+
+      it { is_expected.to be_allowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with developer' do
+      let(:current_user) { developer }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+
+      context 'when security dashboard features is not available' do
+        before do
+          stub_licensed_features(security_dashboard: false)
+        end
+
+        it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+      end
+    end
+
+    context 'with reporter' do
+      let(:current_user) { reporter }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with guest' do
+      let(:current_user) { guest }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with non member' do
+      let(:current_user) { create(:user) }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+    end
+
+    context 'with anonymous' do
+      let(:current_user) { nil }
+
+      it { is_expected.to be_disallowed(:read_group_credentials_inventory) }
+    end
+  end
+
   describe 'read_group_security_dashboard' do
     before do
       stub_licensed_features(security_dashboard: true)

ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -8,6 +8,7 @@ describe 'layouts/nav/sidebar/_group' do
   end
 
   let(:group) { create(:group) }
+  let(:user) { create(:user) }
 
   describe 'contribution analytics tab' do
     it 'is not visible when there is no valid license and we dont show promotions' do
@@ -96,6 +97,21 @@ describe 'layouts/nav/sidebar/_group' do
         stub_licensed_features(group_level_compliance_dashboard: true)
       end
 
+      context 'when the user does not have access to Compliance dashboard' do
+        it 'is not visible' do
+          render
+
+          expect(rendered).not_to have_link 'Security & Compliance'
+          expect(rendered).not_to have_link 'Compliance'
+        end
+      end
+
+      context 'when the user has access to Compliance dashboard' do
+        before do
+          group.add_owner(user)
+          allow(view).to receive(:current_user).and_return(user)
+        end
+
         it 'is visible' do
           render
 
@@ -103,6 +119,50 @@ describe 'layouts/nav/sidebar/_group' do
           expect(rendered).to have_link 'Compliance'
         end
       end
+    end
+
+    context 'when credentials inventory feature is enabled' do
+      shared_examples_for 'Credentials tab is not visible' do
+        it 'does not show the `Credentials` tab' do
+          render
+
+          expect(rendered).not_to have_link 'Security & Compliance'
+          expect(rendered).not_to have_link 'Credentials'
+        end
+      end
+
+      before do
+        stub_licensed_features(credentials_inventory: true)
+      end
+
+      context 'when the group does not enforce managed accounts' do
+        it_behaves_like 'Credentials tab is not visible'
+      end
+
+      context 'when the group enforces managed accounts' do
+        before do
+          allow(group).to receive(:enforced_group_managed_accounts?).and_return(true)
+        end
+
+        context 'when the user has privileges to view Credentials' do
+          before do
+            group.add_owner(user)
+            allow(view).to receive(:current_user).and_return(user)
+          end
+
+          it 'is visible' do
+            render
+
+            expect(rendered).to have_link 'Security & Compliance'
+            expect(rendered).to have_link 'Credentials'
+          end
+        end
+
+        context 'when the user does not have privileges to view Credentials' do
+          it_behaves_like 'Credentials tab is not visible'
+        end
+      end
+    end
 
     context 'when security dashboard feature is disabled' do
       let(:group) { create(:group, plan: :bronze_plan) }

lib/api/keys.rb

@@ -26,7 +26,7 @@ module API
       get do
         authenticated_with_can_read_all_resources!
 
-        key = KeysFinder.new(current_user, params).execute
+        key = KeysFinder.new(params).execute
 
         not_found!('Key') unless key
 

spec/finders/keys_finder_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe KeysFinder do
-  subject { described_class.new(user, params).execute }
+  subject { described_class.new(params).execute }
 
   let(:user) { create(:user) }
   let(:params) { {} }
@@ -20,15 +20,6 @@ describe KeysFinder do
   let!(:key_2) { create(:personal_key, last_used_at: nil, user: user) }
   let!(:key_3) { create(:personal_key, last_used_at: 2.days.ago) }
 
-  context 'with a regular user' do
-    it 'raises GitLabAccessDeniedError' do
-      expect { subject }.to raise_error(KeysFinder::GitLabAccessDeniedError)
-    end
-  end
-
-  context 'with an admin user' do
-    let(:user) {create(:admin)}
-
   context 'key_type' do
     let!(:deploy_key) { create(:deploy_key) }
 
@@ -160,7 +151,7 @@ describe KeysFinder do
 
     context 'with user' do
       before do
-          params[:user] = user
+        params[:users] = user
       end
 
       it 'contains ssh_keys of only the specified users' do
@@ -174,5 +165,4 @@ describe KeysFinder do
       expect(subject).to eq([key_3, key_1, key_2])
     end
   end
-  end
 end