fullcube / loopback-component-access-groups

Access controls for Loopback.
59 stars 21 forks source link

Crash with mongodb connector due to getCurrentContext() #12

Open GregAujay opened 8 years ago

GregAujay commented 8 years ago

Your component is great but I run into crashes when I use the mongodb connector.

It happens when trying to call set() on a null object returned from this.app.loopback.getCurrentContext() https://github.com/fullcube/loopback-component-access-groups/blob/master/lib/utils.js#L298

To reproduce it you can change the datasource in your simple-app using this configuration:

{
  "db": {
    "host": "127.0.0.1",
    "port": 27017,
    "database": "group-access-test",
    "name": "db",
    "connector": "mongodb"
  }
}

There are many issues on loopback's github project about mongodb and getCurrentContext(), they are probably related, but they are all "Closed". https://github.com/strongloop/loopback/pull/885 https://github.com/strongloop/loopback/issues/809 and more

Using the memory connector works fine.

I am using the latest packages:

    "loopback": "^2.29.1",
    "loopback-boot": "^2.21.0",
    "loopback-component-access-groups": "^0.2.0",
    "loopback-connector-mongodb": "^1.15.2",
    "loopback-datasource-juggler": "^2.47.0",
GregAujay commented 8 years ago

Nevermind, it is the same issue as https://github.com/strongloop/loopback/issues/2519 So downgrading to loopback-connector-mongodb@1.15.1 seems to fix this issue.

Apparently they think about deprecating the getCurrentContext() method as it creates a lot of issues: https://github.com/strongloop/loopback/issues/1676

BuchyOne commented 7 years ago

@mrfelton

Apologies for a late addition to this issue. I tried your patch as mentioned here and it did not seem to help us.

We can use the component in memory. That works as expected. If we then attach a Mongo or Postgres data source it fails. We can authenticate but any permitted request is rejected. The logs took us to L316 of utils.

LoopBackContext.getCurrentContext({ bind: true }).set('groupAccessApplied', true)

Can you point us in the right direction?

...

loopback:security:access-context getUserId() companyAdminA +0ms loopback:security:access-context isAuthenticated() true +0ms loopback:security:role Custom resolver found for role $everyone +0ms loopback:security:role isInRole(): $group:member +0ms loopback:security:access-context ---AccessContext--- +0ms loopback:security:access-context principals: +0ms loopback:security:access-context principal: {"type":"USER","id":"companyAdminA"} +0ms loopback:security:access-context modelName Transaction +0ms loopback:security:access-context modelId undefined +0ms loopback:security:access-context property find +0ms loopback:security:access-context method find +0ms loopback:security:access-context accessType READ +0ms loopback:security:access-context --Context scopes of Transaction.find()-- +0ms loopback:security:access-context method-level: ["DEFAULT"] +0ms loopback:security:access-context accessScopes ["DEFAULT"] +0ms loopback:security:access-context accessToken: +0ms loopback:security:access-context id "fReKp5QDSzxpMY4Yc6AEEmwxEKoHim3dZcL7ZhCTKeUmp98ewlFqQ2r9CwjqQ9xw" +0ms loopback:security:access-context ttl 1209600 +0ms loopback:security:access-context scopes ["DEFAULT"] +0ms loopback:security:access-context getUserId() companyAdminA +0ms loopback:security:access-context isAuthenticated() true +0ms loopback:security:role Custom resolver found for role $group:member +0ms loopback:component:access Role resolver for $group:member: evaluate Transaction with id: undefined for user: companyAdminA +1ms /...I/node_modules/mongodb/lib/utils.js:123 process.nextTick(function() { throw err; }); ^

TypeError: Cannot read property 'set' of null at Role.registerResolver (/.../node_modules/loopback-component-access-groups/lib/utils.js:316:56) at Function.Role.isInRole (/.../node_modules/loopback/common/models/role.js:384:21) at /...I/node_modules/loopback/common/models/acl.js:502:23 at /.../node_modules/async/dist/async.js:3853:24 at eachOfArrayLike (/.../node_modules/async/dist/async.js:1003:9) at eachOf (/.../node_modules/async/dist/async.js:1051:5) at _parallel (/.../node_modules/async/dist/async.js:3852:5) at Object.parallelLimit [as parallel] (/.../node_modules/async/dist/async.js:3935:5) at /.../node_modules/loopback/common/models/acl.js:516:13 at /.../node_modules/loopback-datasource-juggler/lib/dao.js:2095:9 at /.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:1012:9 at /.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:359:16 at eachOfArrayLike (/.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:928:9) at eachOf (/.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:990:5) at _asyncMap (/.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:1005:5) at Object.map (/.../node_modules/loopback-datasource-juggler/node_modules/async/dist/async.js:995:16) [nodemon] app crashed - waiting for file changes before starting...

mrfelton commented 7 years ago

Hey @BuchyOne

In the end - after many days of trying - we ditched loopback-context completely and no longer use it.

Instead, we now do things the recommended way in loopback 3 (see https://loopback.io/doc/en/lb3/Using-current-context.html) which is a horrible hacky solution that relies on manually passing the context throughout your app. Unfortunately, this seems to be the only reliable way to do things.

The loopback-component-access-groups component does still use loopback-context under the hood, but we have also introduced a custom strong-remoting phase (see https://loopback.io/doc/en/lb3/Using-current-context.html#use-a-custom-strong-remoting-phase) which makes the current user and user groups available via loopback's optionsFromRequest handler.

see https://github.com/fullcube/loopback-component-access-groups/blob/75ad90190898305839b7cd76ef138dd875b1cc20/lib/utils.js#L42-L56.

Much as I hate to promote a horrible workaround, I would suggest that you look to follow the loopback 3.x recommendations for handing current context.

BuchyOne commented 7 years ago

@GregAujay @mrfelton

Overlooked to mention that we encountered this after upgrading from loopback-connector-mongodb@1.15.1

So Greg's comments are still applicable.

BuchyOne commented 7 years ago

Hi Tom,

Thanks for the speedy reply! Reading those docs now, I get the gist ...

Cheers from downunder

elviocb commented 5 years ago

Someone found the solution for this problem? Using new versions of loopback-connector-mongodb still loses context (null).

The problem happens on the utils.js on (loopback-component-access-groups) (setupRoleResolver() method)