From c9ec915d7d16b88f53493c85928d463d070df472 Mon Sep 17 00:00:00 2001
From: lukegleeson <luke.gleeson@est.tech>
Date: Wed, 7 Sep 2022 14:21:36 +0100
Subject: Fix security bug in logs

When a method with signature containing "AuthPassword" is passed, the value returned is changed to *********** in logs
e.g...  Execution time of : DmiProperties.getAuthPassword() with argument[s] = *********** ...

Legacy code cleaning also included

Issue-ID: CPS-1226
Signed-off-by: lukegleeson <luke.gleeson@est.tech>
Change-Id: Ic4914eae7e5ed6d361287413d17abfe50a3788ae
---
 .../cps/aop/CpsLoggingAspectServiceSpec.groovy     | 54 ++++++++++++++--------
 1 file changed, 36 insertions(+), 18 deletions(-)

(limited to 'cps-service/src/test')

diff --git a/cps-service/src/test/groovy/org/onap/cps/aop/CpsLoggingAspectServiceSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/aop/CpsLoggingAspectServiceSpec.groovy
index 1308bd729..d0e19ad8d 100644
--- a/cps-service/src/test/groovy/org/onap/cps/aop/CpsLoggingAspectServiceSpec.groovy
+++ b/cps-service/src/test/groovy/org/onap/cps/aop/CpsLoggingAspectServiceSpec.groovy
@@ -22,34 +22,31 @@ package org.onap.cps.aop
 
 import org.aspectj.lang.ProceedingJoinPoint
 import org.aspectj.lang.reflect.MethodSignature
-import org.onap.cps.spi.exceptions.DataValidationException
 import spock.lang.Specification
 import java.util.logging.Level
 import java.util.logging.Logger
 
 class CpsLoggingAspectServiceSpec extends Specification {
 
-    private static final Logger logger = Logger.getLogger("org.onap.cps")
+    private static final Logger logger = Logger.getLogger('org.onap.cps')
 
     def mockProceedingJoinPoint = Mock(ProceedingJoinPoint)
-    def mockMethodSignature = Mock(MethodSignature);
-    def objectUnderTest = new CpsLoggingAspectService()
+    def mockMethodSignature = Mock(MethodSignature)
+    def objectUnderTest = Spy(CpsLoggingAspectService)
 
     def setup() {
         mockMethodSignature.getDeclaringType() >> this.getClass()
-        mockMethodSignature.getDeclaringType().getSimpleName() >> 'CpsLoggingAspectServiceSpec'
-        mockMethodSignature.getName() >> 'logMethodExecutionTime'
         mockProceedingJoinPoint.getSignature() >> mockMethodSignature
     }
 
     def 'Log method execution time for log level : #logLevel.'() {
-        given: 'mock valid pointcut arguments and set log level to #logLevel'
-            mockProceedingJoinPoint.getArgs() >> 'dataspace-name'
+        given: 'normal method and log level of #logLevel'
+            mockMethodSignature.getName() >> 'some method'
             logger.setLevel(logLevel)
-        when: 'aop intercepts cps method'
-            objectUnderTest.logMethodExecutionTime(mockProceedingJoinPoint)
-        then: 'expected number of method execution'
-            expectedNumberOfMethodExecution * mockMethodSignature.getName()
+        when: 'cps method is intercepted'
+            objectUnderTest.interceptMethodCall(mockProceedingJoinPoint)
+        then: 'logging is only done for correct levels'
+            expectedNumberOfMethodExecution * objectUnderTest.logMethodCall(*_)
         where: 'the following log levels are used'
             logLevel     || expectedNumberOfMethodExecution
             Level.INFO   || 0
@@ -58,11 +55,32 @@ class CpsLoggingAspectServiceSpec extends Specification {
     }
 
     def 'Exception thrown during method execution.'() {
-        given: 'some exception is created'
-            mockProceedingJoinPoint.proceed() >> { throw new Exception("some exception") }
-        when: 'aop intercepts cps method and start calculation of time'
-            objectUnderTest.logMethodExecutionTime(mockProceedingJoinPoint)
-        then: 'some exception is thrown'
-            thrown Exception
+        given: 'some exception is thrown'
+            def originalException = new Exception('some exception')
+            mockProceedingJoinPoint.proceed() >> {
+                throw originalException
+            }
+        when: 'cps method is intercepted'
+            objectUnderTest.interceptMethodCall(mockProceedingJoinPoint)
+        then: 'the same exception is still thrown'
+            def thrownException = thrown(Exception)
+            assert thrownException == originalException
     }
+
+    def 'Masking sensitive data.'() {
+        given: 'method named #methodName returns some value'
+            mockMethodSignature.getName() >> methodName
+            mockProceedingJoinPoint.proceed() >> 'original return value'
+        and: 'the logger level is set to FINE'
+            logger.setLevel(Level.FINE)
+        when: 'cps method is intercepted'
+           objectUnderTest.interceptMethodCall(mockProceedingJoinPoint)
+        then: 'the expected value is being logged'
+            1 * objectUnderTest.logMethodCall(_, _, _, expectedLogValue)
+        where: 'the following method names are used'
+            methodName        || expectedLogValue
+            'normalMethod'    || 'original return value'
+            'getAuthPassword' || '***********'
+    }
+
 }
-- 
cgit 1.2.3-korg