From 4ae237fd7ba6dde28e9f1bc5fa5e21ddccdebc68 Mon Sep 17 00:00:00 2001
From: Gabriel Omar Cotelli <g.cotelli@gmail.com>
Date: Mon, 2 Dec 2019 16:36:05 -0300
Subject: [PATCH] Fix sensitive arguments handling in the application
 configuration plugin

---
 ...onfigurationRESTfulControllerTest.class.st | 11 +++--
 ...ionConfigurationRESTfulController.class.st | 43 +++++++++++--------
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/source/Stargate-Application-Configuration-Tests/ApplicationConfigurationRESTfulControllerTest.class.st b/source/Stargate-Application-Configuration-Tests/ApplicationConfigurationRESTfulControllerTest.class.st
index 7e7c0c7..fe5b19f 100644
--- a/source/Stargate-Application-Configuration-Tests/ApplicationConfigurationRESTfulControllerTest.class.st
+++ b/source/Stargate-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
index ca12964..90e8159 100644
--- 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 ]
 			]