Skip to content

Add certificate callback. #538

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Add certificate callback. #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b452b2
Add certificate callback.
alexchamberlain Jun 19, 2015
d40cd5f
Add basic test.
alexchamberlain Jun 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion pygit2/remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def __init__(self, tp):
self.received_bytes = tp.received_bytes
""""Number of bytes received up to now"""


class Remote(object):

def sideband_progress(self, string):
Expand Down Expand Up @@ -192,6 +191,7 @@ def fetch(self, signature=None, message=None):
callbacks.transfer_progress = self._transfer_progress_cb
callbacks.update_tips = self._update_tips_cb
callbacks.credentials = self._credentials_cb
callbacks.certificate_check = self._certificate_cb
# We need to make sure that this handle stays alive
self._self_handle = ffi.new_handle(self)
callbacks.payload = self._self_handle
Expand Down Expand Up @@ -304,6 +304,7 @@ def push(self, specs, signature=None, message=None):
callbacks.transfer_progress = self._transfer_progress_cb
callbacks.update_tips = self._update_tips_cb
callbacks.credentials = self._credentials_cb
callbacks.certificate_check = self._certificate_cb
callbacks.push_update_reference = self._push_update_reference_cb
# We need to make sure that this handle stays alive
self._self_handle = ffi.new_handle(self)
Expand Down Expand Up @@ -414,6 +415,25 @@ def _credentials_cb(cred_out, url, username, allowed, data):

return 0

@ffi.callback('int (*git_transport_certificate_check_cb)'
'(git_cert *cert, int valid, const char *host, void *payload)')
def _certificate_cb(cert_i, valid, host, data):
self = ffi.from_handle(data)

if not hasattr(self, 'certificate_check') or not self.certificate_check:
return 0

try:
val = self.certificate_cb(None, bool(valid), ffi.string(host))
if not val:
return C.GIT_ECERTIFICATE
except Exception as e:
self._stored_exception = e
return C.GIT_EUSER

return 0



def get_credentials(fn, url, username, allowed):
"""Call fn and return the credentials object"""
Expand Down
10 changes: 10 additions & 0 deletions test/test_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,16 @@ def test_fetch(self):
self.assertEqual(stats.indexed_objects, REMOTE_REPO_OBJECTS)
self.assertEqual(stats.received_objects, REMOTE_REPO_OBJECTS)

def test_fetch_insecure(self):
def no_check(certificate, valid, host):
return True
remote = self.repo.remotes[0]
remote.certificate_check = no_check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm this isn't being called, as the remote isn't https, but there don't appear to be any https tests?

Copy link
Member

Choose a reason for hiding this comment

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

There's a few remotes which do use HTTPS, but this test needs to make sure it's using HTTPS, not assume that some unspecified remote is going to be that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry; missed this update. Any idea which test case uses HTTPS?

Copy link
Member

Choose a reason for hiding this comment

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

Most should get redirected automatically through GitHub, though some credential tests use it explicitly. That shouldn't matter too much. If you need HTTPS in your test, use a HTTPS url, you're creating a new test, you don't need to use an existing url or remote.

stats = remote.fetch()
self.assertEqual(stats.received_bytes, REMOTE_REPO_BYTES)
self.assertEqual(stats.indexed_objects, REMOTE_REPO_OBJECTS)
self.assertEqual(stats.received_objects, REMOTE_REPO_OBJECTS)

def test_transfer_progress(self):
self.tp = None
def tp_cb(stats):
Expand Down