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.

Sign up for GitHub

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

Jump to bottom

[NEW] Support client certificates for SSL (two-way authentication) #1125

Merged
merged 22 commits into from
Sep 2, 2019
Merged

[NEW] Support client certificates for SSL (two-way authentication) #1125

merged 22 commits into from
Sep 2, 2019

Conversation

djorkaeffalexandre
Copy link
Collaborator

@djorkaeffalexandre djorkaeffalexandre commented Aug 9, 2019 •
edited
Loading

@RocketChat/ReactNative

This pull-request adds the ability to connect in servers that uses two-way SSL connections.
We need to make possible to pin certificate on App and use it on WebSocket & Rest API.
Here you can read about SSL Pinning: iOS SSL Pinning
Here we have an issue about that on Rocket.Chat.iOS: RocketChat/Rocket.Chat.iOS#708

  • RestAPI working with SSL Pinning.
  • WebSocket working with SSL Pinning.
  • XHR working with SSL Pinning.
  • Save certificate on App Dir.
  • Can choose certificate on new server.
  • Set certificate to server when login.
  • Remove certificate when logout.

Closes #1100

@djorkaeffalexandre djorkaeffalexandre self-assigned this Aug 10, 2019
@djorkaeffalexandre djorkaeffalexandre changed the title [WIP] Support client certificates for SSL (WebSocket & REST) [FEATURE] Support client certificates for SSL (WebSocket & REST) Aug 12, 2019
@djorkaeffalexandre djorkaeffalexandre marked this pull request as ready for review August 12, 2019 23:47
@lorek123
Copy link

What if user by mistake removes the certificate from DocumentDir? Couldn't we use app cache to store it?

@djorkaeffalexandre
Copy link
Collaborator Author

@lorek123 the documents dir is the application dir, not documents folder from iOS.

IlarionHalushka
IlarionHalushka suggested changes Aug 15, 2019
View reviewed changes
Copy link
Contributor

@IlarionHalushka IlarionHalushka left a comment

Choose a reason for hiding this comment

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

To tell you the truth I didn't understand much what is going on in this PR but from me a couple of code style comments.

diegolmello
diegolmello requested changes Aug 19, 2019
View reviewed changes
@diegolmello diegolmello changed the title [FEATURE] Support client certificates for SSL (WebSocket & REST) [NEW] Support client certificates for SSL (WebSocket & REST) Aug 23, 2019
@diegolmello diegolmello changed the title [NEW] Support client certificates for SSL (WebSocket & REST) [NEW] Support client certificates for SSL (two-way authentication) Sep 2, 2019
@diegolmello diegolmello merged commit 2515295 into develop Sep 2, 2019
@diegolmello diegolmello deleted the feature.ssl-pinning branch September 2, 2019 16:19
@lgtm-com
Copy link

lgtm-com bot commented Sep 2, 2019

This pull request introduces 2 alerts when merging d972c7e into 48cdd3b - view on LGTM.com

new alerts:

  • 2 for Superfluous trailing arguments

@almandin almandin mentioned this pull request Feb 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegolmello diegolmello diegolmello approved these changes

@IlarionHalushka IlarionHalushka Awaiting requested review from IlarionHalushka

Assignees

@djorkaeffalexandre djorkaeffalexandre

Labels
🎉 feature 👀 needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[NEW] Support client certificates for SSL (WebSocket & REST)
4 participants
@djorkaeffalexandre @lorek123 @diegolmello @IlarionHalushka