Protect the app from importing keys and values that are not supported according to the json scheme

[grzesiek2010]

Closes #5096
Closes #4939
Closes #5176

What has been done to verify that this works as intended?

I've tested the app manually and improved automated tests.

Why is this the best possible solution? Were any other approaches considered?

I've extended JsonSchemaSettingsValidator and added methods to verify if existing keys/values are supported.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Please verify the case form the issue #5096

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

• run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[seadowg]

@grzesiek2010 is this meant to close #4939?

[grzesiek2010]

@grzesiek2010 is this meant to close #4939?

Yes, I've just fixed the description.

[dbemke]

Tested with Success!

Verified on Android 8.1, 11

Verified cases:

• It is possible to add a project without setting current project id #5176
  Protect the app from importing preferences that are not specified in our json scheme #4939
  Handle unsupported settings that might exist in QR codes #5096 are no longer reproducing
• importing QR code from a file, Google Drive and Central
• exporting QR code via Google Drive and email
• scanning QR code in horizontal and vertical way
• resetting all settings
• reconfiguring settings with QR code
• reconfiguring settings with big sized image
• reconfiguring settings with QR code containing sensitive information- admin and server password on Aggregate and ODK
• scanning valid, invalid QR code and inverted QR code
• updating settings from older versions
• adding duplicate project
• switching to existing project

[srujner]

Tested with Success!

Verified on Android 10 and 12

Today I found one strange issue related to Android 5.1 only: #5178

[grzesiek2010]

@seadowg there are new changes (for #5176) here so you might want to take a look.

In the issue @lognaturel asked if #5176 is a fix for this Crashlytics report. I said that the issue was probably responsible for just a part of the reports.
Now I'm thinking more about those crashes and I think that it might be caused by similar problems with QR codes that we are not able to detect and our scheme validator also does not deal with.

I wanted to propose one additional improvements which is wrapping this into try-catch and catch anything there. My thinking is that the mentioned method consists of many steps that could potentialy fail (for example creating JSONObjects).
If that method somehow throws an exception we end up with that problem with no project id. So although we do not now for sure what might throw such exceptions it seeps to be possible.

What do you think @seadowg @lognaturel?

[seadowg]

I wanted to propose one additional improvements which is wrapping this into try-catch and catch anything there. My thinking is that the mentioned method consists of many steps that could potentialy fail (for example creating JSONObjects).
If that method somehow throws an exception we end up with that problem with no project id. So although we do not know for sure what might throw such exceptions it seeps to be possible.

This probably makes sense. I think we'd also want to clear the new shared preferences bucket in the catch as well. A potentially nicer alternative would be to wrap ProjectCreator#createNewProject's implementation in a "catch all" try-catch (and perform project repo/shared prefs clean up in the catch). Then we could remove the try-catch at QrCodeProjectCreatorDialog#createProjectOrError and just show an error when createNewProject returns false (which simplifies our UI code).

[grzesiek2010]

A potentially nicer alternative would be to wrap ProjectCreator#createNewProject's implementation in a "catch all" try-catch (and perform project repo/shared prefs clean up in the catch). Then we could remove the try-catch at QrCodeProjectCreatorDialog#createProjectOrError and just show an error when createNewProject returns false (which simplifies our UI code).

I already removed try-catch from QrCodeProjectCreatorDialog#createProjectOrError in this pr.
I think we should rather use try-catch in ODKAppSettingsImporter#fromJSON because this methods is the only place where SettingsImporter.fromJSON (the risky one) is called but ODKAppSettingsImporter#fromJSON itself is called from different places so i think it should catch exceptions and return false.

Then in ProjectCreator#createNewProject we can delete project / clear prefs if the returned value is false.

What do you think?

[seadowg]

@grzesiek2010 ah sorry I just realized I was looking at the current master. Yeah, that sounds good!

[grzesiek2010]

@seadowg new changes have been added. Please review.