prisma / prisma

Next-generation ORM for Node.js & TypeScript | PostgreSQL, MySQL, MariaDB, SQL Server, SQLite, MongoDB and CockroachDB
https://www.prisma.io
Apache License 2.0
39.86k stars 1.56k forks source link

Add way to exclude fields via query arguments or globally #5042

Open pantharshit00 opened 4 years ago

pantharshit00 commented 4 years ago

Problem

Sometimes it is helpful to just remove some fields from the selection set like password so that we accidentally don't expose them to the frontend.

Proposals

Adding a exclude key in the arguments of the queries to easily exclude fields from the selectionSet just like include or select.

prisma.user.findMany({
  exclude:{
    password: true
  }
})

Or

await prisma.user.findMany({
  omit: {
    password: true,
  },
})

Alternatives

  1. Use query and list all the fields you want
  2. Schema annotation
  3. Manual omission via lodash or something !?

Related

thecodeah commented 4 years ago

@pantharshit00 Despite our suggestions being very similar, I still believe the method I had suggested is better. Instead of having to remember to exclude certain fields every single time we fetch something (Which can still accidentally be forgotten) you specify it once in the Prisma Schema and explicitly request it when it's needed.

https://github.com/prisma/prisma/issues/2498

pantharshit00 commented 4 years ago

Hey @thecodeah, I think I linked the same thing twice in your issue. Added a note in your issue: https://github.com/prisma/prisma/issues/2498#issuecomment-630722344

ghost commented 4 years ago

At least for me, this is a very wanted feature now because TS forbids using the delete operator since TS 4.0:

https://github.com/microsoft/vscode/issues/96022#issuecomment-635729089

20200825184009

As a workaround I'm using the object rest syntax { password, ...userWithoutPassword } = user, but the unused variable warnings are kinda annoying.

darioielardi commented 4 years ago

This becomes an even bigger problem when the column I want to hide is in a nested object as a result of an include query.

I think an omit clause would work but a schema annotation would be even better, as suggested by @thecodeah.

matthewmueller commented 4 years ago

@thecodeah, @darioielardi, and @flybayer expressed concerns with this proposal. Each query you write needs to explicitly exclude private fields. They would prefer an "exclude-by-default" approach.

To complicate matters, being able to somehow access excluded fields from the Prisma Client is important for tests. Any solution we decide on needs to have this feature.

Solution A: add an attribute to the schema

One idea proposed by @darioielardi is to add an attribute in the schema that will exclude a field. @flybayer suggested that if you want to include an excluded-by-default field (for tests) you could select that field explicitly.

Concretely, that would look like this:

model User {
  id Int @id
  password String @hidden // or @private or @exclude
}
prisma.user.findFirst({})
// { id: 1 }

prisma.user.findFirst({
  select: { 
    password: true
  }
})
// { password: "8674305" }

The downside of this is that when you do want to include an excluded field, you need to explicitly select all the other fields, like id.

Solution B: exclude when you initialize the client

Another possibility is excluding fields when you initialize the client. Something like:

const prisma = new PrismaClient({
  exclude: { 
    user: {
      password: true 
    }
  }
})

What do you think? Do you have other ideas on how we could solve this?

flybayer commented 4 years ago

I would prefer it in the schema so you can see everything in one view and easily see if you forgot to hide a certain field. Whereas if it's in another file (and one that is rarely viewed), you can easily forget about it.

darioielardi commented 4 years ago

I agree with @flybayer, I would definitely prefer to have it in the schema.

However I'm concerned about the need to add every field to the select object to explicitly select a hidden column.

What about something like an additional expose field exclusively aimed to select hidden columns? Something like:

model User {
  id Int @id
  name String
  password String @hidden
}
prisma.user.findMany({
  expose: { password: true }, // no need to explicitly select the other fields
})

That's probably a bit too much, but it might work.

ValentinFunk commented 4 years ago

I think a feature where you list the fields that are included by default would be better than a list of fields that are excluded, because as you add new fields to the db someone will forget to add @hidden and accidentally expose it in some API that simply returns say the result of the findOne as JSON. Maybe you could instead explicitly list the fields that are returned by default? Adding to that list will make it intentional, whereas when just adding a field to the DB you probably didn't want to change your API.

You could also leave the decision (list include or list exclude) up to the developer. Algolia does this for example here: https://www.algolia.com/doc/api-reference/api-parameters/attributesToRetrieve/. On an "index" which is basically a table you can specify which fields will be retreived by the client by default. It can be overwritten at query level. Here is how they solved it:

'attributesToRetrieve' => [
  'attribute1', // list of attributes to retrieve
  'attribute2'
]
'attributesToRetrieve' => [
  '*' // retrieves all attributes
]
'attributesToRetrieve' => [
  '*', // retrieves all attributes
  '-attribute1', // except this list of attributes (starting with a '-')
  '-attribute2'
]

Maybe this approach could work as well?

scriptcoded commented 4 years ago

I think @Kamshak's point about forgetting to add @hidden to a field makes sense to some degree, although at the same time I believe that the majority of fields on any given model will not be hidden. This means that for every field added you also have to explicitly mark those fields as non-hidden.

const prisma = new PrismaClient({
  exclude: { password: true }
})

I also believe that there is an issue with this method of excluding fields suggested by @matthewmueller. There might might be multiple models with the same field, but all models shouldn't necessarily have them be hidden. Also, it might become unclear that a field is hidden when creating a new model and the creator of that model is unaware of a field being excluded "globally".

The one time I could see this being useful would be when you want to exclude something on all models, like createdAt. (see https://github.com/prisma/prisma-client-js/issues/649#issuecomment-712228596)

Personally I think adding a @hidden attribute makes a lot more sense as it reduces the amount of code necessary for each model, keeps the option at model level, and is only adding an attribute which makes it backwards compatible.

matthewmueller commented 4 years ago

Thanks @scriptcoded, that's indeed a mistake in the code on my part, the proposal should be:

const prisma = new PrismaClient({
  exclude: { 
    user: {
      password: true 
    }
  }
})
samrith-s commented 4 years ago

While this is good, we would still have to manually exclude things everywhere we use it, right?

Or does this mean we simply override parameters in the middleware?

YannSuissa commented 4 years ago

Any progress on this exclude issue? I'm struggling to hide a private field.. on client side it's easy to hide it but if you use a graphql tool like playground it's easy to hack. An exclude keyword would be perfect. Thanks

benjibuiltit commented 3 years ago

Came here looking for a similar feature. I'll add my input that I'd also like to see an attribute at the model level and then an explicit opt-in when necessary to avoid any accidental leaking.

I'll also offer up an alternative workaround for those who may stumble across this issue before the feature is released. You can create a model for the sensitive data which you want to hide and use a relation to map between the main model and the sensitive data. The sensitive data will live in an alternate table which you can query for when you do need it, but your query for your main model won't have to be "massaged" so frequently. For example:

model Users {
  id  String  @id @default(uuid())
  email String
}

model HashedPasswords {
  userId String @id
  user Users @relation(fields: [userId], references: [id])
  password String
}

This will allow you to execute a nested query w/ a transactional guarantee that your sensitive data is written/created w/ your main model.

const result = await prisma.hashedPasswords.create({
      data: {
        password: hashedPassword,
        user: {
          create: {
            email
          }
        }
      }
    });

Because the sensitive data exists in a separate table you can query your main table w/o worrying about leaking anything.

And finally, to explicitly fetch the password for validation...

const password = await prisma.hashedPasswords.findUnique({
      where: {
        userId: {
          equals: userId
        }
      }
})
ragokan commented 3 years ago

I do totally agree on this, especially for the things like password that you don't want to show to users. Mongoose have a great feature, -select which just removes password, I hope that prisma listens us.

muhdfaiz commented 3 years ago

Hi, I'm looking for a solution to hide the field and found this topic. Just want to give input that I prefer the way mentioned by @matthewmueller by specifying @hidden or @exclude or @private in the schema.

IamManchanda commented 3 years ago

exclude is really needed especially for hiding "column id's" & passwords

We can do it either like

    const users = await prisma.user.findMany({
      include: { posts: true },
      exclude: { id: true, password: true }
    });
    return res.status(200).json(users);

or, as @matthewmueller specified, using @hidden or @exclude or @private in the schema

itsjbecks commented 3 years ago

I'd vote in favour of the @private on the schema.

I'd argue that if I'm adding sensitive data to the database I'm generally aware it's sensitive at that point. I imagine it would be far easier to neglect explicitly writing exclude: { password: true } when another developer (or let's face it, even me) is creating an endpoint related about fetching posts & comments

Having to also explicitly exclude sensitive data fields each time feels like extra work to get baseline security into products.

That all being said, something I think important to consider is the unintended side-effects of naming collisions.

const users = await prisma.user.findMany({
  include: {
    private: true
  },
})

const users = await prisma.user.findMany({
  include: {
    private: {
      password: true,
    },
  },
})

This no longer would allow private/hidden/exclude to be columns or relations.

model User {
  id        Int     @id
  name      String
  password  String  @private
  private   Boolean
}
omar-dulaimi commented 3 years ago

Can't wait to test this feature in 2.18.0 this tuesday I'm currently facing a use case regarding hiding certain fields from playground/client

janpio commented 3 years ago

Can't wait to test this feature in 2.18.0 this tuesday

This issue was not closed, so this will unfortuately not be part of 2.18.0.

HartS commented 3 years ago

At least for me, this is a very wanted feature now because TS forbids using the delete operator since TS 4.0:

microsoft/vscode#96022 (comment)

20200825184009

As a workaround I'm using the object rest syntax { password, ...userWithoutPassword } = user, but the unused variable warnings are kinda annoying.

Thanks for the super helpful tip @joaopaulobdac . Just starting with prisma, but with eslint the warnings can be turned off if you prefix unused vars with _ and set something like no-unused-vars: ["warning", { "argsIgnorePattern": "^_" }] in your eslintrc . I'd hope most other linters would have a similar option. The convention of using a _ prefix just makes too much sense to me, as there are so many situation where unused vars are required with JS/TS

faisalsayed10 commented 3 years ago

ooh waiting for this to be released soon. This is a really important feature so that I can exclude the password field from the user.

WilliamBlais commented 3 years ago

this would be incredibly useful!

faisalsayed10 commented 3 years ago

Um so this didn't came out with prisma 2.19.0?

mhdalmajid commented 3 years ago

the first query I typed in Prisma , asked my self whyyyy there is no exclude !!!!! I have to select all fields and pass false to password

export const getUserById = (id: number, includePassword = false) =>
  db.user.findUnique({
    where: {
      id,
    },
    select: {
      email: true,
      password: includePassword,
      id: true,
      name: true,
      createdAt: true,
      role: true,
      posts: true,
      profile: true,
      updatedAt: true,
    },
  })

would be greet if I just do

export const getUserById = (id: number) =>
  db.user.findUnique({
    where: {
      id,
    },
    exec: ["password"]
    //or 
    execlude:{ 
       password : true
     }
  })

for now I will use this method

export const getUserById = async (id: number, includePassword = false) => {
  const user = await db.user.findUnique({
    where: {
      id,
    },
  })
  if (includePassword) delete user.password
  return user
}
ovidb commented 3 years ago

I had a go trying to solve this at the middleware level, but sadly it's not currently possible because there is no way of passing custom parameters to the query functions.

Instead of me pasting how would someone go about doing it I suggest upvoting the feature request that @samrith-s has opened. It precisely describes this use case https://github.com/prisma/prisma/issues/4217

kuasha420 commented 3 years ago

It's so sad that this important feature keeps getting pushed back :(

Gouet commented 3 years ago

Doing : const allUsers = await prisma.user.findMany({ select: { password: false, id: true, name: true, profile: true } }) works. But you have to set all attributes to true...

Dammmien commented 3 years ago

Guys if you want to be the "Next-generation ORM for Node.js and TypeScript", you already have to equal the current generation. There are different ways to exclude fields in Sequelize and TypeORM. I can't continue to list the 30 fields of my entities except one. Anyway I would be glad to help.

raymclee commented 3 years ago

Guys if you want to be the "Next-generation ORM for Node.js and TypeScript", you already have to equal the current generation. There are different ways to exclude fields in Sequelize and TypeORM. I can't continue to list the 30 fields of my entities except one. Anyway I would be glad to help.

yep. this is a must feature for the Next-Generation ORM 😅

matthewmueller commented 3 years ago

Hey all, sorry for the lack of updates on this one. The TL;DR is we're still working on it.

To clarify the possible solutions:

1. Exclude per query

prisma.user.findMany({
  exclude:{
    password: true
  }
})

The original suggested solution. This is the easiest solution, but we probably won't go with it because it doesn't add much value over delete user.password.

2. Exclude by default in the Schema

model User {
  id        Int     @id
  name      String
  password  String  @exclude
  private   Boolean
}

This is what most people want, but we're trying to figure out how and the ramifications of doing that. It's definitely the most complicated. How does it play with cursors, order by, etc.

3. Top-level Prisma Client Settings

const prisma = new PrismaClient({
  exclude: { 
    user: {
      password: true 
    }
  }
})

People seem lukewarm about this solution and it's also not clear how we'd make this type-safe.


Hopefully this clarifies the hold up! If you have any additional thoughts please chime in!

camsloanftc commented 3 years ago

Personally I would be happy with number 1 (per query) and it does add more than delete user.password in the case of querying relationships. Currently I need to map over all returned relation items to delete the unwanted properties.

Number 2 (Excluding it in the schema) would work as well as long as I still have the capability to "include" something on a per-query basis. Would that be in the plans?

HartS commented 3 years ago

@camsloanftc said it perfectly. 1 is really what I wanted/expected here. 2 and 3 are not as intuitive, but if they make getting the password opt-in only, they might be even better. A very typical use case is to only ever access the password when creating new sessions (comparing the hash with the login password), creating the user, and handling a password update/reset.

scriptcoded commented 3 years ago

Thanks for the update! I've made my voice heard before, but just wanted to bring something up. Most other traditional ORMs make use of user written models to map the data to the code. This step usually gives the user the ability to do pretty much what they want with the data before it's consumed, including removing sensitive data. Since Prisma doesn't provide this functionality (although rightfully so) I think it's important that features like this one actually make it in to the Schema, and not only the query.

With that said I do believe what @camsloanftc has to say about excluding fields in the query is still valid, as it makes the returned data easier to work with.

luxaritas commented 3 years ago

Also: delete user.password does not play well with TypeScript! Doing so with my config at least throws an error: The operand of a 'delete' operator must be optional.ts(2790)

flybayer commented 3 years ago

While (1) does provide some benefit, it doesn't address the security issue of default behavior. Whereas (2) allows us at Blitz to mark tokens and passwords as private by default for all queries. Without this, users write new queries against the user table and they don't even think about the need to filter sensitive information, resulting in security leaks.

Dammmien commented 3 years ago

If 1. is the easiest solution, it can be a good workaround waiting 2.

Samuel-Sorial commented 3 years ago

The second solution is the best one from security perspective

BuistvoPloti commented 3 years ago

don't use delete operator as it is slow. use lodash instead: _.omit(object, [paths])

bencun commented 3 years ago

How about you guys provide us with solution number 1 to ease the pain of doing something as simple as this - since it would be a non-breaking change - and then work on the number 2 or whatever you decide? This is extremely important to have as we probably don't want to explicitly rely on using delete everywhere.

It seems to me like the first solution wouldn't be very technically challenging to implement at this point and it would be an awesome starting point for a lot of us that jumped on the "next-gen ORM" train.

bencun commented 3 years ago

Also, this is a middleware we came up with to try and partially resolve this - we wanted to be able to hide the user password field by default. A flaw here is that you have to manually maintain the list of properties to select when params.args.select is undefined. Please note that this code is not thoroughly tested:

async connectHidePasswordMiddleware() {
  this.$use(async (params, next) => {
    if (params.model === 'User') {
      if (
        params.action === 'findUnique' ||
        params.action === 'findMany' ||
        params.action === 'findFirst'
      ) {
        if (!params.args['select']) {
          params.args['select'] = {
            uuid: true,
            email: true,
            password: false,
            // don't forget all of your other model fields here...
          };
        }
        else if (params.args['select']) {
          params.args['select'] = {
            ...params.args['select'],
            password: params.args['select'].password || false,
          };
        }

        return next(params);
      }
    }
  });
}
janpio commented 3 years ago

An alternative approach in a middleware could be to manipulate the returned data after executing await next(params). There you could just overwrite e.g. password with REDACTED or similar. Many opportunities, depending on your specific use case. (Still just a workaround of course)

un33k commented 3 years ago

The best thing about Prisma is Typescript and as soon as we use omit() or any use of fields as strings like 'firstName', we'd lose types, and there goes with it is intelisense which is one of the major value proposition of Prisma. Everything is typed.

The select with its current implementation is almost perfect, and with a tiny behaviour change we can have it do what is asked for in this ticket.

My recommendation is:

const users = await this.prisma.user.findMany({
  where: { // some condition },
  select: { id: true, password: false },   // <- exception raised:  (`true & false` cannot be combined)
});
const users = await this.prisma.user.findMany({
  where: { // some condition },
  select: { password: false },   // <- this will return all fields `except` for `password`
});
const users = await this.prisma.user.findMany({
  where: { // some condition },
  select: { email: true, id: true, username: true },   // <- this will `only` return `email`, `id` and `username`
});

We just have to realize that, we either want to exclude a few fields like password, email, or include a few fields like name DoB etc, or simply have all fields returned. If so, the above 3 situations will serve us well, which will be optimal, typed and logically easy to wrap one's head around.

GrzegorzWidla commented 3 years ago

We just have to realize that, we either want to exclude a few fields like password, email, or include a few fields like name DoB etc, or simply have all fields returned. If so, the above 3 situations will serve us well, which will be optimal, typed and logically easy to wrap one's head around.

@un33k how would you handle relationships?

Let's say we have three models: User, UserType, UserGroup. Users belong to user types and user types belong to user groups.

We want to select all fields but the password from user, get all fields from user type and name field from user group. Right now I can do that this way:

await prisma.user.findFirst({
  where: { id: 1 },
  select: {
    id: true,
    email: true,
    name: true,
    password: false,
    type: {
      select: {
        id: true,
        name: true,
        group: {
          select: {
            id: false,
            key: false,
            name: true
          }
        }
      }
    }
  }
})
flybayer commented 3 years ago

The best thing about Prisma is Typescript and as soon as we use omit() or any use of fields as strings like 'firstName', we'd lose types, and there goes with it is intelisense which is one of the major value proposition of Prisma. Everything is typed.

With the preferred solution of using a @private schema attribute (comment above https://github.com/prisma/prisma/issues/5042#issuecomment-830191195), the typescript can be generated correctly with no loss of type safety.

pantharshit00 commented 3 years ago

This issue describe the proposal which we are going to implement, we will appreciate feedback there: https://github.com/prisma/prisma/issues/7380

rpedroni commented 3 years ago

Hey, any updates on this? Couldn't find in the documentation if this implemented at any level

ghyath5 commented 2 years ago

This is very important issue! it should be resolved now.

Any updates ?

weslleydsg commented 2 years ago

I still have this problem. No solution implemented?

rpedroni commented 2 years ago

I ended up separating password and any critical value in a separate model/table and intentionally did not build relations between the models so any query that requires the sensitive information has to be very explicit, while avoiding sending back this information accidentally on the multiple User queries I do in the API

ikbenignace commented 2 years ago

I still think this issue needs the necessary attention