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

Hidden properties #1914

Open
wohldani opened this issue Oct 25, 2018 · 37 comments · Fixed by #5468
Open

Hidden properties #1914

wohldani opened this issue Oct 25, 2018 · 37 comments · Fixed by #5468
Assignees
Labels
feature parity feature Repository Issues related to @loopback/repository package

Comments

@wohldani
Copy link

Description / Steps to reproduce / Feature proposal

Does Loopback 4 supports hidden properties like LB3 did?

Current Behavior

I could not find any docs or resources about this.

Expected Behavior

https://loopback.io/doc/en/lb3/Model-definition-JSON-file.html#hidden-properties

@bajtos
Copy link
Member

bajtos commented Oct 30, 2018

@wohldani In theory, since LB4 is using mostly the same juggler under the hood as LB3, hidden and protected properties should be supported out of the box. Have you tried to define them in @model decorator?

@model({
  settings: {hidden: ['password']}
})
class User extends Entity {
  // ...
  @property({
    type: 'string',
    required: true
  })
  password: string;
}

@bajtos bajtos added Docs Repository Issues related to @loopback/repository package labels Oct 30, 2018
@hacksparrow
Copy link
Contributor

I can understand @wohldani's confusion. I encountered the same initially.

As part of our Migration from LoopBack 3.x epic, we should have a "How to do in LB4, what you used to do in LB3" guide.

@wohldani
Copy link
Author

@bajtos thank you for your answer. I have tried to define the hidden properties in @model decorator .


Account model:

import {Entity, model, property} from '@loopback/repository';

@model({
  settings: {hidden: ['password']},
})
export class Account extends Entity {
  @property({
    type: 'string',
    id: true,
    generated: true,
  })
  id: string;

  @property({
    type: 'string',
    required: true
  })
  password: string;

  @property({
    type: 'string',
  })
  email: string;

  constructor(data?: Partial<Account>) {
    super(data);
  }
}

Http response:
screen shot 2018-10-30 at 18 49 48

@raymondfeng
Copy link
Contributor

There is an issue in LB4 to honor hidden/hiddenProperties. It will be fixed by #1947.

@bajtos
Copy link
Member

bajtos commented Nov 9, 2018

Based on the discussion in #1947, this issue is not as easy to fix as it seems.

Cross-posting a part of #1947 (comment):

I propose the following:

  • Improve juggler so that options to control how to honor secret properties can be set at method level too in addition to datasource/model scope.

-Update CRUD controller code template/base class to hide secret properties for REST requests.

  • For juggler.next, we need to reevaluate such property modifiers.

Personally, I am thinking about a different approach that may be easier to implement: modify LB4's Model.prototype.toJSON() method to exclude hidden properties configured via definition.settings.hiddenProperties. IMO, such change should fix our immediate needs and serve as a reasonable short-term workaround.

I agree that we should reevaluate property modifiers in LB4 for long term.

We already have some ideas encoded in LB4 repository types, see https://github.com/strongloop/loopback-next/blob/0e1b06f546aea97855556f985b39e50cd3e7956e/packages/repository/src/model.ts#L26-L45

I am not sure if these interfaces are a good solution for what we need though. I think we should follow scenario-based approach here and start by defining somewhat realistic use cases for these properties.

@wohldani what are your use cases for hidden properties, besides the password property?

Based on my experience, it's better to store user credentials like password and 2FA tokens in a related model (e.g. Credentials) and use hasOne relation to access them.

Cross-posting from #1996:

In LoopBack 3.x, we are storing storing user's password together with user data in the same model (table), which opens a lot of security vulnerabilities to deal with. For example:

  • When returning user data in HTTP response, the password property must be filtered out.
    -When searching for users, the password must be excluded from the query.
  • It is not possible to do a full replace of user data from a REST client (e.g. "save()") because a) the client usually does not know the password b) password must be changed using a different flow.
  • A policy preventing users from reusing the same password is difficult to implement because old password are not preserved.

LB4 does not support hasOne relation yet, but we are actively working on this feature - see #1422, #1956 and the related pull requests.

@bajtos bajtos added feature and removed Docs labels Nov 9, 2018
@serhiihordiichuk

This comment has been minimized.

1 similar comment
@SmirnovM91

This comment has been minimized.

@dkrantsberg

This comment has been minimized.

@ahmedsaab

This comment has been minimized.

@karianpour
Copy link

I override the toEntity method in the user.repository.ts ( or any repository )

   protected toEntity(model: juggler.PersistedModel): Player {
     const r = super.toEntity(model);
     r.password = '****';
     return r;
   }

@Realetive
Copy link

@karianpour can you show all code? Or prev steps reproduced official method?

@karianpour
Copy link

karianpour commented Apr 8, 2019

@Realetive

Let's say that we have a User model having password property that we want to hide.

export class UserRepository extends DefaultCrudRepository<
  User,
  typeof User.prototype.id
  > {
  constructor(
    @inject('datasources.somedb') dataSource: SomeDbDataSource,
  ) {
    super(User, dataSource);
  }

  // here we can hide the properties
  protected toEntity(model: juggler.PersistedModel): Player {
     const r = super.toEntity(model);
     r.password = '****';
     return r;
   }
}

Actually, for the case of password property, I do not suggest to hide it, the problem is what happens if someone gets an User from the server and put it back? Will it save the missing password back to the database?!

@bolner
Copy link

bolner commented May 6, 2019

To sum it up: hidden properties are not supported in LB4?

@hazimdikenli
Copy link

I aggree with;

Personally, I am thinking about a different approach that may be easier to implement: modify LB4's Model.prototype.toJSON() method to exclude hidden properties configured via definition.settings.hiddenProperties. IMO, such change should fix our immediate needs and serve as a reasonable short-term workaround.

@bajtos
Copy link
Member

bajtos commented Jun 14, 2019

hidden properties are not supported in LB4?

Yes, that's the case right now.

Any volunteers to contribute the proposal I have described earlier?

Personally, I am thinking about a different approach that may be easier to implement: modify LB4's Model.prototype.toJSON() method to exclude hidden properties configured via definition.settings.hiddenProperties. IMO, such change should fix our immediate needs and serve as a reasonable short-term workaround.

We are happy to help you along the way. See https://loopback.io/doc/en/contrib/code-contrib-lb4.html to get started.

@frbuceta
Copy link
Contributor

hidden properties are not supported in LB4?

Yes, that's the case right now.

Any volunteers to contribute the proposal I have described earlier?

Personally, I am thinking about a different approach that may be easier to implement: modify LB4's Model.prototype.toJSON() method to exclude hidden properties configured via definition.settings.hiddenProperties. IMO, such change should fix our immediate needs and serve as a reasonable short-term workaround.

We are happy to help you along the way. See https://loopback.io/doc/en/contrib/code-contrib-lb4.html to get started.

The hiddenProperties setting would not be the same as this #2782 ?
How could it help?

@bajtos
Copy link
Member

bajtos commented Jun 20, 2019

The hiddenProperties setting would not be the same as this #2782 ?

Hidden and read-only properties are different concepts. It makes sense to use a similar configuration mechanism for both of them, but there is no need to implement them at the same time.

@frbuceta
Copy link
Contributor

frbuceta commented Jun 26, 2019

The hiddenProperties setting would not be the same as this #2782 ?

Hidden and read-only properties are different concepts. It makes sense to use a similar configuration mechanism for both of them, but there is no need to implement them at the same time.

  toJSON(): Object {
    const def = (<typeof Model>this.constructor).definition;
    if (def == null || def.settings.strict === false) {
      return this.toObject({ignoreUnknownProperties: false});
    }

    const copyPropertyAsJson = (key: string) => {
      json[key] = asJSON((this as AnyObject)[key]);
    };

    const json: AnyObject = {};
    for (const p in def.properties) {
      const hiddenProperties = ['remindAtGeo'];
      console.log('p <-Param->:', p);
      console.log('this <-Param->:', this);
      console.log('----> INDEX OF <----', hiddenProperties.indexOf(p));
      if (p in this && hiddenProperties.indexOf(p) === -1) {
        copyPropertyAsJson(p);
      }
    }

    for (const r in def.relations) {
      const relName = def.relations[r].name;
      if (relName in this) {
        copyPropertyAsJson(relName);
      }
    }

    console.log(json);
    return json;
  }
p <-Param->: remindAtGeo
this <-Param->: Todo {
  id: 4,
  title: 'do a thing',
  desc: 'There are some things that need doing',
  isComplete: false,
  remindAtAddress: undefined,
  remindAtGeo: undefined
}
----> INDEX OF <---- 0
{
  id: 4,
  title: 'do a thing',
  desc: 'There are some things that need doing',
  isComplete: false,
  remindAtAddress: undefined
}

Can I prove this somehow? In the explorer would appear? I do not quite understand the flow

@bhupesh-sf
Copy link

@bajtos I would like to contribute to this feature. Can you assign this to me?

@bajtos
Copy link
Member

bajtos commented Jun 28, 2019

@bhupesh-sf There is already a pull request in progress contributed by @frbuceta, see #3265. Can you work with @frbuceta to see how you can help?

@bhupesh-sf
Copy link

@bajtos Thanks. Will see how can I help @frbuceta on this. I am on vacation on coming few days. Will start on it next week.

@bhupesh-sf
Copy link

@frbuceta let me know how can I help you on this. The current solution is a short-term solution but I believe we should enhance it to full blown solution?

@frbuceta
Copy link
Contributor

@bhupesh-sf I'm beginning to understand how this is going. But yes, we can work on a better solution

@bhupesh-sf
Copy link

bhupesh-sf commented Jun 28, 2019

@bajtos Although its not related to the current issue but do you think to have internal properties for the model. Use case could be some kind of calculated properties which can be only internally created/updated and I don't want them to be the part of request but of the response??

Syntax could be something like

{ name: orderTotal, isInternal: true }

@dhmlau
Copy link
Member

dhmlau commented Dec 16, 2019

@frbuceta, since your PR #3304 has landed, is this issue good to close? Thanks.

@frbuceta
Copy link
Contributor

@frbuceta, since your PR #3304 has landed, is this issue good to close? Thanks.

Yes, it can be closed. Something provisional has been launched to my liking but someday the time will come to do it better

@joel-izaguirre
Copy link

joel-izaguirre commented May 14, 2020

This problem is solved? I still have the same problem.
I put the decorator in model:

@model({
  settings: {
    hiddenProperties: ['Password'],
    strict: false,
  },
})

and the response still show de Password property.

image

@frbuceta
Copy link
Contributor

This problem is solved? I still have the same problem.
I put the decorator in model:

@model({
  settings: {
    hiddenProperties: ['Password'],
    strict: false,
  },
})

and the response still show de Password property.

image

I don't know what exactly is happening but it works for me

@model({
  name: 'users',
  settings: {
    hiddenProperties: ['password'],
  },
})
export class User extends Entity {
...

  @property({
    type: 'string',
    required: true,
    jsonSchema: {
      minLength: 8,
    },
  })
  password: string;

...
}

@joel-izaguirre
Copy link

The problem is with de setting "strict: false", if eliminate that, works well, Is it normal or is an error?

@frbuceta
Copy link
Contributor

The problem is with de setting "strict: false", if eliminate that, works well, Is it normal or is an error?

I investigate it

@frbuceta
Copy link
Contributor

@joel-izaguirre In the next version of LoopBack (in a few days) it will work

@mycolaos
Copy link

mycolaos commented Oct 7, 2020

@joel-izaguirre In the next version of LoopBack (in a few days) it will work

Still not working, or I miss something?

@dhmlau
Copy link
Member

dhmlau commented Oct 7, 2020

@mycolaos, how are you using the hiddenProperties? Could you please take a look at #6385?

@mycolaos
Copy link

mycolaos commented Oct 7, 2020

@dhmlau like this:

@model({
  settings: {
    hiddenProperties: ['password']
  }
})

@dhmlau dhmlau reopened this Oct 7, 2020
@dhmlau
Copy link
Member

dhmlau commented Oct 7, 2020

Reopening this issue to be able to track this issue.

@mycolaos, I just tried it with the latest LB4 modules and seems to be working for me. My Customer model has:

  • custId
  • name
  • password <-- hidden property, defined the same way as you did.

Then I tested it with API explorer,

  1. POST
{
  "name": "name1",
  "password": "pwd1"
}
  1. GET
[
  {
    "custId": "1",
    "name": "name1"
  }
]

Is it what you're seeing too?

@mycolaos
Copy link

mycolaos commented Oct 8, 2020

@dhmlau I believe I found the problem. I have an abstract class MyEntity that is than extended by other models. MyEntity has the hiddenProperties options, but it doesn't work. If I decorate with hiddenProperties the final model that extends MyEntity - it works.

Then, how I can make it work with MyEntity?

@durai001
Copy link

durai001 commented Jun 13, 2021

@model({ settings: { hiddenProperties: ['OTP', 'providerId', 'password', 'isDelete'], }, })

This is worked for me., try this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature parity feature Repository Issues related to @loopback/repository package
Projects
None yet
Development

Successfully merging a pull request may close this issue.