Cannot reproduce Node canView permission not checked on forum view

[Snog]

Affected version

2.0

Like the title says, the canView permission for the node is not checked when a forum is viewed.

This can be checked by just adding this to the node entity at the beginning of the canView function and pasting a direct link to a forum in the browser:

$data = $this->Data;
print_r($data);
die();

The canView function is never called.

This leads to having to extend the assertViewableForum in forum.php. There may be other things that need to be extended to account for that, but I'm not sure.

 

[Chris D]

On the forum view we call assertViewableForum and that does the canView check:

if (!$forum->canView($error))
{
   throw $this->exception($this->noPermission($error));
}

 

[Snog]

On the forum view we call assertViewableForum and that does the canView check:

if (!$forum->canView($error))
{
   throw $this->exception($this->noPermission($error));
}

What I add is simply to dump the data contents to indicate when it's called.

It is called when compiling the list of forums, it's not called on an actual view of the forum.

IE: It's not called if you paste the link to this forum directly in the browser.
https://xenforo.com/community/forums/bug-reports-2-x.91/

Unless I'm missing somewhere else where there is a canView?

 

[Chris D]

Like I just said, when you view a forum, e.g. Forum::actionForum() the first thing we do is:

$forum = $this->assertViewableForum($params->node_id ?: $params->node_name, $this->getForumViewExtraWith());

And that method checks the forum record exists, and is viewable, including checking the canView() method as I quoted in my last post. It's definitely called.

 

[Snog]

Like I just said, when you view a forum, e.g. Forum::actionForum() the first thing we do is:

$forum = $this->assertViewableForum($params->node_id ?: $params->node_name, $this->getForumViewExtraWith());

And that method checks the forum record exists, and is viewable, including checking the canView() method as I quoted in my last post. It's definitely called.

All I can do is ask that you try what I said if you can.

If you do, you'll find no data is dumped and the forum is displayed without dieing.

It will dump and die if you just check the entire forum. Do an actual individual forum node itself.

 

[Chris D]

I have tried it, hence the "it's definitely called" comment and closing the report.

Can't you see that we call the assertViewableForum method, and that calls the $forum->canView() method?

I suppose it's worth noting that the node canView method wouldn't be called here, only the forum canView method. Maybe that's where you've been testing the code.

 

[Snog]

I have tried it, hence the "it's definitely called" comment and closing the report.

Can't you see that we call the assertViewableForum method, and that calls the $forum->canView() method?

Yes, I can. That's why I reported it.

I am trying to extend the Node entity canView function. I can get it to work on a full list of the forums, but it doesn't seem to be called when an individual forum is pasted into the browser. Thus allowing someone that knows the link to paste it into their browser.

I'll double check that there's nothing else interfering, but I don't think there is.

 

[Chris D]

We don't call the node canView method there. We call the forum canView method. On the forum entity, not the node entity.

 

[Snog]

LOL, I must be blind.

There is no canView function in my copy of the forum entity.

 

[Chris D]

No, but it extends the AbstractNode entity, and there's one in there.

 

[Snog]

Ok, I disabled every add-on I have and tried it again with the same result.

I'll have another look at this in the morning, but I swear it's happening to me.

 

[Chris D]

Which file have you edited to test it?

 

[Snog]

Which file have you edited to test it?

Just having my morning coffee and haven't had a chance to look closer at this yet.

But, as I said in the first post, XF\Entity\Node.php

I believe AbstractNode.php only supplies the user permission. It doesn't do any other checking if I recall correctly.

 

[Chris D]

But, as I said in the first post, XF\Entity\Node.php

Yes, but I've said several times that it's not that Entity method which we check to check viewability of a forum when viewing a forum.

$forum->canView() is the canView() method on the AbstractNode entity. It's the same function called by Node::canView().

If you add your die() to AbstractNode::canView() you will see that it does kill the script when viewing the forum.

 

[Snog]

Yes, but I've said several times that it's not that Entity method which we check to check viewability of a forum when viewing a forum.

$forum->canView() is the canView() method on the AbstractNode entity. It's the same function called by Node::canView().

If you add your die() to AbstractNode::canView() you will see that it does kill the script when viewing the forum.

That's why I said I'd have to look at it closer.

What was throwing me is the node canView I'm talking about is called when building the forum LIST, but not when viewing the forum itself.

So, this would make two places I need to extend to forbid access. Can you think of any others that might be needed?

 

[Mike]

If you look at what Node::canView() does, all it does it call the type-specific canView function. It's simply a wrapper -- it does nothing and shouldn't be extended. You need to change the type-specific methods.

 

[Chris D]

That's why I said I'd have to look at it closer.

What was throwing me is the node canView I'm talking about is called when building the forum LIST, but not when viewing the forum itself.

So, this would make two places I need to extend to forbid access. Can you think of any others that might be needed?

That's really just because in that context we're working with a mixed list of node types; we start with a $node entity, and load the forum/page/category data via the Data getter. But that Node canView method still calls the canView method of the AbstractNode (i.e. the actual entity for the data). If you're wanting to target the viewability of a specific node type, and not touch the others, you could extend the e.g. Forum Entity class and add the canView method there (which would be overriding the AbstractNode version of it).

In many other contexts, we're just working with a single record, so we go straight for the forum/page/category data, and then join on to the node table when we need it.