Looks like the latest dev and beta have changed how access is handled and now return an array to quiz_take_access() which is the access callback to node/%quiz_menu/take. This access callback expect a boolean and therefor is always TRUE and will let anybody take the quiz regardless of permissions.

Consider reverting this logic build up or do some kind of boolean grab from the key'success' => TRUE

CommentFileSizeAuthor
#11 unintended_anonymous-2797513-11.patch3.48 KBdjdevin
#8 unintended_anonymous-2797513-8.patch3.21 KBjoelpittet
#6 unintended_anonymous-2797513-6.patch1.15 KBdjdevin
#4 2797513-2.patch943 bytesjoelpittet
#2 2797513-1.patch551 bytesjoelpittet

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
StatusFileSize
new 2797513-1.patch551 bytes

Here's a patch to resolve this, hope I took the right route?

Status: Needs review » Needs work

The last submitted patch, 2: 2797513-1.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new 2797513-2.patch943 bytes

Actually two problems,
If they don't have access 'success' should be FALSE, no?

   if (!user_access('access quiz') || !node_access('view', $quiz)) {
     $hooks['node_perm_access'] = array(
-      'success' => TRUE,
+      'success' => FALSE,
       'message' => t('You are not allowed to take this @quiz.', array('@quiz' => QUIZ_NAME)),
     )

Status: Needs review » Needs work

The last submitted patch, 4: 2797513-2.patch, failed testing.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new unintended_anonymous-2797513-6.patch1.15 KB

Just changed one of the calls, should fix the tests.

Status: Needs review » Needs work

The last submitted patch, 6: unintended_anonymous-2797513-6.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
StatusFileSize
new unintended_anonymous-2797513-8.patch3.21 KB

Thank you @djdevin for helping fix most of the test failures!

The test that failed needed a user with 'access quiz' permission and anonymous doesn't have that by default, so I logged in a user that does (as mentioned in comment).

Also added some extra tests to ensure the message that says they are blocked isn't showing by accident (consider it a regression test maybe?)

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Seems to be good now. Thoughts @djdevin?

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new unintended_anonymous-2797513-11.patch3.48 KB

Looks good, I just extended the check to /take as well to make sure that page is blocked as well.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Fixed

Thanks, fixed!

  • djdevin committed 0a4f9a6 on 7.x-5.x authored by joelpittet 
    Issue #2797513 by joelpittet, djdevin: Unintended anonymous access to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.