From 82d88191bb450a153a217ed8227dd1865c48016d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 26 Mar 2014 20:23:04 +0100 Subject: [PATCH] credentials: use more ducks Instead of making everyone inherit from our credential types, use an interface with two attributes, which makes the C code much shorter and simpler. --- pygit2/credentials.py | 33 +++++- src/credentials.c | 225 --------------------------------------- src/pygit2.c | 6 -- src/remote.c | 2 - src/types.h | 19 ---- src/utils.c | 52 ++++++--- test/test_credentials.py | 15 +-- 7 files changed, 69 insertions(+), 283 deletions(-) delete mode 100644 src/credentials.c diff --git a/pygit2/credentials.py b/pygit2/credentials.py index dad966b..1789022 100644 --- a/pygit2/credentials.py +++ b/pygit2/credentials.py @@ -26,10 +26,9 @@ # Boston, MA 02110-1301, USA. # Import from pygit2 -from _pygit2 import CredUsernamePassword, CredSshKey -from _pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT +from _pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT, GIT_CREDTYPE_SSH_KEY -class UserPass(CredUsernamePassword): +class UserPass: """Username/Password credentials This is an object suitable for passing to a remote's credentials @@ -37,10 +36,22 @@ class UserPass(CredUsernamePassword): """ + def __init__(self, username, password): + self._username = username + self._password = password + + @property + def credential_type(self): + return GIT_CREDTYPE_USERPASS_PLAINTEXT + + @property + def credential_tuple(self): + return (self._username, self._password) + def __call__(self, _url, _username, _allowed): return self -class Keypair(CredSshKey): +class Keypair: """SSH key pair credentials This is an object suitable for passing to a remote's credentials @@ -48,5 +59,19 @@ class Keypair(CredSshKey): """ + def __init__(self, username, pubkey, privkey, passphrase): + self._username = username + self._pubkey = pubkey + self._privkey = privkey + self._passphrase = passphrase + + @property + def credential_type(self): + return GIT_CREDTYPE_SSH_KEY + + @property + def credential_tuple(self): + return (self._username, self._pubkey, self._privkey, self._passphrase) + def __call__(self, _url, _username, _allowed): return self diff --git a/src/credentials.c b/src/credentials.c deleted file mode 100644 index c3127ae..0000000 --- a/src/credentials.c +++ /dev/null @@ -1,225 +0,0 @@ -/* - * Copyright 2010-2014 The pygit2 contributors - * - * This file is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License, version 2, - * as published by the Free Software Foundation. - * - * In addition to the permissions in the GNU General Public License, - * the authors give you unlimited permission to link the compiled - * version of this file into combinations with other programs, - * and to distribute those combinations without any restriction - * coming from the use of this file. (The General Public License - * restrictions do apply in other respects; for example, they cover - * modification of the file, and distribution when not linked into - * a combined executable.) - * - * This file is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; see the file COPYING. If not, write to - * the Free Software Foundation, 51 Franklin Street, Fifth Floor, - * Boston, MA 02110-1301, USA. - */ - -#define PY_SSIZE_T_CLEAN -#include -#include -#include "error.h" -#include "types.h" -#include "utils.h" -#include "oid.h" -#include "refspec.h" -#include "remote.h" - -int -CredUsernamePassword_init(CredUsernamePassword *self, PyObject *args, PyObject *kwds) -{ - char *username, *password; - - if (kwds && PyDict_Size(kwds) > 0) { - PyErr_SetString(PyExc_TypeError, "CredUsernamePassword takes no keyword arguments"); - return -1; - } - - if (!PyArg_ParseTuple(args, "ss", &username, &password)) - return -1; - - self->parent.credtype = GIT_CREDTYPE_USERPASS_PLAINTEXT; - - self->username = strdup(username); - if (!self->username) { - PyErr_NoMemory(); - return -1; - } - - self->password = strdup(password); - if (!self->password) { - free(self->username); - PyErr_NoMemory(); - return -1; - } - - return 0; -} - -void -CredUsernamePassword_dealloc(CredUsernamePassword *self) -{ - free(self->username); - free(self->password); - - Py_TYPE(self)->tp_free(self); -} - -PyMemberDef CredUsernamePassword_members[] = { - MEMBER(CredUsernamePassword, username, T_STRING, "username"), - MEMBER(CredUsernamePassword, password, T_STRING, "password"), - {NULL}, -}; - -PyDoc_STRVAR(CredUsernamePassword__doc__, - "Credential type for username/password combination"); - -PyTypeObject CredUsernamePasswordType = { - PyVarObject_HEAD_INIT(NULL, 0) - "_pygit2.CredUsernamePassword", /* tp_name */ - sizeof(CredUsernamePassword), /* tp_basicsize */ - 0, /* tp_itemsize */ - (destructor)CredUsernamePassword_dealloc, /* tp_dealloc */ - 0, /* tp_print */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_compare */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - 0, /* tp_call */ - 0, /* tp_str */ - 0, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ - CredUsernamePassword__doc__, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - 0, /* tp_methods */ - CredUsernamePassword_members, /* tp_members */ - 0, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - (initproc)CredUsernamePassword_init, /* tp_init */ - 0, /* tp_alloc */ - 0, /* tp_new */ -}; - -int -CredSshKey_init(CredSshKey *self, PyObject *args, PyObject *kwds) -{ - char *username, *pubkey, *privkey, *passphrase; - - if (kwds && PyDict_Size(kwds) > 0) { - PyErr_SetString(PyExc_TypeError, "CredSshKey takes no keyword arguments"); - return -1; - } - - if (!PyArg_ParseTuple(args, "ssss", &username, &pubkey, - &privkey, &passphrase)) - return -1; - - self->parent.credtype = GIT_CREDTYPE_SSH_KEY; - self->username = self->pubkey = self->privkey = self->passphrase = NULL; - - self->username = strdup(username); - self->pubkey = strdup(pubkey); - self->privkey = strdup(privkey); - self->passphrase = strdup(passphrase); - - if (!(self->username && self->pubkey && self->privkey && self->passphrase)) - goto on_oom; - - return 0; - - on_oom: - free(self->username); - free(self->pubkey); - free(self->privkey); - free(self->passphrase); - PyErr_NoMemory(); - return -1; -} - -void -CredSshKey_dealloc(CredSshKey *self) -{ - free(self->username); - free(self->pubkey); - free(self->privkey); - free(self->passphrase); - - Py_TYPE(self)->tp_free(self); -} - -PyMemberDef CredSshKey_members[] = { - MEMBER(CredSshKey, username, T_STRING, "username"), - MEMBER(CredSshKey, pubkey, T_STRING, "pubkey"), - MEMBER(CredSshKey, privkey, T_STRING, "privkey"), - MEMBER(CredSshKey, passphrase, T_STRING, "passphrase"), - {NULL}, -}; - -PyDoc_STRVAR(CredSshKey__doc__, - "Credential type for an SSH keypair"); - -PyTypeObject CredSshKeyType = { - PyVarObject_HEAD_INIT(NULL, 0) - "_pygit2.CredSshKey", /* tp_name */ - sizeof(CredSshKey), /* tp_basicsize */ - 0, /* tp_itemsize */ - (destructor)CredSshKey_dealloc, /* tp_dealloc */ - 0, /* tp_print */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_compare */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - 0, /* tp_call */ - 0, /* tp_str */ - 0, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ - CredSshKey__doc__, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - 0, /* tp_methods */ - CredSshKey_members, /* tp_members */ - 0, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - (initproc)CredSshKey_init, /* tp_init */ - 0, /* tp_alloc */ - 0, /* tp_new */ -}; diff --git a/src/pygit2.c b/src/pygit2.c index 199bbac..bda9545 100644 --- a/src/pygit2.c +++ b/src/pygit2.c @@ -74,8 +74,6 @@ extern PyTypeObject RefLogEntryType; extern PyTypeObject BranchType; extern PyTypeObject SignatureType; extern PyTypeObject RemoteType; -extern PyTypeObject CredUsernamePasswordType; -extern PyTypeObject CredSshKeyType; extern PyTypeObject RefspecType; extern PyTypeObject TransferProgressType; extern PyTypeObject NoteType; @@ -460,13 +458,9 @@ moduleinit(PyObject* m) /* Remotes */ INIT_TYPE(RemoteType, NULL, NULL) - INIT_TYPE(CredUsernamePasswordType, NULL, PyType_GenericNew) - INIT_TYPE(CredSshKeyType, NULL, PyType_GenericNew) INIT_TYPE(RefspecType, NULL, NULL) INIT_TYPE(TransferProgressType, NULL, NULL) ADD_TYPE(m, Remote) - ADD_TYPE(m, CredUsernamePassword) - ADD_TYPE(m, CredSshKey) ADD_TYPE(m, Refspec) ADD_TYPE(m, TransferProgress) /* Direction for the refspec */ diff --git a/src/remote.c b/src/remote.c index b654a8c..ffbe17d 100644 --- a/src/remote.c +++ b/src/remote.c @@ -39,8 +39,6 @@ extern PyObject *GitError; extern PyTypeObject RepositoryType; extern PyTypeObject TransferProgressType; -extern PyTypeObject CredUsernamePasswordType; -extern PyTypeObject CredSshKeyType; PyObject * wrap_transfer_progress(const git_transfer_progress *stats) diff --git a/src/types.h b/src/types.h index 1a73fc5..a36fad6 100644 --- a/src/types.h +++ b/src/types.h @@ -207,25 +207,6 @@ typedef struct { PyObject *update_tips; } Remote; -typedef struct { - PyObject_HEAD - git_credtype_t credtype; -} Cred; - -typedef struct { - Cred parent; - char *username; - char *password; -} CredUsernamePassword; - -typedef struct { - Cred parent; - char *username; - char *pubkey; - char *privkey; - char *passphrase; -} CredSshKey; - /* git_refspec */ typedef struct { PyObject_HEAD diff --git a/src/utils.c b/src/utils.c index 59e7ad5..24b6bbd 100644 --- a/src/utils.c +++ b/src/utils.c @@ -31,8 +31,6 @@ #include "utils.h" extern PyTypeObject ReferenceType; -extern PyTypeObject CredUsernamePasswordType; -extern PyTypeObject CredSshKeyType; /** * py_str_to_c_str() returns a newly allocated C string holding the string @@ -159,42 +157,62 @@ on_error: static int py_cred_to_git_cred(git_cred **out, PyObject *py_cred, unsigned int allowed) { - Cred *base_cred; - int err; + PyObject *py_type, *py_tuple; + long type; + int err = -1; - if (!PyObject_TypeCheck(py_cred, &CredUsernamePasswordType) && - !PyObject_TypeCheck(py_cred, &CredSshKeyType)) { - PyErr_SetString(PyExc_TypeError, "unkown credential type"); - return -1; + py_type = PyObject_GetAttrString(py_cred, "credential_type"); + py_tuple = PyObject_GetAttrString(py_cred, "credential_tuple"); + + if (!py_type || !py_tuple) { + printf("py_type %p, py_tuple %p\n", py_type, py_tuple); + PyErr_SetString(PyExc_TypeError, "credential doesn't implement the interface"); + goto cleanup; } - base_cred = (Cred *) py_cred; + if (!PyLong_Check(py_type)) { + PyErr_SetString(PyExc_TypeError, "credential type is not a long"); + goto cleanup; + } + + type = PyLong_AsLong(py_type); /* Sanity check, make sure we're given credentials we can use */ - if (!(allowed & base_cred->credtype)) { + if (!(allowed & type)) { PyErr_SetString(PyExc_TypeError, "invalid credential type"); - return -1; + goto cleanup; } - switch (base_cred->credtype) { + switch (type) { case GIT_CREDTYPE_USERPASS_PLAINTEXT: { - CredUsernamePassword *cred = (CredUsernamePassword *) base_cred; - err = git_cred_userpass_plaintext_new(out, cred->username, cred->password); + const char *username, *password; + + if (!PyArg_ParseTuple(py_tuple, "ss", &username, &password)) + goto cleanup; + + err = git_cred_userpass_plaintext_new(out, username, password); break; } case GIT_CREDTYPE_SSH_KEY: { - CredSshKey *cred = (CredSshKey *) base_cred; - err = git_cred_ssh_key_new(out, cred->username, cred->pubkey, cred->privkey, cred->passphrase); + const char *username, *pubkey, *privkey, *passphrase; + + if (!PyArg_ParseTuple(py_tuple, "ssss", &username, &pubkey, &privkey, &passphrase)) + goto cleanup; + + err = git_cred_ssh_key_new(out, username, pubkey, privkey, passphrase); break; } default: PyErr_SetString(PyExc_TypeError, "unsupported credential type"); - err = -1; break; } +cleanup: + Py_XDECREF(py_type); + Py_XDECREF(py_tuple); + return err; } diff --git a/test/test_credentials.py b/test/test_credentials.py index c1b95e5..3bdeb6f 100644 --- a/test/test_credentials.py +++ b/test/test_credentials.py @@ -30,7 +30,6 @@ import unittest import pygit2 -from pygit2 import CredUsernamePassword, CredSshKey from pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT from pygit2 import UserPass, Keypair from . import utils @@ -49,9 +48,8 @@ class CredentialCreateTest(utils.NoRepoTestCase): username = "git" password = "sekkrit" - cred = CredUsernamePassword(username, password) - self.assertEqual(username, cred.username) - self.assertEqual(password, cred.password) + cred = UserPass(username, password) + self.assertEqual((username, password), cred.credential_tuple) def test_ssh_key(self): username = "git" @@ -59,11 +57,8 @@ class CredentialCreateTest(utils.NoRepoTestCase): privkey = "id_rsa" passphrase = "bad wolf" - cred = CredSshKey(username, pubkey, privkey, passphrase) - self.assertEqual(username, cred.username) - self.assertEqual(pubkey, cred.pubkey) - self.assertEqual(privkey, cred.privkey) - self.assertEqual(passphrase, cred.passphrase) + cred = Keypair(username, pubkey, privkey, passphrase) + self.assertEqual((username, pubkey, privkey, passphrase), cred.credential_tuple) class CredentialCallback(utils.RepoTestCase): def test_callback(self): @@ -79,7 +74,7 @@ class CredentialCallback(utils.RepoTestCase): def test_bad_cred_type(self): def credentials_cb(url, username, allowed): self.assertTrue(allowed & GIT_CREDTYPE_USERPASS_PLAINTEXT) - return CredSshKey("git", "foo.pub", "foo", "sekkrit") + return Keypair("git", "foo.pub", "foo", "sekkrit") remote = self.repo.create_remote("github", "https://github.com/github/github") remote.credentials = credentials_cb