mattermost-push-proxy icon indicating copy to clipboard operation
mattermost-push-proxy copied to clipboard

Add support for Apple APNS AuthKey

Open enahum opened this issue 3 years ago • 2 comments

Summary

This PR adds support for Apple AuthKey with this we will be using the same key for all Apple topics (one per-app) which means that the same key will WORK with every app we ship, also this removes the need to having to renew certificates every year. Also for customers the setup for APN is going to be much easier than the current certificate conversion to PEM.

Ticket Link

N/A

enahum avatar Apr 14 '22 16:04 enahum

Unit Test Results

  1 files  ±0    5 suites  ±0   16s :stopwatch: ±0s 18 tests ±0  18 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit 58956563. ± Comparison against base commit be6dd111.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 14 '22 16:04 github-actions[bot]

@spirosoik friendly reminder to review

enahum avatar Apr 29 '22 23:04 enahum

@spirosoik @stylianosrigas the current apple certificate will expire in the next 30 days, this PR has been sitting here for a long while now, I'll be back from PTO on the 14th of April, can we coordinate so that this PR gets merged and deployed to our HPNS and TPNS as well as start using the API keys instead of the certs. The benefit of doing that is that we won't need to deploy updated certs every year.

The one downside would be that we would need to rotate the API in case there is a security breach of some sort.

Note: The previous method of using the cert is still valid after this PR gets merged and deployed

enahum avatar Apr 08 '23 16:04 enahum

@enahum Sounds good to me. Once we merge and cut a release, SRE Team can work on updating the Push Proxy helm chart and deploy where needed in cloud.

stylianosrigas avatar Apr 10 '23 06:04 stylianosrigas

@enahum is security team aware for this change?

cc @iyampaul @DSchalla

spirosoik avatar Apr 10 '23 07:04 spirosoik

@enahum is security team aware for this change?

cc @iyampaul @DSchalla

News to me, but I have no issues with this change 👍

iyampaul avatar Apr 10 '23 15:04 iyampaul