Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add encryption for content on cache file #165

Closed

Conversation

argaasasta
Copy link

For fix this issue
#164

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 24, 2025

Seems like it will work for one specific instance of CachingAndroid while if encrypted content is stored yesterday, restored today it won't decrypt because no guarantee it's the same object and the same secret key

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 24, 2025

Seems like it was tried in Plain text; GrowthBook provides encryption in PRO:

image

@argaasasta
Copy link
Author

@Bohdan-Kim is the PRO will encrypt this cache, so it not visible to pentester? so all i need is just use the PRO?

@Bohdan-Kim
Copy link
Collaborator

yes encryption in PRO

@argaasasta
Copy link
Author

@Bohdan-Kim after i see the implementation, only android that have this FeatureCache.txt, can we add encrypt just for the local cache on android?

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 30, 2025

@argaasasta we can but it's overhead for this case. can we just add ability for you to disable the local cache on android?
cachingEnabled: Boolean = false

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 30, 2025

If user of the SDK provides localEncryptionKey it seems allow to achieve encryption of FeatureCache.txt in not PRO, but it would complicate caching layer

@@ -24,6 +24,7 @@ abstract class SDKBuilder(
val attributes: Map<String, Any>,
val trackingCallback: GBTrackingCallback,
val encryptionKey: String?,
val localEncryptionKey: String? = null,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is publicly exposed API change. It should be discussed with maintainers @Auz @jdorn

@argaasasta
Copy link
Author

@argaasasta we can but it's overhead for this case. can we just add ability for you to disable the local cache on android?
cachingEnabled: Boolean = false

@Bohdan-Kim yes please, can you add this? so the FeatureCache.txt will not be there
thank you

@argaasasta
Copy link
Author

internal fun gbSerialize(): JsonElement = when(this) {
is GBBoolean -> JsonPrimitive(this.value)
is GBString -> JsonPrimitive(this.value)
is GBNumber -> JsonPrimitive(this.value)
is GBJson -> JsonObject(
this.mapValues { it.value.gbSerialize() }
)
is Unknown -> JsonNull
}

@Bohdan-Kim could you make this function is not internal, so we can still use on the outside?

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 30, 2025

@argaasasta I think yes

@argaasasta
Copy link
Author

i still want to use the GBFeatureResult for return, and i cant change the GBValue to json that i want

I have copy this
return when(val gbResultValue = gbFeatureResult.gbValue) {
is GBBoolean -> gbResultValue.value as? V
is GBString -> gbResultValue.value as? V
is GBNumber -> gbResultValue.value as? V
is GBJson -> gbResultValue as? V
is GBValue.Unknown -> null
null -> null
}

    but the JSon cannot be transform, and i change to this
    
  val jsonObject: JsonObject = (feature.gbValue as GBJson).gbSerialize() as JsonObject
  Gson().fromJson(jsonObject.toString(), T::class.java)
  
  and it success, so i need the gbSerialize

@Bohdan-Kim

@Bohdan-Kim
Copy link
Collaborator

Bohdan-Kim commented Jan 30, 2025

@argaasasta Are you sure that you feature type is JSON? Maybe safe cast.
Seems like we should to move GBValue.gbSerialize() to growthbook-kotlinx-serialization module as Gson using possible instead of kotlinx-serialization

@argaasasta
Copy link
Author

argaasasta commented Jan 30, 2025

@Bohdan-Kim yes, it works previously with this code
val feature = getInstance().feature(featureId)
generateTracker(feature)
val jsonObject: JsonObject = feature.value as JsonObject
Gson().fromJson(jsonObject.toString(), T::class.java)

since value is change to gbValue on new version, we need to make some refactor too

@Bohdan-Kim
Copy link
Collaborator

About the next steps with this pull request.
I'm concerned that this would quite sophisticate caching layer.
Seems like was agreed for simpler way to achieve absence of cache file.

@argaasasta
Copy link
Author

Sure, i will close this PR

@argaasasta argaasasta closed this Feb 24, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants