Skip to content

Some changes to allow commection with rsa keys authentication#6

Open
AdriwanKenoby wants to merge 3 commits intosteevanb:masterfrom
AdriwanKenoby:master
Open

Some changes to allow commection with rsa keys authentication#6
AdriwanKenoby wants to merge 3 commits intosteevanb:masterfrom
AdriwanKenoby:master

Conversation

@AdriwanKenoby
Copy link

@AdriwanKenoby AdriwanKenoby commented Mar 2, 2018

Hi @steevanb,

I have made some changes

  • rsa key auth on Profile
  • throw error on ssh command error
  • change constructor for dynamic profile

@@ -1,127 +0,0 @@
<?php

Choose a reason for hiding this comment

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

Do not move this file from Entity to Model. Model is usefull when you need to extend it, but in this case, it's a BC and we don't need this feature

Copy link
Author

Choose a reason for hiding this comment

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

What does mean "BC" acronym ? I think it is not an entity as an entity is persisted in DB then I have moved it to Model.

@@ -1,28 +0,0 @@
<?php

Choose a reason for hiding this comment

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

I want to keep this class: if i need to throw an exception who is not a ConnectionException, i will use this one

Copy link
Author

Choose a reason for hiding this comment

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

Maybe better to create an Interface Exception

/**
* Service for SSH2 connections
*/
class ConnectionFactory

Choose a reason for hiding this comment

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

Pattern Factory is a good one, but when there is just 2 lines of code, not sure it's usefull to create a service just for it.

Copy link
Author

Choose a reason for hiding this comment

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

You did the same, I just move the Service\Connection.php class to Factory\ConnectionFactory.php

$this->state = self::STATE_INVALID_ADDRESS;
} else {
$auth = @ssh2_auth_password($this->connection, $profile->getLogin(), $profile->getPassword());
$auth = @ssh2_auth_pubkey_file($this->connection, $profile->getLogin(), $profile->getRsaPubKey(), $profile->getRsaPemKey(), $profile->getPassphrase());

Choose a reason for hiding this comment

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

Maybe do it only when a RsaPubKey is specified, instead of try to connect with an empty Rsa, then connect without Rsa ?

* @return string
* @param $command
* @return bool|string
* @throws ConnectionException

Choose a reason for hiding this comment

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

i don't add this PHPDoc, always wrong :)

* @param string $separator Separator
* @param mixed $default Default value if $index is not found
* @return string
* @throws ConnectionException

Choose a reason for hiding this comment

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

I don't add this PHPDoc

*
* @param type $command
* @return array
* @throws ConnectionException

Choose a reason for hiding this comment

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

I don't add this PHPDoc

* @param int $index Index of the line to return
* @param string $default Default value
* @return string
* @throws ConnectionException

Choose a reason for hiding this comment

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

I don't add this PHPDoc

*
* @return string
*/
public function getAddress() : string

Choose a reason for hiding this comment

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

this lib is for PHP 5, so we can't use PHP 7 syntax :(

Copy link
Author

Choose a reason for hiding this comment

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

Ok i will remove type

# only if XX-ssh2.ini doesn't exists in /etc/php5/cli/conf.d/
cp /etc/php5/conf.d/ssh2.ini /etc/php5/cli/conf.d/
service apache2 restart
apt-get install libssh2-1

Choose a reason for hiding this comment

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

Ok to add this documentation, but keep old one : we need it on some servers

@AdriwanKenoby
Copy link
Author

I will work on it and I would provide some phpUnit test too maybe a .travis.yml

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants