HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Default SomeModel::GetSomething() behaviour

R_JR_J Ex-FanboyMunich Admin

Whenever I write a plugin that uses DiscussionModel::GetID or CommentModel::Get at some time during code writing, I remember I have to restrict the results based on the permissions of the current user. But I finish my plugin first and one of the last steps is the implementation of the restriction. I know that this is a bad habit and I should change that, but to be honest: I think permission check should be part of the models functions.

Is it on purpose, that nearly all Get-queries do not respect permissions by themselves?

I would welcome something like it could be found in decho() for all models get functions public function Get(..., $Permission = false) and default results will be filtered by user permission. Only if there is the need for unfiltered results, the calling function has to specify a $SomeModel->Get(...,true).

I'd happily begin creating pull requests if the Vanilla developer team says, that those PRs have a chance to be accepted. It would break the current logic and thus being a major change, but I can only think of very exotic cases, when those queries shouldn't respect the users permissions and so I think the impact of that change would be very small. What do you think about that?

Comments

  • LincLinc Detroit Admin

    In Vanilla, we do most permission checks in the controller. Models, in our paradigm, shouldn't question what you want or insert extra logic silently, they're an interface to the database. If you ask it for a discussion, it should return it, no questions asked.

  • R_JR_J Ex-Fanboy Munich Admin

    @Linc said:
    Models, in our paradigm, shouldn't question what you want or insert extra logic silently, they're an interface to the database.

    Thanks for the answer! I must admit it sounds reasonable and I haven't thought about that. That's exactly my understanding of a model.

    But nevertheless if I were doing a framework, I wouldn't be sure if I wouldn't break that logic in favour to security concerns. And I'm happy that I'm not even thinking about developing a framework and so I do not have to make such a decision ;)

  • LincLinc Detroit Admin

    Yeah, and in many situations you will find that it's not necessarily that we're espousing this to be the One True Way, it's just the way we chose and now we're sticking with it. :)

  • LincLinc Detroit Admin
    edited July 2014

    What happens when the model inserts its own logic: https://github.com/vanilla/vanilla/issues/1691 :neutral_face:

    Wrote a nice way to check whether stub content should be inserted (empty forum). Now it gnerates incorrectly on private communities because it gets the wrong count.

Sign In or Register to comment.