Question

I have app/controllers/UsersController.php that does a simple Users::find('all'); in the index action.

The path /users/index renders plain 'ol HTML output of the users data. The path /users/index.json render the JSON equivalent of the HTML output which is great except for the fact that it also exposes the password (which is hashed, but still...).

I see two options to avoid this:

  1. Explicitly specify fields in my finder.
  2. Filter Media::render() and unset any sensitive data.

I feel #2 may be easier to maintain, in the long run. Any opinions? Is there a third, better, alternative?

This is how I've implemented #2:

<?php

namespace app\controllers;

use \lithium\net\http\Media;

class UsersController extends \lithium\action\Controller {
    protected function _init() {
        Media::applyFilter('render', function($self, $params, $chain) {
            if ($params['options']['type'] === 'json') {
                foreach ($params['data']['users'] as $user) {
                    $user->set([
                        'password' => null,
                        'salt' => null
                    ]);
                }
            }
            return $chain->next($self, $params, $chain);
        });
        parent::_init();
    }
}
?>

Any advice would be appreciated.

Was it helpful?

Solution

This question could have a lot of answers and ways to do it, depending on your app, maintainability, elegance of your architecture, etc... In the case you want only to remove sensible fields like the user password, your solutions do the job.

But!

Filtering Media::render() doesn't seems to be a good idea at all. You are mixing concerns here, and you'll end up with a bloated filter where you tweak an object to remove what you don't want to expose in your json responses.

using fields could be not good enough if you have to dot it each time, for each controller in your app. And worse, if your entities have 30+ fields, and depending on the current user, show different pieces of information (OMG)! You'll end up with a bloated controller, where, again, you are mixing concerns and responsibilities: find() is responsible of reading your data, and fields thing is only to change the presentation (sort of view) of your data.

So? What could we do?

duplication controller logic
You could separate the filtering logic in your controller by enclosing it into a if ($this->request->is('json')) { ... } That means the same controller action respond differently if the request is html or json (a public api).
This isn't good too :)
A slightly better approach, is to split things a bit by having duplicated controllers => The first set is responsible for you json api, and the second for your "classic" controllers that respond to html.
You could do this easily with Lithium by adding a controllers/api namespace, and reconfiguring the Dispatcher to use this path in case of a json request/response.

li3_jbuilder
I'm not that happy with duplicating controllers in some cases. A better approach is to use the V part of the MVC but this time to render json responses, and handle those as first class objects: json views !
This could be done easily by tweaking Media class configuration, and having a fallback mechanism (if a *.json.php is not found, json_encode the object without filtering fields).
I built li3_jbuilder for Lithium, to make it easy to build json responses, nest objects, make use of helpers, and move the "presentation" aspect to the view layer.
Jbuilder is inspired by Rails' jbuilder. FYI, the ruby community got RABL too.

Presenter Pattern
While this approach seems simple, there is another interesting one, more object oriented: Use Presenter pattern (or Decorator).
A User Model, is associated to a UserPresenter class (plain old php class), responsible for providing objects to be "presented", especially in json responses (or anywhere in your app).
Presenters help you to clean up complex view logic too, are testable, and very flexible.
The presenter needs to know about the model and the view it will be dealing with so you'll pass these in to an initialize method and assign them to instance variables.
Just google for "Presenter pattern", or "Rails presenters" (the only framework I used that make use of this pattern), to know more on the subject

OTHER TIPS

Specifying fields explicitly has several advantages:

  • you don't get data that you don't need, so it can be potentially faster
  • you can't leak the data by accident if you forget to unset it
  • as you specify which fields you need, you'll get an early warning if the JSON format would change

It's a similar principle as not doing SELECT * FROM in SQL.

I had the same problem, I was printing emails and passwords when you added .json to the path.

So, since I am using MySql and I declared my $_schema in all models I did a little trick... I added a 'public' => true to all fields that I want to be requested from the database and used that in all queries like this:

$users = Users::find(array('fields' => Users::publicFields()));

and the publicFields method looks like this:

public static function publicFields() {
    $self = static::_object();

    $className = $self->meta()['name'];
    $schema = $self->schema();

    $fields = array_filter($schema->fields(), function($var) {
        return !empty($var['public']);
    });

    $names = array_keys($fields);

    for ($i = 0, $iMax = count($names); $i < $iMax; $i++) {
        $names[$i] = $className . '.' . $names[$i];
    }

    return $names;
}

Same here. The unset(var) method is really dirty and dangerous.

I need my users object in all of my views for rendering the main menu and do some user interaction. The Controller::Render Method provides this extra class via $this->set().

In my userscontroller I created a new php class "DSMember". This object takes some public properties I need in my views. Passwords and security related stuff is not provided here.

So you have a clear cut between user presentation logic (DS = Display) and core related stuff.

class DSMember
{
    public $id;
    public $profile;
    public $uuid;
    public $messages;

    function __construct ($user) //$user is the Users::Object
    {
        $this->id = $user->id;
        $this->uuid = $user->uuid;
        $this->profile = $user->user_profile;
        $this->messages = $user->messages;
    }

}

The render method is overloaded:::

public function render (array $options = array())
    {
        if ($this->session)
        {
            $member = new DSMember ($this->member);
            $this->set (compact ('member'));
        }
        parent::render ($options);
    }

So the DSMember Object is available in all HTML Views and JSON render outputs. The hot users model is hidden.

There are a lot of different methods mentioned above. For my app this way seems to be good.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top