Merge pull request #79 from ba-st/sensitive-arguments-on-configuratio…

…n-plugin Fix sensitive arguments handling in the application configuration plugin
ba-st · Dec 2, 2019 · c396568 · c396568
2 parents f08e625 + 4ae237f
commit c396568
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 20 deletions.
diff --git a/...te-Application-Configuration-Tests/ApplicationConfigurationRESTfulControllerTest.class.st b/...te-Application-Configuration-Tests/ApplicationConfigurationRESTfulControllerTest.class.st
@@ -14,6 +14,7 @@ ApplicationConfigurationRESTfulControllerTest >> configurationDefinitions [
 		with: ( OptionalArgument named: 'port' defaultingTo: 4000 )
 		with: ( MandatoryArgument named: 'base-url' )
 		with: ( FlagArgument named: 'debug-mode' )
+		with: ( MandatoryArgument named: 'secret' ) asSensitive
 ]
 
 { #category : #'private - support' }
@@ -23,6 +24,7 @@ ApplicationConfigurationRESTfulControllerTest >> configurationProvider [
 		at: 'port' put: 6000;
 		at: 'base-url' put: 'https://api.example.com';
 		at: 'debug-mode' put: true;
+		at: 'secret' put: 'XXX';
 		yourself
 	]
 ]
@@ -64,7 +66,7 @@ ApplicationConfigurationRESTfulControllerTest >> testGetConfigurationInJson [
 		withJsonFromContentsIn: response
 			do: [ :configs | 
 			self
-				assert: configs size equals: 3;
+				assert: configs size equals: 4;
 				withConfigurationNamed: 'port'
 					in: configs
 					do: [ :config | 
@@ -77,7 +79,10 @@ ApplicationConfigurationRESTfulControllerTest >> testGetConfigurationInJson [
 					do: [ :config | self assert: ( config at: #'current-value' ) equals: 'https://api.example.com' ];
 				withConfigurationNamed: 'debug-mode'
 					in: configs
-					do: [ :config | self assert: ( config at: #'current-value' ) ]
+					do: [ :config | self assert: ( config at: #'current-value' ) ];
+				withConfigurationNamed: 'secret'
+					in: configs
+					do: [ :config | self assert: ( config at: #'current-value' ) equals: '********' ]
 			]
 ]
 
@@ -95,7 +100,7 @@ ApplicationConfigurationRESTfulControllerTest >> testGetConfigurationInPlainText
 		assert: response contentType asMediaType equals: ( ZnMimeType textPlain version: '1.0.0' );
 		assert: response contents
 			equals:
-			( 'port = 6000<1s><2s>base-url = https://api.example.com<1s><2s>debug-mode = true'
+			( 'port = 6000<1s><2s>base-url = https://api.example.com<1s><2s>debug-mode = true<1s><2s>secret = ********'
 				expandMacrosWith: String cr
 				with: String lf )
 ]

diff --git a/source/Stargate-Application-Configuration/ApplicationConfigurationRESTfulController.class.st b/source/Stargate-Application-Configuration/ApplicationConfigurationRESTfulController.class.st
@@ -20,32 +20,41 @@ ApplicationConfigurationRESTfulController class >> over: aPlugin configuredBy: c
 { #category : #private }
 ApplicationConfigurationRESTfulController >> configureConfigurationEncodingOn: writer [
 
+	CommandLineArgument
+		subclassesDo: [ :subclass | 
+			writer
+				for: subclass
+				do: [ :mapper | 
+					mapper
+						mapAccessor: #name;
+						mapProperty: #'current-value'
+							getter: [ :definition | self currentValueOrMaskFor: definition ]
+					]
+			].
+
 	writer
 		for: MandatoryArgument
-			do: [ :mapper | 
-			mapper
-				mapProperty: #type getter: [ :definition | 'mandatory' ];
-				mapAccessor: #name;
-				mapProperty: #'current-value' getter: [ :definition | plugin currentValueFor: definition ]
-			];
-		for: FlagArgument
-			do: [ :mapper | 
-			mapper
-				mapProperty: #type getter: [ :definition | 'flag' ];
-				mapAccessor: #name;
-				mapProperty: #'current-value' getter: [ :definition | plugin currentValueFor: definition ]
-			];
+			do: [ :mapper | mapper mapProperty: #type getter: [ :definition | 'mandatory' ] ];
+		for: FlagArgument do: [ :mapper | mapper mapProperty: #type getter: [ :definition | 'flag' ] ];
 		for: OptionalArgument
 			do: [ :mapper | 
 			mapper
 				mapProperty: #type getter: [ :definition | 'optional' ];
-				mapAccessor: #name;
-				mapProperty: #'current-value' getter: [ :definition | plugin currentValueFor: definition ];
 				mapAccessor: #default
 			];
+		for: SensitiveArgument
+			do: [ :mapper | mapper mapProperty: #type getter: [ :definition | 'sensitive' ] ];
 		for: ZnUrl customDo: [ :mapping | mapping encoder: [ :url | url printString ] ]
 ]
 
+{ #category : #private }
+ApplicationConfigurationRESTfulController >> currentValueOrMaskFor: definition [
+
+	^ ( definition isA: SensitiveArgument )
+		then: [ '********' ]
+		otherwise: [ plugin currentValueFor: definition ]
+]
+
 { #category : #routes }
 ApplicationConfigurationRESTfulController >> declareGetApplicationConfigurationRoute [
 
@@ -56,7 +65,7 @@ ApplicationConfigurationRESTfulController >> declareGetApplicationConfigurationR
 			[ :httpRequest :requestContext | self getApplicationConfigurationBasedOn: httpRequest within: requestContext ]
 ]
 
-{ #category : #initialization }
+{ #category : #private }
 ApplicationConfigurationRESTfulController >> encodeConfigurationInTextFormat: configurationDefinitions [
 
 	^ String
@@ -68,7 +77,7 @@ ApplicationConfigurationRESTfulController >> encodeConfigurationInTextFormat: co
 						space;
 						nextPut: $=;
 						space;
-						nextPutAll: ( plugin currentValueFor: definition ) asString
+						nextPutAll: ( self currentValueOrMaskFor: definition ) asString
 					]
 				separatedBy: [ stream crlf ]
 			]