From a07d77de29e941e3439899eab28b9f194e81b4c6 Mon Sep 17 00:00:00 2001 From: Anantha Krishna Bhatta Date: Fri, 17 Sep 2021 11:47:09 -0700 Subject: [PATCH] Removed tenant/user/role based filtering [Tests] Updated tests Signed-off-by: @akbhatta --- .../src/main/config/notifications.yml | 12 -- .../index/ConfigIndexingActions.kt | 17 +-- .../notifications/index/ConfigOperations.kt | 2 - .../index/EventIndexingActions.kt | 7 +- .../notifications/index/EventOperations.kt | 2 - .../index/NotificationConfigIndex.kt | 4 - .../index/NotificationEventIndex.kt | 4 - .../notifications/model/DocMetadata.kt | 10 +- .../notifications/security/UserAccess.kt | 21 +-- .../security/UserAccessManager.kt | 127 ++---------------- .../send/SendMessageActionHelper.kt | 1 - .../notifications-config-mapping.yml | 4 +- .../resources/notifications-event-mapping.yml | 4 +- ...nsearch-notifications-counter-settings.yml | 32 ----- .../opensearch-notifications-counter.yml | 36 ----- .../send/SendTestMessageRestHandlerIT.kt | 4 - .../notifications/ObjectEqualsHelpers.kt | 2 - .../notifications/model/DocMetadataTests.kt | 40 +----- .../model/NotificationConfigDocTests.kt | 12 +- .../model/NotificationEventDocTests.kt | 12 +- 20 files changed, 37 insertions(+), 316 deletions(-) delete mode 100644 notifications/notifications/src/main/resources/opensearch-notifications-counter-settings.yml delete mode 100644 notifications/notifications/src/main/resources/opensearch-notifications-counter.yml diff --git a/notifications/notifications/src/main/config/notifications.yml b/notifications/notifications/src/main/config/notifications.yml index 44759a47..acd6e183 100644 --- a/notifications/notifications/src/main/config/notifications.yml +++ b/notifications/notifications/src/main/config/notifications.yml @@ -31,15 +31,3 @@ opensearch.notifications: general: operationTimeoutMs: 60000 # 60 seconds, Minimum 100ms defaultItemsQueryCount: 100 # default number of items to query - access: - adminAccess: "All" - # adminAccess values: - ## Standard -> Admin user access follows standard user - ## All -> Admin user with "all_access" role can see all data of all users. - filterBy: "NoFilter" # Applied when tenant != __user__ - # filterBy values: - ## NoFilter -> everyone see each other's configurations - ## User -> configurations are visible to only themselves - ## Roles -> configurations are visible to users having any one of the role of creator - ## BackendRoles -> configurations are visible to users having any one of the backend role of creator - ignoreRoles: ["own_index", "kibana_user", "notifications_full_access", "notifications_read_access"] diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt index e8435e16..ed59e778 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt @@ -151,7 +151,7 @@ object ConfigIndexingActions { } // Validate that the user has access to underlying configurations as well. val currentMetadata = it.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationConfig ${it.docInfo.id}", @@ -219,7 +219,6 @@ object ConfigIndexingActions { val metadata = DocMetadata( currentTime, currentTime, - userAccess.getUserTenant(user), userAccess.getAllAccessInfo(user) ) val configDoc = NotificationConfigDoc(metadata, request.notificationConfig) @@ -255,7 +254,7 @@ object ConfigIndexingActions { } val currentMetadata = currentConfigDoc.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationConfig ${request.configId}", @@ -306,7 +305,7 @@ object ConfigIndexingActions { throw OpenSearchStatusException("NotificationConfig $configId not found", RestStatus.NOT_FOUND) } val metadata = configDoc.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, metadata.tenant, metadata.access)) { + if (!userAccess.doesUserHasAccess(user, metadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException("Permission denied for NotificationConfig $configId", RestStatus.FORBIDDEN) } @@ -314,7 +313,6 @@ object ConfigIndexingActions { configId, metadata.lastUpdateTime, metadata.createdTime, - metadata.tenant, configDoc.configDoc.config ) return GetNotificationConfigResponse(NotificationConfigSearchResult(configInfo)) @@ -340,7 +338,7 @@ object ConfigIndexingActions { } configDocs.forEach { val currentMetadata = it.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationConfig ${it.docInfo.id}", @@ -353,7 +351,6 @@ object ConfigIndexingActions { it.docInfo.id!!, it.configDoc.metadata.lastUpdateTime, it.configDoc.metadata.createdTime, - it.configDoc.metadata.tenant, it.configDoc.config ) } @@ -369,7 +366,6 @@ object ConfigIndexingActions { private fun getAll(request: GetNotificationConfigRequest, user: User?): GetNotificationConfigResponse { log.info("$LOG_PREFIX:NotificationConfig-getAll") val searchResult = operations.getAllNotificationConfigs( - userAccess.getUserTenant(user), userAccess.getSearchAccessInfo(user), request ) @@ -392,7 +388,6 @@ object ConfigIndexingActions { ) val getAllRequest = GetNotificationConfigRequest(filterParams = filterParams) val getAllResult = operations.getAllNotificationConfigs( - userAccess.getUserTenant(user), userAccess.getSearchAccessInfo(user), getAllRequest ) @@ -435,7 +430,7 @@ object ConfigIndexingActions { } val currentMetadata = currentConfigDoc.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationConfig $configId", @@ -473,7 +468,7 @@ object ConfigIndexingActions { } configDocs.forEach { val currentMetadata = it.configDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationConfig ${it.docInfo.id}", diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigOperations.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigOperations.kt index 4e29e896..13f56e81 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigOperations.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigOperations.kt @@ -61,13 +61,11 @@ interface ConfigOperations { /** * Query index for NotificationConfigDocs for given access details - * @param tenant the tenant of the user * @param access the list of access details to search NotificationConfigDocs for. * @param request [GetNotificationConfigRequest] object * @return search result of NotificationConfigDocs */ fun getAllNotificationConfigs( - tenant: String, access: List, request: GetNotificationConfigRequest ): NotificationConfigSearchResult diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventIndexingActions.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventIndexingActions.kt index 99f6841d..30020b06 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventIndexingActions.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventIndexingActions.kt @@ -84,7 +84,7 @@ object EventIndexingActions { throw OpenSearchStatusException("NotificationEvent $eventId not found", RestStatus.NOT_FOUND) } val metadata = eventDoc.eventDoc.metadata - if (!userAccess.doesUserHasAccess(user, metadata.tenant, metadata.access)) { + if (!userAccess.doesUserHasAccess(user, metadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException("Permission denied for NotificationEvent $eventId", RestStatus.FORBIDDEN) } @@ -92,7 +92,6 @@ object EventIndexingActions { eventId, metadata.lastUpdateTime, metadata.createdTime, - metadata.tenant, eventDoc.eventDoc.event ) return GetNotificationEventResponse(NotificationEventSearchResult(eventInfo)) @@ -118,7 +117,7 @@ object EventIndexingActions { } eventDocs.forEach { val currentMetadata = it.eventDoc.metadata - if (!userAccess.doesUserHasAccess(user, currentMetadata.tenant, currentMetadata.access)) { + if (!userAccess.doesUserHasAccess(user, currentMetadata.access)) { Metrics.NOTIFICATIONS_PERMISSION_USER_ERROR.counter.increment() throw OpenSearchStatusException( "Permission denied for NotificationEvent ${it.docInfo.id}", @@ -131,7 +130,6 @@ object EventIndexingActions { it.docInfo.id!!, it.eventDoc.metadata.lastUpdateTime, it.eventDoc.metadata.createdTime, - it.eventDoc.metadata.tenant, it.eventDoc.event ) } @@ -147,7 +145,6 @@ object EventIndexingActions { private fun getAll(request: GetNotificationEventRequest, user: User?): GetNotificationEventResponse { log.info("$LOG_PREFIX:NotificationEvent-getAll") val searchResult = operations.getAllNotificationEvents( - userAccess.getUserTenant(user), userAccess.getSearchAccessInfo(user), request ) diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventOperations.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventOperations.kt index a2253dde..223a5320 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventOperations.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/EventOperations.kt @@ -61,13 +61,11 @@ interface EventOperations { /** * Query index for NotificationEventDocs for given access details - * @param tenant the tenant of the user * @param access the list of access details to search NotificationEventDocs for. * @param request [GetNotificationEventRequest] object * @return search result of NotificationEventDocs */ fun getAllNotificationEvents( - tenant: String, access: List, request: GetNotificationEventRequest ): NotificationEventSearchResult diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationConfigIndex.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationConfigIndex.kt index 80d83195..1ac324d6 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationConfigIndex.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationConfigIndex.kt @@ -43,7 +43,6 @@ import org.opensearch.common.unit.TimeValue import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.notifications.NotificationConstants.TENANT_TAG import org.opensearch.commons.notifications.action.GetNotificationConfigRequest import org.opensearch.commons.notifications.model.NotificationConfigInfo import org.opensearch.commons.notifications.model.NotificationConfigSearchResult @@ -92,7 +91,6 @@ internal object NotificationConfigIndex : ConfigOperations { searchHit.id, doc.metadata.lastUpdateTime, doc.metadata.createdTime, - doc.metadata.tenant, doc.config ) } @@ -213,7 +211,6 @@ internal object NotificationConfigIndex : ConfigOperations { * {@inheritDoc} */ override fun getAllNotificationConfigs( - tenant: String, access: List, request: GetNotificationConfigRequest ): NotificationConfigSearchResult { @@ -224,7 +221,6 @@ internal object NotificationConfigIndex : ConfigOperations { .size(request.maxItems) .from(request.fromIndex) val query = QueryBuilders.boolQuery() - query.filter(QueryBuilders.termsQuery("$METADATA_TAG.$TENANT_TAG", tenant)) if (access.isNotEmpty()) { query.filter(QueryBuilders.termsQuery("$METADATA_TAG.$ACCESS_LIST_TAG", access)) } diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationEventIndex.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationEventIndex.kt index 3dedb0db..c95682b5 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationEventIndex.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/NotificationEventIndex.kt @@ -44,7 +44,6 @@ import org.opensearch.common.unit.TimeValue import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.notifications.NotificationConstants.TENANT_TAG import org.opensearch.commons.notifications.action.GetNotificationEventRequest import org.opensearch.commons.notifications.model.NotificationEventInfo import org.opensearch.commons.notifications.model.NotificationEventSearchResult @@ -92,7 +91,6 @@ internal object NotificationEventIndex : EventOperations { searchHit.id, doc.metadata.lastUpdateTime, doc.metadata.createdTime, - doc.metadata.tenant, doc.event ) } @@ -213,7 +211,6 @@ internal object NotificationEventIndex : EventOperations { * {@inheritDoc} */ override fun getAllNotificationEvents( - tenant: String, access: List, request: GetNotificationEventRequest ): NotificationEventSearchResult { @@ -224,7 +221,6 @@ internal object NotificationEventIndex : EventOperations { .size(request.maxItems) .from(request.fromIndex) val query = QueryBuilders.boolQuery() - query.filter(QueryBuilders.termsQuery("$METADATA_TAG.$TENANT_TAG", tenant)) if (access.isNotEmpty()) { query.filter(QueryBuilders.termsQuery("$METADATA_TAG.$ACCESS_LIST_TAG", access)) } diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/model/DocMetadata.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/model/DocMetadata.kt index 760c0dcf..3c1b8a37 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/model/DocMetadata.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/model/DocMetadata.kt @@ -31,11 +31,9 @@ import org.opensearch.common.xcontent.ToXContent import org.opensearch.common.xcontent.XContentBuilder import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils -import org.opensearch.commons.notifications.NotificationConstants.TENANT_TAG import org.opensearch.commons.notifications.NotificationConstants.UPDATED_TIME_TAG import org.opensearch.commons.utils.logger import org.opensearch.commons.utils.stringList -import org.opensearch.notifications.security.UserAccessManager.DEFAULT_TENANT import java.time.Instant /** @@ -44,8 +42,7 @@ import java.time.Instant data class DocMetadata( val lastUpdateTime: Instant, val createdTime: Instant, - val tenant: String, - val access: List // "User:user", "Role:sample_role", "BERole:sample_backend_role" + val access: List ) : ToXContent { companion object { private val log by logger(DocMetadata::class.java) @@ -61,7 +58,6 @@ data class DocMetadata( fun parse(parser: XContentParser): DocMetadata { var lastUpdateTime: Instant? = null var createdTime: Instant? = null - var tenant: String? = null var access: List = listOf() XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser) while (XContentParser.Token.END_OBJECT != parser.nextToken()) { @@ -70,7 +66,6 @@ data class DocMetadata( when (fieldName) { UPDATED_TIME_TAG -> lastUpdateTime = Instant.ofEpochMilli(parser.longValue()) CREATED_TIME_TAG -> createdTime = Instant.ofEpochMilli(parser.longValue()) - TENANT_TAG -> tenant = parser.text() ACCESS_LIST_TAG -> access = parser.stringList() else -> { parser.skipChildren() @@ -80,11 +75,9 @@ data class DocMetadata( } lastUpdateTime ?: throw IllegalArgumentException("$UPDATED_TIME_TAG field absent") createdTime ?: throw IllegalArgumentException("$CREATED_TIME_TAG field absent") - tenant = tenant ?: DEFAULT_TENANT return DocMetadata( lastUpdateTime, createdTime, - tenant, access ) } @@ -97,7 +90,6 @@ data class DocMetadata( return builder!!.startObject() .field(UPDATED_TIME_TAG, lastUpdateTime.toEpochMilli()) .field(CREATED_TIME_TAG, createdTime.toEpochMilli()) - .field(TENANT_TAG, tenant) .field(ACCESS_LIST_TAG, access) .endObject() } diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccess.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccess.kt index a5e67108..b85e62d0 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccess.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccess.kt @@ -31,22 +31,10 @@ import org.opensearch.commons.authuser.User interface UserAccess { /** * Validate User if eligible to do operation - * If filterBy == NoFilter - * -> No validation - * If filterBy == User - * -> User name should be present - * If filterBy == Roles - * -> roles should be present - * If filterBy == BackendRoles - * -> backend roles should be present + * If filter by BackendRoles is enabled then backend roles should be present */ fun validateUser(user: User?) - /** - * Get tenant info from user object. - */ - fun getUserTenant(user: User?): String - /** * Get all user access info from user object. */ @@ -60,10 +48,5 @@ interface UserAccess { /** * validate if user has access based on given access list */ - fun doesUserHasAccess(user: User?, tenant: String, access: List): Boolean - - /** - * Check if user has all info access. - */ - fun hasAllInfoAccess(user: User?): Boolean + fun doesUserHasAccess(user: User?, access: List): Boolean } diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccessManager.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccessManager.kt index f70fe52a..e905939b 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccessManager.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/security/UserAccessManager.kt @@ -29,160 +29,53 @@ package org.opensearch.notifications.security import org.opensearch.OpenSearchStatusException import org.opensearch.commons.authuser.User -import org.opensearch.notifications.settings.PluginSettings import org.opensearch.rest.RestStatus -import java.util.stream.Collectors /** * Class for checking/filtering user access. */ internal object UserAccessManager : UserAccess { - private const val USER_TAG = "User:" - private const val ROLE_TAG = "Role:" - private const val BACKEND_ROLE_TAG = "BERole:" - private const val ALL_ACCESS_ROLE = "all_access" - private const val PRIVATE_TENANT = "__user__" - const val DEFAULT_TENANT = "" + private const val USE_RBAC = false /** * {@inheritDoc} */ override fun validateUser(user: User?) { - if (isUserPrivateTenant(user) && user?.name == null) { + if (USE_RBAC && user?.backendRoles.isNullOrEmpty()) { throw OpenSearchStatusException( - "User name not provided for private tenant access", + "User doesn't have backend roles configured. Contact administrator.", RestStatus.FORBIDDEN ) } - when (PluginSettings.filterBy) { - PluginSettings.FilterBy.NoFilter -> { // No validation - } - PluginSettings.FilterBy.User -> { // User name must be present - user?.name - ?: run { - throw OpenSearchStatusException( - "Filter-by enabled with security disabled", - RestStatus.FORBIDDEN - ) - } - } - PluginSettings.FilterBy.Roles -> { // backend roles must be present - if (user == null || user.roles.isNullOrEmpty()) { - throw OpenSearchStatusException( - "User doesn't have roles configured. Contact administrator.", - RestStatus.FORBIDDEN - ) - } else if (user.roles.stream().filter { !PluginSettings.ignoredRoles.contains(it) }.count() == 0L) { - throw OpenSearchStatusException( - "No distinguishing roles configured. Contact administrator.", - RestStatus.FORBIDDEN - ) - } - } - PluginSettings.FilterBy.BackendRoles -> { // backend roles must be present - if (user?.backendRoles.isNullOrEmpty()) { - throw OpenSearchStatusException( - "User doesn't have backend roles configured. Contact administrator.", - RestStatus.FORBIDDEN - ) - } - } - } - } - - /** - * {@inheritDoc} - */ - override fun getUserTenant(user: User?): String { - return when (val requestedTenant = user?.requestedTenant) { - null -> DEFAULT_TENANT - else -> requestedTenant - } } /** * {@inheritDoc} */ override fun getAllAccessInfo(user: User?): List { - if (user == null) { // Security is disabled + if (user == null) { // Filtering is disabled return listOf() } - val retList: MutableList = mutableListOf() - if (user.name != null) { - retList.add("$USER_TAG${user.name}") - } - user.roles.forEach { retList.add("$ROLE_TAG$it") } - user.backendRoles.forEach { retList.add("$BACKEND_ROLE_TAG$it") } - return retList + return user.backendRoles } /** * {@inheritDoc} */ override fun getSearchAccessInfo(user: User?): List { - if (user == null) { // Security is disabled + if (user == null || !USE_RBAC) { // Filtering is disabled return listOf() } - if (isUserPrivateTenant(user)) { - return listOf("$USER_TAG${user.name}") // No sharing allowed in private tenant. - } - if (canAdminViewAllItems(user)) { - return listOf() - } - return when (PluginSettings.filterBy) { - PluginSettings.FilterBy.NoFilter -> listOf() - PluginSettings.FilterBy.User -> listOf("$USER_TAG${user.name}") - PluginSettings.FilterBy.Roles -> user.roles.stream() - .filter { !PluginSettings.ignoredRoles.contains(it) } - .map { "$ROLE_TAG$it" } - .collect(Collectors.toList()) - PluginSettings.FilterBy.BackendRoles -> user.backendRoles.map { "$BACKEND_ROLE_TAG$it" } - } + return user.backendRoles } /** * {@inheritDoc} */ - override fun doesUserHasAccess(user: User?, tenant: String, access: List): Boolean { - if (user == null) { // Security is disabled - return true - } - if (getUserTenant(user) != tenant) { - return false - } - if (canAdminViewAllItems(user)) { + override fun doesUserHasAccess(user: User?, access: List): Boolean { + if (user == null || !USE_RBAC) { // Filtering is disabled return true } - return when (PluginSettings.filterBy) { - PluginSettings.FilterBy.NoFilter -> true - PluginSettings.FilterBy.User -> access.contains("$USER_TAG${user.name}") - PluginSettings.FilterBy.Roles -> user.roles.stream() - .filter { !PluginSettings.ignoredRoles.contains(it) } - .map { "$ROLE_TAG$it" } - .anyMatch { it in access } - PluginSettings.FilterBy.BackendRoles -> user.backendRoles.map { "$BACKEND_ROLE_TAG$it" }.any { it in access } - } - } - - /** - * {@inheritDoc} - */ - override fun hasAllInfoAccess(user: User?): Boolean { - if (user == null) { // Security is disabled - return true - } - return isAdminUser(user) - } - - private fun canAdminViewAllItems(user: User): Boolean { - return PluginSettings.adminAccess == PluginSettings.AdminAccess.All && isAdminUser(user) - } - - private fun isAdminUser(user: User): Boolean { - return user.roles.contains(ALL_ACCESS_ROLE) - } - - private fun isUserPrivateTenant(user: User?): Boolean { - return getUserTenant(user) == PRIVATE_TENANT + return user.backendRoles.any { it in access } } } diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt index 0fb34a2a..371f0abe 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt +++ b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt @@ -101,7 +101,6 @@ object SendMessageActionHelper { val docMetadata = DocMetadata( updatedTime, createdTime, - userAccess.getUserTenant(user), userAccess.getAllAccessInfo(user) ) val event = NotificationEvent(eventSource, eventStatusList) diff --git a/notifications/notifications/src/main/resources/notifications-config-mapping.yml b/notifications/notifications/src/main/resources/notifications-config-mapping.yml index 26eb0b68..6d62b75a 100644 --- a/notifications/notifications/src/main/resources/notifications-config-mapping.yml +++ b/notifications/notifications/src/main/resources/notifications-config-mapping.yml @@ -39,9 +39,7 @@ properties: created_time_ms: type: date format: epoch_millis - tenant: - type: keyword - access: # Array of access details like user,role,backend_role etc + access: # Array of backend_role who has access to this document type: keyword config: # dynamic structure to store notifications configuration type: object diff --git a/notifications/notifications/src/main/resources/notifications-event-mapping.yml b/notifications/notifications/src/main/resources/notifications-event-mapping.yml index 3fc176ab..bfbc3f97 100644 --- a/notifications/notifications/src/main/resources/notifications-event-mapping.yml +++ b/notifications/notifications/src/main/resources/notifications-event-mapping.yml @@ -39,9 +39,7 @@ properties: created_time_ms: type: date format: epoch_millis - tenant: - type: keyword - access: # Array of access details like user,role,backend_role etc + access: # Array of backend_role who has access to this document type: keyword event: # dynamic structure to store notifications event type: object diff --git a/notifications/notifications/src/main/resources/opensearch-notifications-counter-settings.yml b/notifications/notifications/src/main/resources/opensearch-notifications-counter-settings.yml deleted file mode 100644 index 2f14e7d0..00000000 --- a/notifications/notifications/src/main/resources/opensearch-notifications-counter-settings.yml +++ /dev/null @@ -1,32 +0,0 @@ ---- -# -# SPDX-License-Identifier: Apache-2.0 -# -# The OpenSearch Contributors require contributions made to -# this file be licensed under the Apache-2.0 license or a -# compatible open source license. -# -# Modifications Copyright OpenSearch Contributors. See -# GitHub history for details. -# - -## -# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"). -# You may not use this file except in compliance with the License. -# A copy of the License is located at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# or in the "license" file accompanying this file. This file is distributed -# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -# express or implied. See the License for the specific language governing -# permissions and limitations under the License. -# -## - -# Settings file for the notifications counter index -index: - number_of_shards: "1" # The data size is not expected to be big so keeping it 1 shard - auto_expand_replicas: "0-2" diff --git a/notifications/notifications/src/main/resources/opensearch-notifications-counter.yml b/notifications/notifications/src/main/resources/opensearch-notifications-counter.yml deleted file mode 100644 index 4ca19b1f..00000000 --- a/notifications/notifications/src/main/resources/opensearch-notifications-counter.yml +++ /dev/null @@ -1,36 +0,0 @@ ---- -# -# SPDX-License-Identifier: Apache-2.0 -# -# The OpenSearch Contributors require contributions made to -# this file be licensed under the Apache-2.0 license or a -# compatible open source license. -# -# Modifications Copyright OpenSearch Contributors. See -# GitHub history for details. -# - -## -# Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"). -# You may not use this file except in compliance with the License. -# A copy of the License is located at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# or in the "license" file accompanying this file. This file is distributed -# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -# express or implied. See the License for the specific language governing -# permissions and limitations under the License. -# -## - -# Schema file for the notifications counter index -# Since we only search based on "counter_day", other fields are not used in mapping to avoid index on those fields. -# Also "dynamic" is set to "false" so that other fields can be added. -dynamic: false -properties: - counter_day: - type: date - format: strict_date # "yyyy-MM-dd" diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt index e220a613..3b4777b7 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt @@ -92,7 +92,6 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() { Assert.assertEquals(1, items.size()) val getResponseItem = items[0].asJsonObject Assert.assertEquals(eventId, getResponseItem.get("event_id").asString) - Assert.assertEquals("", getResponseItem.get("tenant").asString) Assert.assertNotNull(getResponseItem.get("event").asJsonObject) Thread.sleep(100) } @@ -153,7 +152,6 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() { Assert.assertEquals(1, items.size()) val getResponseItem = items[0].asJsonObject Assert.assertEquals(eventId, getResponseItem.get("event_id").asString) - Assert.assertEquals("", getResponseItem.get("tenant").asString) Assert.assertNotNull(getResponseItem.get("event").asJsonObject) Thread.sleep(100) } @@ -217,7 +215,6 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() { Assert.assertEquals(1, items.size()) val getResponseItem = items[0].asJsonObject Assert.assertEquals(eventId, getResponseItem.get("event_id").asString) - Assert.assertEquals("", getResponseItem.get("tenant").asString) Assert.assertNotNull(getResponseItem.get("event").asJsonObject) Thread.sleep(100) } @@ -320,7 +317,6 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() { Assert.assertEquals(1, items.size()) val getResponseItem = items[0].asJsonObject Assert.assertEquals(eventId, getResponseItem.get("event_id").asString) - Assert.assertEquals("", getResponseItem.get("tenant").asString) Assert.assertNotNull(getResponseItem.get("event").asJsonObject) Thread.sleep(100) } diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/ObjectEqualsHelpers.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/ObjectEqualsHelpers.kt index 81c876cc..5e26ba47 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/ObjectEqualsHelpers.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/ObjectEqualsHelpers.kt @@ -133,7 +133,6 @@ fun verifySingleConfigEquals( Assert.assertEquals(1, items.size()) val getResponseItem = items[0].asJsonObject Assert.assertEquals(configId, getResponseItem.get("config_id").asString) - Assert.assertEquals("", getResponseItem.get("tenant").asString) verifyEquals(config, getResponseItem.get("config").asJsonObject) } @@ -163,7 +162,6 @@ fun verifyMultiConfigEquals( Assert.assertNotNull(configId) val config = objectMap[configId] Assert.assertNotNull(config) - Assert.assertEquals("", item.get("tenant").asString) verifyEquals(config!!, item.get("config").asJsonObject) } } diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/DocMetadataTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/DocMetadataTests.kt index edabd0b9..050fe19d 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/DocMetadataTests.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/DocMetadataTests.kt @@ -43,35 +43,13 @@ internal class DocMetadataTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "tenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val jsonString = getJsonString(metadata) val recreatedObject = createObjectFromJsonString(jsonString) { DocMetadata.parse(it) } assertEquals(metadata, recreatedObject) } - @Test - fun `DocMetadata should take default tenant when field is absent in json object`() { - val lastUpdatedTimeMs = Instant.ofEpochMilli(Instant.now().toEpochMilli()) - val createdTimeMs = lastUpdatedTimeMs.minusSeconds(1000) - val metadata = DocMetadata( - lastUpdatedTimeMs, - createdTimeMs, - "", // Default tenant - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") - ) - val jsonString = """ - { - "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", - "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] - } - """.trimIndent() - val recreatedObject = createObjectFromJsonString(jsonString) { DocMetadata.parse(it) } - assertEquals(metadata, recreatedObject) - } - @Test fun `DocMetadata should take empty list when access field is absent in json object`() { val lastUpdatedTimeMs = Instant.ofEpochMilli(Instant.now().toEpochMilli()) @@ -79,14 +57,12 @@ internal class DocMetadataTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "selectedTenant", listOf() ) val jsonString = """ { "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", - "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant" + "created_time_ms":"${createdTimeMs.toEpochMilli()}" } """.trimIndent() val recreatedObject = createObjectFromJsonString(jsonString) { DocMetadata.parse(it) } @@ -100,15 +76,13 @@ internal class DocMetadataTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "selectedTenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val jsonString = """ { "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"], + "access":["br1", "br2", "br3"], "extra_field_1":["extra", "value"], "extra_field_2":{"extra":"value"}, "extra_field_3":"extra value 3" @@ -125,8 +99,7 @@ internal class DocMetadataTests { val jsonString = """ { "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] } """.trimIndent() Assertions.assertThrows(IllegalArgumentException::class.java) { @@ -140,8 +113,7 @@ internal class DocMetadataTests { val jsonString = """ { "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] } """.trimIndent() Assertions.assertThrows(IllegalArgumentException::class.java) { diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt index 4e50e97d..34224720 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt @@ -46,8 +46,7 @@ internal class NotificationConfigDocTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "tenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val sampleSlack = Slack("https://domain.com/sample_url#1234567890") val config = NotificationConfig( @@ -70,8 +69,7 @@ internal class NotificationConfigDocTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "selectedTenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val sampleSlack = Slack("https://domain.com/sample_url#1234567890") val config = NotificationConfig( @@ -87,8 +85,7 @@ internal class NotificationConfigDocTests { "metadata":{ "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] }, "config":{ "name":"name", @@ -135,8 +132,7 @@ internal class NotificationConfigDocTests { "metadata":{ "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] } } """.trimIndent() diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationEventDocTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationEventDocTests.kt index e7a82082..3f147fdb 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationEventDocTests.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationEventDocTests.kt @@ -49,8 +49,7 @@ internal class NotificationEventDocTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "tenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val sampleEventSource = EventSource( "title", @@ -79,8 +78,7 @@ internal class NotificationEventDocTests { val metadata = DocMetadata( lastUpdatedTimeMs, createdTimeMs, - "selectedTenant", - listOf("User:user", "Role:sample_role", "BERole:sample_backend_role") + listOf("br1", "br2", "br3") ) val eventSource = EventSource( "title", @@ -102,8 +100,7 @@ internal class NotificationEventDocTests { "metadata":{ "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] }, "event":{ "event_source":{ @@ -176,8 +173,7 @@ internal class NotificationEventDocTests { "metadata":{ "last_updated_time_ms":"${lastUpdatedTimeMs.toEpochMilli()}", "created_time_ms":"${createdTimeMs.toEpochMilli()}", - "tenant":"selectedTenant", - "access":["User:user", "Role:sample_role", "BERole:sample_backend_role"] + "access":["br1", "br2", "br3"] } } """.trimIndent()