-
Notifications
You must be signed in to change notification settings - Fork 259
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
[WIP] Implement end-to-end encryption #225
base: develop
Are you sure you want to change the base?
Conversation
'axolotl-crypto': 'libs/axolotl/axolotl-crypto', | ||
'axolotlCryptoCurve25519': 'libs/axolotl/axolotl-crypto-curve25519', | ||
'ByteBuffer': 'libs/axolotl/ByteBufferAB.min', | ||
'Long': 'libs/axolotl/Long.min', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these libraries depend on each other? If they do make sure to add those dependencies below, to avoid possible load order errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libaxolotl-javascript
has support for require.js
, but defining the dependencies explicitly here won't hurt...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better not to have the dependencies, if the libs handle that itself correctly and it gets picket up right by r.js.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Nice! In addition to the inline comments i suggest to name things End2EndEncryption rather than just Encryption. Also can you check how to make it possible to also encrypt data sent to multiple receivers (when processed by the broadcast server function)? |
- Show own fingerprint in settings. - Show notification in chat while peer identity is requested. - Show lock icon on encrypted chat messages. - Show button in chat menu to open dialog with fingerprint of remote peer. Needs rebuilding of styles after merging.
@@ -349,6 +362,15 @@ | |||
width: 12px; | |||
} | |||
|
|||
&:after { | |||
font-family: FontAwesome; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Properties should be ordered float, font-family, left, margin-left, text-align, width
@@ -329,6 +356,14 @@ define(['jquery', 'underscore', 'text!partials/chat.html', 'text!partials/chatro | |||
console.error("Failed to receive geolocation", err); | |||
}); | |||
}; | |||
subscope.showFingerprint = function() { | |||
if (subscope.fingerprint) { | |||
// TODO(fancycode): Show a nicer notification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be a nicer notification? I guess it should avoid a modal popup and rather show this somewhere inside the chat frame.
With that also get rid of the "Api.send2" and "Api.apply" hacks. Now the respective methods take an optional function to call for sending the data as first argument. No more magic object binding, yay!
_.defer(_.bind(function() { | ||
var ready_state = READY_NONE; | ||
var check_ready = _.bind(function() { | ||
if (ready_state === READY_ALL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess it would be better to use $q.all([this.getLocalIdentityKeyPair(), this.getLocalRegistrationId(), this.getLastResortPreKey()]).then(function() {
here instead, so we can get rid of the bitflags – Much cleaner imo.
This is not ready for merging yet, but should work as base for further discussion.
Features:
libaxolotl-javascript
library).Missing stuff:
localStorage
).Feedback welcome!