Ticket #1243 (new defect)

Opened 20 months ago

Last modified 16 months ago

ACL vs l10n

Reported by: Massimiliano Owned by:
Priority: critical Milestone: 0.8
Component: Habari Core Software Version: SVN
Keywords: acl, tokens, installation, i18n, l10n Cc:

Description

Some token names in system/classes/acl.php - function create_default_tokens() - are wrapped into _t()'s.

If a user installs Habari in one language, then switches to another one, the names of tokens in the DB won't match (reported in  http://groups.google.com/group/habari-users/msg/73fac4bacbad9c41).

References: r3196, #874 (there might be other similar instances in the code).

Change History

comment:1 Changed 19 months ago by mikelietz

I'm not seeing where the token names are _t()ed. Some of the token groups are wrapped, but from what I can see (they get used on the group admin page) that's safe enough.

 https://trac.habariproject.org/habari/browser/trunk/htdocs/system/classes/acl.php?rev=4287#L757

The descriptions in 760 and 785 should probably get _t()ed also.

comment:2 Changed 19 months ago by Massimiliano

  • Keywords installation, added
  • Summary changed from Token names should not be localizable to ACL vs l10n

OK, we figured something out in IRC ( here and  here)

Sure thing: there is a bug about ACL and localisation, probably introduced at the installation.

What I wrote above is wrong. It is safe to wrap in _t()'s the token group names, along with the descriptions. Which means that many are missing in function create_default_tokens() and in other lines of acl.php.

Instead, installhandler.php creates three standard user groups: admin, anonymous and authenticated, all of which are localised. One installs Habari in French. Groups are stored in the database as "adminFrench", "anonyFrench" and "authentiFrench". One switches to German. acl.php looks for the groups as localised like "adminGerman", "anonyGerman" and "authentiGerman"; but it can't find them, so it stores groups again as "adminGerman", "anonyGerman" and "authentiGerman", with some consequences.

ringmaster's words: «The function UserGroup::get_by_name() shouldn't exist. And a cascade of changes from that change should be applied.»

comment:3 Changed 19 months ago by ringmaster

To clarify...

There's no good reason to search for a group by name. The name of a group does not guarantee its purpose.

Here are options that I would consider for fixing this issue:

1) Add a meaningless token to groups during install to mark them as the groups to target when new permissions are added. So we'd add a token "group_authenticated" to the "Authenticated" group, and whenever core or a plugin wanted to grant a permission to authenticated users, it would find all groups that had the "group_authenticated" token already, and add the new token to them. We'd do similar things for "group_anonymous" and "group_administrator".

Note that this allows admins to remove those tokens, and then new tokens wouldn't get automatically granted to anyone. Also, this may be difficult to retcon into the existing installer. I think I like this option best, but I dislike the idea of a token that does nothing permission-wise, because in addition to appearing in the UI and serving no real purpose, it would tempt plugin authors to use this token as a basis for permissions rather than adding their own permission for an explicit purpose. For example, a plugin may erroneously check for "group_administrator" rather than create a token for "update_fluxinator" which would be the preferred way to check for someone's permission to update the fluxinator.

2) We use existing, regular tokens to determine which groups are anonymous, authenticated, and admin. I think this may be hard or impossible.

3) We just ignore the problem, assuming that if you change the locale of your blog mid-use, you're aware that upgrades aren't going to automatically assign new core permissions unless you change the names of the groups in your database.

To some degree this makes additional sense because there's no reason that the groups in the system (even if you never change the locale) need to be explicitly "Anonymous", "Authenticated", and "Administrator". You could easily rename the "Administrator" group to "Bad-Asses" and then no new permissions would be assigned to them at core upgrades or plugin installs. It could just be a known issue.

comment:4 Changed 16 months ago by mikelietz

  • Milestone changed from 0.7 to 0.8
Note: See TracTickets for help on using tickets.