Cancan port

[jforberg]

The thread where we discuss the cancan port

[jforberg]

@Zofz said

To attend the authentication issue, I can fix it, but we should also get going with cancancan, I have
my last exam tomorrow and will hopefully get going some more this.

I think maybe it would be best if someone with experience of Cancan did the initial port. There are some pitfalls as usual and it's just easier to do if you've done it a few times before.

AFAIK that would mean either me, @lham or @henrikssn. It's not fun work, I can do it after my exams meaning next week at the earliest.

It would be nice to have some controller test coverage first, so we know that we don't break anything. Everyone can help there.

[davidwessman]

@jforberg I am not going to argue with you there :)
When the CafeWork-module is live I can try to get the test base up.
Created #68.

[jforberg]

The Plan:

• All controllers use cancan. First step is to remove the_role dependency and then refactor until it builds.
• All Activerecord transactions use the bang form of methods (Model.create! not Model.create). These throw exceptions when they fail unlike the normal ones which just fail silently. Bad.
• No Json rendering in controllers unless it's needed by the views. By serving Json you commit to an API and changing it will break dependent applications. If we want to expose an api it should be done in a separate, versioned api.

[jforberg]

Also:

• Only set a single instance variable in each method if at all possible. This is because, if we have a form error then we need to render the form again and then we have to Repeat setting the instance variables again otherwise the form will not build.

[jforberg]

@henrikssn This is what we have to fix:

johan@starbuck ~/code/fsek $ egrep -Ri 'the_?role' app config lib spec
app/assets/javascripts/application.js.erb://= require the_role_editinplace
app/controllers/events_controller.rb:        flash[:error] = t('the_role.access_denied')
app/controllers/work_posts_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/admin/elections_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/admin/rents_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/albums_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/contacts_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/councils_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/documents_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/menus_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/notices_controller.rb:      redirect_to(root_path, alert: t('the_role.access_denied')) unless current_user &&  (current_user.moderator?(:notiser))
app/controllers/posts_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/profiles_controller.rb:        redirect_to(root_path, alert: t('the_role.access_denied')) unless current_user &&  (current_user == @profile.user)
app/controllers/users_controller.rb:  include TheRole::Controller
app/controllers/users_controller.rb:      flash[:error] = t('the_role.access_denied')
app/controllers/users_controller.rb:    # TheRole: You should define OWNER CHECK OBJECT
app/controllers/static_pages_controller.rb:  include TheRole::Controller
app/controllers/githook_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/news_controller.rb:  include TheRole::Controller  
app/controllers/news_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/application_controller.rb:    flash[:error] = t('the_role.access_denied')
app/controllers/application_controller.rb:    flash[:error] = t('the_role.access_denied')
app/models/role.rb:  include TheRole::Role  
config/locales/sv.yml:  the_role:

[davidwessman]

We also have a lot of current_user.moderator?(:different_rules) in views+controllers.

[jforberg]

The cancan way is to check against actions instead of roles. So

can? :modify, @object

instead of

a_user.has_some_role?

The access rules are all declared in one single place, namely app/models/ability.rb

[davidwessman]

👍

[henrikssn]

I managed to get cancan-port running now on dev.fsektionen.se

We are currently failing on about 25 tests, we should start fixing that and later write tests for what does not work on the website.

See travis for failing tests: https://travis-ci.org/fsek/web/builds/55520580

[davidwessman]

The login_required should be easy to fix?
I'm trying to study for a while, will do some work later on.

[henrikssn]

I fixed a lot of login_required in a80bc9e. Please click around at dev.fsektionen.se and report stuff which is not working.

We can also start fixing tests as authentication is now working. I.e. for foo to edit a Bar then

1. Foo must have a post FooPost
2. FooPost must have the permission (subject_class: 'Bar', action: 'edit') or (subject_class: 'Bar', action: 'manage')

You can modify permissions of a post with set_permissions()

[davidwessman]

We should also add a Fancy menu showing everything where people have permissions 😄

[henrikssn]

Now I have solved a rather big merge with master. We really need to merge this asap.

There are still 4 tests which fail. I will fix them later and then we can merge this. I will need some help adding permissions to all posts.

[davidwessman]

Really good work @henrikssn

Are the permissions added in the code only?

[jforberg]

Do it.

[henrikssn]

@davidwessman Yes, we will need to actually grant the permissions to the posts. There are some (ugly) code added in edit_post to do this.

[henrikssn]

Denna är nu live i production, alla spindelmän är nu admins (och jag lade till @jforberg som spindelman).

Ni kan hjälpa till att editera permissions under http://fsektionen.se/poster, kilcka på redigera och klicka i relevanta permissions.

Lägg upp issues om ni hittar något som folk inte har tillgång till.

[davidwessman]

Har inte kollat detta - men det vore toppen om vi kunde styra vilka utskott som folk kan redigera.
Sanningsminsteriet vill kunna redigera allt men endast låta övriga poster redigera sitt eget utskott.
Hur löser vi det? 😄
#127

[davidwessman]

Alla User-permissions bör väl endast vara satta till sig själv - inte kunna sättas på en Post.
#126

[jforberg]

@davidwessman Lätt i cancan. Bara skicka en stänging (block) till can och returnera bool.

can :modify, Council { |c| user.should_be_able? }

[davidwessman]

The port is done and it is working good, ready to close?