From e24b27c79eb20b7fd8dbe568d08b61fc111436cf Mon Sep 17 00:00:00 2001 From: Marko Strukelj Date: Fri, 21 Oct 2016 17:03:54 +0200 Subject: [PATCH] KEYCLOAK-3801 Remove --unsafe from 'kcreg update' --- .../registration/cli/commands/UpdateCmd.java | 12 ++----- .../cli/registration/AbstractCliTest.java | 35 ++++++++++++++----- .../cli/registration/KcRegUpdateTest.java | 17 --------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/integration/client-cli/client-registration-cli/src/main/java/org/keycloak/client/registration/cli/commands/UpdateCmd.java b/integration/client-cli/client-registration-cli/src/main/java/org/keycloak/client/registration/cli/commands/UpdateCmd.java index f028c46591..23ccbe412f 100644 --- a/integration/client-cli/client-registration-cli/src/main/java/org/keycloak/client/registration/cli/commands/UpdateCmd.java +++ b/integration/client-cli/client-registration-cli/src/main/java/org/keycloak/client/registration/cli/commands/UpdateCmd.java @@ -85,9 +85,6 @@ public class UpdateCmd extends AbstractAuthOptionsCmd { @Option(shortName = 'm', name = "merge", description = "Merge new values with existing configuration on the server", hasValue = false) private boolean mergeMode = true; - @Option(shortName = 'u', name = "unsafe", description = "Allow updating without registration access token - no optimistic locking", hasValue = false) - private boolean allowUnsafe = true; - @Option(shortName = 'o', name = "output", description = "After update output the new client configuration", hasValue = false) private boolean outputClient = false; @@ -280,8 +277,6 @@ public class UpdateCmd extends AbstractAuthOptionsCmd { saveMergeConfig(cfg -> { setRegistrationToken(cfg.ensureRealmConfigData(server, realm), clientToUpdate, newToken); }); - } else if (!allowUnsafe) { - throw new RuntimeException("No Registration Access Token found for client: " + clientId + ". Provide one or use --unsafe."); } // merge local representation over remote one @@ -373,7 +368,6 @@ public class UpdateCmd extends AbstractAuthOptionsCmd { out.println(" -f, --file FILENAME Use the file or standard input if '-' is specified"); out.println(" -m, --merge Merge new values with existing configuration on the server"); out.println(" Merge is automatically enabled unless --file is specified"); - out.println(" -u, --unsafe Allow updating without registration access token - no optimistic locking"); out.println(" -o, --output After update output the new client configuration"); out.println(" -c, --compressed Don't pretty print the output"); out.println(); @@ -389,9 +383,9 @@ public class UpdateCmd extends AbstractAuthOptionsCmd { out.println("and the following items are shifted."); out.println(); out.println("Merged mode fetches current configuration from the server, applies attribute changes to it, and sends it"); - out.println("back to the server, overwriting existing configuration there. To ensure there are no unexpected changes"); - out.println("Registration Access Token is used for authorization when doing changes. Alternatively, one can specify to use"); - out.println("unsafe mode in which case login session's authorization is used - user requires manage-clients permission."); + out.println("back to the server, overwriting existing configuration there. If available, Registration Access Token is used "); + out.println("for authorization when doing changes. Otherwise current session's authorization is used in which case user needs"); + out.println("manage-clients permission for update to work."); out.println(); out.println(); out.println("Examples:"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/AbstractCliTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/AbstractCliTest.java index e44a3485fc..ce6cd0513d 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/AbstractCliTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/AbstractCliTest.java @@ -430,7 +430,7 @@ public abstract class AbstractCliTest extends AbstractKeycloakTest { exe = execute("get test-client --no-config --server " + serverUrl + " --realm test " + credentials + " " + extraOptions); - Assert.assertEquals("exitCode == 0", 0, exe.exitCode()); + assertExitCodeAndStdErrSize(exe, 0, 1); ClientRepresentation client2 = JsonSerialization.readValue(exe.stdout(), ClientRepresentation.class); Assert.assertEquals("clientId", "test-client", client2.getClientId()); @@ -449,7 +449,7 @@ public abstract class AbstractCliTest extends AbstractKeycloakTest { // because the previous invocation didn't use a registration access token exe = execute("get test-client --no-config --server " + serverUrl + " --realm test " + extraOptions + " -t " + client.getRegistrationAccessToken()); - Assert.assertEquals("exitCode == 0", 0, exe.exitCode()); + assertExitCodeAndStdErrSize(exe, 0, 0); ClientRepresentation client3 = JsonSerialization.readValue(exe.stdout(), ClientRepresentation.class); Assert.assertEquals("clientId", "test-client", client3.getClientId()); @@ -466,9 +466,9 @@ public abstract class AbstractCliTest extends AbstractKeycloakTest { exe = execute("update test-client --no-config --server " + serverUrl + " --realm test " + - credentials + " " + extraOptions + " -s enabled=false -o --unsafe"); + credentials + " " + extraOptions + " -s enabled=false -o"); - Assert.assertEquals("exitCode == 0", 0, exe.exitCode()); + assertExitCodeAndStdErrSize(exe, 0, 1); ClientRepresentation client4 = JsonSerialization.readValue(exe.stdout(), ClientRepresentation.class); Assert.assertEquals("clientId", "test-client", client4.getClientId()); @@ -488,7 +488,7 @@ public abstract class AbstractCliTest extends AbstractKeycloakTest { exe = execute("update test-client --no-config --server " + serverUrl + " --realm test " + extraOptions + " -s enabled=true -o -t " + client3.getRegistrationAccessToken()); - Assert.assertEquals("exitCode == 0", 0, exe.exitCode()); + assertExitCodeAndStdErrSize(exe, 0, 0); ClientRepresentation client5 = JsonSerialization.readValue(exe.stdout(), ClientRepresentation.class); Assert.assertEquals("clientId", "test-client", client5.getClientId()); @@ -527,10 +527,29 @@ public abstract class AbstractCliTest extends AbstractKeycloakTest { Assert.assertEquals("config file not modified", lastModified, lastModified2); } + void assertExitCodeAndStdOutSize(KcRegExec exe, int exitCode, int stdOutLineCount) { + assertExitCodeAndStreamSizes(exe, exitCode, stdOutLineCount, -1); + } + + void assertExitCodeAndStdErrSize(KcRegExec exe, int exitCode, int stdErrLineCount) { + assertExitCodeAndStreamSizes(exe, exitCode, -1, stdErrLineCount); + } + void assertExitCodeAndStreamSizes(KcRegExec exe, int exitCode, int stdOutLineCount, int stdErrLineCount) { Assert.assertEquals("exitCode == " + exitCode, exitCode, exe.exitCode()); - Assert.assertTrue("stdout output has " + stdOutLineCount + " lines", exe.stdoutLines().size() == stdOutLineCount); - Assert.assertTrue("stderr output has " + stdErrLineCount + " lines", exe.stderrLines().size() == stdErrLineCount); - + if (stdOutLineCount != -1) { + try { + Assert.assertTrue("stdout output has " + stdOutLineCount + " lines", exe.stdoutLines().size() == stdOutLineCount); + } catch (Throwable e) { + throw new AssertionError("STDOUT: " + exe.stdoutString(), e); + } + } + if (stdErrLineCount != -1) { + try { + Assert.assertTrue("stderr output has " + stdErrLineCount + " lines", exe.stderrLines().size() == stdErrLineCount); + } catch (Throwable e) { + throw new AssertionError("STDERR: " + exe.stderrString(), e); + } + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegUpdateTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegUpdateTest.java index d21c1a3e62..cebc5fc56e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegUpdateTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegUpdateTest.java @@ -148,23 +148,6 @@ public class KcRegUpdateTest extends AbstractCliTest { Assert.assertTrue("stderr is empty", exe.stderrLines().isEmpty()); Assert.assertNull("my_client registration token", handler.loadConfig().ensureRealmConfigData(serverUrl, realm).getClients().get("my_client")); - - - - // test update without registration access token to produce 'unsafe' error - exe = execute("update my_client --config '" + configFile.getName() + "' -o -s bearerOnly=true"); - - Assert.assertEquals("exitCode == 1", 1, exe.exitCode()); - Assert.assertFalse("stderr is not empty", exe.stderrLines().isEmpty()); - Assert.assertEquals("error message", "No Registration Access Token found for client: my_client. Provide one or use --unsafe.", exe.stderrLines().get(0)); - - - // test using unsafe to perform update - exe = execute("update my_client --config '" + configFile.getName() + "' -o -s bearerOnly=true --unsafe"); - - Assert.assertEquals("exitCode == 0", 0, exe.exitCode()); - Assert.assertTrue("stderr is empty", exe.stderrLines().isEmpty()); - } } }