Beta

Superb resource access control configuration

Description
Loading description...
Recursion
Algorithms
  • Please sign in or sign up to leave a comment.
  • Voile Avatar

    Random tests calls user solution twice per test case for some reasons.

    Also, mutating the input breaks the random tests.

  • Voile Avatar

    Reference solution is demonstrably wrong:

    configs = [
      {},
      {
        configChild: { resource: { value: [ 7, 4, 6 ] } },
        resource: { value: [] }
      },
      {
        allow: [
          { '3': [ 3, 1 ] },
          { '5': [ 0, 1 ] },
          { '1': [ 2, 3 ], '2': [], '3': [] }
        ],
        deny: { '5': [ 1 ] }
      },
      { resource: { value: [ 9, 1 ] } },
      {
        allow: [
          { '1': [ 0 ], '2': [ 0, 1 ], '4': [ 3 ], '5': [] },
          { '3': [ 3 ], '4': [ 1 ] },
          { '3': [ 2, 0 ], '4': [] }
        ]
      },
      {},
      {
        resource: { value: [ 8, 9, 5 ] },
        deny: { '2': [], '3': [], '4': [ 2, 3, 0 ], '5': [ 2 ] }
      },
      { configChild: {} },
      {},
      { allow: [] }
    ]
    
    credentials = [ { '5': 0 } ]
    
    expected {} to deeply equal null
    

    [ { '5': 0 } ] matches the entry

    {
      allow: [
        { '3': [ 3, 1 ] },
        { '5': [ 0, 1 ] },
        { '1': [ 2, 3 ], '2': [], '3': [] }
      ],
      deny: { '5': [ 1 ] }
    }
    

    and so the result shouldn't be null.

    More specifically, reference solution consistently gives incorrect results when credentials contains a falsy value (which, by how random tests are generated, are always 0).

  • Voile Avatar

    The tests doesn't specify what separates between a value and a nested key, and what happens if a collision happens, such as having resource: { value: 1 } and resource: { value: { nested: 1 } } together. If this won't happen, this still needs to be specified.

  • Voile Avatar

    There are multiple test assertions in a it block, but they aren't distinguished by test failure message, so it's very hard to figure out which test failed inside a it block.

  • Voile Avatar

    The kata is still woefully under-specified on the actual requirements, and the test block descriptions are very undescriptive about what the actual requirements are. Taking some examples from the sample tests:

      it('if deny admits same types as allow', () => {
        const configs1 = [
          {
            allow: [{}],
            deny: {
              user: 'guess',
            },
          },
        ];
        Test.assertDeepEquals(resourceChecker(configs1)({}), null);
        Test.assertDeepEquals(resourceChecker(configs1)({ user: 'guess' }), null);
        Test.assertDeepEquals(resourceChecker(configs1)({ user: 'root' }), {});
        
        ...
    

    What does if deny admits same types as allow even mean? Why is the first check denied if it doesn't match { user: 'guess' }?


    Alternatively, why aren't all the requirements listed in the description? TDD is one thing, but having the actual spec scattered in a million test blocks is definitely an issue. TDD is not a substitute for proper specs documentation.

  • JohanWiltink Avatar

    I can understand you don't want to tediously explain each and every configuration option in the description. But it would be nice if you gave a high-level overview of all the gazillion possibilities in the description.

    The Example tests do a nice job of building bottom-up, but I like to go into a kata with a top-down initial design, so I hopefully don't have to change the basic design too much to allow for surprise requirements. Normally, I would want the specs in the description. All the specs. But I can see that's not going to work in this case.

    Incidentally, I started reading the Example tests after noticing the size of the scroll bar, thus the humongous size of the tests, and made it to two thirds before just plain giving up for now. This kata is a lot more involved and bigger than the description would have you believe ( but for that last little line - I took it with a grain of salt, but it was orders of magnitude bigger than I took it for ).

  • JohanWiltink Avatar

    In the last description example,

    mychecker({user: 'guess', ip: '127.0.0.1'}) // returns { value: [1, 2] }

    why would guess get access to { value: 1 } ?

  • ZED.CWT Avatar

    Needs Random Tests.

  • ZED.CWT Avatar

    There are many more configuration possibilities that would be very tedious to explain here

    Totally wrong. You, the author, are responsible for specifying the requirement clearly. Just look at your long long solution, how the hell are we able to guess what you thought? Like

    By merging, why {...:[2,3]} and {...:1} got {...:[2,3,1]} instead of {...:[[2,3],1]}?

    ['root', [user => user === 'guess', /^s/]] Seriously? Nested array? How deep could it be? And what should we do if credential value to match is an array?

        const configs2 = [
          {
            allow: [{}],
            deny: {
              user: /^g/,
            },
          },
        ];
        Test.assertDeepEquals(resourceChecker(configs2)({}), null);
    

    Let's see, the config says that any users start with 'g' will be denied, then why {} is denied? It doesnt even have a user field (maybe it does have a undefined user field, but undefined starts with u)

     deny: [
              {
                ip: ip => ip !== '127.0.0.1',
                user: user => !user !== 'root',
              },
              { isSuperUser: false },
            ],
    

    What the heck is !user?

    ... ...

  • Voile Avatar

    In the description the value of allowed property is an object, however in the tests it's an array.

  • Voile Avatar

    It's not a good idea to return either a value or an array of values based on the amount of the final results.