-
Notifications
You must be signed in to change notification settings - Fork 5
Add policy and authorization for host dashboard #978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||||
| # frozen_string_literal: true | ||||||
|
|
||||||
| # app/policies/better_together/host_dashboard_policy.rb | ||||||
|
|
||||||
| module BetterTogether | ||||||
| class HostDashboardPolicy < ApplicationPolicy # rubocop:todo Style/Documentation | ||||||
| def index? | ||||||
| user.present? && user.permitted_to?('manage_platform') | ||||||
|
||||||
| user.present? && user.permitted_to?('manage_platform') | |
| permitted_to?('manage_platform') |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe BetterTogether::HostDashboardController, type: :controller do | ||
| include Devise::Test::ControllerHelpers | ||
| include BetterTogether::DeviseSessionHelpers | ||
|
|
||
| routes { BetterTogether::Engine.routes } | ||
|
|
||
| before do | ||
| configure_host_platform | ||
| @request.env['devise.mapping'] = Devise.mappings[:user] | ||
| end | ||
|
|
||
| describe 'GET #index' do | ||
| context 'when user can manage platform' do | ||
| let(:user) { BetterTogether::User.find_by(email: '[email protected]') } | ||
|
|
||
| before { sign_in user } | ||
|
|
||
| it 'returns http success' do | ||
| get :index, params: { locale: I18n.default_locale } | ||
| expect(response).to be_successful | ||
| end | ||
| end | ||
|
|
||
| context 'when user cannot manage platform' do | ||
| let(:user) { create(:user, :confirmed) } | ||
|
|
||
| before { sign_in user } | ||
|
|
||
| it 'raises Pundit::NotAuthorizedError' do | ||
| expect do | ||
| get :index, params: { locale: I18n.default_locale } | ||
| end.to raise_error(Pundit::NotAuthorizedError) | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
Comment on lines
+1
to
+40
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe BetterTogether::HostDashboardPolicy, type: :policy do | ||
| subject(:policy) { described_class.new(user, nil) } | ||
|
|
||
| context 'when user can manage platform' do | ||
| let(:user) { create(:user, :confirmed, :platform_manager) } | ||
|
|
||
| it 'permits access' do | ||
| expect(policy.index?).to be(true) | ||
| end | ||
| end | ||
|
|
||
| context 'when user cannot manage platform' do | ||
| let(:user) { create(:user, :confirmed) } | ||
|
|
||
| it 'denies access' do | ||
| expect(policy.index?).to be(false) | ||
| end | ||
| end | ||
|
|
||
| context 'when no user is present' do | ||
| let(:user) { nil } | ||
|
|
||
| it 'denies access' do | ||
| expect(policy.index?).to be(false) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file path comment on line 3 is redundant and violates the DRY principle. The file path is already evident from the actual file location, and this type of comment is not used consistently across the codebase. It should be removed to maintain consistency with other policy files like
HubPolicy,GuestAccessPolicy, etc.