Review Request 123593: [WIP] Refactored code to implement ssl and provide backward compatibility

[Albert Vaca Cintora]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123593/#review79824
-----------------------------------------------------------


I like to see how Device and NetworkPackage lost some weight :) There are some issues with your approach, though, so you will need to make some changes. To start with, we need to make the sftp plugin to work. As Aleix said we can't have regresions in the master branch: "master" == "stable" in most KDE projects. If it needs to use a private/public key and can't get it from the Device anymore, maybe the plugin can create its own key and send it to the other device when it connects?


src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java (line 78)
<https://git.reviewboard.kde.org/r/123593/#comment54671>

    Refactor?



src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java (line 109)
<https://git.reviewboard.kde.org/r/123593/#comment54670>

    The loopback link doesn't need encryption. Simplify it by removing the encryption code.



src/org/kde/kdeconnect/Device.java (lines 75 - 81)
<https://git.reviewboard.kde.org/r/123593/#comment54668>

    Don't commit commented code.



src/org/kde/kdeconnect/Device.java (line 95)
<https://git.reviewboard.kde.org/r/123593/#comment54667>

    I don't think we want this "type". See below.



src/org/kde/kdeconnect/Device.java (lines 132 - 141)
<https://git.reviewboard.kde.org/r/123593/#comment54666>

    This does not allow us to have more than one link per device, please change it. The device should not know what types of link to use: it should try them all until one works.



src/org/kde/kdeconnect/Device.java (line 253)
<https://git.reviewboard.kde.org/r/123593/#comment54669>

    Right now the LinkProvider receives a pairing package, passes it to the Device, and the Device passes it back to the PairingHandler. What if the LinkProvider passes identity packages directly to the PairingHandler instead of the device? This way the Device won't need to know about the different PairingHandlers of the different LinkProviders. (Because every LinkProvider knows its own PairingHandler).


- Albert Vaca Cintora


On mai. 2, 2015, 6:31 a.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123593/
> -----------------------------------------------------------
> 
> (Updated mai. 2, 2015, 6:31 a.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-android
> 
> 
> Description
> -------
> 
> * Moved private key and public key from Device to Link
> * Encrypt and decrypt functions moved from NetworkPackage class to LanLink class
> * Added a new ParingHandler class so that each type of link can handle its own pairing process
> * Added a field of "type" in device so that we can differentiate between devices based on types
> * Now device need to send which type of link they will prefer in identity package, if nothing is send, the type is considered "normal". No change is done in createIdentityPackage now.
> 
> ISSUES
> * SFTP plugin is not working now, because it uses public key from device which is not present now
> * There is an instance of Public Key in pairing handler also, which needs to be removed, this instance is only used to save public key when pairing is done
> 
> 
> Diffs
> -----
> 
>   src/org/kde/kdeconnect/Backends/BaseLink.java 579a7af 
>   src/org/kde/kdeconnect/Backends/BaseLinkProvider.java cfaf621 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java 5994142 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java ae26958 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java add92f8 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLinkProvider.java bd9c41b 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/PairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/BackgroundService.java 855a52d 
>   src/org/kde/kdeconnect/Device.java 9fe583c 
>   src/org/kde/kdeconnect/NetworkPackage.java e5a777e 
>   src/org/kde/kdeconnect/Plugins/SftpPlugin/SftpImpl.java ec41060 
>   src/org/kde/kdeconnect/Plugins/SftpPlugin/SftpPlugin.java af7c66d 
>   src/org/kde/kdeconnect/UserInterface/PairActivity.java 7a45751 
> 
> Diff: https://git.reviewboard.kde.org/r/123593/diff/
> 
> 
> Testing
> -------
> 
> Testing done with pairing and unpairing both pc and mobile, working fine
> Tested using Ping plugin and Touchpad plugin, working fine.
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20150504/6cc3c55f/attachment-0001.html>