Skip to content
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

Fixes #30387 - Remove view_bookmarks permission #8486

Merged
merged 1 commit into from Jun 16, 2021

Conversation

tbrisker
Copy link
Member

@tbrisker tbrisker commented May 2, 2021

Users should always be able to see public bookmarks and their own
bookmarks, a permission is not needed for this case.

@theforeman-bot
Copy link
Member

Issues: #30387

Comment on lines -8 to +12
map.permission :my_account, { :users => [:edit],
map.permission :my_account, {
:users => [:edit],
:notification_recipients => [:index, :update, :destroy, :update_group_as_read, :destroy_group],
:"api/v2/table_preferences" => [:show, :create, :edit, :delete, :index]}, :public => true
:"api/v2/table_preferences" => [:show, :create, :edit, :delete, :index],
}, :public => true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just reformatted as an easier to read multi-line hash, no changes.

ezr-ondrej
ezr-ondrej previously approved these changes Jun 12, 2021
Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tbrisker!
As a followup, should we move Bookmarks to different menu?
I've seen people wondering why very unpriviledged user sees Administer menu.

@@ -0,0 +1,9 @@
class DropViewBookmarksPermission < ActiveRecord::Migration[6.0]
def up
Permission.where(name: 'view_bookmarks').destroy_all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might result in empty Filters (if the view_bookmarks was the only permission granted by that filter), but I'd say that's very rare and quite harmless.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, thanks! actually realized my dev db had a bunch of empty filters left behind.

Users should always be able to see public bookmarks and their own
bookmarks, a permission is not needed for this case.
@tbrisker
Copy link
Member Author

As a followup, should we move Bookmarks to different menu?

I think it makes sense to move it to the user menu.

@ezr-ondrej
Copy link
Member

[test katello]

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @tbrisker ! :)

@ezr-ondrej ezr-ondrej merged commit 81512f7 into theforeman:develop Jun 16, 2021
@tbrisker
Copy link
Member Author

theforeman/theforeman.org#1844 has the upgrade warning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants