Toggle bug and fix

rouge75

Hello,

I discovered a bug and have a fix for it :

If a model has a "active" column but a user does not have the "Publish" permission on this model, the user can still toggle the yes/no column.

The reason is in the Module.php, line 2068, which should be :

if ( ! $this->fuel->auth->module_has_action('save') OR (($field == 'publish' OR $field=='active') AND !$this->fuel->auth->has_permission($this->permission, 'publish')))

instead of :

if ( ! $this->fuel->auth->module_has_action('save') OR ($field == 'publish' AND !$this->fuel->auth->has_permission($this->permission, 'publish')))

admin

Fuel treats "publish" and "active" fields similarly and it's not recommended to have both on a model for FUEL.

rouge75

I agree, it's just that if the column is named "active", the permission will not apply to that column which allows you to toggle between yes/no.

And if it is named "publish", the toggle function does not appear

admin

What if you add the "publish" permission to your module?

rouge75

If I remember correctly, the Publish column just displays a 0 or a 1,
I will try again later and tell you.

admin

It's usually an enum column named 'published' with options of 'yes','no'

rouge75

So !
Did several tests :

1) if the column is named "published"
the yes/no toggle appears BUT, if a user does not have the publish permissions, he can still toggle between yes and no

2) if the column is named "publish"
the yes/no toggle does not appear, the column just displays a 0/1

3) if the column is named "active"
the yes/no toggle appears, and with the fix I propose, the "publish" permission works correctly

Now that these 3 tests are done, I think the right fix for case 1 and 3 to work would be :

if ( ! $this->fuel->auth->module_has_action('save') OR (($field == 'published' OR $field=='active') AND !$this->fuel->auth->has_permission($this->permission, 'publish')))

Cheers,
Xavier

admin

Thanks for the report. I've pushed a fix to the develop branch:
https://github.com/daylightstudio/FUEL-CMS/commit/20819072c089f3e49c9528ff8f248432f750eab5

rouge75

You're welcome.
I see you didn't want to include the "active" column
In other places in the Module class, it checks for both "published" and "active" columns, so I thought it was more consistent, but, it's up to you

admin

I just pushed another update to include the "active" column but requires an "activate" permission for toggling.

rouge75

OK, perfect ! thanks !