Fix bug in owner selection when owner is present in serde (#529)

Netflix · Feb 3, 2023 · 8203a08 · 8203a08
1 parent dd680fd
commit 8203a08
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 0 deletions.
diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java
@@ -170,6 +170,18 @@ private void logOwnershipDiagnosticDetails(final QualifiedName name, final Table
 
                 log.info("Create table with invalid owner: {}. name: {}, table-dto: {}, context: {}, headers: {}",
                     tableOwner, name, tableDto, MetacatContextManager.getContext(), getHttpHeaders());
+            } else if (!isOwnerValid(MetacatContextManager.getContext().getUserName())) {
+                registry.counter(
+                    "unauth.user.createTable.requestContext",
+                    "catalog", name.getCatalogName(),
+                    "database", name.getDatabaseName(),
+                    "owner", StringUtils.isBlank(MetacatContextManager.getContext().getUserName()) ? "null" : tableOwner
+                ).increment();
+
+                log.info("Create table called with invalid owner: {} in request context."
+                             + " name: {}, table-dto: {}, context: {}, headers: {}",
+                    MetacatContextManager.getContext().getUserName(), name, tableDto,
+                    MetacatContextManager.getContext(), getHttpHeaders());
             }
         } catch (Exception ex) {
             log.warn("Error when logging diagnostic data for invalid owner table creation. name: {}, table: {}",
@@ -247,6 +259,7 @@ private void setOwnerIfNull(final TableDto tableDto) {
         final String serdeOwner = tableDto.getSerde().getOwner();
         if (isOwnerValid(serdeOwner)) {
             updateTableOwner(tableDto, serdeOwner);
+            return;
         }
 
         // At this point, if we still have not found a valid user,

diff --git a/...t-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy b/...t-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy
@@ -347,6 +347,7 @@ class TableServiceImplSpec extends Specification {
         initialDefinitionMetadata              | sessionUser | initialSerde                      || expectedDefMetadata                     | expectedSerde
         null                                   | null        | null                              || "{}"                                    | new StorageDto()
         null                                   | "ssarma"    | null                              || "{\"owner\":{\"userId\":\"ssarma\"}}"   | new StorageDto()
+        null                                   | "root"      | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"swaranga\"}}" | new StorageDto(owner: "swaranga")
         "{\"owner\":{\"userId\":\"ssarma\"}}"  | "asdf"      | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}"   | new StorageDto(owner: "swaranga")
         "{\"owner\":{\"userId\":\"metacat\"}}" | "ssarma"    | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}"   | new StorageDto(owner: "swaranga")
         "{\"owner\":{\"userId\":\"root\"}}"    | "ssarma"    | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}"   | new StorageDto(owner: "swaranga")