[GitHub] [openwhisk] style95 commented on a change in pull request #4855: Improvements to parameter encryption to support per-namespace keys

[GitBox <gi...@apache.org>]

style95 commented on a change in pull request #4855:
URL: https://github.com/apache/openwhisk/pull/4855#discussion_r499346321



##########
File path: common/scala/src/main/scala/org/apache/openwhisk/core/entity/ParameterEncryption.scala
##########
@@ -26,79 +26,85 @@ import javax.crypto.spec.{GCMParameterSpec, SecretKeySpec}
 import org.apache.openwhisk.core.ConfigKeys
 import pureconfig.loadConfig
 import spray.json.DefaultJsonProtocol._
-import spray.json.{JsNull, JsString}
+import spray.json._
 import pureconfig.generic.auto._
 import spray.json._
-case class ParameterStorageConfig(current: String = "", aes128: String = "", aes256: String = "")
-
-object ParameterEncryption {
-  private val storageConfigLoader = loadConfig[ParameterStorageConfig](ConfigKeys.parameterStorage)
-  var storageConfig = storageConfigLoader.getOrElse(ParameterStorageConfig.apply())
-  def lock(params: Parameters): Parameters = {
-    val configuredEncryptors = new encrypters(storageConfig)
-    new Parameters(
-      params.getMap
-        .map(({
-          case (paramName, paramValue) if paramValue.encryption == JsNull =>
-            paramName -> configuredEncryptors.getCurrentEncrypter().encrypt(paramValue)
-          case (paramName, paramValue) => paramName -> paramValue
-        })))
-  }
-  def unlock(params: Parameters): Parameters = {
-    val configuredEncryptors = new encrypters(storageConfig)
-    new Parameters(
-      params.getMap
-        .map(({
-          case (paramName, paramValue)
-              if paramValue.encryption != JsNull && !configuredEncryptors
-                .getEncrypter(paramValue.encryption.convertTo[String])
-                .isEmpty =>
-            paramName -> configuredEncryptors
-              .getEncrypter(paramValue.encryption.convertTo[String])
-              .get
-              .decrypt(paramValue)
-          case (paramName, paramValue) => paramName -> paramValue
-        })))
+
+protected[core] case class ParameterStorageConfig(current: String = ParameterEncryption.NO_ENCRYPTION,
+                                                  aes128: Option[String] = None,
+                                                  aes256: Option[String] = None)
+
+protected[core] class ParameterEncryption(val default: Option[Encrypter], encryptors: Map[String, Encrypter]) {
+
+  /**
+   * Gets the coder for the given scheme name.
+   *
+   * @param name the name of the encryption algorithm (defaults to current from last configuration)
+   * @return the coder if there is one else no-op encryptor
+   */
+  def encryptor(name: String): Encrypter = {
+    encryptors.get(name).getOrElse(ParameterEncryption.noop)
   }
-}
 
-private trait encrypter {
-  def encrypt(p: ParameterValue): ParameterValue
-  def decrypt(p: ParameterValue): ParameterValue
-  val name: String
 }
 
-private class encrypters(val storageConfig: ParameterStorageConfig) {
-  private val availableEncrypters = Map("" -> new NoopCrypt()) ++
-    (if (!storageConfig.aes256.isEmpty) Some(Aes256.name -> new Aes256(getKeyBytes(storageConfig.aes256))) else None) ++
-    (if (!storageConfig.aes128.isEmpty) Some(Aes128.name -> new Aes128(getKeyBytes(storageConfig.aes128))) else None)
+protected[core] object ParameterEncryption {
 
-  protected[entity] def getCurrentEncrypter(): encrypter = {
-    availableEncrypters.get(ParameterEncryption.storageConfig.current).get
+  val NO_ENCRYPTION = "noop"
+  val AES128_ENCRYPTION = "aes-128"
+  val AES256_ENCRYPTION = "aes-256"
+
+  val noop = new Encrypter {
+    override val name = NO_ENCRYPTION
+  }
+
+  val singleton: ParameterEncryption = {

Review comment:
       It's not directly related to the change.
   I am just curious if there is any benefit to having this explicit singleton while an `object` is already a singleton.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org