From 22afa1455e509b09d60745e758bd7b2aecafe646 Mon Sep 17 00:00:00 2001 From: Udo Walter Date: Thu, 14 May 2020 00:26:58 +0200 Subject: Fix two ways a client could be listed as free but be not connectable. 1. User gets assigned a client but never connects to the vnc. Client is "in use" but ist actually idleing in the login screen. The client gets updated and the state set to IDLE but the inUseBy is not reset because the vnc password stays the same for some time. Clients in IDLE state with no user connected to the vnc tunnel now get reset to not in use. 2. If a client is missing from the JSON list and had IDLE as the last status it was still listed as IDLE until the timeout (5 minutes) where it is removed entirely. Missing clients waiting to timeout are now set to OFFLINE state. --- .../de/bwlehrpool/bwlp_guac/AvailableClient.java | 26 +++++++++++-- .../de/bwlehrpool/bwlp_guac/ConnectionManager.java | 45 +++++++++++++++++----- .../java/de/bwlehrpool/bwlp_guac/JsonGroup.java | 3 -- .../de/bwlehrpool/bwlp_guac/TunnelListener.java | 40 +++++++++++++++++++ .../de/bwlehrpool/bwlp_guac/WrappedConnection.java | 12 +++++- src/main/resources/guac-manifest.json | 1 + 6 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 src/main/java/de/bwlehrpool/bwlp_guac/TunnelListener.java diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java index 0a9e8ec..5d551d9 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java @@ -28,6 +28,8 @@ public class AvailableClient implements Cloneable { private String inUseBy; + private WrappedConnection connection; + private long deadline; private long lastConnectionCheck; @@ -52,6 +54,13 @@ public class AvailableClient implements Cloneable { * can't possibly be in use by the old user anymore. */ public synchronized void update(JsonClient source) { + if (this.inUseBy != null && source.state != State.OCCUPIED && !TunnelListener.hasTunnel(this.inUseBy)) { + LOGGER.info("Free client blocked by a disconnected user detected."); + LOGGER.info("Client " + this + " is available again"); + this.inUseBy = null; + if (this.connection != null) this.connection.invalidate(); + } + if (this.password == null || !this.password.equals(source.password)) { if (source.state != State.OCCUPIED) { if (this.inUseBy != null) { @@ -62,7 +71,10 @@ public class AvailableClient implements Cloneable { this.lastConnectionCheck = 0; this.password = source.password; } - this.state = source.state; + + if (this.inUseBy == null || source.state != State.IDLE) + this.state = source.state; + this.deadline = 0; } @@ -82,13 +94,17 @@ public class AvailableClient implements Cloneable { } public synchronized WrappedConnection getConnection(String expectedOwner) { - if (isInUseBy(expectedOwner)) - return new WrappedConnection(this.clientip + "/" + CON_ID.incrementAndGet(), this); + if (isInUseBy(expectedOwner)) { + if (this.connection == null || !this.connection.isValid()) + this.connection = new WrappedConnection(this.clientip + "/" + CON_ID.incrementAndGet(), this); + return this.connection; + } return null; } public synchronized void releaseConnection(String expectedOwner) { if (isInUseBy(expectedOwner)) { + if (this.connection != null) this.connection.invalidate(); LOGGER.info("Prematurely releasing client " + this); this.inUseBy = null; } else { @@ -121,6 +137,10 @@ public class AvailableClient implements Cloneable { return locationid; } + public void markAsMissing() { + this.state = State.OFFLINE; + } + public GuacamoleConfiguration toGuacConfig() { GuacamoleConfiguration cfg = new GuacamoleConfiguration(); cfg.setProtocol("vnc"); diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java index 00ec284..97dd851 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java @@ -82,6 +82,7 @@ public class ConnectionManager { } for (AvailableClient ac : clients) { + LOGGER.info("Looking for existing mapping: Checking client " + ac); if (ac.isInUseBy(user)) { LOGGER.info("Client " + ac + " is in use by " + user); freeClient = ac; @@ -90,6 +91,7 @@ public class ConnectionManager { } if (freeClient == null) { for (AvailableClient ac : clients) { + LOGGER.info("Looking for free client: Checking client " + ac); if (ac.claim(user)) { LOGGER.info("Claiming client " + ac + " for " + user); freeClient = ac; @@ -168,7 +170,6 @@ public class ConnectionManager { private static void populateList(byte[] data) { ObjectMapper mapper = new ObjectMapper(); - JsonClient[] list; JsonRoot root; try { root = mapper.readValue(data, JsonRoot.class); @@ -185,6 +186,7 @@ public class ConnectionManager { LOGGER.info("Group list null"); } synchronized (groupPool) { + HashSet processedGroups = new HashSet(); for (JsonGroup gnew : groups) { JsonGroup existing = groupPool.get(gnew.id); boolean redoClientMapping = false; @@ -210,12 +212,17 @@ public class ConnectionManager { } } } - existing.checked = true; + processedGroups.add(existing); LOGGER.info("Group " + gnew.name + " with pw " + gnew.password); } - for (JsonGroup group : groupPool.values()) { - if (group.checked) group.checked = false; - else groupPool.remove(group.id); + for (Iterator it = groupPool.values().iterator(); it.hasNext();) { + JsonGroup g = it.next(); + if (!processedGroups.contains(g)) { + for (AvailableClient client : g.clientList) { + client.groupList.remove(g); + } + it.remove(); + } } } @@ -223,32 +230,51 @@ public class ConnectionManager { LOGGER.info("Client list null"); } synchronized (clientPool) { + HashSet processedClients = new HashSet(); for (JsonClient cnew : root.clients) { if (cnew.password == null || cnew.clientip == null) continue; // Invalid AvailableClient existing = clientPool.get(cnew.clientip); if (existing == null) { // New client - AvailableClient newClient = new AvailableClient(cnew); - clientPool.put(cnew.clientip, newClient); + existing = new AvailableClient(cnew); + clientPool.put(cnew.clientip, existing); for (JsonGroup group : groupPool.values()) { for (int id : group.locationids) { if (id == cnew.locationid) { - group.clientList.add(newClient); - newClient.groupList.add(group); + group.clientList.add(existing); + existing.groupList.add(group); break; } } } LOGGER.info("New client " + cnew.clientip); } else { + if (existing.getLocationid() != cnew.locationid) { + for (JsonGroup group : groupPool.values()) { + for (int id : group.locationids) { + if (id == existing.getLocationid()) { + group.clientList.remove(existing); + existing.groupList.remove(group); + } + if (id == cnew.locationid) { + group.clientList.add(existing); + existing.groupList.add(group); + } + } + } + } existing.update(cnew); } + processedClients.add(existing); } final long NOW = System.currentTimeMillis(); for (Iterator it = clientPool.values().iterator(); it.hasNext();) { AvailableClient c = it.next(); + if (!processedClients.contains(c)) { + c.markAsMissing(); + } if (c.isTimeout(NOW)) { LOGGER.info("Removing client " + c + " from list"); for (JsonGroup group : c.groupList) { @@ -256,7 +282,6 @@ public class ConnectionManager { } it.remove(); } - } LOGGER.info("List updated. " + clientPool.size() + " clients."); } diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java index 27e769c..c5adf9e 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java @@ -39,7 +39,4 @@ public class JsonGroup { @JsonIgnore public ArrayList clientList = new ArrayList(); - @JsonIgnore - public boolean checked = false; - } diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/TunnelListener.java b/src/main/java/de/bwlehrpool/bwlp_guac/TunnelListener.java new file mode 100644 index 0000000..26aa93c --- /dev/null +++ b/src/main/java/de/bwlehrpool/bwlp_guac/TunnelListener.java @@ -0,0 +1,40 @@ +package de.bwlehrpool.bwlp_guac; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.event.TunnelCloseEvent; +import org.apache.guacamole.net.event.TunnelConnectEvent; +import org.apache.guacamole.net.event.listener.Listener; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashSet; + +/** + * A listener to track whether a user currently is connected to a tunnel. + */ +public class TunnelListener implements Listener { + + private static final Logger LOGGER = LoggerFactory.getLogger(TunnelListener.class); + + private static final HashSet usersWithTunnel = new HashSet(); + + public static boolean hasTunnel(String username) { + return usersWithTunnel.contains(username); + } + + @Override + public void handleEvent(Object event) throws GuacamoleException { + if (event instanceof TunnelConnectEvent) { + String username = ((TunnelConnectEvent)event).getCredentials().getUsername(); + LOGGER.info("User " + username + " connected to a tunnel."); + usersWithTunnel.add(username); + } + else if (event instanceof TunnelCloseEvent) { + String username = ((TunnelCloseEvent)event).getCredentials().getUsername(); + LOGGER.info("User " + username + " closed a tunnel."); + usersWithTunnel.remove(username); + } + + } + +} diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/WrappedConnection.java b/src/main/java/de/bwlehrpool/bwlp_guac/WrappedConnection.java index 3d46e73..a2ecb52 100644 --- a/src/main/java/de/bwlehrpool/bwlp_guac/WrappedConnection.java +++ b/src/main/java/de/bwlehrpool/bwlp_guac/WrappedConnection.java @@ -9,6 +9,8 @@ public class WrappedConnection extends SimpleConnection { private final AvailableClient ac; + private boolean valid = true; + public WrappedConnection(String name, AvailableClient ac) { super(name, name, makeConfig(ac)); this.ac = ac.clone(); @@ -20,7 +22,15 @@ public class WrappedConnection extends SimpleConnection { } public boolean checkConnection(int retries) { - return ac.checkConnection(retries); + return this.valid && ac.checkConnection(retries); + } + + public boolean isValid() { + return this.valid; + } + + public void invalidate() { + this.valid = false; } } diff --git a/src/main/resources/guac-manifest.json b/src/main/resources/guac-manifest.json index a71d8a9..178140b 100644 --- a/src/main/resources/guac-manifest.json +++ b/src/main/resources/guac-manifest.json @@ -5,6 +5,7 @@ "smallIcon" : "images/Logo_bwLehrpool_symbol.png", "largeIcon" : "images/Logo_bwLehrpool_symbol.png", "authProviders": ["de.bwlehrpool.bwlp_guac.BwlpAuthenticationProvider"], + "listeners" : ["de.bwlehrpool.bwlp_guac.TunnelListener"], "html" : [ "disclaimer.html" ], "translations" : [ "translations/en.json" -- cgit v1.2.3-55-g7522