Skip to content

Commit

Permalink
Removed tenant/user/role based filtering
Browse files Browse the repository at this point in the history
[Tests]
Updated tests

Signed-off-by: @akbhatta
  • Loading branch information
akbhatta committed Sep 17, 2021
1 parent 6ba70a9 commit a07d77d
Show file tree
Hide file tree
Showing 20 changed files with 37 additions and 316 deletions.
12 changes: 0 additions & 12 deletions notifications/notifications/src/main/config/notifications.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down Expand Up @@ -219,7 +219,6 @@ object ConfigIndexingActions {
val metadata = DocMetadata(
currentTime,
currentTime,
userAccess.getUserTenant(user),
userAccess.getAllAccessInfo(user)
)
val configDoc = NotificationConfigDoc(metadata, request.notificationConfig)
Expand Down Expand Up @@ -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}",
Expand Down Expand Up @@ -306,15 +305,14 @@ 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)
}
val configInfo = NotificationConfigInfo(
configId,
metadata.lastUpdateTime,
metadata.createdTime,
metadata.tenant,
configDoc.configDoc.config
)
return GetNotificationConfigResponse(NotificationConfigSearchResult(configInfo))
Expand All @@ -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}",
Expand All @@ -353,7 +351,6 @@ object ConfigIndexingActions {
it.docInfo.id!!,
it.configDoc.metadata.lastUpdateTime,
it.configDoc.metadata.createdTime,
it.configDoc.metadata.tenant,
it.configDoc.config
)
}
Expand All @@ -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
)
Expand All @@ -392,7 +388,6 @@ object ConfigIndexingActions {
)
val getAllRequest = GetNotificationConfigRequest(filterParams = filterParams)
val getAllResult = operations.getAllNotificationConfigs(
userAccess.getUserTenant(user),
userAccess.getSearchAccessInfo(user),
getAllRequest
)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
request: GetNotificationConfigRequest
): NotificationConfigSearchResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,14 @@ 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)
}
val eventInfo = NotificationEventInfo(
eventId,
metadata.lastUpdateTime,
metadata.createdTime,
metadata.tenant,
eventDoc.eventDoc.event
)
return GetNotificationEventResponse(NotificationEventSearchResult(eventInfo))
Expand All @@ -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}",
Expand All @@ -131,7 +130,6 @@ object EventIndexingActions {
it.docInfo.id!!,
it.eventDoc.metadata.lastUpdateTime,
it.eventDoc.metadata.createdTime,
it.eventDoc.metadata.tenant,
it.eventDoc.event
)
}
Expand All @@ -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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
request: GetNotificationEventRequest
): NotificationEventSearchResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,7 +91,6 @@ internal object NotificationConfigIndex : ConfigOperations {
searchHit.id,
doc.metadata.lastUpdateTime,
doc.metadata.createdTime,
doc.metadata.tenant,
doc.config
)
}
Expand Down Expand Up @@ -213,7 +211,6 @@ internal object NotificationConfigIndex : ConfigOperations {
* {@inheritDoc}
*/
override fun getAllNotificationConfigs(
tenant: String,
access: List<String>,
request: GetNotificationConfigRequest
): NotificationConfigSearchResult {
Expand All @@ -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))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,7 +91,6 @@ internal object NotificationEventIndex : EventOperations {
searchHit.id,
doc.metadata.lastUpdateTime,
doc.metadata.createdTime,
doc.metadata.tenant,
doc.event
)
}
Expand Down Expand Up @@ -213,7 +211,6 @@ internal object NotificationEventIndex : EventOperations {
* {@inheritDoc}
*/
override fun getAllNotificationEvents(
tenant: String,
access: List<String>,
request: GetNotificationEventRequest
): NotificationEventSearchResult {
Expand All @@ -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))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -44,8 +42,7 @@ import java.time.Instant
data class DocMetadata(
val lastUpdateTime: Instant,
val createdTime: Instant,
val tenant: String,
val access: List<String> // "User:user", "Role:sample_role", "BERole:sample_backend_role"
val access: List<String>
) : ToXContent {
companion object {
private val log by logger(DocMetadata::class.java)
Expand All @@ -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<String> = listOf()
XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser)
while (XContentParser.Token.END_OBJECT != parser.nextToken()) {
Expand All @@ -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()
Expand All @@ -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
)
}
Expand All @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -60,10 +48,5 @@ interface UserAccess {
/**
* validate if user has access based on given access list
*/
fun doesUserHasAccess(user: User?, tenant: String, access: List<String>): Boolean

/**
* Check if user has all info access.
*/
fun hasAllInfoAccess(user: User?): Boolean
fun doesUserHasAccess(user: User?, access: List<String>): Boolean
}
Loading

0 comments on commit a07d77d

Please sign in to comment.