Asserts, that contain root and prior(root), behave differently depending on their place

[serhiipalash]

I have two helper-functions

hasWritePermission(resource, record_id) {
  prior(root).permissions[resource][record_id][auth.uid].level >= 20
}

and

isCreatedByCurrentUser(resource, record_id) {
  root[resource][record_id].createdBy == auth.uid
}

and I have a write check

write() { isCreatedByCurrentUser('children', childId) || hasWritePermission('children', childId) }

If I place hasWritePermission() before isCreatedByCurrentUser() permission will be denied.

If I place isCreatedByCurrentUser() before hasWritePermission() permission will be granted.

Bug?

[rockwotj]

Can you also put the final generated rules here for both? Thanks!

[serhiipalash]

I tested again and the problem is in hasWritePermission function in >=

hasWritePermission(resource, record_id) {
  prior(root).permissions[resource][record_id][auth.uid].level >= 20
}

== works; > doesn't work < doesn't work >= doesn't work

I am not sure why is this happening

generated rules

".write": "newData.parent().parent().child('permissions').child('children').child($childId).child(auth.uid).child('level').val() >= 20 || (root.child('children').child($childId).child('createdBy').val() == auth.uid || newData.parent().parent().child('children').child($childId).child('createdBy').val() == auth.uid)"

[serhiipalash]

By "doesn't work" I mean that hasWritePermission always returns false, no matter what I do. Even in this case write is false

hasWritePermission(resource, record_id) {
  prior(root).permissions[resource][record_id][auth.uid].level >= 20
}

write() { 
    hasWritePermission("children", 1) || true 
}

[serhiipalash]

And prior(...) is not related to this bug, without it all works the same

[rockwotj]

Is it because you're trying to lookup a key with record_id, which is a number?

Can you test using the console simulator and see what if it tells you exactly what fails? Other things is that auth.uid isn't set?

There must be an error because error || true => error

[serhiipalash]

Sorry, I used number just for shorthand. In my rules it is

 /{childId} {
    write() {
      hasWritePermission("children", childId) || isCreatedByCurrentUser("children", childId)
    }
  }

[serhiipalash]

Success

Error

[rockwotj]

Can you please open a bug here: https://firebase.google.com/support/contact/bugs-features/

I'll look into this in our rules engine to see what the bug is. Can you look in your database and let me know the value of level please?

[serhiipalash]

I'll create bug report tomorrow.

The level is missing in database when I run this query. Even all /permissions collection is missing in database when I do this.

I'll try to run it tomorrow with existing /permissions in database and with level less then 20.

Maybe the bug happens because of missing path entries.

[serhiipalash]

@rockwotj I have sent bug report just now.

[rockwotj]

I'm going to close this issue since it's not specific to bolt. I'll updated you via the bug, I got the report.

[serhiipalash]

Thanks!

[rockwotj]

Sorry just dug into this, seems that null >= 20 is an error, it seems like there is no data in your database for this location? Can you confirm what data is located here?

Otherwise you should make your check:

hasWritePermission(resource, record_id) {
  prior(root).permissions[resource][record_id][auth.uid].level.isNumber && 
    prior(root).permissions[resource][record_id][auth.uid].level >= 20
}

The issue is null >= 20 is an error and stops all rules evaluation. This isn't "real" javascript that's being evaluated, so things are more strongly typed.

[serhiipalash]

Yes, in the moment of testing there was no data in database.

[serhiipalash]

/permissions collections was missing

[serhiipalash]

Otherwise you should make your check:

Ok. Thanks!

Maybe need to write about this case in docs for others.

[serhiipalash]

Btw, if you are still working on this bug you can try to test rules like this

hasWritePermission(resource, record_id) {
    prior(root).permissions[resource][record_id][auth.uid].level >= false
}

It is known bug in JS that null >= false is true 😄

[rockwotj]

The simulator should give an error back that you have incorrect types, but we only support string and number as valid data types for >=, >, <=, and <

[rockwotj]

I'll file a bug to update the docs

[serhiipalash]

Ok, thanks!

[serhiipalash]

Hi @rockwotj ! Maybe you will be interesting, I just submitted this bug report to Firebase.

Target: Web SDK

Bug: firebase.auth().currentUser.metadata.lastSignInTime is not updating on next time user signs in if not much time has passed.
It looks strange when I login first time, then logout, then login again and I see that my user lastSignInTime is not current session sign in time, but previous one.

Test example: https://fir-last-sign-in-time-bug.firebaseapp.com/ with auth in Firebase project https://console.firebase.google.com/u/0/project/fir-last-sign-in-time-bug

Test example code: https://github.com/serhiipalash/firebase-last-sign-in-time-bug

Why is it important. 
Before authenticate user into our app, we need to know is this login first one, to redirect user on /onboarding, or it is not the first one login. To find this info the reasonable way is to compare 

firebase.auth().currentUser.metadata.lastSignInTime === firebase.auth().currentUser.metadata.creationTime

But these timestamps work in strange way and we have to use async and time expensive function to find the same result

firebase
      .auth()
      .currentUser.getIdToken()
      .then(jwtToken => {
        const session = jwtDecode(jwtToken);

        const isFirstTimeSignIn = session.auth_time === firebase.auth().currentUser.metadata.creationTime

        console.info("Is this first time user signs in ", isFirstTimeSignIn); 
      })
      .catch(err => console.log("err: ", err));

It takes 0.5-1 second each time user signs in.

It is very important for us to allow users to enter the app as faster as possible. Can you please fix firebase.auth().currentUser.metadata.lastSignInTime behavior, so then we can minimize our app start time using sync values comparison instead of async very long function?

Thanks in advance!

P.S. You can check test example console logs for more info
P.P.S. I know that for native SDKs there is isNewUser method on User object. Maybe you can add the same for Web SDK?

Serhii

[rockwotj]

Serhii, sorry I can't help as much here, can you please open this issue at: https://github.com/firebase/firebase-js-sdk

[serhiipalash]

can you please open this issue at:

Ok, will do.