From a6047fcf086746b4779e66b3fdeb144c313eb4a7 Mon Sep 17 00:00:00 2001 From: Julien Bourdeau Date: Wed, 22 May 2024 10:56:42 +0200 Subject: [PATCH 1/2] feat(rbac): Allow role to be edited in graphql --- app/graphql/mutations/invites/revoke.rb | 2 +- app/graphql/mutations/invites/update.rb | 26 +++++++ app/graphql/mutations/memberships/update.rb | 26 +++++++ app/graphql/types/mutation_type.rb | 3 + app/services/invites/update_service.rb | 31 ++++++++ app/services/memberships/update_service.rb | 36 +++++++++ spec/graphql/mutations/invites/update_spec.rb | 73 +++++++++++++++++++ .../mutations/memberships/revoke_spec.rb | 2 +- .../mutations/memberships/update_spec.rb | 49 +++++++++++++ spec/services/invites/update_service_spec.rb | 57 +++++++++++++++ .../memberships/update_service_spec.rb | 42 +++++++++++ 11 files changed, 345 insertions(+), 2 deletions(-) create mode 100644 app/graphql/mutations/invites/update.rb create mode 100644 app/graphql/mutations/memberships/update.rb create mode 100644 app/services/invites/update_service.rb create mode 100644 app/services/memberships/update_service.rb create mode 100644 spec/graphql/mutations/invites/update_spec.rb create mode 100644 spec/graphql/mutations/memberships/update_spec.rb create mode 100644 spec/services/invites/update_service_spec.rb create mode 100644 spec/services/memberships/update_service_spec.rb diff --git a/app/graphql/mutations/invites/revoke.rb b/app/graphql/mutations/invites/revoke.rb index d0dc2cf84d5..f1269bcf437 100644 --- a/app/graphql/mutations/invites/revoke.rb +++ b/app/graphql/mutations/invites/revoke.rb @@ -9,7 +9,7 @@ class Revoke < BaseMutation REQUIRED_PERMISSION = 'organization:members:delete' graphql_name 'RevokeInvite' - description 'Revokes a invite' + description 'Revokes an invite' argument :id, ID, required: true diff --git a/app/graphql/mutations/invites/update.rb b/app/graphql/mutations/invites/update.rb new file mode 100644 index 00000000000..7f2f8448e97 --- /dev/null +++ b/app/graphql/mutations/invites/update.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Mutations + module Invites + class Update < BaseMutation + include AuthenticableApiUser + include RequiredOrganization + + REQUIRED_PERMISSION = 'organization:members:update' + + graphql_name 'UpdateInvite' + description 'Update an invite' + + argument :id, ID, required: true + argument :role, Types::Memberships::RoleEnum, required: true + + type Types::Invites::Object + + def resolve(**args) + invite = current_organization.invites.pending.find_by(id: args[:id]) + result = ::Invites::UpdateService.call(invite:, params: args) + result.success? ? result.invite : result_error(result) + end + end + end +end diff --git a/app/graphql/mutations/memberships/update.rb b/app/graphql/mutations/memberships/update.rb new file mode 100644 index 00000000000..ab57f4cdb9c --- /dev/null +++ b/app/graphql/mutations/memberships/update.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Mutations + module Memberships + class Update < BaseMutation + include AuthenticableApiUser + include RequiredOrganization + + REQUIRED_PERMISSION = 'organization:members:update' + + graphql_name 'UpdateMembership' + description 'Update a membership' + + argument :id, ID, required: true + argument :role, Types::Memberships::RoleEnum, required: true + + type Types::MembershipType + + def resolve(**args) + membership = current_organization.memberships.find_by(id: args[:id]) + result = ::Memberships::UpdateService.call(membership:, params: args) + result.success? ? result.membership : result_error(result) + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index a6d6e7abd5d..31913b3e582 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -92,7 +92,10 @@ class MutationType < Types::BaseObject field :accept_invite, mutation: Mutations::Invites::Accept field :create_invite, mutation: Mutations::Invites::Create field :revoke_invite, mutation: Mutations::Invites::Revoke + field :update_invite, mutation: Mutations::Invites::Update + field :revoke_membership, mutation: Mutations::Memberships::Revoke + field :update_membership, mutation: Mutations::Memberships::Update field :create_password_reset, mutation: Mutations::PasswordResets::Create field :reset_password, mutation: Mutations::PasswordResets::Reset diff --git a/app/services/invites/update_service.rb b/app/services/invites/update_service.rb new file mode 100644 index 00000000000..78b05b11884 --- /dev/null +++ b/app/services/invites/update_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Invites + class UpdateService < BaseService + def initialize(invite:, params:) + @invite = invite + @params = params + + super + end + + def call + return result.not_found_failure!(resource: 'invite') unless invite + return result.forbidden_failure!(code: 'cannot_update_accepted_invite') if invite.accepted? + return result.forbidden_failure!(code: 'cannot_update_revoked_invite') if invite.revoked? + + invite.update!( + role: params[:role], + ) + + result.invite = invite + result + rescue ActiveRecord::RecordInvalid => e + result.record_validation_failure!(record: e.record) + end + + private + + attr_reader :invite, :params + end +end diff --git a/app/services/memberships/update_service.rb b/app/services/memberships/update_service.rb new file mode 100644 index 00000000000..b757668b35c --- /dev/null +++ b/app/services/memberships/update_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Memberships + class UpdateService < BaseService + def initialize(membership:, params:) + @membership = membership + @params = params + + super + end + + def call + return result.not_found_failure!(resource: 'membership') unless membership + return result.not_allowed_failure!(code: 'last_admin') if changing_role_of_last_admin? + + membership.update!( + role: params[:role], + ) + + result.membership = membership + result + rescue ActiveRecord::RecordInvalid => e + result.record_validation_failure!(record: e.record) + end + + private + + attr_reader :membership, :params + + def changing_role_of_last_admin? + membership.organization.memberships.admin.count == 1 && + membership.admin? && + params[:role] != 'admin' + end + end +end diff --git a/spec/graphql/mutations/invites/update_spec.rb b/spec/graphql/mutations/invites/update_spec.rb new file mode 100644 index 00000000000..1097bf7f02c --- /dev/null +++ b/spec/graphql/mutations/invites/update_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Mutations::Invites::Update, type: :graphql do + let(:required_permission) { 'organization:members:update' } + let(:membership) { create(:membership) } + let(:organization) { membership.organization } + let(:user) { membership.user } + + let(:mutation) do + <<-GQL + mutation($input: UpdateInviteInput!) { + updateInvite(input: $input) { + id + role + } + } + GQL + end + + it_behaves_like 'requires current user' + it_behaves_like 'requires current organization' + it_behaves_like 'requires permission', 'organization:members:update' + + describe 'Invite update mutation' do + context 'with an existing invite' do + let(:invite) { create(:invite, organization:, role: :admin) } + + it 'returns the updated invite' do + result = execute_graphql( + current_organization: organization, + current_user: user, + permissions: required_permission, + query: mutation, + variables: { + input: { + id: invite.id, + role: 'finance' + } + }, + ) + + data = result['data']['updateInvite'] + + expect(data['id']).to eq(invite.id) + expect(data['role']).to eq('finance') + end + end + + context 'when the invite accepted' do + let(:invite) { create(:invite, organization:, status: :accepted) } + + it 'returns an error' do + result = execute_graphql( + current_organization: organization, + permissions: required_permission, + current_user: user, + query: mutation, + variables: { + input: {id: invite.id, role: 'finance'} + }, + ) + + aggregate_failures do + expect(result['errors'].first['message']).to eq('Resource not found') + expect(result['errors'].first['extensions']['code']).to eq('not_found') + expect(result['errors'].first['extensions']['details']['invite']).to eq(['not_found']) + end + end + end + end +end diff --git a/spec/graphql/mutations/memberships/revoke_spec.rb b/spec/graphql/mutations/memberships/revoke_spec.rb index 755f352518d..e7bc8b36ffd 100644 --- a/spec/graphql/mutations/memberships/revoke_spec.rb +++ b/spec/graphql/mutations/memberships/revoke_spec.rb @@ -67,7 +67,7 @@ permissions: required_permission, query: mutation, variables: { - input: {id: membership.id}, + input: {id: membership.id} }, ) diff --git a/spec/graphql/mutations/memberships/update_spec.rb b/spec/graphql/mutations/memberships/update_spec.rb new file mode 100644 index 00000000000..96e756ca028 --- /dev/null +++ b/spec/graphql/mutations/memberships/update_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Mutations::Memberships::Update, type: :graphql do + let(:required_permission) { 'organization:members:update' } + let(:membership) { create(:membership) } + let(:organization) { membership.organization } + let(:user) { membership.user } + + let(:mutation) do + <<-GQL + mutation($input: UpdateMembershipInput!) { + updateMembership(input: $input) { + id + role + } + } + GQL + end + + it_behaves_like 'requires current user' + it_behaves_like 'requires current organization' + it_behaves_like 'requires permission', 'organization:members:update' + + describe 'Membership update mutation' do + let(:membership_to_edit) { create(:membership, organization:, role: :finance) } + + it 'returns the updated membership' do + result = execute_graphql( + current_organization: organization, + current_user: user, + permissions: required_permission, + query: mutation, + variables: { + input: { + id: membership_to_edit.id, + role: 'admin' + } + }, + ) + + data = result['data']['updateMembership'] + + expect(data['id']).to eq(membership_to_edit.id) + expect(data['role']).to eq('admin') + end + end +end diff --git a/spec/services/invites/update_service_spec.rb b/spec/services/invites/update_service_spec.rb new file mode 100644 index 00000000000..9525ee6ec12 --- /dev/null +++ b/spec/services/invites/update_service_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Invites::UpdateService, type: :service do + let(:membership) { create(:membership) } + let(:organization) { membership.organization } + let(:invite) { create(:invite, organization:) } + let(:params) { {role: 'manager'} } + + describe '#call' do + context 'when invite is pending' do + let(:invite) { create(:invite, organization:, status: 'pending', role: :admin) } + let(:params) { {role: 'manager'} } + + it 'update the role' do + result = described_class.call(invite:, params:) + + expect(result).to be_success + expect(result.invite.reload.role).to eq('manager') + end + end + + context 'when invite is not found' do + let(:invite) { nil } + + it 'returns an error' do + result = described_class.call(invite:, params:) + + expect(result).not_to be_success + expect(result.error.error_code).to eq('invite_not_found') + end + end + + context 'when invite is revoked' do + let(:invite) { create(:invite, organization:, status: 'revoked') } + + it 'returns an error' do + result = described_class.call(invite:, params:) + + expect(result).not_to be_success + expect(result.error.code).to eq('cannot_update_revoked_invite') + end + end + + context 'when invite is accepted' do + let(:invite) { create(:invite, organization:, status: 'accepted') } + + it 'returns an error' do + result = described_class.call(invite:, params:) + + expect(result).not_to be_success + expect(result.error.code).to eq('cannot_update_accepted_invite') + end + end + end +end diff --git a/spec/services/memberships/update_service_spec.rb b/spec/services/memberships/update_service_spec.rb new file mode 100644 index 00000000000..d824b894044 --- /dev/null +++ b/spec/services/memberships/update_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Memberships::UpdateService, type: :service do + let(:membership) { create(:membership, role: 'admin') } + let(:organization) { membership.organization } + let(:params) { {role: 'manager'} } + + describe '#call' do + context 'when another admin exists' do + it 'update the role' do + create(:membership, organization: organization, role: 'admin') + + result = described_class.call(membership:, params:) + + expect(result).to be_success + expect(result.membership.reload.role).to eq('manager') + end + end + + context 'when membership is the last admin' do + it 'returns an error' do + result = described_class.call(membership:, params:) + + expect(result).not_to be_success + expect(result.error.code).to eq('last_admin') + end + end + + context 'when membership is not found' do + let(:membership) { nil } + + it 'returns an error' do + result = described_class.call(membership:, params:) + + expect(result).not_to be_success + expect(result.error.error_code).to eq('membership_not_found') + end + end + end +end From 6a0db801c12b6cc9b4cad6b9d621addcf85cd635 Mon Sep 17 00:00:00 2001 From: Julien Bourdeau Date: Wed, 22 May 2024 10:57:38 +0200 Subject: [PATCH 2/2] dump schema --- schema.graphql | 46 ++++++++++++- schema.json | 170 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 214 insertions(+), 2 deletions(-) diff --git a/schema.graphql b/schema.graphql index 9f6b7f1e7bb..edaa5cc7f14 100644 --- a/schema.graphql +++ b/schema.graphql @@ -4495,7 +4495,7 @@ type Mutation { ): Webhook """ - Revokes a invite + Revokes an invite """ revokeInvite( """ @@ -4664,6 +4664,16 @@ type Mutation { input: UpdateIntegrationMappingInput! ): Mapping + """ + Update an invite + """ + updateInvite( + """ + Parameters for UpdateInvite + """ + input: UpdateInviteInput! + ): Invite + """ Update an existing invoice """ @@ -4674,6 +4684,16 @@ type Mutation { input: UpdateInvoiceInput! ): Invoice + """ + Update a membership + """ + updateMembership( + """ + Parameters for UpdateMembership + """ + input: UpdateMembershipInput! + ): Membership + """ Update Netsuite integration """ @@ -6726,6 +6746,18 @@ input UpdateIntegrationMappingInput { mappableType: MappableTypeEnum } +""" +Autogenerated input type of UpdateInvite +""" +input UpdateInviteInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + id: ID! + role: MembershipRole! +} + """ Update Invoice input arguments """ @@ -6739,6 +6771,18 @@ input UpdateInvoiceInput { paymentStatus: InvoicePaymentStatusTypeEnum } +""" +Autogenerated input type of UpdateMembership +""" +input UpdateMembershipInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + id: ID! + role: MembershipRole! +} + """ Autogenerated input type of UpdateNetsuiteIntegration """ diff --git a/schema.json b/schema.json index e55aee76474..08a3864d6b4 100644 --- a/schema.json +++ b/schema.json @@ -21165,7 +21165,7 @@ }, { "name": "revokeInvite", - "description": "Revokes a invite", + "description": "Revokes an invite", "type": { "kind": "OBJECT", "name": "Invite", @@ -21656,6 +21656,35 @@ } ] }, + { + "name": "updateInvite", + "description": "Update an invite", + "type": { + "kind": "OBJECT", + "name": "Invite", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null, + "args": [ + { + "name": "input", + "description": "Parameters for UpdateInvite", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "UpdateInviteInput", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ] + }, { "name": "updateInvoice", "description": "Update an existing invoice", @@ -21685,6 +21714,35 @@ } ] }, + { + "name": "updateMembership", + "description": "Update a membership", + "type": { + "kind": "OBJECT", + "name": "Membership", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null, + "args": [ + { + "name": "input", + "description": "Parameters for UpdateMembership", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "UpdateMembershipInput", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ] + }, { "name": "updateNetsuiteIntegration", "description": "Update Netsuite integration", @@ -32290,6 +32348,61 @@ ], "enumValues": null }, + { + "kind": "INPUT_OBJECT", + "name": "UpdateInviteInput", + "description": "Autogenerated input type of UpdateInvite", + "interfaces": null, + "possibleTypes": null, + "fields": null, + "inputFields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "id", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "role", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "ENUM", + "name": "MembershipRole", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "enumValues": null + }, { "kind": "INPUT_OBJECT", "name": "UpdateInvoiceInput", @@ -32361,6 +32474,61 @@ ], "enumValues": null }, + { + "kind": "INPUT_OBJECT", + "name": "UpdateMembershipInput", + "description": "Autogenerated input type of UpdateMembership", + "interfaces": null, + "possibleTypes": null, + "fields": null, + "inputFields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "id", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "role", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "ENUM", + "name": "MembershipRole", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "enumValues": null + }, { "kind": "INPUT_OBJECT", "name": "UpdateNetsuiteIntegrationInput",