From 53f2a31547d161e9081daa5366ca8cae10013f70 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Fri, 27 Sep 2013 15:53:39 -0400 Subject: [PATCH 1/3] Fix some bugs with the permissions API. Prevent the user from removing themelves as admin. --- data/model.py | 56 ++++++++++++++++++++++++++++++++++-------------- endpoints/api.py | 20 +++++++++++------ 2 files changed, 54 insertions(+), 22 deletions(-) diff --git a/data/model.py b/data/model.py index 6e7b65e48..42dac1ea1 100644 --- a/data/model.py +++ b/data/model.py @@ -8,6 +8,10 @@ from database import * logger = logging.getLogger(__name__) +class DataModelException(Exception): + pass + + def create_user(username, password, email): pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) new_user = User.create(username=username, password_hash=pw_hash, @@ -130,7 +134,7 @@ def set_image_metadata(image_id, namespace_name, repository_name, Image.image_id == image_id)) if not image_list: - raise RuntimeError('No image with specified id and repository') + raise DataModelException('No image with specified id and repository') fetched = image_list[0] fetched.created = dateutil.parser.parse(created_date_str) @@ -160,7 +164,7 @@ def get_tag_image(namespace_name, repository_name, tag_name): RepositoryTag.name == tag_name)) if not fetched: - raise Image.DoesNotExist('Unable to find image for tag.') + raise DataModelException('Unable to find image for tag.') return fetched[0] @@ -202,23 +206,39 @@ def get_user_repo_permissions(user, repository): RepositoryPermission.repository == repository) -def get_user_reponame_permission(user_obj, namespace_name, repository_name): - repo = Repository.get(Repository.name == repository_name, - Repository.namespace == namespace_name) - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, - RepositoryPermission.repository == repo) - return perm +def user_permission_repo_query(username, namespace_name, repository_name): + selected = RepositoryPermission.select(User, Repository, Role, + RepositoryPermission) + with_user = selected.join(User) + with_role = with_user.switch(RepositoryPermission).join(Role) + with_repo = with_role.switch(RepositoryPermission).join(Repository) + return with_repo.where(Repository.name == repository_name, + Repository.namespace == namespace_name, + User.username == username) -def set_user_repo_permission(user_obj, namespace_name, repository_name, +def get_user_reponame_permission(username, namespace_name, repository_name): + fetched = list(user_permission_repo_query(username, namespace_name, + repository_name)) + if not fetched: + raise DataModelException('User does not have permission for repo.') + + return fetched[0] + + +def set_user_repo_permission(username, namespace_name, repository_name, role_name): + if username == namespace_name: + raise DataModelException('Namespace owner must always be admin.') + + user = User.get(User.username == username) repo = Repository.get(Repository.name == repository_name, Repository.namespace == namespace_name) new_role = Role.get(Role.name == role_name) # Fetch any existing permission for this user on the repo try: - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, + perm = RepositoryPermission.get(RepositoryPermission.user == user, RepositoryPermission.repository == repo) perm.role = new_role perm.save() @@ -228,9 +248,13 @@ def set_user_repo_permission(user_obj, namespace_name, repository_name, role=new_role) return new_perm -def delete_user_permission(user_obj, namespace_name, repository_name): - repo = Repository.get(Repository.name == repository_name, - Repository.namespace == namespace_name) - perm = RepositoryPermission.get(RepositoryPermission.user == user_obj, - RepositoryPermission.repository == repo) - perm.delete_instance() +def delete_user_permission(username, namespace_name, repository_name): + if username == namespace_name: + raise DataModelException('Namespace owner must always be admin.') + + fetched = list(user_permission_repo_query(username, namespace_name, + repository_name)) + if not fetched: + raise DataModelException('User does not have permission for repo.') + + fetched[0].delete_instance() diff --git a/endpoints/api.py b/endpoints/api.py index 375b2d791..a925d34ff 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -153,8 +153,7 @@ def get_permissions(namespace, repository, username): (namespace, repository, username)) permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - user = current_user.db_user - perm = model.get_user_reponame_permission(user, namespace, repository) + perm = model.get_user_reponame_permission(username, namespace, repository) return jsonify(role_view(perm)) abort(403) # Permission denied @@ -169,11 +168,15 @@ def change_permissions(namespace, repository, username): if permission.can(): new_permission = request.get_json() - user = current_user.db_user logger.debug('Setting permission to: %s for user %s' % (new_permission['role'], username)) - perm = model.set_user_repo_permission(user, namespace, repository, - new_permission['role']) + + try: + perm = model.set_user_repo_permission(username, namespace, repository, + new_permission['role']) + except model.DataModelException: + logger.warning('User tried to remove themselves as admin.') + abort(409) resp = jsonify(role_view(perm)) if request.method == 'POST': @@ -189,7 +192,12 @@ def change_permissions(namespace, repository, username): def delete_permissions(namespace, repository, username): permission = AdministerRepositoryPermission(namespace, repository) if permission.can(): - model.delete_user_permission(current_user.db_user, namespace, repository) + try: + model.delete_user_permission(username, namespace, repository) + except model.DataModelException: + logger.warning('User tried to remove themselves as admin.') + abort(409) + return make_response('Deleted', 204) abort(403) # Permission denied From 7e666570e35edea645337066d842e1c3db5c2bd4 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Fri, 27 Sep 2013 16:00:24 -0400 Subject: [PATCH 2/3] Update the test db to have another user with their own repo. --- test.db | Bin 29696 -> 30720 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test.db b/test.db index dc560799b6e042c3a3d86578f12e339f66d9323e..a46e7baafa4d05aec0717bafa0d8d03dccd2a5bc 100644 GIT binary patch delta 1878 zcmZWpOKcNY6n*dA`DBcp5CW7!eri%tJ~p1QXY4TxGPJSd5a;8>aT2vfPU40%AptuC zSU?!qbitz0Y>HG>U33X*gBO($txC61Q&p|{p+Yne+7zfDsD#q6=zC*BsIkURZ?4Wc z_q==Wyi<$#)P4N^7Cw#;s-CXhLpSalTW1D87E1`E5L|@RjG2I`HX@=rlP{A}W8)JC z(wQUKbf#WO)yqo#q29s7yPjlnq_L%^H++1gxj8){w;gF!vV-Y_67cP6Hztoq2lOLv z9Q0&bld*}(#*RpT3l>oo6C6xWPNe!r(*?DG4kaE>wxznpJIC8R!l?st_QW~5^a2CFW&vWJ6NqBP`7Eo0k-tp92CY2c;8*=xjvT4$FU^rV# zpH%HcIy>Rn2m-3xfYEcgY*+=M5;Xo)!VXfGmD9$iQf3$69)cBk2=}Prd9dz}ICCpC z66XN8g5V1L2#eHA4tB5SsD6<=lyg-4f7&l`Rdvr!STLM>vem(r&LNnC4`CQIAY^XF zyhNrHK|pSdjn?j_SZCO0bjA9DUE#h60mUo(++N+S1Zrh9r0OAA_o%*rqNxg>=fvZY zy_!EB?QQc%yAu117dXL?DtlyKz#sI=9OX#Z(4)PvM6kCpnT)q9!RS<&6y*tq6rV>^ z{kl(+X+;?)5oq!Sbh$0r-PzRK=i6IM34~P5p6BHmQo2w%Dc^(I&lT-|Nv%#T$$Dbr(_8kYDp?UcaK~tZ4$AL2vIz`DJAGo8If1dm~f z&7SirNFwkEV~Id+X-@?k%?E`%x$?COVEwk6+XNg)m;gvpeyW!WX{f+W=0V^XAh$B~ z7%Bm5J2slPKiOrvqH6jbEK|ilp{}ZgzE=X1a=`5Bp%&pLT%&dW4rZzIPi^O0!NKG} zvPDbmRb$BHM?R$_?> z@W?JKVQt{x&a=Uxj9+a=4cH8_+I$ya1k?up1@oXyn~$7k2a%BJGPlJHiJj6qI{u#P z^sCsB51K<#u54@zX8F`iku5;OLWCR*Hh(AFxx;t&wIL)9~ zfFjHaZ(SQ5a}rZ6DM4Xoi1_%5waaYBiTMUdt52Wo0y+T2flNH6%n@E;$q&l3Ou delta 1519 zcmah}-)mcS6#ss|=iZxp)1@}Hg;^J@S}IZ!?)T>2+}tQEVQZ5%ZTcgI{y^+G z@WxX946TUCi@B_RDGY|nF zw^r?SvZ6{73Mxv1j)lki)&3!MDms+0a{Zc0!$H*wYLQNA#0(>*(vWGYrfyJ?GjJj` zF_D}a8`m;@;mPqnD@+#==@ir_@j_mCcm*63q@rj~!3tEIOLaWzt%M zFdEasp{PkMO4VNWQ*lZ{wT7c~C^KRv5*dwV8Yn88Ku9;Oh^6r*kl-qUtFQ?V;a&I) zzFxI2!b%$iapM#v4mJ)-xR=#o- zmsh(A#3=+oQTRCi&-;jTAGuuT_hG4xxJ=Q#wSD%YK>ecRjw1q3DZ}lmP**1W0zblS zCiY8kc7}ykBD-om?km-meWmrHscx#4W#|U!6&2g3 zIHPaIn|-VcfK3E{!LP8%w0{St4@C3~#@nehm%xsH?pr^pX5T<)1Aj Date: Fri, 27 Sep 2013 16:04:10 -0400 Subject: [PATCH 3/3] Give anotheruser some permissions on one of the repos. --- test.db | Bin 30720 -> 30720 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test.db b/test.db index a46e7baafa4d05aec0717bafa0d8d03dccd2a5bc..0d99022c0cba088349e8c89359a15f36b02d07c3 100644 GIT binary patch delta 128 zcmV-`0Du30@Bx7E0gxL3Q;{4)1ycYm;v=zSpgRl%1K$7x`2+C-=mX!g5d_Zz2?qoP z009XB0s^z>KO6)L1Ow~<1N;N`1M<5pW>0AOyVwlkiUj0RofpHxsi2XzK$40tA!s iOA`zP1M2_-`~&L)_5<*<5fIx01q1^L0R*$~PY@_9fhXJm delta 127 zcmZqpz}WDCae_2s&_o$$)*uGmN9r3>7FcsJGrwS9{>l82`Nd{Ifm_U*U)amA00loW zF#lox&in}|_=H)8g_)C)8ALEKGjnosF)%PNGk=7r_y|<-n0fMhdA7;ly?KE;KY+x) UF@FGxKVqJI-k