Add FXIOS-14901 [Summarizer] Add support for dynamic bundle identifiers#97
Add FXIOS-14901 [Summarizer] Add support for dynamic bundle identifiers#97
Conversation
| challenge: bytes, | ||
| attestation_obj: bytes, | ||
| use_qa_certificates: bool, | ||
| bundle_id: str, |
There was a problem hiding this comment.
Thanks a lot for PR and for reopening it internally ❤️
As soon as we merge it the requests without bundle_id will start failing. Is it fine on your side? Should me maintain backward compatibility and have a fallback?
There was a problem hiding this comment.
Yes this fine to make it required. We haven't shipped any code yet that is running on user devices that makes requests to MLPA.
| key_id_b64: str | ||
| challenge_b64: str | ||
| attestation_obj_b64: str | ||
| bundle_id: str |
There was a problem hiding this comment.
Question:
Do we have some superset of these bundle_ids or they are can have some unlimited variations?
Because I was thinking if there are 2-3 possible values we could add some validation.
There was a problem hiding this comment.
We have the standard ones org.mozilla.ios.Fennec, org.mozilla.ios.FirefoxBeta, org.mozilla.ios.Firefox. But we have some non used ones and we can always have new ones. I am fine of adding these for now as a start and if we ever have new ones we can deploy again.
| @@ -0,0 +1,44 @@ | |||
| import base64 | |||
There was a problem hiding this comment.
Nitpick:
This could be a file under src/tests/integration/appattest_helpers.py 🙌
subpath
left a comment
There was a problem hiding this comment.
I've realized that we are having outdated swagger documentations, I will update it in the following PRs, so we could see the required fields here: https://firefox-ai.github.io/MLPA/#tag/App-Attest/operation/attest_verify_attest_post
Thanks again : )
📜 Tickets
Jira ticket
Github issue
💡 Description
This PR:
Fennec,FirefoxBetaandFirefox.Right now, we hardcode the bundle identifier via an env that defauts to
org.mozilla.ios.Fennec. This means verification will pass on local builds but not on production builds.