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.

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

Add httpapi password change driver to connect to generic HTTP/HTTPS APIs #7439

Merged
merged 1 commit into from
Jan 16, 2021

Conversation

davidc
Copy link
Contributor

@davidc davidc commented Jun 16, 2020

No description provided.

Copy link
Member

@alecpl alecpl left a comment

Choose a reason for hiding this comment

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

I think that usefulness of this is very limited because of the lack of authentication. Is there any API that works this way? Could you add that?

* @param string $curpass Current password
* @param string $newpass New password
* @param string $username Login username
* @returns int PASSWORD_SUCCESS|PASSWORD_ERROR|PASSWORD_CONNECT_ERROR
Copy link
Member

Choose a reason for hiding this comment

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

It should be @return.


$curl = curl_init();

curl_setopt($curl, CURLOPT_CONNECTTIMEOUT , 5);
Copy link
Member

Choose a reason for hiding this comment

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

Spacing incorrect.

if ($ssl_verify) {
curl_setopt($curl, CURLOPT_SSL_VERIFYHOST , 2);
curl_setopt($curl, CURLOPT_SSL_VERIFYPEER , true);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

We prefer else in a new line.

}
} else {
rcube::raise_error(array(
'code' => 600,
Copy link
Member

Choose a reason for hiding this comment

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

Again, not our style. It should be +8 spaces from the rcube:: above.

return PASSWORD_CONNECT_ERROR;
}

curl_setopt($curl, CURLOPT_URL , $url);
Copy link
Member

Choose a reason for hiding this comment

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

Spacing.

@@ -49,6 +49,7 @@
2.1.21. Kpasswd
2.1.22. Modoboa
2.1.23. LDAP - Password Modify Extended Operation (ldap_exop)
2.1.24. HTTP-API
Copy link
Member

Choose a reason for hiding this comment

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

The driver name is httpapi so add (httpapi) in the section title.

@davidc
Copy link
Contributor Author

davidc commented Jun 21, 2020

Hi, thanks for these tips. The coding style is not mine, I tried to follow the style of the plesk driver I used as a template, but now realise that file wasn't actually in your own code style. I will correct these.

The reason for writing this was to write password changes to a simple API on our backend, but I decided to do it in a generic manner so it might be useful to other service providers, rather than requiring many similar or custom drivers.

The (simple) API on our end just verifies the host is the webmail server, and as it already checks that the user's current password is correct, this was thought sufficient in terms of authentication.

I will add in the ability to set custom headers and custom GET/POST variables (which could then be used for further authentication/API keys) in the hope that it is more universally useful.

@dereks
Copy link

dereks commented Aug 20, 2020

Thank you for submitting a Pull Request (PR) to the Roundcube GitHub project.

You are receiving this message because your PR has conflicts which need to be resolved. We are trying to catch up on our backlog of old PRs and get them merged in (where appropriate). Therefor, we request the following:

Step 1. Rebase from the latest Roundcube branch master into your PR.

git fetch upstream
git checkout your_feature_branch_name
git rebase upstream/master
git push -f origin your_feature_branch_name

Step 2. Re-test to make sure your new code still works as expected

Step 3. Comment back here once it has been tested and will merge cleanly.

Once this has been done we will treat it like a new Pull Request and consider it for acceptance.

Apologies for the inconvenience. Thank you for contributing to Roundcube!

@alecpl
Copy link
Member

alecpl commented Dec 21, 2020

Any progress with this? Since Roundcube uses Guzzle HTTP Client it would be nice to use it here too.

@alecpl alecpl merged commit a49b47b into roundcube:master Jan 16, 2021
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.

3 participants