Allow user to update expired password non-interactively #202
Conversation
pontusmelke
force-pushed
the
4.0-allow-password-change
branch
from
d460a9e to
ba16407
Compare
cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private boolean isAttemptingPasswordChange( CliArgs args ) { | ||
| return args.getCypher().map( (s) -> s.toUpperCase().startsWith( "ALTER CURRENT USER SET PASSWORD FROM" ) ).orElse( false ); |
There was a problem hiding this comment.
Isn't this detection a bit weak w.r.t e.g. comments in the beginning of a .cypher file or formatting whitespace, etc. ?
|
|
||
| SessionConfig.Builder builder = SessionConfig.builder() | ||
| .withDefaultAccessMode(AccessMode.WRITE) | ||
| .withDatabase(SYSTEM_DB_NAME); |
There was a problem hiding this comment.
Isn't this a bit weird? I think we will end up here, using the system-db, even if the user invoked it like this: cypher-shell -d my-db-which-is-not-the-system-db -u user -p pass 'ALTER CURRENT USER SET PASSWORD from "pass" to "newPass" ';
Also it is weird to have such connection things going on here, which usually happens in Main. Can we not use the same code (i.e. runUntilEnd), but parameterize connectMaybeInteractively somehow?
When a user ran ``` cypher-shell -d system -u neo4j -p neo4j --format=plain 'ALTER CURRENT USER SET PASSWORD from "neo4j" to "quality" '; ``` and the password `neo4j` was expired we still prompted the user for providing a new password. This also meant that the change password command later failed since the password had already been changed.
pontusmelke
force-pushed
the
4.0-allow-password-change
branch
from
ffe353f to
a18f541
Compare
pontusmelke
force-pushed
the
4.0-allow-password-change
branch
from
470010f to
a18bcd6
Compare
sherfert
left a comment
sherfert
left a comment
There was a problem hiding this comment.
I like this new design, and the more comprehensive tests.
Just some minor nitpicks.
| int majorVersion = getVersionAndCreateUserWithPasswordChangeRequired(); | ||
|
|
||
| //in 4.0 and later when the user attempts a non-interactive password update | ||
| if (majorVersion >= 4 ) { |
There was a problem hiding this comment.
You could use assume here, since this test has no value if running with neo4j 3.5 or older.
| int majorVersion = getVersionAndCreateUserWithPasswordChangeRequired(); | ||
|
|
||
| //in 4.0 and later when the user attempts a non-interactive password update | ||
| if (majorVersion >= 4 ) { |
| int majorVersion = getVersionAndCreateUserWithPasswordChangeRequired(); | ||
|
|
||
| //in 4.0 and later when the user attempts a non-interactive password update | ||
| if (majorVersion >= 4 ) { |
| shellRunner.runUntilEnd(); | ||
|
|
||
| main.runShell(cliArgs, shell, logger); | ||
| // ConnectionConfig connectionConfig = sac.connectionConfig; |
|
|
||
| CypherShell shell = new CypherShell( logger, prettyConfig, ShellRunner.shouldBeInteractive( cliArgs ), | ||
| cliArgs.getParameters() ); | ||
| System.exit( runShell( cliArgs, shell, logger ) ); |
There was a problem hiding this comment.
Can you split this into two lines? I know this is identical, but I think it will be easier to follow.
2 participants
When a user ran
and the password
neo4jwas expired we still prompted the user forproviding a new password. This also meant that the change password
command later failed since the password had already been changed.
changelog: Fix a bug where a user attempting to update an expired password non-interactively still was prompted for new password.