Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to enable SSL pinning on mobile #1389

Merged
merged 5 commits into from
Jul 23, 2024
Merged

How to enable SSL pinning on mobile #1389

merged 5 commits into from
Jul 23, 2024

Conversation

enahum
Copy link
Contributor

@enahum enahum commented Jul 4, 2024

Summary

Enable SSL Pinning on the mobile apps when you build your own.

Do not merge until this functionality is released.
Related PR's:
mattermost/react-native-network-client#129
mattermost/mattermost-mobile#8055

Ticket Link

https://mattermost.atlassian.net/browse/MM-59055

@enahum enahum added Do Not Merge Should not be merged until this label is removed 1: Dev Review Requires review by a core commiter 2: Editor Review Requires review by an editor labels Jul 4, 2024
Copy link

github-actions bot commented Jul 4, 2024

Newest code from enahum has been published to preview environment for Git SHA 62a4d2d

Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just one typo

Co-authored-by: Joram Wilander <jwawilander@gmail.com>
Copy link

github-actions bot commented Jul 5, 2024

Newest code from enahum has been published to preview environment for Git SHA 3316760

Copy link
Member

@cwarnermm cwarnermm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent update. Thanks, @enahum!

@cwarnermm cwarnermm removed the 2: Editor Review Requires review by an editor label Jul 8, 2024
Copy link
Member

@coltoneshaw coltoneshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Only feedback would be really underpinning the warning there of requiring coordination between teams.

@enahum
Copy link
Contributor Author

enahum commented Jul 8, 2024

@cwarnermm any idea how to achieve Colton's suggestion ?

@cwarnermm
Copy link
Member

cwarnermm commented Jul 9, 2024

@enahum @coltoneshaw - My latest commit adds an Important Note directly after the disadvantages list:
Screenshot 2024-07-09 at 3 45 37 PM

Feel free to suggest changes directly in this PR if needed.

Copy link

github-actions bot commented Jul 9, 2024

Newest code from cwarnermm has been published to preview environment for Git SHA 4178e6d

Copy link

Newest code from cwarnermm has been published to preview environment for Git SHA b855d0d

@enahum enahum requested a review from sbishel July 15, 2024 23:51
Copy link
Member

@sbishel sbishel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, now I know what SSL Pinning is...

@enahum enahum removed the Do Not Merge Should not be merged until this label is removed label Jul 20, 2024
@enahum enahum added 4: Reviews Complete All reviewers have approved the pull request and removed 1: Dev Review Requires review by a core commiter labels Jul 23, 2024
Copy link

Newest code from enahum has been published to preview environment for Git SHA cbabf64

@enahum enahum merged commit 8922918 into master Jul 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants