Updates force password change flow to use Cypher first, falling back to dbms function call

[daveajrussell]

This PR updates the force password change flow that users go through when their password requires resetting upon login.

Currently, the flow will use the app's detected version to determine whether to use the dbms function call if the app is on a version lower than 4.0, otherwise it will use the Cypher query.

This is problematic since the server's version will not be in state if this is a first login attempt, so it will try and query the database to get the server version.

Only the user is not authorized to query the database, because their password requires a reset.. so we cannot use the server version to accurately determine which version of the query to use.

If we still don't have a version at this point, we fallback to using a function on the driver to determine if there is multidb support - if there is then we use the Cypher query, otherwise we will use the dbms function call.

If, for whatever reason, the server is running >4.0 and multidb returns false, the legacy query is used, and will fail since it does not exist.

In this refactored approach, we no longer attempt to determine the version, and simply attempt to execute the Cypher query, falling back to the dbms function call if a specific error is encountered executing the Cypher - a much more straightforward approach, with less margin of error.

There is also a small update in the bolt service to target the system database when verifying connectivity.

This will resolve a problem some users encounter upon logging in when the default neo4j database is unavailable.. deleted or otherwise.

Repro

• Run a version 3 server
• Drop the default neo4j database
• Create a user requiring a password changeCALL dbms.security.createUser('user', 'changeme', true)
• Log in as this user - you will be prompted to change your password - change the password
• You should be able to log in successfully

---

• Run a version >4.0 server
• Drop the default neo4j database
• Create a user requiring a password change CREATE USER user IF NOT EXISTS SET PASSWORD 'changeme' CHANGE REQUIRED
• Log in as this user - you will be prompted to change your password - change the password
• You should be able to log in successfully

[daveajrussell]

this feels brittle.. is there a better way to determine if the Cypher call failed.. it'd be nice to have a GQL status code, but this currently returns the generic code

[OskarDamkjaer]

The "try and see if it works" mechanism is always a compromise so I think we should just always re-run. I think it's important the behavior is simple/understandable (for someone debugging from logs for example) and less brittle, even if it results in some unnecessary tries

[daveajrussell]

the reason I put this in is so that errors from the cypher call are presented, and we don't try the dbms call if the user has, for example, tried to set an invalid password.. we'd then show an error that a procedure doesn't exist, rather than the password not meeting requirements

[daveajrussell]

so something that I've thought about making other changes, is now that we're attempting the Cypher query assuming that it will always support multi databases, is we get back a specific error from the JS driver..

Driver is connected to the database that does not support multiple databases. Please upgrade to neo4j 4.0.0 or later in order to use this functionality

I was thinking, we could create an error class for this in the query helper (e.g. class MultiDatabaseNotSupportedError extends Error, then check for this message when we try to execute a query, throwing an instance of MultiDatabaseNotSupportedError if we get this error message.

Then when we check the response we could do

if (isError(res)) {
   if (res instanceof MultiDatabaseNotSupportedError) {
      // fallback
   } else {
      // show other errors in the UI, e.g. password requirement error
   }
}

wdyt?

[OskarDamkjaer]

the reason I put this in is so that errors from the cypher call are presented, and we don't try the dbms call if the user has, for example, tried to set an invalid password.. we'd then show an error that a procedure doesn't exist, rather than the password not meeting requirements

ah of course. Interesting. I think the approach you outlined makes sense. The other option is to do a supportsMultidatabase call before and use the old/new syntax depending on that. It's a little problematic though because some old version (4.1, 4.2) are reported as not supporting multi database after they went out of support...

[daveajrussell]

😬

okay cool, thanks for confirming!

I've pushed the change and streamlined the epic slightly, less repetition of certain bits and added some accompanying comments

[OskarDamkjaer]

The "try and see if it works" mechanism is always a compromise so I think we should just always re-run. I think it's important the behavior is simple/understandable (for someone debugging from logs for example) and less brittle, even if it results in some unnecessary tries