Skip to content

Added sorting keys before checking.#3

Open
artzub wants to merge 1 commit into
futurechan:masterfrom
artzub:patch-1
Open

Added sorting keys before checking.#3
artzub wants to merge 1 commit into
futurechan:masterfrom
artzub:patch-1

Conversation

@artzub

@artzub artzub commented Jun 21, 2019

Copy link
Copy Markdown

The reason is

const roles = [
  {
    roles: ['readonly'],
    allows: [{
      resources: ['/api/v1/user/.*'],
      permissions: ['get'],
    }],
  },
  {
    roles: ['admin'],
    allows: [
      {
        resources: ['/api/v1/user(/.*)?'],
        permissions: ['get', 'put', 'post', 'delete'],
      }
    ],
  },
  {
    roles: ['superadmin'],
    allows: [{
      resources: '.*',
      permissions: '*'
    }]
  }
];

Keys of buckets will be incorrectly sorted.

if make a request (role admin or superadmin):

DELETE /api/v1/user/someid,

then union method will find a wrong bucket /api/v1/user/.* that has just only one role readonly,
but it is not expected behavior.

After keys sorting we have at least the needed bucket = allows_/api/v1/user(/.*)? or for superadmin allows_.*

The reason is 
```
const roles = [
  {
    roles: ['readonly'],
    allows: [{
      resources: ['/api/v1/user/.*'],
      permissions: ['get'],
    }],
  },
  {
    roles: ['admin'],
    allows: [
      {
        resources: ['/api/v1/user(/.*)?'],
        permissions: ['get', 'put', 'post', 'delete'],
      }
    ],
  },
  {
    roles: ['superadmin'],
    allows: [{
      resources: '.*',
      permissions: '*'
    }]
  }
];
```

Keys of buckets will be incorrectly sorted.

if make a request (role `admin` or `superadmin`):
`DELETE /api/v1/user/someid`
then `union` method will find a wrong `bucket`
`/api/v1/user/.*` that has just only one role `readonly`
but it is not expected behavior.

After keys sorting we have at least the needed `bucket` = `allows_/api/v1/user(/.*)?` or for superadmin `allows_.*`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant