Skip to content

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

Merged
diegolmello merged 22 commits into
developfrom
feature.ssl-pinning
Sep 2, 2019
Merged

diegolmello merged 22 commits into
developfrom
feature.ssl-pinning

Conversation

@djorkaeffalexandre

@djorkaeffalexandre djorkaeffalexandre commented Aug 9, 2019 •

Copy link
Copy Markdown
Contributor

@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
Copy Markdown

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

@djorkaeffalexandre

Copy link
Copy Markdown
Contributor Author

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

@IlarionHalushka IlarionHalushka left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread app/views/NewServerView.js Outdated
Comment thread app/views/NewServerView.js Outdated
Comment thread app/utils/server.js
Comment thread app/views/NewServerView.js Outdated
Comment thread app/views/NewServerView.js Outdated
@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

lgtm-com Bot commented Sep 2, 2019

Copy link
Copy Markdown

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

new alerts:

  • 2 for Superfluous trailing arguments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NEW] Support client certificates for SSL (WebSocket & REST)

4 participants