User property is not being sent with 2.0 master

[ianjevans]

I'm testing 2.0 master with our internal docs review system, and it appears that the identity module is not properly setting the user property of the annotation when creating it.

Here is our setup:

function addAnnotations() {

    var username = readCookie("dsdocs.review.username");
        if ((!username)||(username=='null')) {
        username = prompt("Enter your email address to comment on the docs: ", "");
        createCookie("dsdocs.review.username", username, 365);
    }

    console.log('username is ' + username);

    var pageUri = function () {
        return {
            beforeAnnotationCreated: function (ann) {
                ann.uri = window.location.href;
            }
        };
    };

    var app = new annotator.App();
    app.include(annotator.ui.main, {element: document.article});
    app.include(annotator.storage.http, {prefix: '/api'});
    app.include(pageUri);
    app.include(annotator.identity.simple);

    app.start()
        .then(function () {
            app.ident.identity = username;
            app.annotations.load({uri: window.location.href});
    });
}

Note that I'm logging the username to the browser's console.

The store is based on annotator-store, and is a Flask app. Here's the POST method, where I'm logging the user from the request.

@app.route('/annotations', methods=['POST'])
def create():
    if request.get_json() is not None:
        cursor = get_db()
        try:
            annotation = Annotation(request.get_json())
        except KeyError:
            return jsonify('Not all fields included.', status=400)

        app.logger.info('user is %s', annotation.user)
        annotation.save(cursor)

        return jsonify(annotation.dict()), 201
    else:
        return jsonify('No JSON annotation sent', status=400)

Annotations are being created, but the user property is always None. E.g.

  {
    "resolved": false, 
    "updated": 1446768162093, 
    "created": 1446768162093, 
    "text": "Why is the username not being stored?", 
    "tags": [], 
    "uri": "http://notus/en/tutorials/kerberos/kerberos/settingUp.html", 
    "schema_version": "1.0", 
    "ranges": [
      {
        "start": "/div[1]/div[2]/div[2]/article[1]/h1[1]", 
        "end": "/div[1]/div[2]/div[2]/article[1]/h1[1]", 
        "startOffset": 8, 
        "endOffset": 27
      }
    ], 
    "user": "None", 
    "quote": "up your environment", 
    "id": "cb5ad0b0-92a0-478c-805b-4820bbc0d247", 
    "permissions": {
      "read": [], 
      "admin": [], 
      "update": [], 
      "delete": []
    }
  }

Despite the console showing that username is being properly set:

username is ievans@datastax.com

[ianjevans]

Added some debug statements to make sure the user is set.

    app.include(annotator.storage.debug);

    app.start()
        .then(function () {
            app.ident.identity = username;
            console.log("user is " + app.ident.who());
            app.annotations.load({uri: window.location.href});
    });

This seems correct:

But user is not being included when the annotation is created:

[tilgovi]

You're correct. It seems this functionality, which was part of the Permissions plugin in v1.2.x, was never ported over.

We could do so. I'm not sure exactly where / how we would want to do this automatically as v2 is on track to enforce few or no requirements about the annotation properties.

You can do it yourself like this, though:

app.include(function() {
  return {
    beforeAnnotationCreated: (annotation) {
      annotation.user = app.ident.who();
    }
  }
});

If you don't need to have the user set client-side before the annotation is saved you can also add it server-side. You should do this anyway to enforce that the client can't set whatever user it wants without the server enforcing that it matches the JWT.

[ianjevans]

I think in 2.0 you've split out the identity and permissions, and include the SimpleIdentityPolicy class to provide similar functionality to 1.2.x. I feel that SimpleIdentityPolicy should allow you to set the user client-side (similar to 1.2.x), and that any server-side identity should be in a different, custom policy.

So, this change to identity.js allows you to set the user on new annotations as I expected.

/**
 * function:: simple()
 *
 * A module that configures and registers an instance of
 * :class:`annotator.identity.SimpleIdentityPolicy`.
 */
exports.simple = function simple() {
    var identity = new SimpleIdentityPolicy();

    return {
        configure: function (registry) {
            registry.registerUtility(identity, 'identityPolicy');
        },
        beforeAnnotationCreated: function (annotation) {
            annotation.user = identity.identity;
        }
    };
};

I can create a pull request if you'd like.

[tilgovi]

Ah, yes! I had forgotten that there was an extension module for the identity policy like this. Yes, the simple identity policy should have such a beforeAnnotationCreated function.

Please, open a PR! That would be lovely. Thanks!

[ianjevans]

PR here: https://github.com/openannotation/annotator/pull/576

[tilgovi]

Fixed on master.