ws: use the failing "init" message as /login result

If we fail a login attempt in response to receiving a

  { "command": "init", "problem", "..." }

control message from the session program, then return the message in
full as the result text for the /login page.

This makes use of an until-now dead code path in cockpithandlers that
pretty-prints and sets JSON data as the body of the GET /cockpit/login
result if it's returned from cockpit_auth_login_finish().  Previously we
were returning a fancy fail.html-formatted page which the XHR request in
login.js was simply ignoring.

Going forward, this will allow us to forward more interesting errors to
the login page.

Adjust the auth unit tests to expect the response blob in the cases where
it's relevant instead of always expecting NULL on the error path.

We also have to adjust two integration tests that manually inspected
that the result of /login contained HTML: it no longer does, because
it's JSON.  Just inspect the request status reason, like login.js does.

Finally, there is a couple of cases where our "more intelligent" message
handling results in a more accurate error being reported: fix those in
the tests.

src/ws/cockpitauth.c

@@ -125,6 +125,9 @@ typedef struct {
  /* An authorize challenge from session */
  JsonObject *authorize;
 
+ /* The failing "init" message from the session */
+ JsonObject *init_failure;
+
  /* The conversation in progress */
  gchar *conversation;
 } CockpitSession;
@@ -156,6 +159,8 @@ cockpit_session_reset (gpointer data)
  session->cookie = NULL;
  self = session->auth;
 
+ g_clear_pointer (&session->init_failure, json_object_unref);
+
  /* No accessing session after this point */
 
  if (cookie)
@@ -750,6 +755,8 @@ on_transport_control (CockpitTransport *transport,
  if (!cockpit_json_get_string (options, "message", NULL, &message))
  message = NULL;
  propagate_problem_to_error (session, options, problem, message, &error);
+ g_clear_pointer (&session->init_failure, json_object_unref);
+ session->init_failure = json_object_ref (options);
  if (session->login_task == NULL)
  {
  g_message ("ignoring failure from session process: %s", error->message);
@@ -1535,13 +1542,17 @@ cockpit_auth_login_finish (CockpitAuth *self,
 
  g_return_val_if_fail (g_task_is_valid (result, self), NULL);
 
+ CockpitSession *session = g_task_get_task_data (G_TASK (result));
+
  if (!g_task_propagate_boolean (G_TASK (result), error))
- goto out;
+ {
+ if (session != NULL)
+ body = g_steal_pointer (&session->init_failure);
+ goto out;
+ }
 
- CockpitSession *session = g_task_get_task_data (G_TASK (result));
  g_return_val_if_fail (session != NULL, NULL);
  g_return_val_if_fail (session->login_task == NULL, NULL);
-
  cockpit_session_reset (session);
 
  if (session->authorize)

src/ws/test-auth.c

@@ -260,7 +260,10 @@ test_userpass_bad (Test *test,
  response = cockpit_auth_login_finish (test->auth, result, NULL, headers, &error);
  g_object_unref (result);
 
- g_assert (response == NULL);
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, "authentication-failed");
+ g_clear_pointer (&response, json_object_unref);
+
  g_assert_error (error, COCKPIT_ERROR, COCKPIT_ERROR_AUTHENTICATION_FAILED);
  g_clear_error (&error);
 
@@ -289,7 +292,10 @@ test_userpass_emptypass (Test *test,
  response = cockpit_auth_login_finish (test->auth, result, NULL, headers, &error);
  g_object_unref (result);
 
- g_assert (response == NULL);
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, "authentication-failed");
+ g_clear_pointer (&response, json_object_unref);
+
  g_assert_error (error, COCKPIT_ERROR, COCKPIT_ERROR_AUTHENTICATION_FAILED);
  g_clear_error (&error);
 
@@ -461,8 +467,10 @@ test_max_startups (Test *test,
  g_main_context_iteration (NULL, TRUE);
  response = cockpit_auth_login_finish (test->auth, result1, NULL, NULL, &error1);
  g_object_unref (result1);
- g_assert (response == NULL);
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, "authentication-failed");
  g_assert_cmpstr ("Authentication failed", ==, error1->message);
+ g_clear_pointer (&response, json_object_unref);
 
  /* Now that first is finished we can successfully run another one */
  g_hash_table_insert (headers_fail, g_strdup ("Authorization"), g_strdup ("testscheme fail"));
@@ -473,7 +481,9 @@ test_max_startups (Test *test,
  g_main_context_iteration (NULL, TRUE);
  response = cockpit_auth_login_finish (test->auth, result3, NULL, NULL, &error3);
  g_object_unref (result3);
- g_assert (response == NULL);
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, "authentication-failed");
+ g_clear_pointer (&response, json_object_unref);
  g_assert_cmpstr ("Authentication failed", ==, error3->message);
 
  g_clear_error (&error1);
@@ -489,6 +499,7 @@ typedef struct {
  const gchar *error_message;
  const gchar *warning;
  const gchar *path;
+ const gchar *init_problem;
  int error_code;
 } ErrorFixture;
 
@@ -530,7 +541,17 @@ test_custom_fail (Test *test,
  response = cockpit_auth_login_finish (test->auth, result, NULL, headers, &error);
  g_object_unref (result);
 
- g_assert (response == NULL);
+ if (fix->init_problem)
+ {
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, fix->init_problem);
+ g_clear_pointer (&response, json_object_unref);
+ }
+ else
+ {
+ g_assert (response == NULL);
+ }
+
  if (fix->error_code)
  g_assert_error (error, COCKPIT_ERROR, fix->error_code);
  else
@@ -679,36 +700,41 @@ static const SuccessFixture fixture_ssh_alt = {
 
 static const ErrorFixture fixture_bad_conversation = {
  .header = "X-Conversation conversation-id xxx",
- .error_message = "Invalid conversation token"
+ .error_message = "Invalid conversation token",
 };
 
 static const ErrorFixture fixture_ssh_basic_failed = {
  .error_message = "Authentication failed",
- .header = "Basic dXNlcjp0aGlzIGlzIHRoZSBwYXNzd29yZA=="
+ .header = "Basic dXNlcjp0aGlzIGlzIHRoZSBwYXNzd29yZA==",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorFixture fixture_ssh_remote_basic_failed = {
  .error_message = "Authentication failed",
  .header = "Basic d3Jvbmc6dGhpcyBpcyB0aGUgbWFjaGluZSBwYXNzd29yZA==",
- .path = "/cockpit+=machine"
+ .path = "/cockpit+=machine",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorFixture fixture_ssh_not_supported = {
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed: authentication-not-supported",
  .header = "testsshscheme not-supported",
+ .init_problem = "authentication-not-supported",
 };
 
 static const ErrorFixture fixture_ssh_auth_failed = {
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed",
  .header = "testsshscheme ssh-fail",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorFixture fixture_ssh_auth_with_error = {
  .error_code = COCKPIT_ERROR_FAILED,
  .error_message = "Authentication failed: unknown: detail for error",
  .header = "testsshscheme with-error",
+ .init_problem = "unknown",
 };
 
 static const SuccessFixture fixture_no_cookie = {
@@ -739,18 +765,21 @@ static const ErrorFixture fixture_auth_failed = {
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed",
  .header = "testscheme fail",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorFixture fixture_auth_denied = {
  .error_code = COCKPIT_ERROR_PERMISSION_DENIED,
  .error_message = "Permission denied",
  .header = "testscheme denied",
+ .init_problem = "access-denied",
 };
 
 static const ErrorFixture fixture_auth_with_error = {
  .error_code = COCKPIT_ERROR_FAILED,
  .error_message = "Authentication failed: unknown: detail for error",
  .header = "testscheme with-error",
+ .init_problem = "unknown",
 };
 
 static const ErrorFixture fixture_auth_none = {
@@ -783,6 +812,7 @@ typedef struct {
  const gchar *error_message;
  const gchar *message;
  int error_code;
+ const gchar *init_problem;
  int pause;
 } ErrorMultiFixture;
 
@@ -1030,7 +1060,17 @@ test_multi_step_fail (Test *test,
  }
  else
  {
- g_assert (response == NULL);
+ if (fix->init_problem)
+ {
+ g_assert (response != NULL);
+ g_assert_cmpstr (json_object_get_string_member (response, "problem"), ==, fix->init_problem);
+ g_clear_pointer (&response, json_object_unref);
+ }
+ else
+ {
+ g_assert (response == NULL);
+ }
+
  if (fix->error_code)
  g_assert_error (error, COCKPIT_ERROR, fix->error_code);
  else
@@ -1076,20 +1116,23 @@ static const ErrorMultiFixture fixture_fail_three_steps = {
  .prompts = three_prompts,
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorMultiFixture fixture_fail_two_steps = {
  .headers = two_steps_wrong,
  .prompts = two_prompts,
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorMultiFixture fixture_fail_ssh_two_steps = {
  .headers = two_steps_ssh_wrong,
  .prompts = two_prompts,
  .error_code = COCKPIT_ERROR_AUTHENTICATION_FAILED,
  .error_message = "Authentication failed",
+ .init_problem = "authentication-failed",
 };
 
 static const ErrorMultiFixture fixture_fail_step_timeout = {

test/verify/check-static-login

@@ -875,7 +875,7 @@ matchrule = <SUBJECT>^DC=LAN,DC=COCKPIT,CN=alice$
  # another certificate gets rejected
  self.allow_journal_messages("cockpit-session: No matching user for certificate")
  do_test(["--cert", self.vm_tmpdir + "/bob.pem", "--key", self.vm_tmpdir + "/bob.key"],
- ["HTTP/1.1 401 Authentication failed", '<h1>Authentication failed</h1>'],
+ ["HTTP/1.1 401 Authentication failed"],
  not_expected=["crsf-token"])
 
  # check expired certificate
@@ -893,7 +893,7 @@ matchrule = <SUBJECT>^DC=LAN,DC=COCKPIT,CN=alice$
  m.write("/etc/cockpit/cockpit.conf", "[Basic]\naction = none\n", append=True)
  do_test(alice_cert_key, ['HTTP/1.1 200 OK', '"csrf-token"'])
  do_test(['-u', 'alice:foobar123'],
- ['HTTP/1.1 401 Authentication disabled', '<h1>Authentication disabled</h1>'],
+ ['HTTP/1.1 401 Authentication disabled'],
  not_expected=["crsf-token"])
 
  # wwithout a CA, alice's cert fails
@@ -906,7 +906,7 @@ matchrule = <SUBJECT>^DC=LAN,DC=COCKPIT,CN=alice$
  self.allow_journal_messages("cockpit-session: Failed to map .* Could not activate remote peer.*")
  self.allow_journal_messages("cockpit-session: Failed to map .* Unit sssd-ifp.service is masked.")
  m.execute("systemctl mask sssd-ifp; systemctl stop sssd-ifp")
- do_test(alice_cert_key, ["HTTP/1.1 401 Authentication failed", '<h1>Authentication failed</h1>'],
+ do_test(alice_cert_key, ["HTTP/1.1 401 Authentication failed"],
  not_expected=["crsf-token"])
  m.execute("systemctl unmask sssd-ifp")
 

test/verify/check-system-realms

@@ -432,7 +432,7 @@ class CommonTests:
  self.allow_journal_messages("cockpit-session: user account access failed: 4 alice: System error")
 
  # cert auth should not be enabled by default
- do_test(alice_cert_key, ["HTTP/1.1 401 Authentication required", '"authorize"'])
+ do_test(alice_cert_key, ["HTTP/1.1 401 Authentication failed", '"authorize"'])
  # password auth should work (but might need to be retried)
  do_test(alice_user_pass, ['HTTP/1.1 200 OK', '"csrf-token"'], session_leader='cockpit-session', retry=True)
 
@@ -458,14 +458,14 @@ class CommonTests:
  self.allow_journal_messages("cockpit-session: No matching user for certificate")
  m.upload(["bob.pem", "bob.key"], "/var/tmp", relative_dir="src/tls/ca/")
  do_test(['--cert', "/var/tmp/bob.pem", '--key', "/var/tmp/bob.key"],
- ["HTTP/1.1 401 Authentication failed", '<h1>Authentication failed</h1>'],
+ ["HTTP/1.1 401 Authentication failed"],
  not_expected=["crsf-token"])
  self.allow_journal_messages("cockpit-session: Failed to map certificate to user: .* Invalid certificate provided")
 
  # disallow password auth
  m.write("/etc/cockpit/cockpit.conf", "[Basic]\naction = none\n", append=True)
  do_test(alice_cert_key, ['HTTP/1.1 200 OK', '"csrf-token"'])
- do_test(alice_user_pass, ['HTTP/1.1 401 Authentication disabled', '<h1>Authentication disabled</h1>'],
+ do_test(alice_user_pass, ['HTTP/1.1 401 Authentication disabled'],
  not_expected=["crsf-token"])
 
  # valid user certificate which fails CA validation