buffet: Handle XMPP authentication failures
For the nightly tests there is a fake GCD server used, and as a result
the XMPP client can't authenticate with the XMPP server (since the
access_token is also fake). This simply closes the XMPP connection if we
see a read fail, so that we don't get spammed with 0 size reads.
TEST=manual and FEATURES=test emerge-whirlwind buffet
BUG=brillo:338
Change-Id: Ie8db114fcc197f816f3cb56673d6f393fa05c8e5
Reviewed-on: https://chromium-review.googlesource.com/251587
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Nathan Bullock <nathanbullock@google.com>
Tested-by: Nathan Bullock <nathanbullock@google.com>
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 5851f04..3e8fd00 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -373,6 +373,17 @@
xmpp_client_->StartStream();
}
+void DeviceRegistrationInfo::OnFileCanReadWithoutBlocking(int fd) {
+ if (xmpp_client_ && xmpp_client_->GetFileDescriptor() == fd) {
+ if (!xmpp_client_->Read()) {
+ // Authentication failed or the socket was closed.
+ if (!fd_watcher_.StopWatchingFileDescriptor()) {
+ LOG(WARNING) << "Failed to stop the watcher";
+ }
+ }
+ }
+}
+
std::unique_ptr<base::DictionaryValue>
DeviceRegistrationInfo::BuildDeviceResource(chromeos::ErrorPtr* error) {
std::unique_ptr<base::DictionaryValue> commands =
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index 780aeb3..4409d25 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -54,11 +54,7 @@
~DeviceRegistrationInfo() override;
- void OnFileCanReadWithoutBlocking(int fd) override {
- if (xmpp_client_ && xmpp_client_->GetFileDescriptor() == fd) {
- xmpp_client_->Read();
- }
- }
+ void OnFileCanReadWithoutBlocking(int fd) override;
void OnFileCanWriteWithoutBlocking(int fd) override {
LOG(FATAL) << "No write watcher is configured";
diff --git a/buffet/xmpp/xmpp_client.cc b/buffet/xmpp/xmpp_client.cc
index d0f4a56..f98c6bb 100644
--- a/buffet/xmpp/xmpp_client.cc
+++ b/buffet/xmpp/xmpp_client.cc
@@ -61,11 +61,11 @@
} // namespace
-void XmppClient::Read() {
+bool XmppClient::Read() {
std::string msg;
if (!connection_->Read(&msg) || msg.size() <= 0) {
- LOG(ERROR) << "Failed to read from stream";
- return;
+ LOG(ERROR) << "Failed to read from stream. The socket was probably closed";
+ return false;
}
// TODO(nathanbullock): Need to add support for TLS (brillo:191).
@@ -82,6 +82,9 @@
if (std::string::npos != msg.find("success")) {
state_ = XmppState::kStreamRestartedPostAuthentication;
connection_->Write(BuildXmppStartStreamCommand());
+ } else if (std::string::npos != msg.find("not-authorized")) {
+ state_ = XmppState::kAuthenticationFailed;
+ return false;
}
break;
case XmppState::kStreamRestartedPostAuthentication:
@@ -114,6 +117,7 @@
default:
break;
}
+ return true;
}
void XmppClient::StartStream() {
diff --git a/buffet/xmpp/xmpp_client.h b/buffet/xmpp/xmpp_client.h
index 9eb5826..6d847ca 100644
--- a/buffet/xmpp/xmpp_client.h
+++ b/buffet/xmpp/xmpp_client.h
@@ -31,7 +31,7 @@
}
// Needs to be called when new data is available from the connection.
- void Read();
+ bool Read();
// Start talking to the XMPP server (authenticate, etc.)
void StartStream();
@@ -41,6 +41,7 @@
kNotStarted,
kStarted,
kAuthenticationStarted,
+ kAuthenticationFailed,
kStreamRestartedPostAuthentication,
kBindSent,
kSessionStarted,
diff --git a/buffet/xmpp/xmpp_client_unittest.cc b/buffet/xmpp/xmpp_client_unittest.cc
index 9736349..ef26f2e 100644
--- a/buffet/xmpp/xmpp_client_unittest.cc
+++ b/buffet/xmpp/xmpp_client_unittest.cc
@@ -34,8 +34,11 @@
"<required/></starttls><mechanisms "
"xmlns=\"urn:ietf:params:xml:ns:xmpp-sasl\"><mechanism>X-OAUTH2</mechanism>"
"<mechanism>X-GOOGLE-TOKEN</mechanism></mechanisms></stream:features>";
-constexpr char kAuthenticationResponse[] =
+constexpr char kAuthenticationSucceededResponse[] =
"<success xmlns=\"urn:ietf:params:xml:ns:xmpp-sasl\"/>";
+constexpr char kAuthenticationFailedResponse[] =
+ "<failure xmlns=\"urn:ietf:params:xml:ns:xmpp-sasl\"><not-authorized/>"
+ "</failure></stream:stream>";
constexpr char kRestartStreamResponse[] =
"<stream:stream from=\"clouddevices.gserviceaccount.com\" "
"id=\"BE7D34E0B7589E2A\" version=\"1.0\" "
@@ -143,12 +146,13 @@
XmppClient::XmppState::kAuthenticationStarted);
}
-TEST_F(XmppClientTest, HandleAuthenticationResponse) {
+TEST_F(XmppClientTest, HandleAuthenticationSucceededResponse) {
TestHelper::SetState(
xmpp_client_.get(),
XmppClient::XmppState::kAuthenticationStarted);
EXPECT_CALL(*connection_, Read(_))
- .WillOnce(DoAll(SetArgPointee<0>(kAuthenticationResponse), Return(true)));
+ .WillOnce(DoAll(SetArgPointee<0>(kAuthenticationSucceededResponse),
+ Return(true)));
EXPECT_CALL(*connection_, Write(kStartStreamMessage))
.WillOnce(Return(true));
xmpp_client_->Read();
@@ -156,6 +160,20 @@
XmppClient::XmppState::kStreamRestartedPostAuthentication);
}
+TEST_F(XmppClientTest, HandleAuthenticationFailedResponse) {
+ TestHelper::SetState(
+ xmpp_client_.get(),
+ XmppClient::XmppState::kAuthenticationStarted);
+ EXPECT_CALL(*connection_, Read(_))
+ .WillOnce(DoAll(SetArgPointee<0>(kAuthenticationFailedResponse),
+ Return(true)));
+ EXPECT_CALL(*connection_, Write(_))
+ .Times(0);
+ xmpp_client_->Read();
+ EXPECT_EQ(TestHelper::GetState(*xmpp_client_),
+ XmppClient::XmppState::kAuthenticationFailed);
+}
+
TEST_F(XmppClientTest, HandleStreamRestartedResponse) {
TestHelper::SetState(
xmpp_client_.get(),
