Skip to content

Commit

Permalink
chore: improve logging around auth (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbutler
jbutler authored May 25, 2022
1 parent 385c9ed commit 5102704
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 17 deletions.
28 changes: 15 additions & 13 deletions integration/src/main/java/com/aws/greengrass/mqttbroker/ClientDeviceAuthorizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,12 @@ public ClientDeviceAuthorizer(ClientDeviceTrustManager trustManager, DeviceAuthC
@Override
public boolean checkValid(String clientId, String username, byte[] password) {
if (username == null || username.isEmpty()) {
LOG.error("No peer certificate provided");
LOG.atWarn().kv(CLIENT_ID, clientId).log("No peer certificate provided");
return false;
}

// Retrieve session ID and construct authorization request for MQTT CONNECT
String sessionId = trustManager.getSessionForCertificate(username);
LOG.atInfo().kv(CLIENT_ID, clientId).kv(SESSION_ID, sessionId).log("Retrieved client session");

try {
deviceAuthClient.attachThing(sessionId, clientId);
} catch (AuthenticationException e) {
Expand All @@ -78,7 +76,7 @@ public boolean checkValid(String clientId, String username, byte[] password) {
return new UserSessionPair(username, sessionId);
});
} else {
LOG.atInfo().kv(CLIENT_ID, clientId).kv(SESSION_ID, sessionId).log("Device isn't authorized to connect");
LOG.atWarn().kv(CLIENT_ID, clientId).kv(SESSION_ID, sessionId).log("Device isn't authorized to connect");
closeSession(sessionId);
}

Expand All @@ -87,14 +85,20 @@ public boolean checkValid(String clientId, String username, byte[] password) {

@Override
public boolean canWrite(Topic topic, String user, String client) {
LOG.atDebug().kv("topic", topic).kv("user", user).kv(CLIENT_ID, client).log("MQTT publish request");
return canDevicePerform(getSessionForClient(client, user), "mqtt:publish", "mqtt:topic:" + topic);
String resource = "mqtt:topic:" + topic;
boolean canPerform = canDevicePerform(getSessionForClient(client, user), "mqtt:publish", resource);
LOG.atDebug().kv("topic", topic).kv("isAllowed", canPerform).kv(CLIENT_ID, client)
.log("MQTT publish request");
return canPerform;
}

@Override
public boolean canRead(Topic topic, String user, String client) {
LOG.atDebug().kv("topic", topic).kv("user", user).kv(CLIENT_ID, client).log("MQTT subscribe request");
return canDevicePerform(getSessionForClient(client, user), "mqtt:subscribe", "mqtt:topicfilter:" + topic);
String resource = "mqtt:topicfilter:" + topic;
boolean canPerform = canDevicePerform(getSessionForClient(client, user), "mqtt:subscribe", resource);
LOG.atDebug().kv("topic", topic).kv("isAllowed", canPerform).kv(CLIENT_ID, client)
.log("MQTT subscribe request");
return canPerform;
}

private void closeSession(String sessionId) {
Expand All @@ -118,8 +122,6 @@ private boolean canDevicePerform(String sessionId, String operation, String reso

private boolean canDevicePerform(UserSessionPair sessionPair, String operation, String resource) {
if (sessionPair == null) {
LOG.atError().kv("operation", operation).kv("resource", resource)
.log("Unknown client request, denying request");
return false;
}

Expand All @@ -131,6 +133,7 @@ UserSessionPair getSessionForClient(String clientId, String username) {
if (pair != null && pair.getUsername().equals(username)) {
return pair;
}
LOG.atDebug().kv(CLIENT_ID, clientId).log("Unable to retrieve authorization session");
return null;
}

Expand All @@ -143,13 +146,11 @@ public String getID() {

@Override
public void onDisconnect(InterceptDisconnectMessage msg) {
LOG.atDebug().kv(CLIENT_ID, msg.getClientID()).log("On disconnect auth session handling");
closeAuthSession(msg.getClientID(), msg.getUsername());
}

@Override
public void onConnectionLost(InterceptConnectionLostMessage msg) {
LOG.atDebug().kv(CLIENT_ID, msg.getClientID()).log("On connection lost auth session handling");
closeAuthSession(msg.getClientID(), msg.getUsername());
}

Expand All @@ -161,7 +162,8 @@ private void closeAuthSession(String clientId, String username) {
try {
deviceAuthClient.closeSession(sessionId);
} catch (AuthorizationException e) {
LOG.atWarn().kv(CLIENT_ID, clientId).kv(SESSION_ID, sessionId).log("Session is already closed");
// This will happen under normal operating circumstances
LOG.atDebug().kv(CLIENT_ID, clientId).kv(SESSION_ID, sessionId).log("Session is already closed");
}
clientToSessionMap.remove(clientId, sessionPair);
}
Expand Down
8 changes: 4 additions & 4 deletions moquette-0.16/broker/src/main/java/io/moquette/interception/BrokerInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,17 @@ public void notifyClientConnected(final MqttConnectMessage msg) {
@Override
public void notifyClientDisconnected(final String clientID, final String username) {
for (final InterceptHandler handler : this.handlers.get(InterceptDisconnectMessage.class)) {
LOG.debug("Notifying MQTT client disconnection to interceptor. CId={}, username={}, interceptorId={}",
clientID, username, handler.getID());
LOG.debug("Notifying MQTT client disconnection to interceptor. CId={}, interceptorId={}",
clientID, handler.getID());
executor.execute(() -> handler.onDisconnect(new InterceptDisconnectMessage(clientID, username)));
}
}

@Override
public void notifyClientConnectionLost(final String clientID, final String username) {
for (final InterceptHandler handler : this.handlers.get(InterceptConnectionLostMessage.class)) {
LOG.debug("Notifying unexpected MQTT client disconnection to interceptor CId={}, username={}, " +
"interceptorId={}", clientID, username, handler.getID());
LOG.debug("Notifying unexpected MQTT client disconnection to interceptor CId={}, " +
"interceptorId={}", clientID, handler.getID());
executor.execute(() -> handler.onConnectionLost(new InterceptConnectionLostMessage(clientID, username)));
}
}
Expand Down

0 comments on commit 5102704

Please sign in to comment.