lifterlms icon indicating copy to clipboard operation
lifterlms copied to clipboard

Custom/Third Party Roles Don't Obey Unrestricted Preview Access

Open nrherron92 opened this issue 2 years ago • 5 comments

Reproduction Steps

  • HS-192885
  • Create a custom user based on subscriber/student role with user role editor and add the role to the unrestricted preview setting
  • or add Memberium basic user roles to the unrestricted preview setting
  • create a course/lesson/drip or some other locked content
  • switch into the custom or third party user role and attempt to access the content

Expected Behavior

  • Restricted content should be accessible via the unrestricted preview

Actual Behavior

  • If the role is based on student, even though you can add them to the preview setting they are still confined to the enroll/drip settings

  • User points to this code as the issue:

if ( ! array_intersect( $user->get_user()->roles, $roles ) ) {
return false;
}

If he turns false to true he says it works for him to use custom/third party roles.

Error Messages / Logs

  • Include any relevant error messages or log files
<!-- Paste error logs / backtraces below this line -->

System and Environment Information

System Report
WordPress
-------------------------------------------

Home Url: [removed]
Site Url: [removed]
Login Url: [removed]/p31/
Version: 5.9.3
Debug Mode: No
Debug Log: No
Debug Display: Yes
Locale: en_US
Multisite: No
Page For Posts: COMPEL Blog (#359) [[removed]/blog/]
Page On Front: Public Home (#25960) [[removed]/]
Permalink Structure: /%postname%/
Show On Front: page
Wp Cron: No


Settings
-------------------------------------------

Version: 6.4.0
Db Version: 6.4.0
Course Catalog: Course Catalog (#75) [[removed]/course-catalog/]
Membership Catalog: Subscription Options (#310) [[removed]/membership-levels/]
Student Dashboard: New MY COMPEL (#27529) [[removed]/new-my-compel/]
Checkout Page: Not Set
Course Catalog Per Page: 12
Course Catalog Sorting: date,DESC
Membership Catalog Per Page: 9
Membership Catalog Sorting: menu_order,ASC
Site Membership: Not Set
Courses Endpoint: my-courses
Edit Endpoint: edit-my-account
Lost Password Endpoint: lost-password
Vouchers Endpoint: redeem-vouchers
Autogenerate Username: no
Password Strength Meter: yes
Minimum Password Strength: strong
Terms Required: yes
Terms Page: Terms & Conditions (#12937) [[removed]/home/terms-conditions/]
Checkout Names: required
Checkout Address: required
Checkout Phone: optional
Checkout Email Confirmation: yes
Open Registration: no
Registration Names: required
Registration Address: optional
Registration Phone: hidden
Registration Voucher: optional
Registration Email Confirmation: no
Account Names: hidden
Account Address: hidden
Account Phone: hidden
Account Email Confirmation: no
Confirmation Endpoint: confirm-payment
Force Ssl Checkout: no
Country: US
Currency: USD
Currency Position: left
Thousand Separator: ,
Decimal Separator: .
Decimals: 2
Trim Zero Decimals: no
Recurring Payments: no
Email From Address: [removed]
Email From Name: [removed]
Email Footer Text:
Email Header Image: 12919
Cert Bg Width: 800
Cert Bg Height: 616
Cert Legacy Compat: no


Constants
-------------------------------------------

LLMS_REMOVE_ALL_DATA: undefined
LLMS_REST_DISABLE: undefined
LLMS_SITE_FEATURE_RECURRING_PAYMENTS: undefined
LLMS_SITE_IS_CLONE: undefined


Gateways
-------------------------------------------

Manual: Enabled
Manual Logging: no
Manual Order: 1


Server
-------------------------------------------

Mysql Version: 5.5.5
Php Curl: Yes
Php Default Timezone: UTC
Php Fsockopen: Yes
Php Max Input Vars: 1000
Php Max Upload Size: 128 MB
Php Memory Limit: 1024M
Php Post Max Size: 128M
Php Soap: Yes
Php Suhosin: No
Php Time Limt: 30
Php Version: 7.4.21
Software: Apache
Wp Memory Limit: 1024M


Browser
-------------------------------------------

HTTP USER AGENT: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36


Theme
-------------------------------------------

Name: Compel Michigan Child
Version: 3.4
Themeuri: https://test.compeltraining.com/themes/michigan
Authoruri: http://themeforest.net/user/WEBNUS
Template: michigan
Child Theme: Yes
Llms Support: No


Plugins
-------------------------------------------

Akismet Anti-Spam: 4.2.3
bbPress: 2.6.9
BP Profile Search: 5.4.5
BuddyPress: 10.2.0
BuddyPress Activity Plus: 1.6.4
Business Directory Plugin: 6.1
Business Directory Plugin - File Attachments Module: 5.1.2
Business Directory Ratings: 5.2.1
Business Directory ZIP Search: 5.4
Classic Editor: 1.6.2
Contact Form 7: 5.5.6
Duplicate Page: 4.4.8
Easy WP SMTP: 1.4.7
i2SDK: 3.99
LifterLMS: 6.4.0
LifterLMS Assignments: 1.3.0
LifterLMS Helper: 3.4.2
LifterLMS Private Areas: 1.1.5
LifterLMS Social Learning: 1.5.0
Memberium: 2.183
Query Monitor: 3.9.0
Really Simple SSL: 5.3.1
Rename wp-login.php: 2.6.0
Simple Banner: 2.11.0
The Events Calendar: 5.14.2.1
Weblizar Pin It Button On Image Hover And Post: 3.4
Webnus Core: 1.0
Webnus Time Table: 1.0.0
WP-PageNavi: 2.94.0
WP-PostRatings: 1.89
WPBakery Page Builder: 6.9.0
WP Crontrol: 1.12.1
Yoast SEO: 18.7


Integrations
-------------------------------------------

BbPress: Yes
BuddyPress: Yes
LifterLMS Private Areas: Yes
LifterLMS Social Learning: Yes


Template Overrides
-------------------------------------------

archive-course.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: )
archive-llms_membership.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
content-no-access-after.php (ver: 4.17.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
content-no-access-before.php (ver: 4.17.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
content-no-access.php (ver: ): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
content-single-lesson-after.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: )
content-single-lesson-before.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
content-single-quiz-after.php (ver: 3.16.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: 3.16.0)
content-single-quiz-before.php (ver: 3.16.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
course/complete-lesson-link.php (ver: 3.33.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.16.1)
course/lesson-navigation.php (ver: 5.7.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
loop/content.php (ver: 3.14.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.14.0)
loop/loop-end.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.0.0)
loop/loop-start.php (ver: 3.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.0.0)
myaccount/dashboard.php (ver: 3.14.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: )
myaccount/form-edit-account.php (ver: 5.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: )
myaccount/navigation.php (ver: 3.17.5): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: 3.0.4)
product/free-enroll-form.php (ver: 5.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.30.0)
product/pricing-table.php (ver: 6.0.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan_child_theme/lifterlms/ (ver: 3.11.1)
quiz/results-attempt-questions-list.php (ver: 5.3.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: 3.16.8)
quiz/results-attempt.php (ver: 3.27.0): /home/p31hos5/stag_compeltraining_com/wp-content/themes/michigan/lifterlms/ (ver: 3.16.8)

This issue has be recreated:

  • [x] Locally
  • [x] On a staging site
  • [ ] On a production website
  • [x] With only LifterLMS and a default theme

Browser, Device, and Operating System Information

  • Browser name and version
  • Operating System name and version
  • Device name and version (if applicable)

nrherron92 avatar Jun 10 '22 18:06 nrherron92

Relevant code in question:

https://github.com/gocodebox/lifterlms/blob/898135c8538e5777c7702bda40345d462d729fe1/includes/functions/llms.functions.person.php#L41-L43

This would be where the user gets if the user has no roles. I think the issue is actually below this where we are requiring the user to have edit access to the post in question in order to bypass restrictions: https://github.com/gocodebox/lifterlms/blob/898135c8538e5777c7702bda40345d462d729fe1/includes/functions/llms.functions.person.php#L45-L47

This will likely not be changed. We've discussed this in previous issues: https://github.com/gocodebox/lifterlms/issues/2133

Essentially the "bug" here is the description of the option on the settings page which currently says "Users with the selected roles will bypass enrollment, drip, and prerequisite restrictions for courses and memberships" but which should add information about this setting applying to roles which can edit the content in order to preview it.

We introduced the requirement to be able to edit in order to bypass as a result of https://github.com/gocodebox/lifterlms/issues/1974 (Related fix https://github.com/gocodebox/lifterlms/pull/1987)

If we remove this restriction in the core we re-introduce a bug/security issue.

Since this issue keeps presenting itself since closing #1974 it seems reasonable that we need to introduce some way to wholly bypass restrictions for those without edit access. I believe we discussed this at some point but I don't recall coming up with anything concrete.

TLDR; This is expected behavior. The issue is that the language of the option didn't update to include information about the edit_post requirement for the roles when we fixed #1974. #2133 is open to track that language change.

The only other thing I could see doing immediately would be to add a filter to the return of the function: https://github.com/gocodebox/lifterlms/blob/898135c8538e5777c7702bda40345d462d729fe1/includes/functions/llms.functions.person.php#L28

We could also maybe change the edit_post requirement to be an or condition with another (custom) capability. If they can edit the post or they have this other capability they could bypass it. That other cap can exist solely to allow users like this one to create a custom role (with this custom cap) to bypass stuff.

I believe @eri-trabiccolo suggested at some point that we add a second option. The existing option continues to require edit access and the second option would be a list of roles that can bypass regardless of whether or not they have edit access. I guess I'm open to that but it feels like overkill.

thomasplevy avatar Jun 10 '22 19:06 thomasplevy

@nrherron92 I don't really have a solution in place for your user as they're reporting a break in functionality that we introduced in order to fix a bug / security issue. What do you do when a feature changes because of a BUG and the user considers that bug to be a feature? We say "we fixed a bug" and they say "we broke a feature". These two things are in direct conflict and as the maintainers and devs I say that this was a bug in the way the feature worked, really from it's introduction into the codebase. It should have ever only applied to users who can edit the content. So we really need to introduce a new feature for this user.

thomasplevy avatar Jun 10 '22 19:06 thomasplevy

The user says "broke my user's access to the lesson content which allowed them to bypass restrictions and not force them to enroll in the course to view the lessons."

If the goal is to not use enrollment and have completely free / open content without the enrollment system, using this is misusing the setting anyway, I think.

Placing a simple filter on llms_page_restricted() might be a better solution: https://github.com/gocodebox/lifterlms/blob/3ecaeb454a242ea32b4dd5167582e89d9983122c/includes/functions/llms.functions.access.php#L108-L116

thomasplevy avatar Jun 10 '22 19:06 thomasplevy

@thomasplevy Thank you for explaining everything! <3

I think you're right, adding roles for custom roles would be overkill or at least something that can be on the back burner, because the bug fix to prevent security violations is way more important for the vast majority of users.

I'll give him the restrictions filter and let him know we will not be changing the code but may consider the idea of adding custom roles at a laster date (?)

nrherron92 avatar Jun 10 '22 19:06 nrherron92

Sounds good... I'm not really sure what the best solution would be. It would be cool to understand his requirements in more detail. It sounds to me like the use-case is that he's using a custom role for all users so that users don't have to enroll in everything. But I might misunderstand.

In any event, understanding the problem / requirement of the user is useful in determining the best way to implement a new feature.

thomasplevy avatar Jun 10 '22 19:06 thomasplevy