Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permissions based on content owner - not just role #1640

Closed
growandwin opened this issue Jan 14, 2021 · 8 comments
Closed

permissions based on content owner - not just role #1640

growandwin opened this issue Jan 14, 2021 · 8 comments

Comments

@growandwin
Copy link

Hi All,

Love your work! I think redwood is going to be the future!
I want to contribute by posting some questions about permissions ;)

How to give permissions i.e. for authors only to the pages/cells/content - they created - not just based on roles ?
Like in github for example. Everyone is a user, but by default only has write access to his own repositories and has access to view and edit his own profile.

I suppose you should make a service to check if a user has the privillege in a database to a given page. Is there a nice standard way to architecture this in redwood not to overkill the database with each page view?

@pi0neerpat
Copy link
Contributor

This would be cool. Here is an example of how I am checking ownership before returning data.

// src/lib/auth.js
export const isOwner = (id) => {
  if (context.currentUser.id !== id) {
    throw new AuthenticationError('Only the dapp creator can do that.')
  }
}

Then in your services

// services/users/users.js

import { requireAuth, restricted, isOwner } from 'src/lib/auth'
...
export const user = async ({ id }) => {
  const user = await db.user.findUnique({ where: { id } })
  isOwner(user.id)
  return db.user.findUnique({
    where: { id },
  })
}

@growandwin
Copy link
Author

growandwin commented Jan 15, 2021

Thank you for your example.

Why don't you just return "user" instead of making the same querry like 2 rows above the return? here:

return db.user.findUnique({
    where: { id },
  })

So... you still need to have a table in the db with each user and his indivdual contents ids ? You don't have a way to store this in netlify identity or auth0?

@pi0neerpat
Copy link
Contributor

Right, you could get id from there. I'm using my own auth solution here, with no external auth client, so I have to get the id from my own database https://github.com/oneclickdapp/ethereum-auth

Also you're right about not having to do the user query twice for this example

@ccnklc
Copy link
Contributor

ccnklc commented Jan 15, 2021

I would suggest anyone, who is looking into managing permissions in graphql, to check https://github.com/maticzav/graphql-shield it is the best library I know, and I am sure that you will love it too and you will not look for anything else.

@thedavidprice
Copy link
Contributor

@dthyresson any thoughts here? Seems like there might be a possible use for adding an example to a related cookbook/doc. And/or maybe adding this as an example to the Forum for future knowledgebase.

@dthyresson
Copy link
Contributor

dthyresson commented Jan 16, 2021

I don't think RedwoodJS or I have a strong opinion yet as to the best pattern -- often data permissions are app/org-specific so generalizing them can be tricky. But, yes, a few thoughts:

1 -

Re @ccnklc

who is looking into managing permissions in graphql, to check https://github.com/maticzav/graphql-shield it is the best library I know

I have been using graphql-shield in most of my projects -- up until a few days ago when Prisma 12.1 was causing some issues with. However, I've upgraded to 12.4 ahead of time and will look at using it again ... the 12.1 issues may have contributed to it. I agree, it's quite good.

2 -

the example @pi0neerpat shows:

// src/lib/auth.js
export const isOwner = (id) => {
  if (context.currentUser.id !== id) {
    throw new AuthenticationError('Only the dapp creator can do that.')
  }
}

// services/users/users.js

import { requireAuth, restricted, isOwner } from 'src/lib/auth'
...
export const user = async ({ id }) => {
  const user = await db.user.findUnique({ where: { id } })
  isOwner(user.id)
  return db.user.findUnique({
    where: { id },
  })
}

is reasonable, however I would:

  • throw a ForbiddenError vs an AuthenticationError

throw new ForbiddenError("You don't have access to do that.")

and also not findUnique twice.

I might consider combining the logic to identify ownership and also requireAuth() in isOwner()

3 - Another approach is to query on the "relationship" that establishes ownership or authorship:

Let's say you can only delete posts you published and that publisherId is a user id stored on the Post:

export const deletePost = ({ id }) => {
  requireAuth({ roles: DELETE_POST_ROLES })

  return db.post.delete({
    where: { id },
  })
}

can become

export const deletePost = ({ id }) => {
  requireAuth({ roles: DELETE_POST_ROLES })

  return db.post.delete({
    where: { id, publisherId:  context.currentUser.id },
  })
}

Or only update a Post that you authored where authorId stores the user's id:

export const updatePost = ({ id, input }) => {
  requireAuth({ roles: UPDATE_POST_ROLES })

  return db.post.update({
    data: {
      ...input,
      updatedAt: new Date(),
    },
    where: { id, authorId: context.currentUser.id },
  })
}

effectively, it cannot find the model to delete or update due to the where clause.

I think you could also join in on the User table if you have one where Author is a User in a one-many relationship to Post.

See: https://www.prisma.io/docs/concepts/components/prisma-client/filtering/#filter-on-related-records

export const updatePost = ({ id, input }) => {
  requireAuth({ roles: UPDATE_POST_ROLES })

  return db.post.update({
    data: {
      ...input,
      updatedAt: new Date(),
    },
    where: { id, author: { id: context.currentUser.id } },
  })
}

I'm not sure if this will raise an exception so may way to try catch and throw a ForbiddenError or if the you would be better off storing in post and if post is null/undefined then throw a ForbiddenError or return the updated post.

@pi0neerpat
Copy link
Contributor

pi0neerpat commented Jan 19, 2021

These are good suggestions @dthyresson

I've also setup graphql-shield for a non-redwood project. I found it to be overly complex for simple tasks, and somewhat confusing to track down errors. It took me a few days to get comfortable with. I guess it could be a good candidate to "opinionated wrap" it with redwood. At that point, now we are talking about making another "auth-connector" for permissions, similar to the way it is for providers.

Until then, I think keeping the current "write your own permissions checks" is totally sufficient + cookbooks for @dthyresson s wisdom of using shield+redwood

Is there a forum discussion for this yet?

@dthyresson
Copy link
Contributor

@thedavidprice I think we can close out this issue -- or create a new one for a cookbook or other docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants