Apple Pass integration

[openprojects]

Original ticket: https://rtce-jira.unog.ch/browse/IND2-3018

[openprojects]

Hi @ThiefMaster , can you please stop reviewing this PR? It is marked in DRAFT and I explicitly required @OmeGak to review it.
Thank you.

[ThiefMaster]

Don't worry, I did not plan to go more in-depth, but those are things I noticed immediately while skimming over it.

[OmeGak]

Towards the end of my review I realized that Apple has stopped publicly using the term "Apple Pass" in favor of "Pass for Apple Wallet". I would suggest that we stick to this name for all variable and setting names in the code (e.g. apple_wallet). The benefit is that both Google and Apple wallet related code will share the _wallet suffix, making it more explicit that they are very closely related.

Besides this, please note that there a couple of issues with header checks.

[openprojects]

Towards the end of my review I realized that Apple has stopped publicly using the term "Apple Pass" in favor of "Pass for Apple Wallet". I would suggest that we stick to this name for all variable and setting names in the code (e.g. apple_wallet). The benefit is that both Google and Apple wallet related code will share the _wallet suffix, making it more explicit that they are very closely related.

Besides this, please note that there a couple of issues with header checks.

I did a quick search about that, and it looks to me that the name Pass still indicates the object created, and Wallet the software to keep/display the passes.
What we are generating in Indico is a Pass, not a Wallet:

https://developer.apple.com/documentation/walletpasses

[ThiefMaster]

I did a quick search about that, and it looks to me that the name Pass still indicates the object created, and Wallet the software to keep/display the passes.
What we are generating in Indico is a Pass, not a Wallet

I think there's still lots of value in using similar terminology in the codebase, ie google_wallet_whatever and apple_wallet_whatever. That way one can easily search for _wallet_whatever to find code that's likely applicable for both places (but too different to be reused).

[openprojects]

I'll squash all the single commits AFTER the whole review process is completed.

[ThiefMaster]

No need to squash since you do not have merge commits int here :) We'll squash+merge at the end anyway (same like in the google wallet PR and most other PRs), so the individual commits in the PR are perfectly fine to leave around.

[OmeGak]

Following @ThiefMaster, let's make sure to rename all apple_pass_whatever variable names to `apple_wallet_whatever.

[openprojects]

Following @ThiefMaster, let's make sure to rename all apple_pass_whatever variable names to `apple_wallet_whatever.

Done.

[bpedersen2]

Just as a hint: pkpass support is not restricted to Apple, on Android e.g.the walletpasses app (check walletpasses.io) happily ingests them as well. So when presenting things to users, keep that in mind.

[OmeGak]

@ThiefMaster, feel free to go ahead with your review. Most of my comments were already resolved. Some of the outstanding ones would help your feedback still.

[bpedersen2]

I am a bit unsure if storing signing keys and their password in the same database is something that is good security practice. I would have made those per-instance fields that are configure in the config file only.

[ThiefMaster]

This is just Apple being Apple. The fact that those passbook files even NEED to be signed is kind of ridiculous. And we want to have this configurable by category, and requiring this go go through indico.conf would not scale.

FWIW, the cleanest option would be some kind of "set credentials" UI that would be write-only, instead of integrating this in the general settings form. Might still be a good idea at a later point (for both the Google and Apple credentials). But overkill for the initial version (even more so while this is gated behind a feature flag in indico.conf).