From 818760aae8c5a252100a72493bcc44d5257981f6 Mon Sep 17 00:00:00 2001 From: Jessie Young <jessieyoung@gitlab.com> Date: Wed, 24 Jan 2024 18:06:04 +0000 Subject: [PATCH] Add `glAbilities` for frontend * So that we can check for abilities in the frontend rather than relying on license or feature flag checks. --- app/assets/javascripts/commons/vue.js | 2 ++ .../vue_shared/gl_abilities_plugin.js | 11 +++++++ .../vue_shared/mixins/gl_abilities_mixin.js | 8 +++++ app/controllers/application_controller.rb | 4 +-- doc/development/fe_guide/vue.md | 25 ++++++++++++++ doc/development/permissions/authorizations.md | 33 +++++++++++++++++++ .../vulnerabilities/components/header.vue | 9 +++-- .../security/vulnerabilities_controller.rb | 1 + .../frontend/vulnerabilities/header_spec.js | 22 ++++++------- lib/gitlab/gon_helper.rb | 8 +++++ .../vue_shared/gl_abilities_plugin_spec.js | 28 ++++++++++++++++ .../mixins/gl_abilities_mixin_spec.js | 33 +++++++++++++++++++ spec/lib/gitlab/gon_helper_spec.rb | 16 +++++++++ 13 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 app/assets/javascripts/vue_shared/gl_abilities_plugin.js create mode 100644 app/assets/javascripts/vue_shared/mixins/gl_abilities_mixin.js create mode 100644 spec/frontend/vue_shared/gl_abilities_plugin_spec.js create mode 100644 spec/frontend/vue_shared/mixins/gl_abilities_mixin_spec.js diff --git a/app/assets/javascripts/commons/vue.js b/app/assets/javascripts/commons/vue.js index cd24a50363135..857d10aad6784 100644 --- a/app/assets/javascripts/commons/vue.js +++ b/app/assets/javascripts/commons/vue.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import GlFeatureFlagsPlugin from '~/vue_shared/gl_feature_flags_plugin'; +import GlAbilitiesPlugin from '~/vue_shared/gl_abilities_plugin'; import Translate from '~/vue_shared/translate'; if (process.env.NODE_ENV !== 'production') { @@ -7,6 +8,7 @@ if (process.env.NODE_ENV !== 'production') { } Vue.use(GlFeatureFlagsPlugin); +Vue.use(GlAbilitiesPlugin); Vue.use(Translate); Vue.config.ignoredElements = ['gl-emoji']; diff --git a/app/assets/javascripts/vue_shared/gl_abilities_plugin.js b/app/assets/javascripts/vue_shared/gl_abilities_plugin.js new file mode 100644 index 0000000000000..4d0272af5fb89 --- /dev/null +++ b/app/assets/javascripts/vue_shared/gl_abilities_plugin.js @@ -0,0 +1,11 @@ +export default (Vue) => { + Vue.mixin({ + provide() { + return { + glAbilities: { + ...window.gon?.abilities, + }, + }; + }, + }); +}; diff --git a/app/assets/javascripts/vue_shared/mixins/gl_abilities_mixin.js b/app/assets/javascripts/vue_shared/mixins/gl_abilities_mixin.js new file mode 100644 index 0000000000000..2efa20bb65d68 --- /dev/null +++ b/app/assets/javascripts/vue_shared/mixins/gl_abilities_mixin.js @@ -0,0 +1,8 @@ +export default () => ({ + inject: { + glAbilities: { + from: 'glAbilities', + default: () => ({}), + }, + }, +}); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d7b005d03b5b4..2ed0b860906ef 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -208,8 +208,8 @@ def after_sign_out_path_for(resource) Gitlab::CurrentSettings.after_sign_out_path.presence || new_user_session_path end - def can?(object, action, subject = :global) - Ability.allowed?(object, action, subject) + def can?(user, action, subject = :global) + Ability.allowed?(user, action, subject) end def access_denied!(message = nil, status = nil) diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index cf7659d0fc0b2..6990a021b2626 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -351,6 +351,31 @@ return new Vue({ }); ``` +#### Accessing abilities + +After pushing an ability to the [frontend](../permissions/authorizations.md#frontend), +use the [`provide` and `inject`](https://v2.vuejs.org/v2/api/#provide-inject) +mechanisms in Vue to make abilities available to any descendant components +in a Vue application. The `glAbilties` object is already provided in +`commons/vue.js`, so only the mixin is required to use the flags: + +```javascript +// An arbitrary descendant component + +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; + +export default { + // ... + mixins: [glAbilitiesMixin()], + // ... + created() { + if (this.glAbilities.someAbility) { + // ... + } + }, +} +``` + #### Accessing feature flags After pushing a feature flag to the [frontend](../feature_flags/index.md#frontend), diff --git a/doc/development/permissions/authorizations.md b/doc/development/permissions/authorizations.md index 97eef8a648b92..daa7e71e77a41 100644 --- a/doc/development/permissions/authorizations.md +++ b/doc/development/permissions/authorizations.md @@ -44,6 +44,39 @@ Logic, like backend worker logic, might not need authorization based on the curr If the service or finder's constructor does not expect `current_user`, then it typically does not check permissions. +### Frontend + +When using an ability check in UI elements, make sure to _also_ use an ability +check for the underlying backend code, if there is any. This ensures there is +absolutely no way to use the feature until the user has proper access. + +If the UI element is HAML, you can use embedded Ruby to check if +`Ability.allowed?(user, action, subject)`. + +If the UI element is JavaScript or Vue, use the `push_frontend_ability` method, +which is available to all controllers that inherit from `ApplicationController`. +You can use this method to expose the ability, for example: + +```ruby +before_action do + push_frontend_ability(ability: :read_project, resource: @project, user: current_user) +end +``` + +You can then check the state of the ability in JavaScript as follows: + +```javascript +if ( gon.abilities.readProject ) { + // ... +} +``` + +The name of the ability in JavaScript is always camelCase, +so checking for `gon.abilities.read_project` would not work. + +To check for an ability in a Vue template, see the +[developer documentation for access abilities in Vue](../fe_guide/vue.md#accessing-abilities). + ### Tips If a class accepts `current_user`, then it may be responsible for authorization. diff --git a/ee/app/assets/javascripts/vulnerabilities/components/header.vue b/ee/app/assets/javascripts/vulnerabilities/components/header.vue index 18dc92655f6ba..1a77405151cdf 100644 --- a/ee/app/assets/javascripts/vulnerabilities/components/header.vue +++ b/ee/app/assets/javascripts/vulnerabilities/components/header.vue @@ -2,6 +2,7 @@ import { GlLoadingIcon, GlButton, GlBadge, GlTooltipDirective as GlTooltip } from '@gitlab/ui'; import { v4 as uuidv4 } from 'uuid'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; import vulnerabilityStateMutations from 'ee/security_dashboard/graphql/mutate_vulnerability_state'; import StatusBadge from 'ee/vue_shared/security_reports/components/status_badge.vue'; import { createAlert } from '~/alert'; @@ -39,12 +40,16 @@ export default { directives: { GlTooltip, }, - mixins: [glFeatureFlagsMixin()], + mixins: [glFeatureFlagsMixin(), glAbilitiesMixin()], props: { vulnerability: { type: Object, required: true, }, + ability: { + type: Object, + required: true, + }, }, data() { @@ -70,7 +75,7 @@ export default { } if ( - this.glFeatures.resolveVulnerability && + this.glAbilities.resolveVulnerabilityAi && this.vulnerability.reportType === REPORT_TYPE_SAST ) { buttons.push(HEADER_ACTION_BUTTONS.mergeRequestCreationAi); diff --git a/ee/app/controllers/projects/security/vulnerabilities_controller.rb b/ee/app/controllers/projects/security/vulnerabilities_controller.rb index ae6d97ba0bd5f..7b2735c897c28 100644 --- a/ee/app/controllers/projects/security/vulnerabilities_controller.rb +++ b/ee/app/controllers/projects/security/vulnerabilities_controller.rb @@ -8,6 +8,7 @@ class VulnerabilitiesController < Projects::ApplicationController before_action do push_frontend_feature_flag(:create_vulnerability_jira_issue_via_graphql, @project) + push_frontend_ability(ability: :resolve_vulnerability_ai, resource: @project, user: current_user) end before_action :vulnerability, except: [:new] diff --git a/ee/spec/frontend/vulnerabilities/header_spec.js b/ee/spec/frontend/vulnerabilities/header_spec.js index 98ad0ff24ec52..fc9609c685cc0 100644 --- a/ee/spec/frontend/vulnerabilities/header_spec.js +++ b/ee/spec/frontend/vulnerabilities/header_spec.js @@ -115,7 +115,7 @@ describe('Vulnerability Header', () => { dropdown.vm.$emit('change', { action }); }; - const createWrapper = ({ vulnerability = {}, apolloProvider, glFeatures }) => { + const createWrapper = ({ vulnerability = {}, apolloProvider, glAbilities }) => { wrapper = shallowMount(Header, { apolloProvider, directives: { @@ -129,9 +129,9 @@ describe('Vulnerability Header', () => { }, provide: { dismissalDescriptions, - glFeatures: { - resolveVulnerability: true, - ...glFeatures, + glAbilities: { + resolveVulnerabilityAi: true, + ...glAbilities, }, }, }); @@ -663,12 +663,12 @@ describe('Vulnerability Header', () => { }); }); - describe('when FF "resolveVulnerability" is disabled', () => { + describe('when user does not have "resolveVulnerabilityAi" ability', () => { describe('split button', () => { it('renders the create merge request and issue button as a split button', async () => { createWrapper({ - glFeatures: { - resolveVulnerability: false, + glAbilities: { + resolveVulnerabilityAi: false, }, vulnerability: getVulnerability({ canCreateMergeRequest: true, @@ -686,8 +686,8 @@ describe('Vulnerability Header', () => { it('does not render the split button if there is only one action', () => { createWrapper({ - glFeatures: { - resolveVulnerability: false, + glAbilities: { + resolveVulnerabilityAi: false, }, vulnerability: getVulnerability({ canCreateMergeRequest: true, @@ -699,8 +699,8 @@ describe('Vulnerability Header', () => { it('does not display if there are no actions', () => { createWrapper({ - glFeatures: { - resolveVulnerability: false, + glAbilities: { + resolveVulnerabilityAi: false, }, vulnerability: getVulnerability({}), }); diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index c4c0e48be3fd2..384143f02c185 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -95,6 +95,14 @@ def push_frontend_feature_flag(name, *args, **kwargs) push_to_gon_attributes(:features, name, enabled) end + def push_frontend_ability(ability:, user:, resource: :global) + push_to_gon_attributes( + :abilities, + ability, + Ability.allowed?(user, ability, resource) + ) + end + # Exposes the state of a feature flag to the frontend code. # Can be used for more complex feature flag checks. # diff --git a/spec/frontend/vue_shared/gl_abilities_plugin_spec.js b/spec/frontend/vue_shared/gl_abilities_plugin_spec.js new file mode 100644 index 0000000000000..af1efa9fd5d9b --- /dev/null +++ b/spec/frontend/vue_shared/gl_abilities_plugin_spec.js @@ -0,0 +1,28 @@ +import { shallowMount } from '@vue/test-utils'; +import Vue from 'vue'; +import GlAbilities from '~/vue_shared/gl_abilities_plugin'; + +describe('GitLab Abilities Plugin', () => { + beforeEach(() => { + window.gon = { + abilities: { + aAbility: true, + bAbility: false, + }, + }; + + Vue.use(GlAbilities); + }); + + it('should provide glAbilities to components', () => { + const component = { + template: `<span></span>`, + inject: ['glAbilities'], + }; + const wrapper = shallowMount(component); + expect(wrapper.vm.glAbilities).toEqual({ + aAbility: true, + bAbility: false, + }); + }); +}); diff --git a/spec/frontend/vue_shared/mixins/gl_abilities_mixin_spec.js b/spec/frontend/vue_shared/mixins/gl_abilities_mixin_spec.js new file mode 100644 index 0000000000000..6d8c8b5c9d643 --- /dev/null +++ b/spec/frontend/vue_shared/mixins/gl_abilities_mixin_spec.js @@ -0,0 +1,33 @@ +import { shallowMount } from '@vue/test-utils'; +import glAbilitiesMixin from '~/vue_shared/mixins/gl_abilities_mixin'; + +describe('GitLab Abilities Mixin', () => { + let wrapper; + + beforeEach(() => { + const gon = { + abilities: { + aAbility: true, + bAbility: false, + }, + }; + + const component = { + template: `<span></span>`, + mixins: [glAbilitiesMixin()], + }; + + wrapper = shallowMount(component, { + provide: { + glAbilities: { ...gon.abilities }, + }, + }); + }); + + it('should provide glAbilities to components', () => { + expect(wrapper.vm.glAbilities).toEqual({ + aAbility: true, + bAbility: false, + }); + }); +}); diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index d9dcae3cdc7e1..4408ef6d54b7d 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -107,6 +107,22 @@ end end + describe '#push_frontend_ability' do + it 'pushes an ability to the frontend' do + user = create(:user) + gon = class_double('Gon') + allow(helper) + .to receive(:gon) + .and_return(gon) + + expect(gon) + .to receive(:push) + .with({ abilities: { 'logIn' => true } }, true) + + helper.push_frontend_ability(ability: :log_in, user: user) + end + end + describe '#push_frontend_feature_flag' do before do skip_feature_flags_yaml_validation -- GitLab