You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by am...@apache.org on 2022/12/07 13:04:52 UTC
[knox] branch master updated: KNOX-2848: Prevent overwriting generated descriptors/providers (#687)
This is an automated email from the ASF dual-hosted git repository.
amagyar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new b8fc2c798 KNOX-2848: Prevent overwriting generated descriptors/providers (#687)
b8fc2c798 is described below
commit b8fc2c7989c0a4702fc5c1836d0e2988df5e958e
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Wed Dec 7 14:04:47 2022 +0100
KNOX-2848: Prevent overwriting generated descriptors/providers (#687)
---
.../new-desc-wizard/new-desc-wizard.component.html | 6 +-
.../new-desc-wizard/new-desc-wizard.component.ts | 14 ++++-
.../provider-config-wizard.component.html | 6 +-
.../provider-config-wizard.component.ts | 15 ++++-
.../gateway/config/impl/GatewayConfigImpl.java | 15 +++++
.../gateway/service/admin/TopologiesResource.java | 34 ++++++++++-
.../org/apache/knox/gateway/GatewayTestConfig.java | 12 ++++
.../apache/knox/gateway/config/GatewayConfig.java | 5 ++
.../knox/gateway/i18n/GatewaySpiMessages.java | 6 ++
.../knox/gateway/GatewayAdminTopologyFuncTest.java | 66 ++++++++++++++++++++++
10 files changed, 169 insertions(+), 10 deletions(-)
diff --git a/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.html b/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.html
index 960f30079..1c9a9770a 100644
--- a/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.html
+++ b/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.html
@@ -1,4 +1,4 @@
-<bs-modal (onClose)="onClose()" #newDescriptorModal xmlns="http://www.w3.org/1999/html">
+<bs-modal #newDescriptorModal xmlns="http://www.w3.org/1999/html">
<bs-modal-header [showDismiss]="true">
<label class="modal-title">Create a New Descriptor</label>
</bs-modal-header>
@@ -12,6 +12,8 @@
<span *ngIf="isMissingDescriptorName()" style="color: red">required</span>
<span *ngIf="!isMissingDescriptorName() && !isValidDescriptorName()"
style="color: red">invalid</span>
+ <span *ngIf="isExistingReadOnlyTopology()"
+ style="color: red">Cannot overwrite existing read-only topology</span>
</div> <!-- Descriptor Name -->
<br>
@@ -141,7 +143,7 @@
<button type="button"
class="btn btn-primary btn-sm"
[disabled]="!validate()"
- (click)="newDescriptorModal.close()">Ok
+ (click)="save()">Ok
</button>
</bs-modal-footer>
</bs-modal>
diff --git a/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.ts b/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.ts
index 453c1e853..4794b26a3 100644
--- a/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.ts
+++ b/gateway-admin-ui/admin-ui/app/new-desc-wizard/new-desc-wizard.component.ts
@@ -23,6 +23,7 @@ import {ResourceService} from '../resource/resource.service';
import {Resource} from '../resource/resource';
import {ResourceTypesService} from '../resourcetypes/resourcetypes.service';
import {ValidationUtils} from '../utils/validation-utils';
+import {HttpErrorResponse} from '@angular/common/http';
@Component({
selector: 'app-new-desc-wizard',
@@ -92,6 +93,8 @@ export class NewDescWizardComponent implements OnInit {
editModePC: boolean;
+ existingReadOnlyTopology = false;
+
constructor(private resourceTypesService: ResourceTypesService, private resourceService: ResourceService) {
}
@@ -110,6 +113,7 @@ export class NewDescWizardComponent implements OnInit {
this.resource = new Resource();
this.descriptor = new Descriptor();
this.descriptorName = '';
+ this.existingReadOnlyTopology = false;
// Reset any previously-selected services
for (let serviceName of NewDescWizardComponent.supportedServices) {
@@ -120,7 +124,7 @@ export class NewDescWizardComponent implements OnInit {
}
}
- onClose() {
+ save() {
// Set the service declarations on the descriptor
for (let serviceName of NewDescWizardComponent.supportedServices) {
if (this.isSelected(serviceName)) {
@@ -153,9 +157,17 @@ export class NewDescWizardComponent implements OnInit {
}
}
});
+ this.childModal.close(); // close the dialog if there was no error
+ }).catch((err: HttpErrorResponse) => {
+ this.existingReadOnlyTopology = (err.status === 409);
+ console.error('Error creating ' + this.descriptorName + ' : ' + err.message);
});
}
+ isExistingReadOnlyTopology(): boolean {
+ return this.existingReadOnlyTopology;
+ }
+
getServiceDisplayColumns() {
let cols = [];
let svcCount = NewDescWizardComponent.supportedServices.length;
diff --git a/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.html b/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.html
index 875cf274a..46cd5dc21 100644
--- a/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.html
+++ b/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.html
@@ -1,4 +1,4 @@
-<bs-modal (onClose)="onClose()" #newProviderConfigModal xmlns="http://www.w3.org/1999/html">
+<bs-modal #newProviderConfigModal xmlns="http://www.w3.org/1999/html">
<bs-modal-header [showDismiss]="true">
<label class="modal-title">Create a New Provider Configuration</label>
</bs-modal-header>
@@ -19,6 +19,8 @@
<span *ngIf="field.errors?.required" style="color: red">
required
</span>
+ <span *ngIf="isExistingReadOnlyProvider()"
+ style="color: red">Cannot overwrite existing read-only topology</span>
<span *ngIf="!field.errors?.required && !isValidProviderConfigName()"
style="color: red">
invalid
@@ -120,7 +122,7 @@
*ngIf="isRootStep() || !hasMoreSteps()"
class="btn btn-primary btn-sm"
[disabled]="(isRootStep() && !name)"
- (click)="isRootStep() ? newProviderConfigModal.close() : onFinishAdd()">Ok
+ (click)="isRootStep() ? save() : onFinishAdd()">Ok
</button>
<button type="button"
*ngIf="!isRootStep() && hasMoreSteps()"
diff --git a/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.ts b/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.ts
index 70f3fa078..a4b03a7e8 100644
--- a/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.ts
+++ b/gateway-admin-ui/admin-ui/app/provider-config-wizard/provider-config-wizard.component.ts
@@ -32,6 +32,7 @@ import {HostMapProviderWizard} from './hostmap-provider-wizard';
import {ProviderContributorWizard} from './provider-contributor-wizard';
import {WebAppSecurityWizard} from './webappsec-wizard';
import {ValidationUtils} from '../utils/validation-utils';
+import {HttpErrorResponse} from '@angular/common/http';
@Component({
selector: 'app-provider-config-wizard',
@@ -79,6 +80,8 @@ export class ProviderConfigWizardComponent implements OnInit {
selectedCategory: string;
+ existingReadOnlyProvider = false;
+
static isProviderConfigValid(pc: ProviderConfig): boolean {
let isValid = true;
if (pc instanceof DisplayBindingProviderConfig) {
@@ -113,6 +116,7 @@ export class ProviderConfigWizardComponent implements OnInit {
this.name = '';
this.providers = [];
this.selectedCategory = ProviderConfigWizardComponent.CATEGORY_AUTHENTICATION;
+ this.existingReadOnlyProvider = false;
}
onFinishAdd() {
@@ -189,7 +193,7 @@ export class ProviderConfigWizardComponent implements OnInit {
}
}
- onClose() {
+ save() {
// Identify the new resource
let newResource = new Resource();
newResource.name = this.name + '.json';
@@ -222,7 +226,11 @@ export class ProviderConfigWizardComponent implements OnInit {
}
}
});
- });
+ this.childModal.close(); // close the dialog if there was no error
+ }).catch((err: HttpErrorResponse) => {
+ this.existingReadOnlyProvider = (err.status === 409);
+ console.error('Error creating ' + newResource + ' : ' + err.message);
+ });
}
onNextStep() {
@@ -392,4 +400,7 @@ export class ProviderConfigWizardComponent implements OnInit {
return this['show' + propertName];
}
+ isExistingReadOnlyProvider(): boolean {
+ return this.existingReadOnlyProvider;
+ }
}
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
index 33bbf7066..fce819a34 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java
@@ -205,6 +205,9 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
public static final String READ_ONLY_OVERRIDE_TOPOLOGIES =
GATEWAY_CONFIG_FILE_PREFIX + ".read.only.override.topologies";
+ public static final String READ_ONLY_OVERRIDE_PROVIDERS =
+ GATEWAY_CONFIG_FILE_PREFIX + ".read.only.override.providers";
+
/* Websocket defaults */
public static final boolean DEFAULT_WEBSOCKET_FEATURE_ENABLED = false;
public static final int DEFAULT_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE = Integer.MAX_VALUE;
@@ -1160,6 +1163,18 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
return topologyNames;
}
+ @Override
+ public List<String> getReadOnlyOverrideProviderNames() {
+ List<String> topologyNames = new ArrayList<>();
+
+ String value = get(READ_ONLY_OVERRIDE_PROVIDERS);
+ if (value != null && !value.isEmpty()) {
+ topologyNames.addAll(Arrays.asList(value.trim().split("\\s*,\\s*")));
+ }
+
+ return topologyNames;
+ }
+
@Override
public String getKnoxAdminGroups() {
return get(KNOX_ADMIN_GROUPS, null);
diff --git a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
index eb3f1d42a..7a6ec5b4d 100644
--- a/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
+++ b/gateway-service-admin/src/main/java/org/apache/knox/gateway/service/admin/TopologiesResource.java
@@ -128,8 +128,8 @@ public class TopologiesResource {
}
// For any read-only override topology, mark it as generated to discourage modification.
- final List<String> ambariManagedTopos = config.getReadOnlyOverrideTopologyNames();
- if (ambariManagedTopos.contains(convertedTopology.getName())) {
+ final List<String> managedTopologies = config.getReadOnlyOverrideTopologyNames();
+ if (managedTopologies.contains(convertedTopology.getName())) {
convertedTopology.setGenerated(true);
}
@@ -361,6 +361,14 @@ public class TopologiesResource {
// extension.
String filename = isUpdate ? existing.getName() : getFileNameForResource(name, headers);
+ GatewayConfig config =
+ (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if (providerExists(name, ts) &&
+ config.getReadOnlyOverrideProviderNames().contains(FilenameUtils.getBaseName(name))) {
+ log.disallowedOverwritingGeneratedProvider(name);
+ return status(Response.Status.CONFLICT).entity("{ \"error\" : \"Cannot overwrite existing generated provider: " + name + "\" }").build();
+ }
+
if (ts.deployProviderConfiguration(filename, content)) {
try {
if (isUpdate) {
@@ -398,8 +406,14 @@ public class TopologiesResource {
GatewayServices gs =
(GatewayServices) request.getServletContext().getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
-
TopologyService ts = gs.getService(ServiceType.TOPOLOGY_SERVICE);
+ GatewayConfig config =
+ (GatewayConfig) request.getServletContext().getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if ((descriptorExists(name, ts) || topologyExists(name, ts))
+ && config.getReadOnlyOverrideTopologyNames().contains(FilenameUtils.getBaseName(name))) {
+ log.disallowedOverwritingGeneratedDescriptor(name);
+ return status(Response.Status.CONFLICT).entity("{ \"error\" : \"Cannot overwrite existing generated topology: " + name + "\" }").build();
+ }
File existing = getExistingConfigFile(ts.getDescriptors(), name);
boolean isUpdate = (existing != null);
@@ -424,6 +438,20 @@ public class TopologiesResource {
return response;
}
+ public boolean topologyExists(String fileName, TopologyService topologyService) {
+ return topologyService.getTopologies().stream()
+ .anyMatch(topology -> topology.getName().equals(FilenameUtils.getBaseName(fileName)));
+ }
+
+ public boolean descriptorExists(String fileName, TopologyService topologyService) {
+ return topologyService.getDescriptors().stream()
+ .anyMatch(file -> FilenameUtils.getBaseName(file.getName()).equals(FilenameUtils.getBaseName(fileName)));
+ }
+
+ public boolean providerExists(String fileName, TopologyService topologyService) {
+ return topologyService.getProviderConfigurations().stream()
+ .anyMatch(file -> FilenameUtils.getBaseName(file.getName()).equals(FilenameUtils.getBaseName(fileName)));
+ }
@GET
@Produces({APPLICATION_JSON})
diff --git a/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java b/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
index fb70ded11..2974ac15c 100644
--- a/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
+++ b/gateway-spi-common/src/main/java/org/apache/knox/gateway/GatewayTestConfig.java
@@ -781,6 +781,18 @@ public class GatewayTestConfig extends Configuration implements GatewayConfig {
return readOnly;
}
+ @Override
+ public List<String> getReadOnlyOverrideProviderNames() {
+ List<String> readOnly = new ArrayList<>();
+
+ String value = get("gateway.read.only.override.providers");
+ if (value != null && !value.isEmpty()) {
+ readOnly.addAll(Arrays.asList(value.trim().split("\\s*,\\s*")));
+ }
+
+ return readOnly;
+ }
+
@Override
public String getKnoxAdminGroups() {
return null;
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
index ced53f013..2030180fd 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java
@@ -626,6 +626,11 @@ public interface GatewayConfig {
*/
List<String> getReadOnlyOverrideTopologyNames();
+ /**
+ * Get the list of those topology names which should be treated as read-only.
+ */
+ List<String> getReadOnlyOverrideProviderNames();
+
/**
* Get the comma separated list of group names that represent Knox Admin users
* @return comma separate list of admin group names
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
index 48f6ca643..b8593643f 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/i18n/GatewaySpiMessages.java
@@ -76,6 +76,12 @@ public interface GatewaySpiMessages {
@Message( level = MessageLevel.ERROR, text = "Topology {0} cannot be manually overwritten because it was generated from a simple descriptor." )
void disallowedOverwritingGeneratedTopology(String topologyName);
+ @Message( level = MessageLevel.INFO, text = "Read-only descriptor {0} cannot be overwritten." )
+ void disallowedOverwritingGeneratedDescriptor(String name);
+
+ @Message( level = MessageLevel.INFO, text = "Read-only provider {0} cannot be overwritten." )
+ void disallowedOverwritingGeneratedProvider(String providerName);
+
@Message(level = MessageLevel.ERROR, text = "Failed to load truststore due to {0}")
void failedToLoadTruststore(String message, @StackTrace(level = MessageLevel.DEBUG) Exception e);
diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
index df3b8ba81..2102eb8d5 100644
--- a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
+++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayAdminTopologyFuncTest.java
@@ -1912,6 +1912,72 @@ public class GatewayAdminTopologyFuncTest {
assertFalse(newDescriptorFile.exists());
}
+ @Test( timeout = TestUtils.LONG_TIMEOUT )
+ public void testPutExistingGeneratedDescriptorFails() throws Exception {
+ LOG_ENTER();
+ try {
+ gateway.stop();
+ GatewayTestConfig conf = new GatewayTestConfig();
+ String topologyName = "test-desc";
+ conf.set(GatewayConfigImpl.READ_ONLY_OVERRIDE_TOPOLOGIES, topologyName);
+ setupGateway(conf);
+ given()
+ .auth().preemptive().basic("admin", "admin-password")
+ .header("Content-type", MediaType.APPLICATION_JSON)
+ .body("{}")
+ .then()
+ .statusCode(HttpStatus.SC_CREATED)
+ .when().put((clusterUrl + "/api/v1/descriptors/" + topologyName));
+ given()
+ .auth().preemptive().basic("admin", "admin-password")
+ .header("Content-type", MediaType.APPLICATION_JSON)
+ .body("new content")
+ .then()
+ .statusCode(HttpStatus.SC_CONFLICT)
+ .when().put((clusterUrl + "/api/v1/descriptors/" + topologyName));
+ } finally {
+ // Restart the gateway with old settings.
+ gateway.stop();
+ setupGateway(new GatewayTestConfig());
+ }
+
+ LOG_EXIT();
+ }
+
+ @Test( timeout = TestUtils.LONG_TIMEOUT )
+ public void testPutExistingGeneratedProviderFails() throws Exception {
+ LOG_ENTER();
+ try {
+ gateway.stop();
+ GatewayTestConfig conf = new GatewayTestConfig();
+ String providerName = "sso";
+ conf.set(GatewayConfigImpl.READ_ONLY_OVERRIDE_PROVIDERS, providerName);
+ setupGateway(conf);
+ given()
+ .auth().preemptive().basic("admin", "admin-password")
+ .header("Content-type", MediaType.APPLICATION_JSON)
+ .body("{}")
+ .then()
+ .statusCode(HttpStatus.SC_CREATED)
+ .when()
+ .put((clusterUrl + "/api/v1/providerconfig/" + providerName));
+ given()
+ .auth().preemptive().basic("admin", "admin-password")
+ .header("Content-type", MediaType.APPLICATION_JSON)
+ .body("new content")
+ .then()
+ .statusCode(HttpStatus.SC_CONFLICT)
+ .when()
+ .put((clusterUrl + "/api/v1/providerconfig/" + providerName));
+ } finally {
+ // Restart the gateway with old settings.
+ gateway.stop();
+ setupGateway(new GatewayTestConfig());
+ }
+
+ LOG_EXIT();
+ }
+
@Test( timeout = TestUtils.LONG_TIMEOUT )
public void testPutDescriptorWithInvalidNameEncodedElement() throws Exception {
LOG_ENTER();