Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null values are passed up the hierarchy #113

Closed
Maradox opened this issue Jan 10, 2017 · 8 comments
Closed

Null values are passed up the hierarchy #113

Maradox opened this issue Jan 10, 2017 · 8 comments

Comments

@Maradox
Copy link

Maradox commented Jan 10, 2017

When writing or patching a null value, the null is passed up the hierarchy which causes tests to fail when they should not.

Example:

{
  "rules": {
    "A": {
      "B": {
        "C": {
          ".read": true,
          ".write": true
        }
      }
    }
  }
}

Chai code:

  1. expect().can.write({C: 1}).to.path('A/B'); - passes
  2. expect().can.write({C: null}).to.path('A/B'); - fails: AssertionError: Expected an unauthenticated user to be able to write `null` to A/B, but the rules denied the write. No .write rule allowed the operation. Write was denied.
  3. expect().can.patch({C: 1}).to.path('A/B'); - passes
  4. expect().can.patch({C: null}).to.path('A/B'); - passes
  5. expect().can.write({B: {C: 1}}).to.path('A'); - passes
  6. expect().can.write({B: {C: null}}).to.path('A'); - fails: AssertionError: Expected an unauthenticated user to be able to write `null` to A, but the rules denied the write. No .write rule allowed the operation. Write was denied.
  7. expect().can.patch({B: {C: 1}}).to.path('A'); - passes
  8. expect().can.patch({B: {C: null}}).to.path('A'); - fails: AssertionError: Expected an unauthenticated user to be able to write `null` to A/B, but the rules denied the write. No .write rule allowed the operation. Write was denied.
@dinoboff
Copy link
Collaborator

dinoboff commented Jan 11, 2017

Hi, which version are your using?

ps: the rules only allow read and write of A/B/C. Anything else than a read of or write to A/B/C. (or patch of A/B with an object having only a C property) would fail:

  • 1, 5, 6 and 7 should fail.
  • 2, 6 and 8 fail as I would expected.
  • 3 and 4 pass as expected.

@dinoboff
Copy link
Collaborator

Please reopen if you can provide more details.

@Maradox
Copy link
Author

Maradox commented Jan 31, 2017

Sorry for the late reply. I used 2.3.3, but I just updated now.

Just to clarify, because maybe I misunderstood the behavior of the patch rule.
Lets say I have a rule which only allows writes to:
A/B/C and A/X/Z
Firebase currently supports multi-location updates, so for example:
root.child("A").update("B: {C: ...}, X: {Z: ...}") does work.

Can this be simulated with a rule using patch?

@dinoboff
Copy link
Collaborator

Targaryen support multi-location updates.

Assuming you have the following rules:

{
  "rules": {
    "A": {
      "$foo": {
        "$bar": {
          ".read": true,
          ".write": true
        }
      }
    }
  }
}

Firebase allows something like that:

root.child("A").update({
  "B/C": "something",
  "X/Y": "something"
})

But not:

root.child("A").update({
  B: {C: "something"},
  X: {Y: "something"}
})

At least that how I understand it and how targaryen@3 simulates it.

https://firebase.google.com/docs/reference/js/firebase.database.Reference#update
https://firebase.googleblog.com/2015/09/introducing-multi-location-updates-and_86.html

@Maradox
Copy link
Author

Maradox commented Feb 1, 2017

Actually this is supported by multi-location updates as far as I know and is also backed by the fact that our setup works in production:
/groupRelations/{uid0}/requests/{uid1} is writeable. Nodes above are not writeable.

Then we do the following:

const updates = {
  [groupId + '/requests/' + userId]: data,
  [userId + '/requests/' + groupId]: data
}
db.ref('groupRelations').update(updates)

dinoboff added a commit to dinoboff/targaryen that referenced this issue Feb 1, 2017
dinoboff added a commit to dinoboff/targaryen that referenced this issue Feb 1, 2017
rule example with:
```
npm i -g targaryen@3 mocha chai
mocha docs/chai/examples/issue-113.js
```

fix goldibex#113
@dinoboff
Copy link
Collaborator

dinoboff commented Feb 1, 2017

Please read again what I wrote; Targaryen@3 support multilocation write operations using the patch assertion tests.

The failing operations I pointed out are failing because they are targeting a level without write permissions.

Please check dinoboff@be60fdf for an example.

@Maradox
Copy link
Author

Maradox commented Feb 2, 2017

I will create an example project to test something I think is not working correctly. I will open a different issue if this is the case.

@dinoboff
Copy link
Collaborator

dinoboff commented Feb 2, 2017

Thanks.

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

No branches or pull requests

2 participants