search

gridly-spreadsheet-CMS / gridly-ue4-plugin

8 stars 5 forks source link

API keys are exported in pak files by default #6

Open sturcotte06 opened 2 years ago

sturcotte06 commented 2 years ago

UGridlyGameSettings is defined in DefaultGame.ini instead of DefaultEditor.ini and will be exported to pak files by default, effectively leaking API keys to players. I think the minimal for a cooked game should be kept in Gridly and move everything that isn't related to gameplay to GridlyEditor. Since no setting in UGridlyGameSettings seems to be gameplay related, it should be moved to GridlyEditor as well and should be changed from Config = Game to Config = Editor so that's it's defined in DefaultEditor.ini and excluded from cooked builds.

yuhe00 commented 2 years ago

The reason for this is that the plugin supports runtime update from Gridly for translators in cooked builds. There are separate API key settings for import and export. The export API key should be private and is only saved locally, but the import one is packaged with the game by default. This should be made more clear though and perhaps solved in a different way.

yuhe00 commented 2 years ago

Simply requiring the API key to be input to the BP function that downloads and updates translations is perhaps a good solution?

sturcotte06 commented 2 years ago

Simply requiring the API key to be input to the BP function that downloads and updates translations is perhaps a good solution?

I'm not entirely sure honestly. It seems like a security hazard to me to export any kind of API key to a client. Within the Gridly web app, I can only see a single API key being generated, which I guess would be a read-write key. Anyone using this value as an import API key could end up leaking a write key and compromising its localization.

I'm not a fan of this live updates for localization to be honest. I'd prefer if the plugin was editor only and had no chance to leak anything in a live build. Live localization is not enough of an issue on a live game to justify this risk.

yuhe00 commented 2 years ago

If it's a BP function, that input could come from any source, eg. an editable text field in the UI so it's not required bundle key with cooked project in any way.

According to Gridly webapp/backend team, it is recommended to create separate users with different levels of access. API keys for these will be different.

EDIT: Actually, it seems like they have added support for creating API keys with restricted access now as well.

sturcotte06 commented 2 years ago

If it's a BP function, that input could come from any source, eg. an editable text field in the UI so it's not required bundle key with cooked project in any way.

According to Gridly webapp/backend team, it is recommended to create separate users with different levels of access. API keys for these will be different.

EDIT: Actually, it seems like they have added support for creating API keys with restricted access now as well.

Yes but then the player would have to input an API key? Either way, live updates will require an API key being included in the build somehow, or known by players.

yuhe00 commented 2 years ago

This feature was not intended for live updates for players, but for external translators/translation teams that do not have access to the Unreal project.