Issue-1762: User profile updates should be only allowed by admin or bots (#1765)

* Issue-1762: User profile updates should be only allowed by admin or bots
This commit is contained in:
Sriharsha Chintalapani 2021-12-15 01:10:44 -08:00 committed by GitHub
parent 18e6d2b2f3
commit 92a8ecd0ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 40 deletions

View File

@ -298,10 +298,12 @@ class TableESIndex extends ElasticSearchIndex {
} }
} }
ParseTags parseTags = new ParseTags(tags); ParseTags parseTags = new ParseTags(tags);
Long updatedTimestamp = table.getUpdatedAt().getTime();
TableESIndexBuilder tableESIndexBuilder = internalBuilder().tableId(tableId) TableESIndexBuilder tableESIndexBuilder = internalBuilder().tableId(tableId)
.name(tableName) .name(tableName)
.displayName(tableName) .displayName(tableName)
.description(description) .description(description)
.lastUpdatedTimestamp(updatedTimestamp)
.fqdn(table.getFullyQualifiedName()) .fqdn(table.getFullyQualifiedName())
.suggest(suggest) .suggest(suggest)
.entityType("table") .entityType("table")
@ -313,7 +315,13 @@ class TableESIndex extends ElasticSearchIndex {
.tier(parseTags.tierTag); .tier(parseTags.tierTag);
if (table.getDatabase() != null) { if (table.getDatabase() != null) {
tableESIndexBuilder.database(table.getDatabase().getName()); String databaseFQN = table.getDatabase().getName();
String[] databaseFQNSplit = databaseFQN.split("\\.");
if (databaseFQNSplit.length == 2) {
tableESIndexBuilder.database(databaseFQNSplit[1]);
} else {
tableESIndexBuilder.database(databaseFQNSplit[0]);
}
} }
if (table.getService() != null) { if (table.getService() != null) {
@ -343,13 +351,13 @@ class TableESIndex extends ElasticSearchIndex {
ESChangeDescription esChangeDescription = null; ESChangeDescription esChangeDescription = null;
if (table.getChangeDescription() != null) { if (table.getChangeDescription() != null) {
esChangeDescription = ESChangeDescription.builder().updatedAt(table.getUpdatedAt().getTime()) esChangeDescription = ESChangeDescription.builder().updatedAt(updatedTimestamp)
.updatedBy(table.getUpdatedBy()).build(); .updatedBy(table.getUpdatedBy()).build();
esChangeDescription.setFieldsAdded(table.getChangeDescription().getFieldsAdded()); esChangeDescription.setFieldsAdded(table.getChangeDescription().getFieldsAdded());
esChangeDescription.setFieldsDeleted(table.getChangeDescription().getFieldsDeleted()); esChangeDescription.setFieldsDeleted(table.getChangeDescription().getFieldsDeleted());
esChangeDescription.setFieldsUpdated(table.getChangeDescription().getFieldsUpdated()); esChangeDescription.setFieldsUpdated(table.getChangeDescription().getFieldsUpdated());
} else if (responseCode == Response.Status.CREATED.getStatusCode()) { } else if (responseCode == Response.Status.CREATED.getStatusCode()) {
esChangeDescription = ESChangeDescription.builder().updatedAt(table.getUpdatedAt().getTime()) esChangeDescription = ESChangeDescription.builder().updatedAt(updatedTimestamp)
.updatedBy(table.getUpdatedBy()) .updatedBy(table.getUpdatedBy())
.fieldsAdded(new ArrayList<>()) .fieldsAdded(new ArrayList<>())
.fieldsUpdated(new ArrayList<>()) .fieldsUpdated(new ArrayList<>())
@ -410,11 +418,13 @@ class TopicESIndex extends ElasticSearchIndex {
topic.getTags().forEach(tag -> tags.add(tag.getTagFQN())); topic.getTags().forEach(tag -> tags.add(tag.getTagFQN()));
} }
ParseTags parseTags = new ParseTags(tags); ParseTags parseTags = new ParseTags(tags);
Long updatedTimestamp = topic.getUpdatedAt().getTime();
TopicESIndexBuilder topicESIndexBuilder = internalBuilder().topicId(topic.getId().toString()) TopicESIndexBuilder topicESIndexBuilder = internalBuilder().topicId(topic.getId().toString())
.name(topic.getName()) .name(topic.getName())
.displayName(topic.getDisplayName()) .displayName(topic.getDisplayName())
.description(topic.getDescription()) .description(topic.getDescription())
.fqdn(topic.getFullyQualifiedName()) .fqdn(topic.getFullyQualifiedName())
.lastUpdatedTimestamp(updatedTimestamp)
.suggest(suggest) .suggest(suggest)
.service(topic.getService().getName()) .service(topic.getService().getName())
.serviceType(topic.getServiceType().toString()) .serviceType(topic.getServiceType().toString())
@ -437,13 +447,13 @@ class TopicESIndex extends ElasticSearchIndex {
ESChangeDescription esChangeDescription = null; ESChangeDescription esChangeDescription = null;
if (topic.getChangeDescription() != null) { if (topic.getChangeDescription() != null) {
esChangeDescription = ESChangeDescription.builder().updatedAt(topic.getUpdatedAt().getTime()) esChangeDescription = ESChangeDescription.builder().updatedAt(updatedTimestamp)
.updatedBy(topic.getUpdatedBy()).build(); .updatedBy(topic.getUpdatedBy()).build();
esChangeDescription.setFieldsAdded(topic.getChangeDescription().getFieldsAdded()); esChangeDescription.setFieldsAdded(topic.getChangeDescription().getFieldsAdded());
esChangeDescription.setFieldsDeleted(topic.getChangeDescription().getFieldsDeleted()); esChangeDescription.setFieldsDeleted(topic.getChangeDescription().getFieldsDeleted());
esChangeDescription.setFieldsUpdated(topic.getChangeDescription().getFieldsUpdated()); esChangeDescription.setFieldsUpdated(topic.getChangeDescription().getFieldsUpdated());
} else if (responseCode == Response.Status.CREATED.getStatusCode()) { } else if (responseCode == Response.Status.CREATED.getStatusCode()) {
esChangeDescription = ESChangeDescription.builder().updatedAt(topic.getUpdatedAt().getTime()) esChangeDescription = ESChangeDescription.builder().updatedAt(updatedTimestamp)
.updatedBy(topic.getUpdatedBy()).build(); .updatedBy(topic.getUpdatedBy()).build();
} }
@ -486,7 +496,7 @@ class DashboardESIndex extends ElasticSearchIndex {
List<ElasticSearchSuggest> suggest = new ArrayList<>(); List<ElasticSearchSuggest> suggest = new ArrayList<>();
suggest.add(ElasticSearchSuggest.builder().input(dashboard.getFullyQualifiedName()).weight(5).build()); suggest.add(ElasticSearchSuggest.builder().input(dashboard.getFullyQualifiedName()).weight(5).build());
suggest.add(ElasticSearchSuggest.builder().input(dashboard.getDisplayName()).weight(10).build()); suggest.add(ElasticSearchSuggest.builder().input(dashboard.getDisplayName()).weight(10).build());
Long updatedTimestamp = dashboard.getUpdatedAt().getTime();
if (dashboard.getTags() != null) { if (dashboard.getTags() != null) {
dashboard.getTags().forEach(tag -> tags.add(tag.getTagFQN())); dashboard.getTags().forEach(tag -> tags.add(tag.getTagFQN()));
} }
@ -501,6 +511,7 @@ class DashboardESIndex extends ElasticSearchIndex {
.displayName(dashboard.getDisplayName()) .displayName(dashboard.getDisplayName())
.description(dashboard.getDescription()) .description(dashboard.getDescription())
.fqdn(dashboard.getFullyQualifiedName()) .fqdn(dashboard.getFullyQualifiedName())
.lastUpdatedTimestamp(updatedTimestamp)
.chartNames(chartNames) .chartNames(chartNames)
.chartDescriptions(chartDescriptions) .chartDescriptions(chartDescriptions)
.entityType("dashboard") .entityType("dashboard")
@ -532,14 +543,14 @@ class DashboardESIndex extends ElasticSearchIndex {
ESChangeDescription esChangeDescription = null; ESChangeDescription esChangeDescription = null;
if (dashboard.getChangeDescription() != null) { if (dashboard.getChangeDescription() != null) {
esChangeDescription = ESChangeDescription.builder() esChangeDescription = ESChangeDescription.builder()
.updatedAt(dashboard.getUpdatedAt().getTime()) .updatedAt(updatedTimestamp)
.updatedBy(dashboard.getUpdatedBy()).build(); .updatedBy(dashboard.getUpdatedBy()).build();
esChangeDescription.setFieldsAdded(dashboard.getChangeDescription().getFieldsAdded()); esChangeDescription.setFieldsAdded(dashboard.getChangeDescription().getFieldsAdded());
esChangeDescription.setFieldsDeleted(dashboard.getChangeDescription().getFieldsDeleted()); esChangeDescription.setFieldsDeleted(dashboard.getChangeDescription().getFieldsDeleted());
esChangeDescription.setFieldsUpdated(dashboard.getChangeDescription().getFieldsUpdated()); esChangeDescription.setFieldsUpdated(dashboard.getChangeDescription().getFieldsUpdated());
} else if (responseCode == Response.Status.CREATED.getStatusCode()) { } else if (responseCode == Response.Status.CREATED.getStatusCode()) {
esChangeDescription = ESChangeDescription.builder() esChangeDescription = ESChangeDescription.builder()
.updatedAt(dashboard.getUpdatedAt().getTime()) .updatedAt(updatedTimestamp)
.updatedBy(dashboard.getUpdatedBy()).build(); .updatedBy(dashboard.getUpdatedBy()).build();
} }
dashboardESIndexBuilder.changeDescriptions(esChangeDescription != null ? List.of(esChangeDescription) : null); dashboardESIndexBuilder.changeDescriptions(esChangeDescription != null ? List.of(esChangeDescription) : null);
@ -577,12 +588,14 @@ class PipelineESIndex extends ElasticSearchIndex {
taskNames.add(task.getDisplayName()); taskNames.add(task.getDisplayName());
taskDescriptions.add(task.getDescription()); taskDescriptions.add(task.getDescription());
} }
Long updatedTimestamp = pipeline.getUpdatedAt().getTime();
ParseTags parseTags = new ParseTags(tags); ParseTags parseTags = new ParseTags(tags);
PipelineESIndexBuilder pipelineESIndexBuilder = internalBuilder().pipelineId(pipeline.getId().toString()) PipelineESIndexBuilder pipelineESIndexBuilder = internalBuilder().pipelineId(pipeline.getId().toString())
.name(pipeline.getDisplayName()) .name(pipeline.getDisplayName())
.displayName(pipeline.getDisplayName()) .displayName(pipeline.getDisplayName())
.description(pipeline.getDescription()) .description(pipeline.getDescription())
.fqdn(pipeline.getFullyQualifiedName()) .fqdn(pipeline.getFullyQualifiedName())
.lastUpdatedTimestamp(updatedTimestamp)
.taskNames(taskNames) .taskNames(taskNames)
.taskDescriptions(taskDescriptions) .taskDescriptions(taskDescriptions)
.entityType("pipeline") .entityType("pipeline")
@ -607,14 +620,14 @@ class PipelineESIndex extends ElasticSearchIndex {
ESChangeDescription esChangeDescription = null; ESChangeDescription esChangeDescription = null;
if (pipeline.getChangeDescription() != null) { if (pipeline.getChangeDescription() != null) {
esChangeDescription = ESChangeDescription.builder() esChangeDescription = ESChangeDescription.builder()
.updatedAt(pipeline.getUpdatedAt().getTime()) .updatedAt(updatedTimestamp)
.updatedBy(pipeline.getUpdatedBy()).build(); .updatedBy(pipeline.getUpdatedBy()).build();
esChangeDescription.setFieldsAdded(pipeline.getChangeDescription().getFieldsAdded()); esChangeDescription.setFieldsAdded(pipeline.getChangeDescription().getFieldsAdded());
esChangeDescription.setFieldsDeleted(pipeline.getChangeDescription().getFieldsDeleted()); esChangeDescription.setFieldsDeleted(pipeline.getChangeDescription().getFieldsDeleted());
esChangeDescription.setFieldsUpdated(pipeline.getChangeDescription().getFieldsUpdated()); esChangeDescription.setFieldsUpdated(pipeline.getChangeDescription().getFieldsUpdated());
} else if (responseCode == Response.Status.CREATED.getStatusCode()) { } else if (responseCode == Response.Status.CREATED.getStatusCode()) {
esChangeDescription = ESChangeDescription.builder() esChangeDescription = ESChangeDescription.builder()
.updatedAt(pipeline.getUpdatedAt().getTime()) .updatedAt(updatedTimestamp)
.updatedBy(pipeline.getUpdatedBy()).build(); .updatedBy(pipeline.getUpdatedBy()).build();
} }
pipelineESIndexBuilder.changeDescriptions(esChangeDescription != null ? List.of(esChangeDescription) : null); pipelineESIndexBuilder.changeDescriptions(esChangeDescription != null ? List.of(esChangeDescription) : null);

View File

@ -41,7 +41,9 @@ import org.openmetadata.catalog.util.ResultList;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import javax.json.JsonObject;
import javax.json.JsonPatch; import javax.json.JsonPatch;
import javax.json.JsonValue;
import javax.validation.Valid; import javax.validation.Valid;
import javax.validation.constraints.Max; import javax.validation.constraints.Max;
import javax.validation.constraints.Min; import javax.validation.constraints.Min;
@ -176,8 +178,7 @@ public class UserResource {
@ApiResponse(responseCode = "200", description = "The user", @ApiResponse(responseCode = "200", description = "The user",
content = @Content(mediaType = "application/json", content = @Content(mediaType = "application/json",
schema = @Schema(implementation = User.class))), schema = @Schema(implementation = User.class))),
@ApiResponse(responseCode = "404", description = "User for instance {id} is not found") @ApiResponse(responseCode = "404", description = "User for instance {id} is not found")})
})
public User get(@Context UriInfo uriInfo, @Context SecurityContext securityContext, @PathParam("id") String id, public User get(@Context UriInfo uriInfo, @Context SecurityContext securityContext, @PathParam("id") String id,
@Parameter(description = "Fields requested in the returned resource", @Parameter(description = "Fields requested in the returned resource",
schema = @Schema(type = "string", example = FIELDS)) schema = @Schema(type = "string", example = FIELDS))
@ -281,7 +282,8 @@ public class UserResource {
public Response createOrUpdateUser(@Context UriInfo uriInfo, public Response createOrUpdateUser(@Context UriInfo uriInfo,
@Context SecurityContext securityContext, @Context SecurityContext securityContext,
@Valid CreateUser create) throws IOException, ParseException { @Valid CreateUser create) throws IOException, ParseException {
if (create.getIsAdmin() != null && create.getIsAdmin()) { if ((create.getIsAdmin() != null && create.getIsAdmin()) ||
(create.getIsBot() != null && create.getIsBot())) {
SecurityUtil.checkAdminOrBotRole(authorizer, securityContext); SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
} }
User user = getUser(securityContext, create); User user = getUser(securityContext, create);
@ -307,6 +309,15 @@ public class UserResource {
"{op:add, path: /b, value: val}" + "{op:add, path: /b, value: val}" +
"]")})) "]")}))
JsonPatch patch) throws IOException, ParseException { JsonPatch patch) throws IOException, ParseException {
for (JsonValue patchOp: patch.toJsonArray()) {
JsonObject patchOpObject = patchOp.asJsonObject();
if (patchOpObject.containsKey("path") && patchOpObject.containsKey("value")) {
String path = patchOpObject.getString("path");
if (path.equals("/isAdmin") || path.equals("/isBot")) {
SecurityUtil.checkAdminOrBotRole(authorizer, securityContext);
}
}
}
User user = dao.get(uriInfo, id, new Fields(FIELD_LIST, null)); User user = dao.get(uriInfo, id, new Fields(FIELD_LIST, null));
SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext,
new UserEntityInterface(user).getEntityReference()); new UserEntityInterface(user).getEntityReference());

View File

@ -223,8 +223,8 @@ public class UserResourceTest extends EntityResourceTest<User> {
public void patch_userNameChange_as_another_user_401(TestInfo test) public void patch_userNameChange_as_another_user_401(TestInfo test)
throws HttpResponseException, JsonProcessingException { throws HttpResponseException, JsonProcessingException {
// Ensure user name can't be changed using patch // Ensure user name can't be changed using patch
User user = createUser(create(test, 6).withName("test2").withDisplayName("displayName") User user = createUser(create(test, 7).withName("test23").withDisplayName("displayName")
.withEmail("test2@email.com"), authHeaders("test2@email.com")); .withEmail("test23@email.com"), authHeaders("test23@email.com"));
String userJson = JsonUtils.pojoToJson(user); String userJson = JsonUtils.pojoToJson(user);
user.setDisplayName("newName"); user.setDisplayName("newName");
HttpResponseException exception = assertThrows(HttpResponseException.class, () -> patchUser(userJson, user, HttpResponseException exception = assertThrows(HttpResponseException.class, () -> patchUser(userJson, user,
@ -232,6 +232,19 @@ public class UserResourceTest extends EntityResourceTest<User> {
assertResponse(exception, FORBIDDEN, "Principal: CatalogPrincipal{name='test100'} does not have permissions"); assertResponse(exception, FORBIDDEN, "Principal: CatalogPrincipal{name='test100'} does not have permissions");
} }
@Test
public void patch_makeAdmin_as_another_user_401(TestInfo test)
throws HttpResponseException, JsonProcessingException {
// Ensure user name can't be changed using patch
User user = createUser(create(test, 6).withName("test2").withDisplayName("displayName")
.withEmail("test2@email.com"), authHeaders("test2@email.com"));
String userJson = JsonUtils.pojoToJson(user);
user.setIsAdmin(Boolean.TRUE);
HttpResponseException exception = assertThrows(HttpResponseException.class, () -> patchUser(userJson, user,
authHeaders("test100@email.com")));
assertResponse(exception, FORBIDDEN, "Principal: CatalogPrincipal{name='test100'} is not admin");
}
@Test @Test
public void patch_userNameChange_as_same_user_200_ok(TestInfo test) throws HttpResponseException, public void patch_userNameChange_as_same_user_200_ok(TestInfo test) throws HttpResponseException,
JsonProcessingException { JsonProcessingException {

View File

@ -13,6 +13,7 @@ import json
import logging import logging
import ssl import ssl
import time import time
from datetime import datetime
from typing import List, Optional from typing import List, Optional
from dateutil import parser from dateutil import parser
@ -52,6 +53,10 @@ from metadata.ingestion.sink.elasticsearch_constants import (
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
def epoch_ms(dt: datetime):
return int(dt.timestamp() * 1000)
class ElasticSearchConfig(ConfigModel): class ElasticSearchConfig(ConfigModel):
es_host: str es_host: str
es_port: int = 9200 es_port: int = 9200
@ -220,7 +225,7 @@ class ElasticsearchSink(Sink[Entity]):
column_descriptions = [] column_descriptions = []
tags = set() tags = set()
timestamp = int(table.updatedAt.__root__.timestamp()) timestamp = epoch_ms(table.updatedAt.__root__)
tier = None tier = None
for table_tag in table.tags: for table_tag in table.tags:
if "Tier" in table_tag.tagFQN: if "Tier" in table_tag.tagFQN:
@ -283,7 +288,7 @@ class ElasticsearchSink(Sink[Entity]):
{"input": [topic_name], "weight": 10}, {"input": [topic_name], "weight": 10},
] ]
tags = set() tags = set()
timestamp = topic.updatedAt.__root__.timestamp() timestamp = epoch_ms(topic.updatedAt.__root__)
service_entity = self.metadata.get_by_id( service_entity = self.metadata.get_by_id(
entity=MessagingService, entity_id=str(topic.service.id.__root__) entity=MessagingService, entity_id=str(topic.service.id.__root__)
) )
@ -322,7 +327,7 @@ class ElasticsearchSink(Sink[Entity]):
dashboard_name = dashboard.name dashboard_name = dashboard.name
suggest = [{"input": [dashboard.displayName], "weight": 10}] suggest = [{"input": [dashboard.displayName], "weight": 10}]
tags = set() tags = set()
timestamp = dashboard.updatedAt.__root__.timestamp() timestamp = epoch_ms(dashboard.updatedAt.__root__)
service_entity = self.metadata.get_by_id( service_entity = self.metadata.get_by_id(
entity=DashboardService, entity_id=str(dashboard.service.id.__root__) entity=DashboardService, entity_id=str(dashboard.service.id.__root__)
) )
@ -383,7 +388,7 @@ class ElasticsearchSink(Sink[Entity]):
fqdn = pipeline.fullyQualifiedName fqdn = pipeline.fullyQualifiedName
suggest = [{"input": [pipeline.displayName], "weight": 10}] suggest = [{"input": [pipeline.displayName], "weight": 10}]
tags = set() tags = set()
timestamp = pipeline.updatedAt.__root__.timestamp() timestamp = epoch_ms(pipeline.updatedAt.__root__)
service_entity = self.metadata.get_by_id( service_entity = self.metadata.get_by_id(
entity=PipelineService, entity_id=str(pipeline.service.id.__root__) entity=PipelineService, entity_id=str(pipeline.service.id.__root__)
) )
@ -481,7 +486,7 @@ class ElasticsearchSink(Sink[Entity]):
version_json = json.loads(version) version_json = json.loads(version)
updatedAt = parser.parse(version_json["updatedAt"]) updatedAt = parser.parse(version_json["updatedAt"])
change_description = ChangeDescription( change_description = ChangeDescription(
updatedBy=version_json["updatedBy"], updatedAt=updatedAt.timestamp() updatedBy=version_json["updatedBy"], updatedAt=epoch_ms(updatedAt)
) )
if "changeDescription" in version_json: if "changeDescription" in version_json:
change_description.fieldsAdded = version_json["changeDescription"][ change_description.fieldsAdded = version_json["changeDescription"][