From 2524172c12b8707a1c1cd0a9a2289547095b774b Mon Sep 17 00:00:00 2001 From: "Michael N. Lipp" Date: Wed, 5 Mar 2025 13:02:00 +0100 Subject: [PATCH] Centralize evaluation of console accessibility. --- .../vmoperator/common/VmDefinition.java | 47 ++++++++------ .../jdrupes/vmoperator/vmaccess/VmAccess.java | 62 +++++++++++++------ .../vmaccess/browser/VmAccess-functions.ts | 24 ++----- .../vmoperator/vmmgmt/VmMgmt-view.ftl.html | 9 ++- .../org/jdrupes/vmoperator/vmmgmt/VmMgmt.java | 9 ++- 5 files changed, 85 insertions(+), 66 deletions(-) diff --git a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java index 6a27723..f763c47 100644 --- a/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java +++ b/org.jdrupes.vmoperator.common/src/org/jdrupes/vmoperator/common/VmDefinition.java @@ -39,6 +39,8 @@ import java.util.function.Function; import java.util.logging.Logger; import java.util.stream.Collectors; import org.jdrupes.vmoperator.common.Constants.Status; +import org.jdrupes.vmoperator.common.Constants.Status.Condition; +import org.jdrupes.vmoperator.common.Constants.Status.Condition.Reason; import org.jdrupes.vmoperator.util.DataPath; /** @@ -363,9 +365,17 @@ public class VmDefinition extends K8sDynamicModel { } /** - * Check if the console is accessible. Returns true if the console is - * currently unused, used by the given user or if the permissions - * allow taking over the console. + * Check if the console is accessible. Always returns `true` if + * the VM is running and the permissions allow taking over the + * console. Else, returns `true` if + * + * * the permissions allow access to the console and + * + * * the VM is running and + * + * * the console is currently unused or used by the given user and + * + * * if user login is requested, the given user is logged in. * * @param user the user * @param permissions the permissions @@ -373,32 +383,29 @@ public class VmDefinition extends K8sDynamicModel { */ @SuppressWarnings("PMD.SimplifyBooleanReturns") public boolean consoleAccessible(String user, Set permissions) { - // If user has takeConsole permission, console is always accessible - if (permissions.contains(VmDefinition.Permission.TAKE_CONSOLE)) { + // Basic checks + if (!conditionStatus(Condition.RUNNING).orElse(false)) { + return false; + } + if (permissions.contains(Permission.TAKE_CONSOLE)) { return true; } - - // Check if an automatic login is requested. If so, allow access only - // if the log in has been established - var wantedLogIn = DataPath. get(spec(), "vm", "display", - "loggedInUser").orElse(null); - if (wantedLogIn != null - && !wantedLogIn.equals(status().get(Status.LOGGED_IN_USER))) { + if (!permissions.contains(Permission.ACCESS_CONSOLE)) { return false; } - // If the console is not in use, allow access - if (!conditionStatus("ConsoleConnected").orElse(true)) { - return true; + // If the console is in use by another user, deny access + if (conditionStatus(Condition.CONSOLE_CONNECTED).orElse(false) + && !consoleUser().map(cu -> cu.equals(user)).orElse(false)) { + return false; } - // If the console is in use by the user, allow access - if (consoleUser().map(cu -> cu.equals(user)).orElse(true)) { + // If no login is requested, allow access, else check if user matches + if (condition(Condition.USER_LOGGED_IN).map(V1Condition::getReason) + .map(r -> Reason.NOT_REQUESTED.equals(r)).orElse(false)) { return true; } - - // Else deny access - return false; + return user.equals(status().get(Status.LOGGED_IN_USER)); } /** diff --git a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java index 5625a4d..91642f1 100644 --- a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java +++ b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/VmAccess.java @@ -524,6 +524,13 @@ public class VmAccess extends FreeMarkerConlet { .assignedTo(user)).get().stream().findFirst(); } + /** + * Returns the permissions from the VM definition. + * + * @param vmDef the VM definition + * @param session the session + * @return the sets the + */ private Set permissions(VmDefinition vmDef, Session session) { var user = WebConsoleUtils.userFromSession(session) .map(ConsoleUser::getName).orElse(null); @@ -532,6 +539,13 @@ public class VmAccess extends FreeMarkerConlet { return vmDef.permissionsFor(user, roles); } + /** + * Returns the permissions from the pool. + * + * @param pool the pool + * @param session the session + * @return the sets the + */ private Set permissions(VmPool pool, Session session) { var user = WebConsoleUtils.userFromSession(session) .map(ConsoleUser::getName).orElse(null); @@ -540,23 +554,33 @@ public class VmAccess extends FreeMarkerConlet { return pool.permissionsFor(user, roles); } - private Set permissions(ResourceModel model, Session session, - VmPool pool, VmDefinition vmDef) throws InterruptedException { + /** + * Returns the permissions from the VM definition or the pool depending + * on the state of the model. + * + * @param session the session + * @param model the model + * @param vmDef the vm def + * @return the sets the + * @throws InterruptedException the interrupted exception + */ + private Set permissions(Session session, ResourceModel model, + VmDefinition vmDef) throws InterruptedException { var user = WebConsoleUtils.userFromSession(session) .map(ConsoleUser::getName).orElse(null); var roles = WebConsoleUtils.rolesFromSession(session) .stream().map(ConsoleRole::getName).toList(); if (model.mode() == ResourceModel.Mode.POOL) { - if (pool == null) { - pool = appPipeline.fire(new GetPools() - .withName(model.name())).get().stream().findFirst() - .orElse(null); - } + // Use permissions from pool + var pool = appPipeline.fire(new GetPools().withName(model.name())) + .get().stream().findFirst().orElse(null); if (pool == null) { return Collections.emptySet(); } return pool.permissionsFor(user, roles); } + + // Use permissions from VM if (vmDef == null) { vmDef = appPipeline.fire(new GetVms().assignedFrom(model.name()) .assignedTo(user)).get().stream().map(VmData::definition) @@ -578,7 +602,7 @@ public class VmAccess extends FreeMarkerConlet { VmDefinition vmDef) throws InterruptedException { channel.respond(new NotifyConletView(type(), model.getConletId(), "updateConfig", model.mode(), model.name(), - permissions(model, channel.session(), null, vmDef).stream() + permissions(channel.session(), model, vmDef).stream() .map(VmDefinition.Permission::toString).toList())); } @@ -589,12 +613,17 @@ public class VmAccess extends FreeMarkerConlet { model.setAssignedVm(null); } else { model.setAssignedVm(vmDef.name()); + var session = channel.session(); + var user = WebConsoleUtils.userFromSession(session) + .map(ConsoleUser::getName).orElse(null); + var perms = permissions(session, model, vmDef); try { - data = Map.of("metadata", - Map.of("namespace", vmDef.namespace(), + data = Map.of( + "metadata", Map.of("namespace", vmDef.namespace(), "name", vmDef.name()), "spec", vmDef.spec(), - "status", vmDef.status()); + "status", vmDef.status(), + "consoleAccessible", vmDef.consoleAccessible(user, perms)); } catch (JsonSyntaxException e) { logger.log(Level.SEVERE, e, () -> "Failed to serialize VM definition"); @@ -635,6 +664,8 @@ public class VmAccess extends FreeMarkerConlet { // Update known conlets for (var entry : conletIdsByConsoleConnection().entrySet()) { var connection = entry.getKey(); + var user = WebConsoleUtils.userFromSession(connection.session()) + .map(ConsoleUser::getName).orElse(null); for (var conletId : entry.getValue()) { var model = stateFromSession(connection.session(), conletId); if (model.isEmpty() @@ -655,9 +686,6 @@ public class VmAccess extends FreeMarkerConlet { } else { // Check if VM is used by pool conlet or to be assigned to // it - var user - = WebConsoleUtils.userFromSession(connection.session()) - .map(ConsoleUser::getName).orElse(null); var toBeUsedByConlet = vmDef.assignment() .map(Assignment::pool) .map(p -> p.equals(model.get().name())).orElse(false) @@ -752,7 +780,7 @@ public class VmAccess extends FreeMarkerConlet { var vmChannel = vmData.get().channel(); var vmDef = vmData.get().definition(); var vmName = vmDef.metadata().getName(); - var perms = permissions(model, channel.session(), null, vmDef); + var perms = permissions(channel.session(), model, vmDef); var resourceBundle = resourceBundle(channel.locale()); switch (event.method()) { case "start": @@ -776,9 +804,7 @@ public class VmAccess extends FreeMarkerConlet { } break; case "openConsole": - if (perms.contains(VmDefinition.Permission.ACCESS_CONSOLE)) { - openConsole(channel, model, vmChannel, vmDef, perms); - } + openConsole(channel, model, vmChannel, vmDef, perms); break; default:// ignore break; diff --git a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/browser/VmAccess-functions.ts b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/browser/VmAccess-functions.ts index 21fadfd..47e6e11 100644 --- a/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/browser/VmAccess-functions.ts +++ b/org.jdrupes.vmoperator.vmaccess/src/org/jdrupes/vmoperator/vmaccess/browser/VmAccess-functions.ts @@ -1,6 +1,6 @@ /* * VM-Operator - * Copyright (C) 2024 Michael N. Lipp + * Copyright (C) 2024,2025 Michael N. Lipp * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -71,12 +71,10 @@ window.orgJDrupesVmOperatorVmAccess.initPreview = (previewDom: HTMLElement, const poolName = computed(() => previewApi.poolName); const vmName = computed(() => previewApi.vmDefinition.name); const configured = computed(() => previewApi.vmDefinition.spec); + const accessible = computed(() => previewApi.vmDefinition.consoleAccessible); const busy = computed(() => previewApi.vmDefinition.spec && (previewApi.vmDefinition.spec.vm.state === 'Running' - && ((previewApi.poolName - && previewApi.vmDefinition.userLoginRequested) - ? !previewApi.vmDefinition.userLoggedIn - : !previewApi.vmDefinition.running) + && (!previewApi.vmDefinition.consoleAccessible) || previewApi.vmDefinition.spec.vm.state === 'Stopped' && previewApi.vmDefinition.running)); const startable = computed(() => previewApi.vmDefinition.spec @@ -88,8 +86,6 @@ window.orgJDrupesVmOperatorVmAccess.initPreview = (previewDom: HTMLElement, previewApi.vmDefinition.spec.vm.state !== 'Stopped' && previewApi.vmDefinition.running); const running = computed(() => previewApi.vmDefinition.running); - const userLoginRequested = computed(() => previewApi.vmDefinition.userLoginRequested); - const userLoggedIn = computed(() => previewApi.vmDefinition.userLoggedIn); const inUse = computed(() => previewApi.vmDefinition.usedBy != ''); const permissions = computed(() => previewApi.permissions); const osicon = computed(() => { @@ -125,8 +121,8 @@ window.orgJDrupesVmOperatorVmAccess.initPreview = (previewDom: HTMLElement, }; return { localize, resourceBase, vmAction, poolName, vmName, - configured, busy, startable, stoppable, running, userLoggedIn, - userLoginRequested, inUse, permissions, osicon }; + configured, accessible, busy, startable, stoppable, running, + inUse, permissions, osicon }; }, template: ` @@ -134,10 +130,7 @@ window.orgJDrupesVmOperatorVmAccess.initPreview = (previewDom: HTMLElement, diff --git a/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java b/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java index ac16803..00df484 100644 --- a/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java +++ b/org.jdrupes.vmoperator.vmmgmt/src/org/jdrupes/vmoperator/vmmgmt/VmMgmt.java @@ -249,14 +249,15 @@ public class VmMgmt extends FreeMarkerConlet { .toBigInteger()); // Build result + var perms = vmDef.permissionsFor(user, roles); return Map.of("metadata", Map.of("namespace", vmDef.namespace(), "name", vmDef.name()), "spec", spec, "status", status, "nodeName", vmDef.extra().map(VmExtraData::nodeName).orElse(""), - "permissions", vmDef.permissionsFor(user, roles).stream() - .map(VmDefinition.Permission::toString).toList()); + "consoleAccessible", vmDef.consoleAccessible(user, perms), + "permissions", perms); } /** @@ -438,9 +439,7 @@ public class VmMgmt extends FreeMarkerConlet { } break; case "openConsole": - if (perms.contains(VmDefinition.Permission.ACCESS_CONSOLE)) { - openConsole(channel, model, vmChannel, vmDef, user, perms); - } + openConsole(channel, model, vmChannel, vmDef, user, perms); break; case "cpus": fire(new ModifyVm(vmName, "currentCpus",
{ if (condition.type === "Running") { vmDefinition.running = condition.status === "True"; vmDefinition.runningConditionSince = new Date(condition.lastTransitionTime); - } else if (condition.type === "UserLoggedIn") { - vmDefinition.userLoggedIn = condition.status === "True"; - vmDefinition.userLoginRequested = condition.reason !== "NotRequested"; } }) } else { diff --git a/org.jdrupes.vmoperator.vmmgmt/resources/org/jdrupes/vmoperator/vmmgmt/VmMgmt-view.ftl.html b/org.jdrupes.vmoperator.vmmgmt/resources/org/jdrupes/vmoperator/vmmgmt/VmMgmt-view.ftl.html index 6ec6ce3..533b2f4 100644 --- a/org.jdrupes.vmoperator.vmmgmt/resources/org/jdrupes/vmoperator/vmmgmt/VmMgmt-view.ftl.html +++ b/org.jdrupes.vmoperator.vmmgmt/resources/org/jdrupes/vmoperator/vmmgmt/VmMgmt-view.ftl.html @@ -60,21 +60,21 @@ @@ -86,8 +86,7 @@ ? 'computer-off.svg' : (entry.usedFrom ? 'computer-in-use.svg' : 'computer.svg'))" :title="localize('Open console')" - :aria-disabled="!entry['running'] - || !(entry.permissions.includes('accessConsole'))" + :aria-disabled="!entry.consoleAccessible" v-on:click="vmAction(entry.name, 'openConsole')">