From 3c2b2170662bb0ade587618a2105267a0b7d544e Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 12 Aug 2022 09:45:48 +0200 Subject: Remove application role management from /user/v1 API --- .../controller/restapi/user/UserApiHandler.java | 20 -------------------- .../hosted/controller/restapi/user/UserApiTest.java | 17 ----------------- .../restapi/user/responses/application-roles.json | 19 ------------------- 3 files changed, 56 deletions(-) delete mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-roles.json diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index e10defb4416..d988d8709d2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -111,7 +111,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse handlePOST(Path path, HttpRequest request) { if (path.matches("/user/v1/tenant/{tenant}")) return addTenantRoleMember(path.get("tenant"), request); - if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return addApplicationRoleMember(path.get("tenant"), path.get("application"), request); return ErrorResponse.notFoundError(Text.format("No '%s' handler at '%s'", request.getMethod(), request.getUri().getPath())); @@ -119,7 +118,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse handleDELETE(Path path, HttpRequest request) { if (path.matches("/user/v1/tenant/{tenant}")) return removeTenantRoleMember(path.get("tenant"), request); - if (path.matches("/user/v1/tenant/{tenant}/application/{application}")) return removeApplicationRoleMember(path.get("tenant"), path.get("application"), request); return ErrorResponse.notFoundError(Text.format("No '%s' handler at '%s'", request.getMethod(), request.getUri().getPath())); @@ -280,15 +278,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { return new MessageResponse(user + " is now a member of " + roles.stream().map(Role::toString).collect(Collectors.joining(", "))); } - private HttpResponse addApplicationRoleMember(String tenantName, String applicationName, HttpRequest request) { - Inspector requestObject = bodyInspector(request); - String roleName = require("roleName", Inspector::asString, requestObject); - UserId user = new UserId(require("user", Inspector::asString, requestObject)); - Role role = Roles.toRole(TenantName.from(tenantName), ApplicationName.from(applicationName), roleName); - users.addUsers(role, List.of(user)); - return new MessageResponse(user + " is now a member of " + role); - } - private HttpResponse removeTenantRoleMember(String tenantName, HttpRequest request) { Inspector requestObject = bodyInspector(request); if (requestObject.field("roles").valid()) { @@ -348,15 +337,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { } } - private HttpResponse removeApplicationRoleMember(String tenantName, String applicationName, HttpRequest request) { - Inspector requestObject = bodyInspector(request); - String roleName = require("roleName", Inspector::asString, requestObject); - UserId user = new UserId(require("user", Inspector::asString, requestObject)); - Role role = Roles.toRole(TenantName.from(tenantName), ApplicationName.from(applicationName), roleName); - users.removeUsers(role, List.of(user)); - return new MessageResponse(user + " is no longer a member of " + role); - } - private boolean hasTrialCapacity() { if (! controller.system().isPublic()) return true; var existing = controller.tenants().asList().stream().map(Tenant::name).collect(Collectors.toList()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 3e9f6256134..85c9405082a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -99,12 +99,6 @@ public class UserApiTest extends ControllerContainerCloudTest { .data("{\"user\":\"developer@tenant\",\"roleName\":\"administrator\"}"), accessDenied, 403); - // POST a headless for a non-existent application fails. - tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", POST) - .roles(Set.of(Role.administrator(TenantName.from("my-tenant")))) - .data("{\"user\":\"headless@app\",\"roleName\":\"headless\"}"), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"role 'headless' of 'my-app' owned by 'my-tenant' not found\"}", 400); - // POST an application is allowed for a tenant developer. tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app", POST) .principal("developer@tenant") @@ -116,22 +110,11 @@ public class UserApiTest extends ControllerContainerCloudTest { .roles(Set.of(Role.administrator(id.tenant()))), accessDenied, 403); - // POST a tenant role is not allowed to an application. - tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app", POST) - .roles(Set.of(Role.hostedOperator())) - .data("{\"user\":\"developer@app\",\"roleName\":\"developer\"}"), - "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Malformed or illegal role name 'developer'.\"}", 400); - // GET tenant role information is available to readers. tester.assertResponse(request("/user/v1/tenant/my-tenant") .roles(Set.of(Role.reader(id.tenant()))), new File("tenant-roles.json")); - // GET application role information is available to tenant administrators. - tester.assertResponse(request("/user/v1/tenant/my-tenant/application/my-app") - .roles(Set.of(Role.administrator(id.tenant()))), - new File("application-roles.json")); - // POST a pem deploy key tester.assertResponse(request("/application/v4/tenant/my-tenant/application/my-app/key", POST) .roles(Set.of(Role.developer(id.tenant()))) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-roles.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-roles.json deleted file mode 100644 index 8497358fe40..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/application-roles.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "tenant": "my-tenant", - "application": "my-app", - "roleNames": [ ], - "users": [ - { - "name": "administrator@tenant", - "email": "administrator@tenant", - "verified": false, - "roles": { } - }, - { - "name": "developer@tenant", - "email": "developer@tenant", - "verified": false, - "roles": { } - } - ] -} -- cgit v1.2.3 From 361cf0516ade8b465b0441f3f8cd409d7680163a Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 12 Aug 2022 09:53:34 +0200 Subject: Only accept the new format for updating user roles --- .../controller/restapi/user/UserApiHandler.java | 35 ---------------------- .../controller/restapi/user/UserApiTest.java | 6 ++-- 2 files changed, 3 insertions(+), 38 deletions(-) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index d988d8709d2..c13f46dacde 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -253,21 +253,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse addTenantRoleMember(String tenantName, HttpRequest request) { Inspector requestObject = bodyInspector(request); - if (requestObject.field("roles").valid()) { - return addMultipleTenantRoleMembers(tenantName, requestObject); - } - return addTenantRoleMember(tenantName, requestObject); - } - - private HttpResponse addTenantRoleMember(String tenantName, Inspector requestObject) { - String roleName = require("roleName", Inspector::asString, requestObject); - UserId user = new UserId(require("user", Inspector::asString, requestObject)); - Role role = Roles.toRole(TenantName.from(tenantName), roleName); - users.addUsers(role, List.of(user)); - return new MessageResponse(user + " is now a member of " + role); - } - - private HttpResponse addMultipleTenantRoleMembers(String tenantName, Inspector requestObject) { var tenant = TenantName.from(tenantName); var user = new UserId(require("user", Inspector::asString, requestObject)); var roles = SlimeStream.fromArray(requestObject.field("roles"), Inspector::asString) @@ -280,26 +265,6 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse removeTenantRoleMember(String tenantName, HttpRequest request) { Inspector requestObject = bodyInspector(request); - if (requestObject.field("roles").valid()) { - return removeMultipleTenantRoleMembers(tenantName, requestObject); - } - return removeTenantRoleMember(tenantName, requestObject); - } - - private HttpResponse removeTenantRoleMember(String tenantName, Inspector requestObject) { - TenantName tenant = TenantName.from(tenantName); - String roleName = require("roleName", Inspector::asString, requestObject); - UserId user = new UserId(require("user", Inspector::asString, requestObject)); - List roles = Collections.singletonList(Roles.toRole(tenant, roleName)); - - enforceLastAdminOfTenant(tenant, user, roles); - removeDeveloperKey(tenant, user, roles); - users.removeFromRoles(user, roles); - - return new MessageResponse(user + " is no longer a member of " + roles.stream().map(Role::toString).collect(Collectors.joining(", "))); - } - - private HttpResponse removeMultipleTenantRoleMembers(String tenantName, Inspector requestObject) { var tenant = TenantName.from(tenantName); var user = new UserId(require("user", Inspector::asString, requestObject)); var roles = SlimeStream.fromArray(requestObject.field("roles"), Inspector::asString) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index 85c9405082a..1344b106bbe 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -84,7 +84,7 @@ public class UserApiTest extends ControllerContainerCloudTest { // POST a hosted operator role is not allowed. tester.assertResponse(request("/user/v1/tenant/my-tenant", POST) .roles(Set.of(Role.administrator(id.tenant()))) - .data("{\"user\":\"evil@evil\",\"roleName\":\"hostedOperator\"}"), + .data("{\"user\":\"evil@evil\",\"roles\":[\"hostedOperator\"]}"), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Malformed or illegal role name 'hostedOperator'.\"}", 400); // POST a tenant developer is available to the tenant owner. @@ -96,7 +96,7 @@ public class UserApiTest extends ControllerContainerCloudTest { // POST a tenant admin is not available to a tenant developer. tester.assertResponse(request("/user/v1/tenant/my-tenant", POST) .roles(Set.of(Role.developer(id.tenant()))) - .data("{\"user\":\"developer@tenant\",\"roleName\":\"administrator\"}"), + .data("{\"user\":\"developer@tenant\",\"roles\":[\"administrator\"]}"), accessDenied, 403); // POST an application is allowed for a tenant developer. @@ -183,7 +183,7 @@ public class UserApiTest extends ControllerContainerCloudTest { // DELETE the last tenant owner is not allowed. tester.assertResponse(request("/user/v1/tenant/my-tenant", DELETE) .roles(operator) - .data("{\"user\":\"administrator@tenant\",\"roleName\":\"administrator\"}"), + .data("{\"user\":\"administrator@tenant\",\"roles\":[\"administrator\"]}"), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Can't remove the last administrator of a tenant.\"}", 400); // DELETE the tenant is not allowed -- cgit v1.2.3 From 58bc70d49ca1bd442c54c06d13fb10ba2c5f8fbf Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 12 Aug 2022 09:55:21 +0200 Subject: Add invalidateUserSessionsBefore to CloudTenant --- .../hosted/controller/tenant/CloudTenant.java | 11 +++++++-- .../vespa/hosted/controller/LockedTenant.java | 26 +++++++++++++--------- .../controller/persistence/TenantSerializer.java | 5 ++++- .../notification/NotificationsDbTest.java | 3 ++- .../controller/notification/NotifierTest.java | 3 ++- .../persistence/TenantSerializerTest.java | 13 ++++++----- .../restapi/filter/SignatureFilterTest.java | 6 +++-- 7 files changed, 44 insertions(+), 23 deletions(-) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java index 54924b9c456..44f9c0ea3b8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/CloudTenant.java @@ -25,17 +25,19 @@ public class CloudTenant extends Tenant { private final TenantInfo info; private final List tenantSecretStores; private final ArchiveAccess archiveAccess; + private final Optional invalidateUserSessionsBefore; /** Public for the serialization layer — do not use! */ public CloudTenant(TenantName name, Instant createdAt, LastLoginInfo lastLoginInfo, Optional creator, BiMap developerKeys, TenantInfo info, - List tenantSecretStores, ArchiveAccess archiveAccess) { + List tenantSecretStores, ArchiveAccess archiveAccess, Optional invalidateUserSessionsBefore) { super(name, createdAt, lastLoginInfo, Optional.empty()); this.creator = creator; this.developerKeys = developerKeys; this.info = Objects.requireNonNull(info); this.tenantSecretStores = tenantSecretStores; this.archiveAccess = Objects.requireNonNull(archiveAccess); + this.invalidateUserSessionsBefore = invalidateUserSessionsBefore; } /** Creates a tenant with the given name, provided it passes validation. */ @@ -44,7 +46,7 @@ public class CloudTenant extends Tenant { createdAt, LastLoginInfo.EMPTY, Optional.ofNullable(creator), - ImmutableBiMap.of(), TenantInfo.empty(), List.of(), new ArchiveAccess()); + ImmutableBiMap.of(), TenantInfo.empty(), List.of(), new ArchiveAccess(), Optional.empty()); } /** The user that created the tenant */ @@ -75,6 +77,11 @@ public class CloudTenant extends Tenant { return archiveAccess; } + /** Returns instant before which all user sessions that have access to this tenant must be refreshed */ + public Optional invalidateUserSessionsBefore() { + return invalidateUserSessionsBefore; + } + @Override public Type type() { return Type.cloud; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java index 4f58e87035b..ac7c6319c1b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedTenant.java @@ -129,25 +129,27 @@ public abstract class LockedTenant { private final TenantInfo info; private final List tenantSecretStores; private final ArchiveAccess archiveAccess; + private final Optional invalidateUserSessionsBefore; private Cloud(TenantName name, Instant createdAt, LastLoginInfo lastLoginInfo, Optional creator, BiMap developerKeys, TenantInfo info, - List tenantSecretStores, ArchiveAccess archiveAccess) { + List tenantSecretStores, ArchiveAccess archiveAccess, Optional invalidateUserSessionsBefore) { super(name, createdAt, lastLoginInfo); this.developerKeys = ImmutableBiMap.copyOf(developerKeys); this.creator = creator; this.info = info; this.tenantSecretStores = tenantSecretStores; this.archiveAccess = archiveAccess; + this.invalidateUserSessionsBefore = invalidateUserSessionsBefore; } private Cloud(CloudTenant tenant) { - this(tenant.name(), tenant.createdAt(), tenant.lastLoginInfo(), tenant.creator(), tenant.developerKeys(), tenant.info(), tenant.tenantSecretStores(), tenant.archiveAccess()); + this(tenant.name(), tenant.createdAt(), tenant.lastLoginInfo(), tenant.creator(), tenant.developerKeys(), tenant.info(), tenant.tenantSecretStores(), tenant.archiveAccess(), tenant.invalidateUserSessionsBefore()); } @Override public CloudTenant get() { - return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess); + return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withDeveloperKey(PublicKey key, Principal principal) { @@ -155,38 +157,42 @@ public abstract class LockedTenant { if (keys.containsKey(key)) throw new IllegalArgumentException("Key " + KeyUtils.toPem(key) + " is already owned by " + keys.get(key)); keys.put(key, principal); - return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withoutDeveloperKey(PublicKey key) { BiMap keys = HashBiMap.create(developerKeys); keys.remove(key); - return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, keys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withInfo(TenantInfo newInfo) { - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, newInfo, tenantSecretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, newInfo, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } @Override public LockedTenant with(LastLoginInfo lastLoginInfo) { - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withSecretStore(TenantSecretStore tenantSecretStore) { ArrayList secretStores = new ArrayList<>(tenantSecretStores); secretStores.add(tenantSecretStore); - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withoutSecretStore(TenantSecretStore tenantSecretStore) { ArrayList secretStores = new ArrayList<>(tenantSecretStores); secretStores.remove(tenantSecretStore); - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, secretStores, archiveAccess, invalidateUserSessionsBefore); } public Cloud withArchiveAccess(ArchiveAccess archiveAccess) { - return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess); + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); + } + + public Cloud withInvalidateUserSessionsBefore(Instant invalidateUserSessionsBefore) { + return new Cloud(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess, Optional.of(invalidateUserSessionsBefore)); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index e7cf0c34511..e91fbe8b1b7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -81,6 +81,7 @@ public class TenantSerializer { private static final String archiveAccessField = "archiveAccess"; private static final String awsArchiveAccessRoleField = "awsArchiveAccessRole"; private static final String gcpArchiveAccessMemberField = "gcpArchiveAccessMember"; + private static final String invalidateUserSessionsBeforeField = "invalidateUserSessionsBefore"; private static final String awsIdField = "awsId"; private static final String roleField = "role"; @@ -123,6 +124,7 @@ public class TenantSerializer { toSlime(tenant.info(), root); toSlime(tenant.tenantSecretStores(), root); toSlime(tenant.archiveAccess(), root); + tenant.invalidateUserSessionsBefore().ifPresent(instant -> root.setLong(invalidateUserSessionsBeforeField, instant.toEpochMilli())); } private void toSlime(ArchiveAccess archiveAccess, Cursor root) { @@ -187,7 +189,8 @@ public class TenantSerializer { TenantInfo info = tenantInfoFromSlime(tenantObject.field(tenantInfoField)); List tenantSecretStores = secretStoresFromSlime(tenantObject.field(secretStoresField)); ArchiveAccess archiveAccess = archiveAccessFromSlime(tenantObject); - return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess); + Optional invalidateUserSessionsBefore = SlimeUtils.optionalInstant(tenantObject.field(invalidateUserSessionsBeforeField)); + return new CloudTenant(name, createdAt, lastLoginInfo, creator, developerKeys, info, tenantSecretStores, archiveAccess, invalidateUserSessionsBefore); } private DeletedTenant deletedTenantFrom(Inspector tenantObject) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index 1a171341ee0..4c1344650f8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -61,7 +61,8 @@ public class NotificationsDbTest { List.of(TenantContacts.Audience.NOTIFICATIONS), email)))), List.of(), - new ArchiveAccess()); + new ArchiveAccess(), + Optional.empty()); private static final List notifications = List.of( notification(1001, Type.deployment, Level.error, NotificationSource.from(tenant), "tenant msg"), notification(1101, Type.applicationPackage, Level.warning, NotificationSource.from(TenantAndApplicationId.from(tenant.value(), "app1")), "app msg"), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java index 00d8c100ced..7c07192d633 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java @@ -41,7 +41,8 @@ public class NotifierTest { List.of(TenantContacts.Audience.NOTIFICATIONS), email)))), List.of(), - new ArchiveAccess()); + new ArchiveAccess(), + Optional.empty()); MockCuratorDb curatorDb = new MockCuratorDb(SystemName.Public); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index e29d4afabff..636620acf07 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -103,8 +103,8 @@ public class TenantSerializerTest { otherPublicKey, new SimplePrincipal("jane")), TenantInfo.empty(), List.of(), - new ArchiveAccess() - ); + new ArchiveAccess(), + Optional.empty()); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.name(), serialized.name()); assertEquals(tenant.creator(), serialized.creator()); @@ -125,11 +125,12 @@ public class TenantSerializerTest { new TenantSecretStore("ss1", "123", "role1"), new TenantSecretStore("ss2", "124", "role2") ), - new ArchiveAccess().withAWSRole("arn:aws:iam::123456789012:role/my-role") - ); + new ArchiveAccess().withAWSRole("arn:aws:iam::123456789012:role/my-role"), + Optional.of(Instant.ofEpochMilli(1234567))); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(tenant.info(), serialized.info()); assertEquals(tenant.tenantSecretStores(), serialized.tenantSecretStores()); + assertEquals(tenant.invalidateUserSessionsBefore(), serialized.invalidateUserSessionsBefore()); } @Test @@ -174,8 +175,8 @@ public class TenantSerializerTest { otherPublicKey, new SimplePrincipal("jane")), TenantInfo.empty(), List.of(), - new ArchiveAccess().withAWSRole("arn:aws:iam::123456789012:role/my-role").withGCPMember("user:foo@example.com") - ); + new ArchiveAccess().withAWSRole("arn:aws:iam::123456789012:role/my-role").withGCPMember("user:foo@example.com"), + Optional.empty()); CloudTenant serialized = (CloudTenant) serializer.tenantFrom(serializer.toSlime(tenant)); assertEquals(serialized.archiveAccess().awsRole().get(), "arn:aws:iam::123456789012:role/my-role"); assertEquals(serialized.archiveAccess().gcpMember().get(), "user:foo@example.com"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java index 9ff79213983..ec9be1f04c3 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/SignatureFilterTest.java @@ -77,7 +77,8 @@ public class SignatureFilterTest { ImmutableBiMap.of(), TenantInfo.empty(), List.of(), - new ArchiveAccess())); + new ArchiveAccess(), + Optional.empty())); tester.curator().writeApplication(new Application(appId, tester.clock().instant())); } @@ -123,7 +124,8 @@ public class SignatureFilterTest { ImmutableBiMap.of(publicKey, () -> "user"), TenantInfo.empty(), List.of(), - new ArchiveAccess())); + new ArchiveAccess(), + Optional.empty())); verifySecurityContext(requestOf(signer.signed(request.copy(), Method.POST, () -> new ByteArrayInputStream(hiBytes)), hiBytes), new SecurityContext(new SimplePrincipal("user"), Set.of(Role.reader(id.tenant()), -- cgit v1.2.3 From 4524b4678e7b5486ba6ff02685a73322893c0b5f Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 12 Aug 2022 09:56:00 +0200 Subject: Update invalidateUserSessionsBefore every time a user role for tenant is removed --- .../yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index c13f46dacde..a407e5aa211 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -275,6 +275,11 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { removeDeveloperKey(tenant, user, roles); users.removeFromRoles(user, roles); + controller.tenants().lockIfPresent(tenant, LockedTenant.class, lockedTenant -> { + if (lockedTenant instanceof LockedTenant.Cloud cloudTenant) + controller.tenants().store(cloudTenant.withInvalidateUserSessionsBefore(controller.clock().instant())); + }); + return new MessageResponse(user + " is no longer a member of " + roles.stream().map(Role::toString).collect(Collectors.joining(", "))); } -- cgit v1.2.3 From e8313cda153f5a4f85f12673b3d1da588940eb38 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 12 Aug 2022 13:26:26 +0200 Subject: Create UserSessionManager --- .../api/integration/user/UserSessionManager.java | 13 +++++ .../security/CloudUserSessionManager.java | 50 +++++++++++++++++ .../security/CloudUserSessionManagerTest.java | 64 ++++++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserSessionManager.java create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManager.java create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManagerTest.java diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserSessionManager.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserSessionManager.java new file mode 100644 index 00000000000..eae62c66b35 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserSessionManager.java @@ -0,0 +1,13 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.user; + +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; + +/** + * @author freva + */ +public interface UserSessionManager { + + /** Returns whether the existing session for the given SecurityContext should be expired */ + boolean shouldExpireSessionFor(SecurityContext context); +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManager.java new file mode 100644 index 00000000000..e2b5083abae --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManager.java @@ -0,0 +1,50 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.security; + +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.flags.LongFlag; +import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.hosted.controller.Controller; +import com.yahoo.vespa.hosted.controller.TenantController; +import com.yahoo.vespa.hosted.controller.api.integration.user.UserSessionManager; +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; +import com.yahoo.vespa.hosted.controller.api.role.TenantRole; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; + +import java.time.Instant; + +/** + * @author freva + */ +public class CloudUserSessionManager implements UserSessionManager { + + private final TenantController tenantController; + private final LongFlag invalidateConsoleSessions; + + public CloudUserSessionManager(Controller controller) { + this.tenantController = controller.tenants(); + this.invalidateConsoleSessions = PermanentFlags.INVALIDATE_CONSOLE_SESSIONS.bindTo(controller.flagSource()); + } + + @Override + public boolean shouldExpireSessionFor(SecurityContext context) { + if (context.issuedAt().isBefore(Instant.ofEpochSecond(invalidateConsoleSessions.value()))) + return true; + + return context.roles().stream() + .filter(TenantRole.class::isInstance) + .map(TenantRole.class::cast) + .map(TenantRole::tenant) + .distinct() + .anyMatch(tenantName -> shouldExpireSessionFor(tenantName, context.issuedAt())); + } + + private boolean shouldExpireSessionFor(TenantName tenantName, Instant contextIssuedAt) { + return tenantController.get(tenantName) + .filter(CloudTenant.class::isInstance) + .map(CloudTenant.class::cast) + .flatMap(CloudTenant::invalidateUserSessionsBefore) + .map(contextIssuedAt::isBefore) + .orElse(false); + } +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManagerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManagerTest.java new file mode 100644 index 00000000000..710e75fb235 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/security/CloudUserSessionManagerTest.java @@ -0,0 +1,64 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.security; + +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.flags.InMemoryFlagSource; +import com.yahoo.vespa.flags.PermanentFlags; +import com.yahoo.vespa.hosted.controller.ControllerTester; +import com.yahoo.vespa.hosted.controller.LockedTenant; +import com.yahoo.vespa.hosted.controller.api.role.Role; +import com.yahoo.vespa.hosted.controller.api.role.SecurityContext; +import com.yahoo.vespa.hosted.controller.api.role.SimplePrincipal; +import com.yahoo.vespa.hosted.controller.api.role.TenantRole; +import org.junit.jupiter.api.Test; + +import java.time.Instant; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author freva + */ +class CloudUserSessionManagerTest { + + private final ControllerTester tester = new ControllerTester(SystemName.Public); + private final CloudUserSessionManager userSessionManager = new CloudUserSessionManager(tester.controller()); + + @Test + void test() { + createTenant("tenant1", null); + createTenant("tenant2", 1234); + createTenant("tenant3", 1543); + createTenant("tenant4", 2313); + + assertShouldExpire(false, 123); + assertShouldExpire(false, 123, "tenant1"); + assertShouldExpire(true, 123, "tenant2"); + assertShouldExpire(false, 2123, "tenant2"); + assertShouldExpire(true, 123, "tenant1", "tenant2"); + + ((InMemoryFlagSource) tester.controller().flagSource()).withLongFlag(PermanentFlags.INVALIDATE_CONSOLE_SESSIONS.id(), 150); + assertShouldExpire(true, 123); + assertShouldExpire(true, 123, "tenant1"); + } + + private void assertShouldExpire(boolean expected, long issuedAtSeconds, String... tenantNames) { + Set roles = Stream.of(tenantNames).map(name -> TenantRole.developer(TenantName.from(name))).collect(Collectors.toSet()); + SecurityContext context = new SecurityContext(new SimplePrincipal("dev"), roles, Instant.ofEpochSecond(issuedAtSeconds)); + assertEquals(expected, userSessionManager.shouldExpireSessionFor(context)); + } + + private void createTenant(String tenantName, Integer invalidateAfterSeconds) { + tester.createTenant(tenantName); + Optional.ofNullable(invalidateAfterSeconds) + .map(Instant::ofEpochSecond) + .ifPresent(instant -> + tester.controller().tenants().lockOrThrow(TenantName.from(tenantName), LockedTenant.Cloud.class, tenant -> + tester.controller().tenants().store(tenant.withInvalidateUserSessionsBefore(instant)))); + } +} -- cgit v1.2.3