diff options
Diffstat (limited to 'app')
30 files changed, 411 insertions, 206 deletions
diff --git a/app/assets/javascripts/layout_nav.js b/app/assets/javascripts/layout_nav.js index 2cb8c37f192..1f58065a505 100644 --- a/app/assets/javascripts/layout_nav.js +++ b/app/assets/javascripts/layout_nav.js @@ -1,5 +1,4 @@ import $ from 'jquery'; -import { setNotification } from './whats_new/utils/notification'; function hideEndFade($scrollingTabs) { $scrollingTabs.each(function scrollTabsLoop() { @@ -86,27 +85,8 @@ function initInviteMembers() { .catch(() => {}); } -function initWhatsNewComponent() { - const appEl = document.getElementById('whats-new-app'); - if (!appEl) return; - - setNotification(appEl); - - const triggerEl = document.querySelector('.js-whats-new-trigger'); - if (!triggerEl) return; - - triggerEl.addEventListener('click', () => { - import(/* webpackChunkName: 'whatsNewApp' */ '~/whats_new') - .then(({ default: initWhatsNew }) => { - initWhatsNew(appEl); - }) - .catch(() => {}); - }); -} - function initDeferred() { initScrollingTabs(); - initWhatsNewComponent(); initInviteMembers(); } diff --git a/app/assets/javascripts/members/components/action_buttons/remove_group_link_button.vue b/app/assets/javascripts/members/components/action_buttons/remove_group_link_button.vue index 2f10a333bf4..c76b928ad3d 100644 --- a/app/assets/javascripts/members/components/action_buttons/remove_group_link_button.vue +++ b/app/assets/javascripts/members/components/action_buttons/remove_group_link_button.vue @@ -36,7 +36,7 @@ export default { :title="$options.i18n.buttonTitle" :aria-label="$options.i18n.buttonTitle" icon="remove" - data-qa-selector="remove_group_link_button" + data-testid="remove-group-link-button" @click="showRemoveGroupLinkModal(groupLink)" /> </template> diff --git a/app/assets/javascripts/members/components/modals/remove_group_link_modal.vue b/app/assets/javascripts/members/components/modals/remove_group_link_modal.vue index 18db8fe9cfb..55f75bc819c 100644 --- a/app/assets/javascripts/members/components/modals/remove_group_link_modal.vue +++ b/app/assets/javascripts/members/components/modals/remove_group_link_modal.vue @@ -15,7 +15,7 @@ export default { text: s__('Members|Remove group'), attributes: { variant: 'danger', - 'data-qa-selector': 'remove_group_button', + 'data-testid': 'remove-group-button', }, }, csrf, @@ -69,7 +69,7 @@ export default { :action-primary="$options.actionPrimary" :action-cancel="$options.actionCancel" size="sm" - data-qa-selector="remove_group_link_modal_content" + data-testid="remove-group-link-modal-content" @primary="handlePrimary" @hide="hideRemoveGroupLinkModal" > diff --git a/app/assets/javascripts/members/components/modals/remove_member_modal.vue b/app/assets/javascripts/members/components/modals/remove_member_modal.vue index ecc769174f4..d3079dc7d0a 100644 --- a/app/assets/javascripts/members/components/modals/remove_member_modal.vue +++ b/app/assets/javascripts/members/components/modals/remove_member_modal.vue @@ -72,7 +72,7 @@ export default { text: this.actionText, attributes: { variant: 'danger', - 'data-qa-selector': 'remove_member_button', + 'data-testid': 'remove-member-button', }, }; }, @@ -104,7 +104,7 @@ export default { :action-primary="actionPrimary" :title="actionText" :visible="removeMemberModalVisible" - data-qa-selector="remove_member_modal" + data-testid="remove-member-modal" @primary="submitForm" @hide="hideRemoveMemberModal" > diff --git a/app/assets/javascripts/members/components/table/max_role.vue b/app/assets/javascripts/members/components/table/max_role.vue index 504e68f4f09..89780108518 100644 --- a/app/assets/javascripts/members/components/table/max_role.vue +++ b/app/assets/javascripts/members/components/table/max_role.vue @@ -107,13 +107,13 @@ export default { :header-text="__('Change role')" :disabled="disabled" :loading="busy" - data-qa-selector="access_level_dropdown" + data-testid="access-level-dropdown" :items="accessLevelOptions.formatted" :selected="selectedRole" @select="handleSelect" > <template #list-item="{ item }"> - <span data-qa-selector="access_level_link">{{ item.text }}</span> + <span data-testid="access-level-link">{{ item.text }}</span> </template> <template #footer> <ldap-dropdown-footer diff --git a/app/assets/javascripts/members/components/table/members_table.vue b/app/assets/javascripts/members/components/table/members_table.vue index 149947397ad..1bccb8a0c4b 100644 --- a/app/assets/javascripts/members/components/table/members_table.vue +++ b/app/assets/javascripts/members/components/table/members_table.vue @@ -143,7 +143,6 @@ export default { ...this.tableAttrs.tr, ...(member?.id && { 'data-testid': `members-table-row-${member.id}`, - 'data-qa-selector': 'member_row', }), }; }, diff --git a/app/assets/javascripts/super_sidebar/components/help_center.vue b/app/assets/javascripts/super_sidebar/components/help_center.vue index e354d82b76a..4e0d05add85 100644 --- a/app/assets/javascripts/super_sidebar/components/help_center.vue +++ b/app/assets/javascripts/super_sidebar/components/help_center.vue @@ -47,6 +47,7 @@ export default { return { showWhatsNewNotification: this.shouldShowWhatsNewNotification(), helpCenterState, + toggleWhatsNewDrawer: null, }; }, computed: { @@ -177,12 +178,11 @@ export default { this.showWhatsNewNotification = false; if (!this.toggleWhatsNewDrawer) { - const appEl = document.getElementById('whats-new-app'); const { default: toggleWhatsNewDrawer } = await import( /* webpackChunkName: 'whatsNewApp' */ '~/whats_new' ); this.toggleWhatsNewDrawer = toggleWhatsNewDrawer; - this.toggleWhatsNewDrawer(appEl); + this.toggleWhatsNewDrawer(this.sidebarData.whats_new_version_digest); } else { this.toggleWhatsNewDrawer(); } diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue index d153c17ea1e..6a1f5f0bb44 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue @@ -1,7 +1,7 @@ <script> -import { GlButton, GlSprintf } from '@gitlab/ui'; +import { GlForm, GlButton, GlSprintf } from '@gitlab/ui'; import { createAlert } from '~/alert'; -import { visitUrl } from '~/lib/utils/url_utility'; +import csrf from '~/lib/utils/csrf'; import { STATUS_MERGED } from '~/issues/constants'; import { BV_SHOW_MODAL } from '~/lib/utils/constants'; import { HTTP_STATUS_UNAUTHORIZED } from '~/lib/utils/http_status'; @@ -23,7 +23,9 @@ export default { StateContainer, GlButton, GlSprintf, + GlForm, }, + csrf, mixins: [approvalsMixin, glFeatureFlagsMixin()], props: { mr: { @@ -169,16 +171,15 @@ export default { .join(', ') .concat('.'); }, + samlApprovalPath() { + return this.mr.samlApprovalPath; + }, requireSamlAuthToApprove() { return this.mr.requireSamlAuthToApprove; }, }, methods: { approve() { - if (this.requireSamlAuthToApprove) { - this.approveWithSamlAuth(); - return; - } if (this.requirePasswordToApprove) { this.$root.$emit(BV_SHOW_MODAL, this.modalId); return; @@ -195,7 +196,7 @@ export default { }, approveWithSamlAuth() { // Intentionally direct to SAML Identity Provider for renewed authorization even if SSO session exists - visitUrl(this.mr.samlApprovalPath); + this.$refs.form.$el.submit(); }, approveWithAuth(data) { this.updateApproval( @@ -270,17 +271,40 @@ export default { <template v-else> <div class="gl-display-flex gl-flex-direction-column"> <div class="gl-display-flex gl-flex-direction-row gl-align-items-center"> - <gl-button - v-if="action" - :variant="action.variant" - :category="action.category" - :loading="isApproving" - class="gl-mr-3" - data-testid="approve-button" - @click="action.action" - > - {{ action.text }} - </gl-button> + <div v-if="requireSamlAuthToApprove && showApprove"> + <gl-form + ref="form" + :action="samlApprovalPath" + method="post" + data-testid="approve-form" + > + <gl-button + v-if="action" + :variant="action.variant" + :category="action.category" + :loading="isApproving" + class="gl-mr-3" + data-testid="approve-button" + type="submit" + > + {{ action.text }} + </gl-button> + <input :value="$options.csrf.token" type="hidden" name="authenticity_token" /> + </gl-form> + </div> + <span v-if="!requireSamlAuthToApprove || showUnapprove"> + <gl-button + v-if="action" + :variant="action.variant" + :category="action.category" + :loading="isApproving" + class="gl-mr-3" + data-testid="approve-button" + @click="action.action" + > + {{ action.text }} + </gl-button> + </span> <approvals-summary-optional v-if="isOptional" :can-approve="hasAction" diff --git a/app/assets/javascripts/whats_new/index.js b/app/assets/javascripts/whats_new/index.js index bc9e2d5c3b1..57db8cde110 100644 --- a/app/assets/javascripts/whats_new/index.js +++ b/app/assets/javascripts/whats_new/index.js @@ -1,34 +1,22 @@ import Vue from 'vue'; -// eslint-disable-next-line no-restricted-imports -import { mapState } from 'vuex'; -import App from './components/app.vue'; +import WhatsNewApp from './components/app.vue'; import store from './store'; -import { getVersionDigest, setNotification } from './utils/notification'; let whatsNewApp; -export default (el) => { +export default (versionDigest) => { if (whatsNewApp) { store.dispatch('openDrawer'); } else { + const el = document.createElement('div'); + document.body.append(el); whatsNewApp = new Vue({ el, store, - components: { - App, - }, - computed: { - ...mapState(['open']), - }, - watch: { - open() { - setNotification(el); - }, - }, render(createElement) { - return createElement('app', { + return createElement(WhatsNewApp, { props: { - versionDigest: getVersionDigest(el), + versionDigest, }, }); }, diff --git a/app/assets/javascripts/whats_new/utils/notification.js b/app/assets/javascripts/whats_new/utils/notification.js index 1621c4d5f27..fb6ce7454dc 100644 --- a/app/assets/javascripts/whats_new/utils/notification.js +++ b/app/assets/javascripts/whats_new/utils/notification.js @@ -1,21 +1 @@ export const STORAGE_KEY = 'display-whats-new-notification'; - -export const getVersionDigest = (appEl) => appEl.dataset.versionDigest; - -export const setNotification = (appEl) => { - const versionDigest = getVersionDigest(appEl); - const notificationEl = document.querySelector('.header-help'); - if (!notificationEl) return; - - let notificationCountEl = notificationEl.querySelector('.js-whats-new-notification-count'); - - if (localStorage.getItem(STORAGE_KEY) === versionDigest || versionDigest === undefined) { - notificationEl.classList.remove('with-notifications'); - if (notificationCountEl) { - notificationCountEl.parentElement.removeChild(notificationCountEl); - notificationCountEl = null; - } - } else { - notificationEl.classList.add('with-notifications'); - } -}; diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index a97516fddff..907ece1a06e 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -8,6 +8,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController include KnownSignIn include AcceptsPendingInvitations include Onboarding::Redirectable + include InternalRedirect + + ACTIVE_SINCE_KEY = 'active_since' after_action :verify_known_sign_in @@ -113,14 +116,21 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController super end + def store_redirect_to + # overridden in EE + end + def omniauth_flow(auth_module, identity_linker: nil) if fragment = request.env.dig('omniauth.params', 'redirect_fragment').presence store_redirect_fragment(fragment) end + store_redirect_to + if current_user return render_403 unless link_provider_allowed?(oauth['provider']) + set_session_active_since(oauth['provider']) if ::AuthHelper.saml_providers.include?(oauth['provider'].to_sym) track_event(current_user, oauth['provider'], 'succeeded') if Gitlab::CurrentSettings.admin_mode @@ -167,6 +177,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end end + # Overrided in EE + def set_session_active_since(id); end + def sign_in_user_flow(auth_user_class) auth_user = build_auth_user(auth_user_class) new_user = auth_user.new? @@ -181,6 +194,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController Gitlab::Tracking.event(self.class.name, "#{oauth['provider']}_sso", user: @user) if new_user set_remember_me(@user) + set_session_active_since(oauth['provider']) if ::AuthHelper.saml_providers.include?(oauth['provider'].to_sym) if @user.two_factor_enabled? && !auth_user.bypass_two_factor? prompt_for_two_factor(@user) diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index 5f8bf423219..855b9824cf2 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -3,7 +3,7 @@ class Projects::GroupLinksController < Projects::ApplicationController layout 'project_settings' before_action :authorize_admin_project!, except: [:destroy] - before_action :authorize_admin_project_group_link!, only: [:destroy] + before_action :authorize_manage_destroy!, only: [:destroy] before_action :authorize_admin_project_member!, only: [:update] feature_category :groups_and_projects @@ -20,8 +20,8 @@ class Projects::GroupLinksController < Projects::ApplicationController else render json: {} end - elsif result.reason == :not_found - render json: { message: result.message }, status: :not_found + else + render json: { message: result.message }, status: result.reason end end @@ -47,7 +47,7 @@ class Projects::GroupLinksController < Projects::ApplicationController end format.js do - render json: { message: result.message }, status: :not_found if result.reason == :not_found + render json: { message: result.message }, status: result.reason end end end @@ -55,8 +55,8 @@ class Projects::GroupLinksController < Projects::ApplicationController protected - def authorize_admin_project_group_link! - render_404 unless can?(current_user, :admin_project_group_link, group_link) + def authorize_manage_destroy! + render_404 unless can?(current_user, :manage_destroy, group_link) end def group_link diff --git a/app/finders/timelogs/timelogs_finder.rb b/app/finders/timelogs/timelogs_finder.rb new file mode 100644 index 00000000000..610dba43317 --- /dev/null +++ b/app/finders/timelogs/timelogs_finder.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +module Timelogs + class TimelogsFinder + attr_reader :issuable, :params + + def initialize(issuable, params = {}) + @issuable = issuable + @params = params + end + + def execute + timelogs = issuable&.timelogs || Timelog.all + timelogs = by_time(timelogs) + timelogs = by_user(timelogs) + timelogs = by_group(timelogs) + timelogs = by_project(timelogs) + apply_sorting(timelogs) + end + + private + + def by_time(timelogs) + return timelogs unless params[:start_time] || params[:end_time] + + validate_time_difference! + + timelogs = timelogs.at_or_after(params[:start_time]) if params[:start_time] + timelogs = timelogs.at_or_before(params[:end_time]) if params[:end_time] + + timelogs + end + + def by_user(timelogs) + return timelogs unless params[:username] + + user = User.find_by_username(params[:username]) + timelogs.for_user(user) + end + + def by_group(timelogs) + return timelogs unless params[:group_id] + + group = Group.find_by_id(params[:group_id]) + raise(ActiveRecord::RecordNotFound, "Group with id '#{params[:group_id]}' could not be found") unless group + + timelogs.in_group(group) + end + + def by_project(timelogs) + return timelogs unless params[:project_id] + + timelogs.in_project(params[:project_id]) + end + + def apply_sorting(timelogs) + return timelogs unless params[:sort] + + timelogs.sort_by_field(params[:sort]) + end + + def validate_time_difference! + return unless end_time_before_start_time? + + raise ArgumentError, 'Start argument must be before End argument' + end + + def end_time_before_start_time? + times_provided? && params[:end_time] < params[:start_time] + end + + def times_provided? + params[:start_time] && params[:end_time] + end + end +end diff --git a/app/graphql/mutations/container_registry/protection/rule/update.rb b/app/graphql/mutations/container_registry/protection/rule/update.rb new file mode 100644 index 00000000000..b4464e5b5f4 --- /dev/null +++ b/app/graphql/mutations/container_registry/protection/rule/update.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Mutations + module ContainerRegistry + module Protection + module Rule + class Update < ::Mutations::BaseMutation + graphql_name 'UpdateContainerRegistryProtectionRule' + description 'Updates a container registry protection rule to restrict access to project containers. ' \ + 'You can prevent users without certain roles from altering containers. ' \ + 'Available only when feature flag `container_registry_protected_containers` is enabled.' + + authorize :admin_container_image + + argument :id, + ::Types::GlobalIDType[::ContainerRegistry::Protection::Rule], + required: true, + description: 'Global ID of the container registry protection rule to be updated.' + + argument :repository_path_pattern, + GraphQL::Types::String, + required: false, + validates: { allow_blank: false }, + description: + 'Container\'s repository path pattern of the protection rule. ' \ + 'For example, `my-scope/my-project/container-dev-*`. ' \ + 'Wildcard character `*` allowed.' + + argument :delete_protected_up_to_access_level, + Types::ContainerRegistry::Protection::RuleAccessLevelEnum, + required: false, + validates: { allow_blank: false }, + description: + 'Maximum GitLab access level prevented from deleting a container. ' \ + 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' + + argument :push_protected_up_to_access_level, + Types::ContainerRegistry::Protection::RuleAccessLevelEnum, + required: false, + validates: { allow_blank: false }, + description: + 'Maximum GitLab access level prevented from pushing a container. ' \ + 'For example, `DEVELOPER`, `MAINTAINER`, `OWNER`.' + + field :container_registry_protection_rule, + Types::ContainerRegistry::Protection::RuleType, + null: true, + description: 'Container registry protection rule after mutation.' + + def resolve(id:, **kwargs) + container_registry_protection_rule = authorized_find!(id: id) + + if Feature.disabled?(:container_registry_protected_containers, container_registry_protection_rule.project) + raise_resource_not_available_error!("'container_registry_protected_containers' feature flag is disabled") + end + + response = ::ContainerRegistry::Protection::UpdateRuleService.new(container_registry_protection_rule, + current_user: current_user, params: kwargs).execute + + { container_registry_protection_rule: response.payload[:container_registry_protection_rule], + errors: response.errors } + end + end + end + end + end +end diff --git a/app/graphql/resolvers/timelog_resolver.rb b/app/graphql/resolvers/timelog_resolver.rb index 4f52db6801d..c2be582742b 100644 --- a/app/graphql/resolvers/timelog_resolver.rb +++ b/app/graphql/resolvers/timelog_resolver.rb @@ -3,6 +3,7 @@ module Resolvers class TimelogResolver < BaseResolver include LooksAhead + include Gitlab::Graphql::Authorize::AuthorizeResource type ::Types::TimelogType.connection_type, null: false @@ -42,21 +43,30 @@ module Resolvers def resolve_with_lookahead(**args) validate_args!(object, args) - timelogs = object&.timelogs || Timelog.all - args = parse_datetime_args(args) - timelogs = apply_user_filter(timelogs, args) - timelogs = apply_project_filter(timelogs, args) - timelogs = apply_time_filter(timelogs, args) - timelogs = apply_group_filter(timelogs, args) - timelogs = apply_sorting(timelogs, args) + timelogs = Timelogs::TimelogsFinder.new(object, finder_params(args)).execute apply_lookahead(timelogs) + rescue ArgumentError => e + raise_argument_error(e.message) + rescue ActiveRecord::RecordNotFound + raise_resource_not_available_error! end private + def finder_params(args) + { + username: args[:username], + start_time: args[:start_time], + end_time: args[:end_time], + group_id: args[:group_id]&.model_id, + project_id: args[:project_id]&.model_id, + sort: args[:sort] + } + end + def preloads { note: [:note] @@ -95,58 +105,6 @@ module Resolvers args[:start_time] && args[:end_time] end - def validate_time_difference!(args) - return unless end_time_before_start_time?(args) - - raise_argument_error('Start argument must be before End argument') - end - - def end_time_before_start_time?(args) - times_provided?(args) && args[:end_time] < args[:start_time] - end - - def apply_project_filter(timelogs, args) - return timelogs unless args[:project_id] - - timelogs.in_project(args[:project_id].model_id) - end - - def apply_group_filter(timelogs, args) - return timelogs unless args[:group_id] - - group = Group.find_by_id(args[:group_id].model_id) - timelogs.in_group(group) - end - - def apply_user_filter(timelogs, args) - return timelogs unless args[:username] - - user = UserFinder.new(args[:username]).find_by_username - timelogs.for_user(user) - end - - def apply_time_filter(timelogs, args) - return timelogs unless args[:start_time] || args[:end_time] - - validate_time_difference!(args) - - if args[:start_time] - timelogs = timelogs.at_or_after(args[:start_time]) - end - - if args[:end_time] - timelogs = timelogs.at_or_before(args[:end_time]) - end - - timelogs - end - - def apply_sorting(timelogs, args) - return timelogs unless args[:sort] - - timelogs.sort_by_field(args[:sort]) - end - def raise_argument_error(message) raise Gitlab::Graphql::Errors::ArgumentError, message end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index d6c064f6882..8ec41af0be4 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -140,6 +140,7 @@ module Types mount_mutation Mutations::ContainerExpirationPolicies::Update mount_mutation Mutations::ContainerRegistry::Protection::Rule::Create, alpha: { milestone: '16.6' } mount_mutation Mutations::ContainerRegistry::Protection::Rule::Delete, alpha: { milestone: '16.7' } + mount_mutation Mutations::ContainerRegistry::Protection::Rule::Update, alpha: { milestone: '16.7' } mount_mutation Mutations::ContainerRepositories::Destroy mount_mutation Mutations::ContainerRepositories::DestroyTags mount_mutation Mutations::Ci::Catalog::Resources::Create, alpha: { milestone: '15.11' } diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 2f042ea6417..e931d95d38a 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -279,7 +279,7 @@ module MergeRequestsHelper target_branch = link_to merge_request.target_branch, project_tree_path(merge_request.target_project, merge_request.target_branch), title: merge_request.target_branch, class: 'ref-container gl-display-inline-block gl-text-truncate gl-max-w-26 gl-mx-2' - _('%{author} requested to merge %{source_branch} %{copy_button} into %{target_branch} %{created_at}').html_safe % { author: link_to_author.html_safe, source_branch: merge_request_source_branch(merge_request).html_safe, copy_button: copy_button.html_safe, target_branch: target_branch.html_safe, created_at: time_ago_with_tooltip(merge_request.created_at, html_class: 'gl-display-inline-block').html_safe } + safe_format('%{author} • %{source_branch} %{copy_button} ➔ %{target_branch} %{created_at}', author: link_to_author, source_branch: merge_request_source_branch(merge_request), copy_button: copy_button, target_branch: target_branch, created_at: time_ago_with_tooltip(merge_request.created_at, html_class: 'gl-display-inline-block')) end def sticky_header_data diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index 69d8c0db55b..de323d98584 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -11,8 +11,7 @@ class ProjectGroupLink < ApplicationRecord validates :project_id, presence: true validates :group, presence: true validates :group_id, uniqueness: { scope: [:project_id], message: N_("already shared with this group") } - validates :group_access, presence: true - validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true + validates :group_access, inclusion: { in: Gitlab::Access.all_values }, presence: true validate :different_group scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) } @@ -22,20 +21,16 @@ class ProjectGroupLink < ApplicationRecord alias_method :shared_with_group, :group alias_method :shared_from, :project - def self.access_options - Gitlab::Access.options - end - - def self.default_access - Gitlab::Access::DEVELOPER - end - def self.search(query) joins(:group).merge(Group.search(query)) end def human_access - self.class.access_options.key(self.group_access) + Gitlab::Access.human_access(self.group_access) + end + + def owner_access? + group_access.to_i == Gitlab::Access::OWNER end private diff --git a/app/policies/project_group_link_policy.rb b/app/policies/project_group_link_policy.rb index 7ad2985ecc5..98d5bdebdc9 100644 --- a/app/policies/project_group_link_policy.rb +++ b/app/policies/project_group_link_policy.rb @@ -1,16 +1,39 @@ # frozen_string_literal: true class ProjectGroupLinkPolicy < BasePolicy # rubocop:disable Gitlab/NamespacedClass + condition(:group_owner) { group_owner? } condition(:group_owner_or_project_admin) { group_owner? || project_admin? } condition(:can_read_group) { can?(:read_group, @subject.group) } condition(:project_member) { @subject.project.member?(@user) } + condition(:can_manage_owners) { can_manage_owners? } + condition(:can_manage_group_link_with_owner_access) do + next true unless @subject.owner_access? - rule { group_owner_or_project_admin }.enable :admin_project_group_link + can_manage_owners? + end + + rule { can_manage_owners }.enable :manage_owners + + rule { can_manage_group_link_with_owner_access }.enable :manage_group_link_with_owner_access + + # `manage_destroy` specifies the very basic permission that a user needs to destroy a link. + rule { group_owner_or_project_admin }.enable :manage_destroy + + # `destroy_project_group_link` combines the basic permission, ie `manage_destroy` AND + # the specific permissions a user needs to destroy a link that has `OWNER` access level. + # link.project's owner, or link.group's owner can delete a link with any access level, including OWNER + rule { can?(:manage_destroy) & (can?(:manage_group_link_with_owner_access) | group_owner) }.policy do + enable :destroy_project_group_link + end rule { can_read_group | project_member }.enable :read_shared_with_group private + def can_manage_owners? + can?(:manage_owners, @subject.project) + end + def group_owner? can?(:admin_group, @subject.group) end diff --git a/app/serializers/group_link/group_group_link_entity.rb b/app/serializers/group_link/group_group_link_entity.rb index f855d89f593..f6989e8afcd 100644 --- a/app/serializers/group_link/group_group_link_entity.rb +++ b/app/serializers/group_link/group_group_link_entity.rb @@ -8,14 +8,22 @@ module GroupLink GroupEntity.represent(group_link.shared_from, only: [:id, :full_name, :web_url]) end - private + expose :valid_roles do |group_link| + group_link.class.access_options + end - def can_admin_group_link?(group_link, options) - can?(current_user, admin_permission_name, group_link.shared_from) + expose :can_update do |group_link, options| + can_admin_group_link?(group_link, options) + end + + expose :can_remove do |group_link, options| + can_admin_group_link?(group_link, options) end - def admin_permission_name - :admin_group_member + private + + def can_admin_group_link?(group_link, options) + direct_member?(group_link, options) && can?(current_user, :admin_group_member, group_link.shared_from) end end end diff --git a/app/serializers/group_link/group_link_entity.rb b/app/serializers/group_link/group_link_entity.rb index 66645e736a9..1b8313c2536 100644 --- a/app/serializers/group_link/group_link_entity.rb +++ b/app/serializers/group_link/group_link_entity.rb @@ -15,10 +15,6 @@ module GroupLink expose :group_access, as: :integer_value end - expose :valid_roles do |group_link| - group_link.class.access_options - end - expose :is_shared_with_group_private do |group_link| !can_read_shared_group?(group_link) end @@ -43,14 +39,6 @@ module GroupLink end end - expose :can_update do |group_link, options| - can_admin_shared_from?(group_link, options) - end - - expose :can_remove do |group_link, options| - direct_member?(group_link, options) && can_admin_group_link?(group_link, options) - end - expose :is_direct_member do |group_link, options| direct_member?(group_link, options) end @@ -68,10 +56,5 @@ module GroupLink def direct_member?(group_link, options) group_link.shared_from == options[:source] end - - def can_admin_shared_from?(group_link, options) - direct_member?(group_link, options) && - can?(current_user, admin_permission_name, group_link.shared_from) - end end end diff --git a/app/serializers/group_link/project_group_link_entity.rb b/app/serializers/group_link/project_group_link_entity.rb index fbad69bf2c5..d763809d123 100644 --- a/app/serializers/group_link/project_group_link_entity.rb +++ b/app/serializers/group_link/project_group_link_entity.rb @@ -8,14 +8,23 @@ module GroupLink ProjectEntity.represent(group_link.shared_from, only: [:id, :full_name]) end - private + expose :valid_roles do |group_link| + if can?(current_user, :manage_owners, group_link) + Gitlab::Access.options_with_owner + else + Gitlab::Access.options + end + end - def can_admin_group_link?(group_link, options) - can?(current_user, :admin_project_group_link, group_link) + expose :can_update do |group_link, options| + direct_member?(group_link, options) && + can?(current_user, :admin_project_member, group_link.project) && + can?(current_user, :manage_group_link_with_owner_access, group_link) end - def admin_permission_name - :admin_project_member + expose :can_remove do |group_link, options| + direct_member?(group_link, options) && + can?(current_user, :destroy_project_group_link, group_link) end end end diff --git a/app/services/container_registry/protection/update_rule_service.rb b/app/services/container_registry/protection/update_rule_service.rb new file mode 100644 index 00000000000..af74e542ac7 --- /dev/null +++ b/app/services/container_registry/protection/update_rule_service.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module ContainerRegistry + module Protection + class UpdateRuleService + include Gitlab::Allowable + + ALLOWED_ATTRIBUTES = %i[ + repository_path_pattern + delete_protected_up_to_access_level + push_protected_up_to_access_level + ].freeze + + def initialize(container_registry_protection_rule, current_user:, params:) + if container_registry_protection_rule.blank? || current_user.blank? + raise ArgumentError, + 'container_registry_protection_rule and current_user must be set' + end + + @container_registry_protection_rule = container_registry_protection_rule + @current_user = current_user + @params = params || {} + end + + def execute + unless can?(current_user, :admin_container_image, container_registry_protection_rule.project) + error_message = _('Unauthorized to update a container registry protection rule') + return service_response_error(message: error_message) + end + + container_registry_protection_rule.update(params.slice(*ALLOWED_ATTRIBUTES)) + + if container_registry_protection_rule.errors.present? + return service_response_error(message: container_registry_protection_rule.errors.full_messages) + end + + ServiceResponse.success(payload: { container_registry_protection_rule: container_registry_protection_rule }) + rescue StandardError => e + service_response_error(message: e.message) + end + + private + + attr_reader :container_registry_protection_rule, :current_user, :params + + def service_response_error(message:) + ServiceResponse.error( + message: message, + payload: { container_registry_protection_rule: nil } + ) + end + end + end +end diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb index c9642fb495a..cc7478540d2 100644 --- a/app/services/projects/group_links/create_service.rb +++ b/app/services/projects/group_links/create_service.rb @@ -11,12 +11,30 @@ module Projects super(project, user, params) end + def execute + if adding_a_group_as_owner? && cannot_assign_owner_responsibilities_to_member_in_project? + error('403 Forbidden', 403) + else + super + end + end + private delegate :root_ancestor, to: :project + def adding_a_group_as_owner? + params[:link_group_access].to_i == Gitlab::Access::OWNER + end + + def cannot_assign_owner_responsibilities_to_member_in_project? + !current_user.can?(:manage_owners, project) + end + def valid_to_create? - can?(current_user, :read_namespace_via_membership, shared_with_group) && sharing_allowed? + can?(current_user, :admin_project, project) && + can?(current_user, :read_namespace_via_membership, shared_with_group) && + sharing_allowed? end def build_link diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb index e0218ae087e..f0ac28c9216 100644 --- a/app/services/projects/group_links/destroy_service.rb +++ b/app/services/projects/group_links/destroy_service.rb @@ -4,8 +4,14 @@ module Projects module GroupLinks class DestroyService < BaseService def execute(group_link, skip_authorization: false) - unless valid_to_destroy?(group_link, skip_authorization) - return ServiceResponse.error(message: 'Not found', reason: :not_found) + return not_found! unless group_link + + unless skip_authorization + return not_found! unless allowed_to_manage_destroy?(group_link) + + unless allowed_to_destroy_link?(group_link) + return ServiceResponse.error(message: 'Forbidden', reason: :forbidden) + end end if group_link.project.private? @@ -30,11 +36,16 @@ module Projects private - def valid_to_destroy?(group_link, skip_authorization) - return false unless group_link - return true if skip_authorization + def not_found! + ServiceResponse.error(message: 'Not found', reason: :not_found) + end + + def allowed_to_manage_destroy?(group_link) + current_user.can?(:manage_destroy, group_link) + end - current_user.can?(:admin_project_group_link, group_link) + def allowed_to_destroy_link?(group_link) + current_user.can?(:destroy_project_group_link, group_link) end def refresh_project_authorizations_asynchronously(project) diff --git a/app/services/projects/group_links/update_service.rb b/app/services/projects/group_links/update_service.rb index 04f1552d929..1d657f2396d 100644 --- a/app/services/projects/group_links/update_service.rb +++ b/app/services/projects/group_links/update_service.rb @@ -10,7 +10,13 @@ module Projects end def execute(group_link_params) - return ServiceResponse.error(message: 'Not found', reason: :not_found) unless allowed_to_update? + if group_link.blank? || !allowed_to_update? + return ServiceResponse.error(message: 'Not found', reason: :not_found) + end + + unless allowed_to_update_to_or_from_owner?(group_link_params) + return ServiceResponse.error(message: 'Forbidden', reason: :forbidden) + end group_link.update!(group_link_params) @@ -24,7 +30,13 @@ module Projects attr_reader :group_link def allowed_to_update? - current_user.can?(:admin_project_member, project) + current_user.can?(:admin_project_member, group_link.project) + end + + def allowed_to_update_to_or_from_owner?(params) + return current_user.can?(:manage_owners, group_link) if upgrading_to_owner?(params) || touching_an_owner? + + true end def refresh_authorizations @@ -41,6 +53,14 @@ module Projects def requires_authorization_refresh?(params) params.include?(:group_access) end + + def upgrading_to_owner?(params) + params[:group_access].to_i == Gitlab::Access::OWNER + end + + def touching_an_owner? + group_link.owner_access? + end end end end diff --git a/app/views/groups/_home_panel.html.haml b/app/views/groups/_home_panel.html.haml index 5d1ebd60413..6b56fe0b75b 100644 --- a/app/views/groups/_home_panel.html.haml +++ b/app/views/groups/_home_panel.html.haml @@ -5,8 +5,7 @@ .group-home-panel .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-sm-flex-direction-column.gl-md-flex-direction-row.gl-gap-3.gl-my-5 .home-panel-title-row.gl-display-flex - .avatar-container.rect-avatar.s48.home-panel-avatar.gl-flex-shrink-0.float-none{ class: 'gl-mr-3!' } - = render Pajamas::AvatarComponent.new(@group, alt: @group.name, size: 48, avatar_options: { itemprop: 'logo' }) + = render Pajamas::AvatarComponent.new(@group, alt: @group.name, size: 48, class: 'float-none gl-flex-shrink-0 gl-mr-3', avatar_options: { itemprop: 'logo' }) %div %h1.home-panel-title.gl-font-size-h-display.gl-mt-3.gl-mb-2.gl-ml-2.gl-display-flex.gl-align-items-center.gl-flex-wrap.gl-gap-3.gl-word-break-word{ itemprop: 'name' } = @group.name diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 06d9a53efdb..a7caa797a46 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -7,9 +7,6 @@ - sidebar_data = super_sidebar_context(current_user, group: group, project: @project, panel: sidebar_panel, panel_type: nav).to_json %aside.js-super-sidebar.super-sidebar.super-sidebar-loading{ data: { root_path: root_path, sidebar: sidebar_data, force_desktop_expanded_sidebar: @force_desktop_expanded_sidebar.to_s, command_palette: command_palette_data(project: @project).to_json } } - - if display_whats_new? - #whats-new-app{ data: { version_digest: whats_new_version_digest } } - = render_if_exists "layouts/tanuki_bot_chat" - elsif defined?(nav) && nav diff --git a/app/views/projects/merge_requests/_code_dropdown.html.haml b/app/views/projects/merge_requests/_code_dropdown.html.haml index bfa33f26453..50f4e313bc5 100644 --- a/app/views/projects/merge_requests/_code_dropdown.html.haml +++ b/app/views/projects/merge_requests/_code_dropdown.html.haml @@ -1,6 +1,6 @@ .gl-md-ml-3.dropdown.gl-dropdown{ class: "gl-display-none! gl-md-display-flex!" } #js-check-out-modal{ data: how_merge_modal_data(@merge_request) } - = button_tag type: 'button', class: "btn dropdown-toggle btn-confirm gl-button gl-dropdown-toggle", data: { toggle: 'dropdown', testid: 'mr-code-dropdown' } do + = button_tag type: 'button', class: "btn dropdown-toggle btn-confirm-secondary gl-button gl-dropdown-toggle", data: { toggle: 'dropdown', testid: 'mr-code-dropdown' } do %span.gl-dropdown-button-text= _('Code') = sprite_icon "chevron-down", size: 16, css_class: "dropdown-icon gl-icon gl-ml-2 gl-mr-0!" .dropdown-menu.dropdown-menu-right diff --git a/app/views/projects/merge_requests/_widget.html.haml b/app/views/projects/merge_requests/_widget.html.haml index 9ec4363fa9a..721446eb017 100644 --- a/app/views/projects/merge_requests/_widget.html.haml +++ b/app/views/projects/merge_requests/_widget.html.haml @@ -22,6 +22,7 @@ window.gl.mrWidgetData.can_view_false_positive = '#{@merge_request.project.licensed_feature_available?(:sast_fp_reduction).to_s}'; window.gl.mrWidgetData.user_preferences_gitpod_path = '#{profile_preferences_path(anchor: 'user_gitpod_enabled')}'; window.gl.mrWidgetData.user_profile_enable_gitpod_path = '#{profile_path(user: { gitpod_enabled: true })}'; + window.gl.mrWidgetData.saml_approval_path = window.gl.mrWidgetData.saml_approval_path %h2#merge-request-widgets-heading.gl-sr-only = _("Merge request reports") |