Role.getRoles inconsistent entities return

[mitsos1os]

I believe Role.getRoles functionality isn't working as expected. When issued for a user, it will return the names of dynamically resolved roles such as $authenticated, $everyone, etc.. and then for normal roles, the role id. So you get an array of names and Ids together. I do not see any reason for this and I find it incorrect.

• You can see that for resolved roles it adds the name here
• But for normal roles it adds the id... here

Any opinions???

[davidcheung]

hi @mitsos1os, it would be really helpful if you could reproduce the issue(where the role name and roleId ends up in same array) in a sand-box

please note that in principalId are strings

"acls": [
    {
      "accessType": "*",
      "principalType": "ROLE",
      "principalId": "$owner",
      "permission": "ALLOW"
    }
  ]

[mitsos1os]

Hello @davidcheung !
I have created the sandbox here: https://github.com/mitsos1os/loopback-sandbox/tree/get-roles-return-mixed
You can see for yourself that in the end of the boot scrip for server/boot/admin.js the roles of an admin user are printed, and you will see that dynamic roles (simple names), are mixed with static role ids....
In my first comment you can see the lines responsible for this, but I haven't issued a pull request because I wanted to discuss first this behavior....
Thanks

[davidcheung]

Thanks for reproducing the issue, I do agree the behavior is unexpected, not sure whats the best approach to resolve this, perhaps changing

loopback/common/models/role.json

Line 8 in 42c83f6

"generated": true

{
  "name": "Role",
  "properties": {

    "id": {
      "type": "string",
      "id": true,
      "generated": true 
    },

right now this forces all ids into numbers
to something like

{
  "name": "Role",
  "idInjection": true,
  "properties": {
    "id": {
      "type": "string",
      "id": true
    },
}

then user would be able to do

Role.create({
  id: '$someRole',
  name: 'someRole'
});

@raymondfeng any thoughts?

[mitsos1os]

@davidcheung I am afraid you are not right about the auto generated key...
idInjection: true what only does is adding an id property with also the generated: true property, as you can see here

The generated property is used by the connector to be auto completed if selected, as you can see here

The fact that the id gets a number for the example is because of the memory connector which uses integers for auto - id generation. If this was in mongoDb an ObjectId would be auto generated...

I believe a solution would be to either

• actually make the id property a string but not generated, so that the user will have to add the id of the role in letters, such as admin which would also make it unique
• or to select to add the name for static roles instead of the role id as seen here, but we shoud also set the name property of the role to required in order to avoid situations where an empty role name is added to the list.

[davidcheung]

you're right, what i wanted to achieve was backwards compatibility, so user can define id, but when its not defined to give them a auto-generated id (number)
but since you pointed out that it assigns the id as a number (in model builder), probably a bad idea to do so

but the better solution might to be just make it required and unique( i think its already unique) then this would be breaking-change, but the behavior would make more sense

---

@mitsos1os on a different note, when you link code it would be helpful if you press y on the page so it changes the URL from master->commit-number, this would allow the code to point to the correct place, given our turnaround time may not be as fast as we would like, and the code might be pointing to a different line in the future. ( i understand you linked it just for the conversation, but if someone picks up the task to fix it would be helpful )

[mitsos1os]

@davidcheung thanks for the notice.... I didn't know about y! I will do it from now on. Any news on this issue?

[davidcheung]

@mitsos1os no news yet on this issue, so proposal is user will setup roles with self defined id
example

Role.create({
  id: '$someRole',
  name: 'someRole'
});

^^ right now this would result in {id:1, name:'$someRole'}

@raymondfeng @bajtos any thoughts?

[bajtos]

no news yet on this issue, so proposal is user will setup roles with self defined id
example

@davidcheung I am afraid I am not familiar enough with this code base. I guess it's up to @raymondfeng and/or @ritch to make the call.

cc @loay @richardpringle, as they are the new owners of everything authentication-related