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

A problem with tag permission for user. #3692

Open
MIKU-N opened this issue Nov 24, 2022 · 3 comments
Open

A problem with tag permission for user. #3692

MIKU-N opened this issue Nov 24, 2022 · 3 comments

Comments

@MIKU-N
Copy link

MIKU-N commented Nov 24, 2022

Current Behavior

I get a problem with some permission about tag.
When I set permission by tag,although the global permission is set for Indefinitely,but tag permission is null,and I can't edit this permission.User also can't get this permission.like rename post himself,delete post himself on this tag.

Steps to Reproduce

1.Create a Tag
2.Create a permission rule by this tag,set global permission Allow renaming to Indefinitely
3.Login with a default users,and create a post with this tag.
4.Now,the user can't edit himself post title,and he can't delete this post.

Expected Behavior

I think all tag permission maybe can edit,If tag permissions not set ,The tag permissions must inherit global permissions.
This rule is also same in Restrict by Tag.When admin set the tag permission,the permissions of 'null' must inherit global permissions.

Screenshots

No response

Environment

  • Flarum version: 1.6.1,Core Version 1.6.2
  • Website URL: All flarum have this problem
  • Webserver: Nginx
  • Hosting environment:
  • PHP version: 8.1.7

Output of php flarum info

Output of "php flarum info", run this in terminal in your Flarum directory.

Flarum core 1.6.2
PHP version: 8.1.7
MySQL version: 8.0.24
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, bcmath, ctype, curl, dom, filter, ftp, gd, gettext, hash, iconv, intl, json, mbstring, SPL, session, pcntl, standard, mysqlnd, PDO, pdo_mysql, pdo_sqlite, Phar, posix, Reflection, mysqli, shmop, SimpleXML, soap, sockets, sodium, sysvsem, tokenizer, xml, xmlreader, xmlwriter, zip, fileinfo, redis, exif, imagick, gmp, memcached, Zend OPcache
+-------------------------------------+--------------+--------+
| Flarum Extensions | | |
+-------------------------------------+--------------+--------+
| ID | Version | Commit |
+-------------------------------------+--------------+--------+
| flarum-flags | v1.6.1 | |
| flarum-approval | v1.6.1 | |
| flarum-tags | v1.6.1 | |
| flarum-suspend | v1.6.1 | |
| askvortsov-auto-moderator | dev-master | |
| flarum-sticky | v1.6.0 | |
| flarum-nicknames | v1.6.0 | |
| flarum-lock | v1.6.0 | |
| fof-byobu | 1.1.7 | |
| flarum-markdown | v1.6.1 | |
| afrux-forum-widgets-core | v0.1.7 | |
| zerosonesfun-direct-links | 3.1 | |
| v17development-user-badges | v1.1.0 | |
| v17development-seo | v1.8.0 | |
| the-turk-stickiest | 3.0.1 | |
| the-turk-flamoji | 1.0.4 | |
| sycho-profile-cover | v1.3.3 | |
| swaggymacro-only-starter | 0.6.4 | |
| pipecraft-id-slug | v1.1.0 | |
| michaelbelgium-discussion-views | v7.1.3 | |
| kilowhat-audit-free | 1.5.1 | |
| fof-user-directory | 1.2.3 | |
| fof-user-bio | 1.1.0 | |
| fof-upload | 1.3.0-beta.1 | |
| fof-terms | 1.2.0 | |
| fof-sitemap | 2.0.1 | |
| fof-recaptcha | 1.1.0 | |
| fof-pretty-mail | 1.1.1 | |
| fof-polls | 1.3.0 | |
| fof-nightmode | 1.5.1 | |
| fof-linguist | 1.0.4 | |
| fof-drafts | 1.1.2 | |
| fof-doorman | 1.1.1 | |
| fof-default-user-preferences | 1.1.1 | |
| fof-best-answer | 1.2.4 | |
| fof-analytics | 1.1.0 | |
| flarum-subscriptions | v1.6.0 | |
| flarum-statistics | v1.6.1 | |
| flarum-mentions | v1.6.1 | |
| flarum-likes | v1.6.1 | |
| flarum-lang-english | v1.6.0 | |
| flarum-lang-chinese-simplified | dev-master | |
| flarum-bbcode | v1.6.0 | |
| darkle-fancybox | 1.1.2 | |
| clarkwinkelmann-see-past-first-post | 1.3.0 | |
| blomstra-database-queue | 0.1-beta.2 | |
| askvortsov-rich-text | v2.1.7 | |
| askvortsov-pwa | v3.1.3 | |
| afrux-online-users-widget | v0.1.6 | |
| afrux-news-widget | v0.1.1 | |
+-------------------------------------+--------------+--------+
Base URL:
Installation path:
Queue driver: redis
Session driver: redis (Code override. Configured to file)
Mail driver: smtp
Debug mode: off

Possible Solution

No response

Additional Context

This problem is old,You can find same problem in flarum discussion.

@SychO9
Copy link
Member

SychO9 commented Jan 14, 2023

Confirmed.

Breakdown of the issue

Rename discussion ability

There is technically two abilities here, a rename own and rename any, but they are implicit.
This ability's logic is as follows:

With no tag restrictions
In the normal case scenario, the ability check does the following:

  • Check that the user is the author (a rename own ability check)
    • Check that they can reply.
    • Check that the allow renaming setting allows them to.
  • Otherwise check if they have renaming permission (a rename any ability check).

With tag restrictions
Tags apply restrictions in a way that creates issues with this type of own/any ability. Here is what happens when the author is allowed to rename their discussion.

  • The first check described above passes.
  • Then the tag DiscussionPoicy kicks in and does the following:
    • If the actor does not have the discussion.rename permission on the said tag. Deny access.

That nullifies the passed own ability check.

Solution

Imo, we need a proper separation of own vs any permissions. But that might require a big refactor.

@MIKU-N
Copy link
Author

MIKU-N commented Jan 21, 2023

Thanks for the confirmation, I'm looking forward to the refactored permissions system

@imdong
Copy link
Contributor

imdong commented Nov 22, 2023

Although there is a problem with the underlying permission design of the Flarum framework, this is actually a problem introduced by the official plug-in Tag. Can the Tag plug-in be patched to temporarily solve this problem?

For example, the following code can allow users to modify the title of their posts (only unlimited, for reference only, please do not copy, Unless you clearly understand the meaning of this code)

if ($actor->id == $discussion->user_id && isset($this->special_ability[$ability])) {
    $setting_value = $this->settings->get($this->special_ability[$ability]);
    switch ($setting_value) {
        case 'reply':
            // Check if it is replied 
            break;

        case '-1':
            return $this->allow();
            break;

        default:
            if (!is_numeric($setting_value) || !$setting_value > 0) {
                break;
            }
            //  Check creation time
    }
}

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

No branches or pull requests

4 participants