Strapi: Create is owner and contributor policy

Created on 12 Feb 2018  路  36Comments  路  Source: strapi/strapi

Node.js version: v8.9.4
npm version: 5.6.0
Strapi version: alpha 9.2
Operating system: Mac OS
Do you want to request a feature or report a bug?
Feature
What is the current behavior?
A user with the update/delete access to a model can update/delete data not created by him.
What is the expected behavior?
Add a policy to prevent user update/delete content not created by him.

medium feature request

Most helpful comment

Is there any workaround to self-implement it for at least destroy and update permissions?

I really miss this feature 馃槥

Edit:

Just did a _very quick basic_ global isOwner policy that I'll use for the DELETE and UPDATE endpoints

Imgur

isOwner.js (global policy for DELETE and UPDATE endpoints)

module.exports = async (ctx, next) => {
  const { role } = ctx.state.user;
  const { url } = ctx.request;

  const fieldName = url.match(/([^\/]+[^\/])/gm)[0]; 
  const fieldId = url.match(/([^\/]+[^\/])/gm)[1];

  const field = ctx.state.user[fieldName];

  if (role.type !== 'root' && field && !field.find(item => item._id.toString() === fieldId)) {
    return ctx.unauthorized(
      'You are not allowed to perform this action! (You are not the Owner)'
    );
  }

  await next();
};

setOwner.js (global policy for POST(create) endpoints)

module.exports = async (ctx, next) => {
  const { _id } = ctx.state.user;
  const { body } = ctx.request;

  body.owner = _id.toString();

  await next();
};

Imgur

All 36 comments

Hey @lucaperret thank you for this proposition.
We plan to make it but not now, it's not in our high priority.

It's possible to create your own policies https://strapi.io/documentation/guides/policies.html to handle your case.

Hi @lauriejim, is it possible to access requested document to check the owner ?

Sadly nop, because your controller can call many different models in the same action.
But if you use the REST url format, you can detect the model that will be called [action] /[model]/[ressource] for GET /article/1337
It's not 100% perfect but can work I think.

There is no proposal in https://strapi.io/vote about it.

Is it enough with 馃憤 the first post?

At the bottom of the page you have a form to submit you proposal.

Yes 馃憤 is a good option too but submit also the feature you want please.

About this isOwner policy... It would be great if it could have multiple owners.

Example: A Book created by multiple Authors. All authors should be able to edit the book.

Thank you for this point @abdonrd

Is there any workaround to self-implement it for at least destroy and update permissions?

I really miss this feature 馃槥

Edit:

Just did a _very quick basic_ global isOwner policy that I'll use for the DELETE and UPDATE endpoints

Imgur

isOwner.js (global policy for DELETE and UPDATE endpoints)

module.exports = async (ctx, next) => {
  const { role } = ctx.state.user;
  const { url } = ctx.request;

  const fieldName = url.match(/([^\/]+[^\/])/gm)[0]; 
  const fieldId = url.match(/([^\/]+[^\/])/gm)[1];

  const field = ctx.state.user[fieldName];

  if (role.type !== 'root' && field && !field.find(item => item._id.toString() === fieldId)) {
    return ctx.unauthorized(
      'You are not allowed to perform this action! (You are not the Owner)'
    );
  }

  await next();
};

setOwner.js (global policy for POST(create) endpoints)

module.exports = async (ctx, next) => {
  const { _id } = ctx.state.user;
  const { body } = ctx.request;

  body.owner = _id.toString();

  await next();
};

Imgur

@pt-br We tried to find a way to implement it. We faced many issues to implement it in a generic way. We should give it a try again with @lauriejim, it could be a really great feature 馃憤

I used policies and changed controller behavior to obtain this. Got insights from @pt-br 's comment.
I don't know somehow the global policies were not working for me so I went with local policies.

I have a Client Model wherein the each client has one user (owner).

Model Relation

isOwner.js (local policy)

module.exports = async (ctx, next) => {
  const { role, id } = ctx.state.user;

  const fieldId = ctx.params._id;

  if (typeof fieldId !== "undefined")
    Client.findOne({ _id: fieldId, owner: id }).then(result => {
      if (!result && role.type !== "administrator")
        return ctx.unauthorized("You are not allowed to perform this action.");
    });

  await next();
};

setOwner.js (local policy)

module.exports = async (ctx, next) => {
  const { _id } = ctx.state.user;
  const { body } = ctx.request;

  body.owner = _id.toString();

  await next();
};

Routes:
setOwner for PUT and POST methods.
isOwner for rest of the methods.

{
  "routes": [
    {
      "method": "GET",
      "path": "/client",
      "handler": "Client.find",
      "config": {
        "policies": ["isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/client/count",
      "handler": "Client.count",
      "config": {
        "policies": ["isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/client/:_id",
      "handler": "Client.findOne",
      "config": {
        "policies": ["isOwner"]
      }
    },
    {
      "method": "POST",
      "path": "/client",
      "handler": "Client.create",
      "config": {
        "policies": ["setOwner"]
      }
    },
    {
      "method": "PUT",
      "path": "/client/:_id",
      "handler": "Client.update",
      "config": {
        "policies": ["setOwner"]
      }
    },
    {
      "method": "DELETE",
      "path": "/client/:_id",
      "handler": "Client.destroy",
      "config": {
        "policies": ["isOwner"]
      }
    }
  ]
}

Hope this helps. :sunglasses:

Thank you @feat7 for these details

based on @feat7 response, i just did a very quick basic global isOwner policy (generic)

isOwner.js (global policy for GET endpoints)

module.exports = async (ctx, next,args) => {
  const { id, role } = ctx.state.user;
  if(role !== "administrator"){
     ctx.query.owner =  id;
  }

  await next();
  if(ctx.params.id){
     let owner = ctx.response.body.get("owner")
     if(owner !== id && role !== "administrator"){
        return ctx.unauthorized("You are not allowed to perform this action.");
     }
  } 
};

routes.json

{
  "routes": [
    {
      "method": "GET",
      "path": "/tests",
      "handler": "Test.find",
      "config": {
        "policies": ["global.isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/tests/count",
      "handler": "Test.count",
      "config": {
        "policies": ["global.isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/tests/:id",
      "handler": "Test.findOne",
      "config": {
        "policies": ["global.isOwner"]
      }
    }, //POST, PUT, DELETE...
  ]
}

Hope this helps!

hi guys, i got really excited about strapi isOwner policy and then find out strapi content-manager plugin don't trigger the any policy. So, a been working to solve the problem. The solution was make a proxy policy form content-manager plugin. This policy aims to be a a proxy between content-manager requests, and at the same time trigger for all the policy (global and scoped) define in each route.json for the api.

Hope this helps!

based on @feat7 response, i just did a very quick basic global isOwner policy (generic)

isOwner.js (global policy for GET endpoints)

module.exports = async (ctx, next,args) => {
  const { id, role } = ctx.state.user;
  if(role !== "administrator"){
     ctx.query.owner =  id;
  }

  await next();
  if(ctx.params.id){
     let owner = ctx.response.body.get("owner")
     if(owner !== id && role !== "administrator"){
        return ctx.unauthorized("You are not allowed to perform this action.");
     }
  } 
};

routes.json

{
  "routes": [
    {
      "method": "GET",
      "path": "/tests",
      "handler": "Test.find",
      "config": {
        "policies": ["global.isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/tests/count",
      "handler": "Test.count",
      "config": {
        "policies": ["global.isOwner"]
      }
    },
    {
      "method": "GET",
      "path": "/tests/:id",
      "handler": "Test.findOne",
      "config": {
        "policies": ["global.isOwner"]
      }
    }, //POST, PUT, DELETE...
  ]
}

Hope this helps!

I've created a card on Product Board (https://portal.productboard.com/strapi/c/44-data-ownership). I'm closing this issue, I think we have enough information and use cases to develop this feature. When we tried to implement it we faced many technical issues due to Mongoose and Bookshelf. I hope we'll find the solution this time.

I've created a card on Product Board (https://portal.productboard.com/strapi/c/44-data-ownership). I'm closing this issue, I think we have enough information and use cases to develop this feature. When we tried to implement it we faced many technical issues due to Mongoose and Bookshelf. I hope we'll find the solution this time.

Where can I keep track of this feature development? I'd really like to follow.

Has this issue been addressed? it is a thing that potentially will make our project abandon strapi to be honest, if the feature isn't no where near.

@deeq14 It has not been started as the Strapi team are entirely focused on the plugin framework rewrite to be able to build more plugins like this.

hi, @mbaez Is this workaround can be used in production?

hi, @mbaez Is this workaround can be used in production?

hi @puneetverma05 i been using this in production with strapi 3.0.0-alpha.14.5, without troubles.If you've found a bug or have a great idea for new feature let me know by adding your suggestion to issues list.

For me it's not working anymore after upgrading to 3.0.0-beta.3. Anyone else having the same problem?

For me it's not working anymore after upgrading to 3.0.0-beta.3. Anyone else having the same problem?

i just implemented it here following the new documentation https://strapi.io/documentation/3.0.0-beta.x/guides/policies.html#how-to-create-a-policy and work well.

If you come from alpha version maybe is because that.

@lucasfontesgaspareto I implemented the same isOwner policy based on above comments but not working I can fetch using POSTMAN and browser as well (from any authenticated user credentials)

@mbaez let me know more about how you implemented as not local nor global policy working for me for find and findOne and also let me know if proxy works for graphql request as well?

I'm not understanding where the issue is guys. Is this not something we can help the project with? Doing this at the policy level probably isn't the right approach, this is a feature that needs to be at the ORM layer.

In Mongoose, you can create filters which are executed post-query for exactly this kind of scenario where an object has a one to one ownership relation with a user. I've done this in a few projects where a custom DAO was needed.

In Bookshelf, everything has a post async function (.then()). The same function could be added in at the ORM layer here too.

What are the technical problems you were having @Aurelsicoko ?

@walkerandco this is why it will be apart of the new DBAL layer as detailed in their sept monthly update

https://strapi.io/blog/monthly-update-september-2019?redirectPage=1

image

Thanks @derrickmehaffy . Maybe the easiest way then to implement for now would be to use data lifecycle (on a per use-case basis by each developer) methods in the models (e.g. beforeFetch) to perform a validation query (is the user the owner of the record). It just creates the pain of a custom implementation for every developer, but at least there are ways of integrating.

Right now the best way is via the policy or at the controller level. Lifecycle callbacks have their own major issues (that will be addressed by the same DBAL layer)

I see, so you're saying extend the actions @derrickmehaffy? Do you know of any example of this working in practice? I'm trying to solve this issue at the moment for my use-case so it would be great reference.

Out of interest, what are the issues with the lifecycle callbacks?

at the controller or policy level just reading the JWT data via ctx.state.user (could be id, name, role, ect) and comparing it to a value in the database. likewise for setting a default user value you could do the same thing ( talking about find/findOne vs create/update ).

Lifecycle callbacks: https://github.com/strapi/strapi/issues/1443

Thank @derrickmehaffy for the info.

I think in that case if we're overriding the methods in the controller, the usual way is to have conditional logic which is feasible to implement by these means (the equivalent of UPDATE ... WHERE...). I would probably say this is more efficient than a policy because the examples above are doing a pre-lookup on the user, so two queries and two round trips - this can't work well at scale. A better way would be to leave that assessment to the DB in conditional logic for the query, so I'll do that and post a gist here for anyone solving the same problem until DBAL is added in future.

3.0.0-beta.16.8

isOwnenr.js

'use strict';

/**
 * `isOwner` policy.
 */

module.exports = async (ctx, next,args) => {
  const { id, role } = ctx.state.user;
  if(role !== "administrator"){
    ctx.query.owner =  id;
  }

  await next();
  if(ctx.params.id){
    const owner = ctx.response.body.owner.id;
    if(owner !== id && role !== "administrator"){
      return ctx.unauthorized("You are not allowed to perform this action.");
    }
  }
};

setOwner.js

'use strict';

/**
 * `setOwner` policy.
 */

module.exports = async (ctx, next) => {
  const { id } = ctx.state.user;
  const { body } = ctx.request;

  body.owner = id.toString();

  await next();
};

I found all the samples above are not working properly and insecure as well.
Here is mine global isOwner policy which supports all API Endpoints (create, update, delete, find all, find one and count).

*Note: Use owner as the field name when defining relation to a user table

strapi/api/message/config/routes.json

{
  "routes": [
    {
      "method": "GET",
      "path": "/messages",
      "handler": "Message.find",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    },
    {
      "method": "GET",
      "path": "/messages/count",
      "handler": "Message.count",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    },
    {
      "method": "GET",
      "path": "/messages/:id",
      "handler": "Message.findOne",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    },
    {
      "method": "POST",
      "path": "/messages",
      "handler": "Message.create",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    },
    {
      "method": "PUT",
      "path": "/messages/:id",
      "handler": "Message.update",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    },
    {
      "method": "DELETE",
      "path": "/messages/:id",
      "handler": "Message.delete",
      "config": {
        "policies": [
          "global.isOwner"
        ]
      }
    }
  ]
}

strapi/config/policies/isOwner.js

'use strict';
/**
 * `isOwner` policy.
 */


var pluralize = require('pluralize')

module.exports = async (ctx, next) => {
    try {

        let errMsg = "You are not allowed to perform this action.";

        const { id, sid } = await strapi.plugins['users-permissions'].services.jwt.getToken(ctx);

        if (typeof id === "undefined" || typeof sid === "undefined" || id === null || sid === null) {
            return ctx.unauthorized(`${errMsg} [1]`);
        }


        // [find, count] Only query entities that owned by the current user
        if (ctx.request.method === 'GET') {
            // Remove everything about owner in the query eg. owner.id_in, owner.id, owner etc.
            for (let key in ctx.query) {
                if (key.includes("owner")) {
                    delete ctx.query[key];
                }
            }

            // Reset owner.id to current user id
            ctx.query = Object.assign({ 'owner.id': id }, ctx.query);
        }

        // [update, delete] Check owner of an existing entity
        if ((ctx.request.method === 'DELETE' || ctx.request.method === 'PUT') && typeof ctx.params !== "undefined" && ctx.params !== null && typeof ctx.params.id !== "undefined" && ctx.params.id !== null && ctx.params.id !== '') {
            // Extract model name from request path eg. /messages/15
            var modelName = ctx.request.path.match(/^\/(.*)\/\d*$/);
            if (Array.isArray(modelName) === false || modelName.length !== 2 || modelName[1] === '') {
                return ctx.unauthorized(`Invalid request ${ctx.request.path}`);
            }
            // Get existing entity and check for ownership
            let existingEntity = await strapi.query(pluralize.singular(modelName[1])).findOne({
                id: ctx.params.id
            });
            if (typeof existingEntity === "undefined" || existingEntity === null) {
                return ctx.notFound('Not Found');
            }

            if (typeof existingEntity.owner === "undefined" || existingEntity.owner === null || typeof existingEntity.owner.id === "undefined" || existingEntity.owner.id === null || existingEntity.owner.id.toString() !== id.toString()) {
                return ctx.unauthorized(`${errMsg} [2]`);
            }

            // Set owner to current user
            ctx.request.body.owner = id.toString();
        }

        // [create] Set owner for a new entity
        if (ctx.request.method === 'POST') {
            ctx.request.body.owner = id.toString();
        }

        await next();

        // [find.one] Only query entities that owned by the current user
        if (ctx.request.method === 'GET') {
            if (Object.prototype.toString.call(ctx.response.body) === '[object Object]') {
                if (typeof ctx.response.body.owner === "undefined" || ctx.response.body.owner === null || typeof ctx.response.body.owner.id === "undefined" || ctx.response.body.owner.id === null || ctx.response.body.owner.id.toString() !== id.toString()) {
                    return ctx.unauthorized(`${errMsg} [3]`);
                }
            }
        }

    } catch (error) {
        return ctx.unauthorized(error);
    }

};

@alidawud Hey, Just wondering, what is SID that the getToken function is supposed to return? because for me it is always undefined and therefore the function gets returns with unauthorized access. However, after removing it from the first if statement everything seems to be fine?

@Arman-AP please remove sid. We added sid (session id) to our project for better tracking user activity. I forgot to remove it from the sample code.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

niallobrien picture niallobrien  路  47Comments

dsheyp picture dsheyp  路  46Comments

djbingham picture djbingham  路  40Comments

dannydtk picture dannydtk  路  43Comments

mnlbox picture mnlbox  路  37Comments