feat(graphql): add allowUnauthenticated parameter to auth rules

[mathix420]

Description

Add allowUnauthenticated parameter in auth rules to avoid throwing exceptions if no auth is provided.

Changes:

• Update documentation auth/authentication.
• If paramValue is undefined inject false instead of the previous condition key = $param, as it should always be falsy
• Fix a small issue: if a paramValue is null, apoc.validate raise an exception if the check is done like this value = null. So I just replaced the condition for it to be value IS null (more infos)
• Fix the 4.3.0 Neo4J deprecation of EXISTS() with recommanded IS NOT NULL. (more infos)

Issue

#345

Checklist

• Documentation has been updated
• TCK tests have been updated
• Integration tests have been updated
• Example applications have been updated
• New files have copyright header
• CLA (https://neo4j.com/developer/cla/) has been signed

[darrellwarde]

Hey @mathix420, thanks for the contribution!

This really needs some integration tests to prove that unexpected errors won't be thrown on execution of the Cypher.

[mathix420]

@darrellwarde Should I open 2 separate issues to explain more in details the EXISTS() change and apoc.validate behavior on null comparisons?

[darrellwarde]

@darrellwarde Should I open 2 separate issues to explain more in details the EXISTS() change and apoc.validate behavior on null comparisons?

Hey @mathix420, sorry for the slow reply here! No need to open 2 PRs, happy to review as a batch - was just waiting on some integration tests but realise now I missed your commit!

[darrellwarde]

It does look like you have some unit test failures though which will need reviewing.

[mathix420]

It does look like you have some unit test failures though which will need reviewing.

Yup, sorry dumb mistake I was focalized on create-auth-and-params tests 😅
Everything should be fix now.

I'm not sure if I should replace all others occurences of EXISTS() in this PR, should I?

[danstarns]

It does look like you have some unit test failures though which will need reviewing.

Yup, sorry dumb mistake I was focalized on create-auth-and-params tests 😅
Everything should be fix now.

I'm not sure if I should replace all others occurences of EXISTS() in this PR, should I?

Hey, no need to replace all other EXISTS we can do this in another PR. Thanks for pointing out its deprecation.

[mathix420]

@danstarns great, have a nice day!

[darrellwarde]

Thanks for PR, always grateful for contributions! 🙂

Please see code comments, and also, I would like to see some integration tests for more complex cases. For example, what happens with allow and allowUnauthenticated if there is one post which is published and one that isn't?

[darrellwarde]

Literally just a single word documentation change, which I think can just be applied from my suggestion once I've finished this review!

[darrellwarde]

As expected, I was able to just apply my word change suggestion.

In which case, this all looks good! 🚀 Thanks for the improvement, and working through changes so quickly!

[danstarns]

Thanks @mathix420