ACL seems to not work when having multiple user models with a base UserModel

[abouroubi]

Description/Steps to reproduce

I'm having trouble using multiple user models in my app, here is my test case.

I create a new loopback app, follow the doc to add multiple user models, with custom AccesToken model (https://loopback.io/doc/en/lb3/Authentication-authorization-and-permissions.html#access-control-with-multiple-user-models).
I have 5 models:

1. BaseUser : that inherit from User and have common functions (not provided in the sample but needed for a DRY code).
2. Admin : inherits from BaseUser
3. Client: inherits from BaseUser
4. CustomAccessToken: inherits from AccessToken
5. TaskItem: a persisted model with these ACL

   "acls": [
       {
         "accessType": "*",
         "principalType": "ROLE",
         "principalId": "$everyone",
         "permission": "DENY"
       },
       {
         "accessType": "*",
         "principalType": "Admin",
         "principalId": "$everyone",
         "permission": "ALLOW",
         "property": "*"
       }
   

I want to forbid everyone from accessing the model except Admins.
I tried reversing the order, change principalType to "ROLE" and principalId to "Admin", tried by changing the case (admin, ADMIN, Admin, etc...)

But whatever I do, when accessing the end point GET TaskItems as an Admin user I have a forbidden response.

Of course if I remove all the acls I have the results.

Link to reproduction sandbox

My code is here : https://github.com/abouroubi/loopbacktest

Expected result

Empty Array when Admin and Unauthorized when Client.

Additional information

[bajtos]

Hello, thank you for the bug report and the loopbacktest repository. I managed to reproduce the problem you have described.

AFAICT, there are several causes:

1] $everyone cannot be used as a user id, it's a role id. Here is the corrected ACL definition:

  "acls": [
    {
      "principalType": "ROLE",
      "principalId": "$everyone",
      "permission": "DENY"
    },
    {
      "principalType": "ROLE",
      "principalId": "admin",
      "permission": "ALLOW"
    }
  ],

2] You need to create "admin" role and assign your admin user to this role, as described in our docs.

Here is how I modified you boot/root.js script:

const credentials = { email: "admin@admin.com", password: "admin" };

  Promise.all([
    Role.create({name: 'admin'}),
    Admin.create(credentials),
  ])
  .then(([role, user]) => {
    return role.principals.create({
      principalType: Admin.modelName,
      principalId: user.id,
    }).then(() => {
      return new Promise((resolve, reject) => {
        user.createAccessToken({}, {}, (err, token) => {
          if (err) reject(err);
          else resolve(token);
        });
      });
    });
  })
  .then(token => {
    console.log('ADMIN TOKEN:', token.id || token);
  })
  .catch(function(err) {
    console.error('ERR WHEN CREATING ADMIN >>', err.stack);
  });

3] There is a bug in AccessContext where to incorrectly add USER instead of Admin/Client principal to the context, see

loopback/lib/access-context.js

Lines 80 to 91 in 6ddf268

	var principalType = context.principalType || Principal.USER;
	var principalId = context.principalId || undefined;
	var principalName = context.principalName || undefined;
	if (principalId != null) {
	this.addPrincipal(principalType, principalId, principalName);
	}
	var token = this.accessToken || {};
	if (token.userId != null) {
	this.addPrincipal(Principal.USER, token.userId);
	}

The pending pull request #3823 is offering the following fix, which seems to address the problems you are observing, but I am worried it may be breaking other things - I need to investigate this more.

diff --git a/lib/access-context.js b/lib/access-context.js
index 310e9bf2..99e66c9e 100644
--- a/lib/access-context.js
+++ b/lib/access-context.js
@@ -87,7 +87,7 @@ function AccessContext(context) {
   var token = this.accessToken || {};

   if (token.userId != null) {
-    this.addPrincipal(Principal.USER, token.userId);
+    this.addPrincipal(token.principalType || Principal.USER, token.userId);
   }
   if (token.appId != null) {
     this.addPrincipal(Principal.APPLICATION, token.appId);

[bajtos]

Yay, I sent a pull request to fix this issue - see #3835. @abouroubi could you please check if the patch fixes the problem for you (together with the change described in my comment above) and leave a comment in the pull request to let us know?

[abouroubi]

Thanks for the reply.

When assigning the role, it fixes the authentication.

But my original problem, was to control access based on principal type and not roles. Wonder if this is possible with the actual implementation ?

I added some code in the BaseUser, that assign role to the models that inherits from it (sets role to Admin when principal is Admin, and to Client when principal is Client). It fixes the problem, but it makes me duplicate code, cause in my case the role will always be the principal type.

[bajtos]

But my original problem, was to control access based on principal type and not roles. Wonder if this is possible with the actual implementation ?

I added some code in the BaseUser, that assign role to the models that inherits from it (sets role to Admin when principal is Admin, and to Client when principal is Client). It fixes the problem, but it makes me duplicate code, cause in my case the role will always be the principal type.

@raymondfeng could you please help? I don't know our ACL/auth system enough to be able to answer this question.