From 3d08ea81a988a93026eaa249225b60abfb56397d Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Sat, 27 Jul 2024 17:21:04 +0200 Subject: [PATCH 1/4] Reuse REST::RoleSerializer in REST::AccountSerializer --- app/serializers/rest/account_serializer.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/app/serializers/rest/account_serializer.rb b/app/serializers/rest/account_serializer.rb index 354d384464d..e2866800f52 100644 --- a/app/serializers/rest/account_serializer.rb +++ b/app/serializers/rest/account_serializer.rb @@ -30,15 +30,7 @@ class REST::AccountSerializer < ActiveModel::Serializer end end - class RoleSerializer < ActiveModel::Serializer - attributes :id, :name, :color - - def id - object.id.to_s - end - end - - has_many :roles, serializer: RoleSerializer, if: :local? + has_many :roles, serializer: REST::RoleSerializer, if: :local? class FieldSerializer < ActiveModel::Serializer include FormattingHelper From becc24a3b17c5ec47d4c8b6523070d8e422b783d Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Sat, 27 Jul 2024 17:26:48 +0200 Subject: [PATCH 2/4] Add spec to ensure Account Serializer doesn't expose the permissions associated with a role --- spec/serializers/rest/account_serializer_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/serializers/rest/account_serializer_spec.rb b/spec/serializers/rest/account_serializer_spec.rb index 15939e484d8..a57b3105d95 100644 --- a/spec/serializers/rest/account_serializer_spec.rb +++ b/spec/serializers/rest/account_serializer_spec.rb @@ -25,6 +25,10 @@ describe REST::AccountSerializer do it 'returns the expected role' do expect(subject['roles'].first).to include({ 'name' => 'Role' }) end + + it 'does not expose the roles permissions' do + expect(subject['roles'].first).to_not include({ 'permissions' => role.computed_permissions.to_s }) + end end context 'when the account has a non-highlighted role' do From 48fafc7389645479d8566c234d3f44bd15a5961b Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Sat, 27 Jul 2024 17:31:14 +0200 Subject: [PATCH 3/4] Fix specs --- .../rest/account_serializer_spec.rb | 8 +++---- .../credential_account_serializer_spec.rb | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 spec/serializers/rest/credential_account_serializer_spec.rb diff --git a/spec/serializers/rest/account_serializer_spec.rb b/spec/serializers/rest/account_serializer_spec.rb index a57b3105d95..1b9937561e3 100644 --- a/spec/serializers/rest/account_serializer_spec.rb +++ b/spec/serializers/rest/account_serializer_spec.rb @@ -5,7 +5,7 @@ require 'rails_helper' describe REST::AccountSerializer do subject { serialized_record_json(account, described_class) } - let(:role) { Fabricate(:user_role, name: 'Role', highlighted: true) } + let(:role) { Fabricate(:user_role, name: 'Fancy User', highlighted: true) } let(:user) { Fabricate(:user, role: role) } let(:account) { user.account } @@ -20,10 +20,10 @@ describe REST::AccountSerializer do end context 'when the account has a highlighted role' do - let(:role) { Fabricate(:user_role, name: 'Role', highlighted: true) } + let(:role) { Fabricate(:user_role, name: 'Fancy User', highlighted: true) } it 'returns the expected role' do - expect(subject['roles'].first).to include({ 'name' => 'Role' }) + expect(subject['roles'].first).to include({ 'name' => 'Fancy User' }) end it 'does not expose the roles permissions' do @@ -32,7 +32,7 @@ describe REST::AccountSerializer do end context 'when the account has a non-highlighted role' do - let(:role) { Fabricate(:user_role, name: 'Role', highlighted: false) } + let(:role) { Fabricate(:user_role, name: 'Fancy User', highlighted: false) } it 'returns empty roles' do expect(subject['roles']).to eq [] diff --git a/spec/serializers/rest/credential_account_serializer_spec.rb b/spec/serializers/rest/credential_account_serializer_spec.rb new file mode 100644 index 00000000000..5ccef6a4121 --- /dev/null +++ b/spec/serializers/rest/credential_account_serializer_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe REST::CredentialAccountSerializer do + subject { serialized_record_json(account, described_class) } + + let(:role) { Fabricate(:user_role, name: 'Fancy User') } + let(:user) { Fabricate(:user, role: role) } + let(:account) { user.account } + + context 'when the account has a role' do + it 'returns the expected role' do + expect(subject['roles'].first).to include({ 'name' => 'Fancy User' }) + end + + it 'exposes the role permissions' do + expect(subject['roles'].first).to include({ 'permissions' => role.computed_permissions.to_s }) + end + end +end From 08235a8da74a999bb6b2a559c143cf3b627a403d Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Sat, 27 Jul 2024 17:40:54 +0200 Subject: [PATCH 4/4] WIP --- app/serializers/initial_state_serializer.rb | 2 +- app/serializers/rest/admin/account_serializer.rb | 2 +- app/serializers/rest/credential_role_serializer.rb | 9 +++++++++ app/serializers/rest/role_serializer.rb | 4 ---- 4 files changed, 11 insertions(+), 6 deletions(-) create mode 100644 app/serializers/rest/credential_role_serializer.rb diff --git a/app/serializers/initial_state_serializer.rb b/app/serializers/initial_state_serializer.rb index 13f332c95c4..bfe41dc6a82 100644 --- a/app/serializers/initial_state_serializer.rb +++ b/app/serializers/initial_state_serializer.rb @@ -10,7 +10,7 @@ class InitialStateSerializer < ActiveModel::Serializer attribute :critical_updates_pending, if: -> { object&.role&.can?(:view_devops) && SoftwareUpdate.check_enabled? } has_one :push_subscription, serializer: REST::WebPushSubscriptionSerializer - has_one :role, serializer: REST::RoleSerializer + has_one :role, serializer: REST::CredentialRoleSerializer def meta store = default_meta_store diff --git a/app/serializers/rest/admin/account_serializer.rb b/app/serializers/rest/admin/account_serializer.rb index 959884c5505..8568d67a7f3 100644 --- a/app/serializers/rest/admin/account_serializer.rb +++ b/app/serializers/rest/admin/account_serializer.rb @@ -11,7 +11,7 @@ class REST::Admin::AccountSerializer < ActiveModel::Serializer has_many :ips, serializer: REST::Admin::IpSerializer has_one :account, serializer: REST::AccountSerializer - has_one :role, serializer: REST::RoleSerializer + has_one :role, serializer: REST::CredentialRoleSerializer def id object.id.to_s diff --git a/app/serializers/rest/credential_role_serializer.rb b/app/serializers/rest/credential_role_serializer.rb new file mode 100644 index 00000000000..905c2297415 --- /dev/null +++ b/app/serializers/rest/credential_role_serializer.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class REST::CredentialRoleSerializer < REST::RoleSerializer + attributes :permissions + + def permissions + object.computed_permissions.to_s + end +end diff --git a/app/serializers/rest/role_serializer.rb b/app/serializers/rest/role_serializer.rb index 5b81c6e0487..8acbb6cd590 100644 --- a/app/serializers/rest/role_serializer.rb +++ b/app/serializers/rest/role_serializer.rb @@ -6,8 +6,4 @@ class REST::RoleSerializer < ActiveModel::Serializer def id object.id.to_s end - - def permissions - object.computed_permissions.to_s - end end