From 23cbcb2979303727047457915818b9971f960c10 Mon Sep 17 00:00:00 2001 From: yackob03 Date: Thu, 26 Sep 2013 15:58:11 -0400 Subject: [PATCH] Make images belong to one repository only. Add a description field to the repository. Fix a bug with access tokens. Fix an embarrasing bug with multiple select criteria in peewee. Update the test db. --- auth/auth.py | 19 ++++++------ data/database.py | 34 +++++++++++---------- data/model.py | 68 +++++++++++++++++++++++++----------------- endpoints/index.py | 21 +++++-------- endpoints/registry.py | 4 +++ test.db | Bin 31744 -> 29696 bytes 6 files changed, 79 insertions(+), 67 deletions(-) diff --git a/auth/auth.py b/auth/auth.py index 8b214c17c..08cc6986b 100644 --- a/auth/auth.py +++ b/auth/auth.py @@ -27,7 +27,7 @@ def process_basic_auth(): normalized = [part.strip() for part in auth.split(' ') if part] if normalized[0].lower() != 'basic' or len(normalized) != 2: logger.debug('Invalid basic auth format.') - return False + return credentials = b64decode(normalized[1]).split(':') @@ -43,10 +43,11 @@ def process_basic_auth(): identity_changed.send(app, identity=Identity(authenticated.username)) - return True + return # We weren't able to authenticate via basic auth. - return False + logger.debug('Basic auth present but could not be validated.') + abort(401) def process_token(): @@ -56,19 +57,19 @@ def process_token(): normalized = [part.strip() for part in auth.split(' ') if part] if normalized[0].lower() != 'token' or len(normalized) != 2: logger.debug('Invalid token format.') - return False + return token_details = normalized[1].split(',') if len(token_details) != 2: logger.debug('Invalid token format.') - return False + return token_vals = {val[0]: val[1] for val in (detail.split('=') for detail in token_details)} if ('signature' not in token_vals or 'repository' not in token_vals): logger.debug('Invalid token components.') - return False + return unquoted = token_vals['repository'][1:-1] namespace, repository = parse_namespace_repository(unquoted) @@ -86,11 +87,11 @@ def process_token(): identity_changed.send(app, identity=Identity(validated.code)) - return True + return # WE weren't able to authenticate the token - logger.debug('Token could not be validated.') - return False + logger.debug('Token present but could not be validated.') + abort(401) def process_auth(f): diff --git a/data/database.py b/data/database.py index e80d7c746..c8c5aab5a 100644 --- a/data/database.py +++ b/data/database.py @@ -31,6 +31,7 @@ class Repository(BaseModel): namespace = CharField() name = CharField() visibility = ForeignKeyField(Visibility) + description = CharField(null=True) class Meta: database = db @@ -66,8 +67,22 @@ class AccessToken(BaseModel): class Image(BaseModel): - image_id = CharField(unique=True) + # This class is intentionally denormalized. Even though images are supposed + # to be globally unique we can't treat them as such for permissions and + # security reasons. So rather than Repository <-> Image being many to many + # each image now belongs to exactly one repository. + image_id = CharField() checksum = CharField(null=True) + created = DateTimeField(null=True) + comment = CharField(null=True) + repository = ForeignKeyField(Repository) + + class Meta: + database = db + indexes = ( + # we don't really want duplicates + (('repository', 'image_id'), True), + ) class RepositoryTag(BaseModel): @@ -76,22 +91,9 @@ class RepositoryTag(BaseModel): repository = ForeignKeyField(Repository) -class RepositoryImage(BaseModel): - repository = ForeignKeyField(Repository) - image = ForeignKeyField(Image) - tag = CharField() - - class Meta: - database = db - indexes = ( - # we don't really want duplicates - (('repository', 'image', 'tag'), True), - ) - - def initialize_db(): - create_model_tables([User, Repository, Image, RepositoryImage, AccessToken, - Role, RepositoryPermission, Visibility, RepositoryTag]) + create_model_tables([User, Repository, Image, AccessToken, Role, + RepositoryPermission, Visibility, RepositoryTag]) Role.create(name='admin') Role.create(name='write') Role.create(name='read') diff --git a/data/model.py b/data/model.py index 14d9f254f..6cae25da5 100644 --- a/data/model.py +++ b/data/model.py @@ -1,5 +1,6 @@ import bcrypt import logging +import dateutil.parser from database import * @@ -34,10 +35,15 @@ def verify_user(username, password): return None +def create_access_token(user, repository): + new_token = AccessToken.create(user=user, repository=repository) + return new_token + + def verify_token(code, namespace_name, repository_name): joined = AccessToken.select(AccessToken, Repository).join(Repository) - tokens = list(joined.where(AccessToken.code == code and - Repository.namespace == namespace_name and + tokens = list(joined.where(AccessToken.code == code, + Repository.namespace == namespace_name, Repository.name == repository_name)) if tokens: return tokens[0] @@ -64,7 +70,7 @@ def get_all_repo_permissions(user): def get_repository(namespace, name): try: - return Repository.get(Repository.name == name and + return Repository.get(Repository.name == name, Repository.namespace == namespace) except Repository.DoesNotExist: return None @@ -88,28 +94,39 @@ def create_repository(namespace, name, owner): return repo -def create_image(image_id): - new_image = Image.create(image_id=image_id) +def create_image(image_id, repository): + new_image = Image.create(image_id=image_id, repository=repository) return new_image -def set_image_checksum(image_id, checksum): - fetched = Image.get(Image.image_id == image_id) +def set_image_checksum(image_id, repository, checksum): + fetched = Image.get(Image.image_id == image_id, + Image.repository == repository) fetched.checksum = checksum fetched.save() return fetched -def assign_image_repository(repository, image, tag): - repo_image = RepositoryImage.create(repository=repository, image=image, - tag=tag) - return repo_image +def set_image_metadata(image_id, namespace_name, repository_name, + created_date_str, comment): + joined = Image.select().join(Repository) + image_list = list(joined.where(Repository.name == repository_name, + Repository.namespace == namespace_name, + Image.image_id == image_id)) + + if not image_list: + raise RuntimeError('No image with specified id and repository') + + fetched = image_list[0] + fetched.created = dateutil.parser.parse(created_date_str) + fetched.comment = comment + fetched.save() + return fetched def get_repository_images(namespace_name, repository_name): - select = Image.select(Image, RepositoryImage) - joined = select.join(RepositoryImage).join(Repository) - return joined.where(Repository.name == repository_name and + joined = Image.select().join(Repository) + return joined.where(Repository.name == repository_name, Repository.namespace == namespace_name) @@ -117,25 +134,25 @@ def list_repository_tags(namespace_name, repository_name): select = RepositoryTag.select(RepositoryTag, Image) with_repo = select.join(Repository) with_image = with_repo.switch(RepositoryTag).join(Image) - return with_image.where(Repository.name == repository_name and + return with_image.where(Repository.name == repository_name, Repository.namespace == namespace_name) def get_tag_image(namespace_name, repository_name, tag_name): joined = Image.select().join(RepositoryTag).join(Repository) - return joined.where(Repository.name == repository_name and - Repository.namespace == namespace_name and + return joined.where(Repository.name == repository_name, + Repository.namespace == namespace_name, RepositoryTag.name == tag_name) def create_or_update_tag(namespace_name, repository_name, tag_name, tag_image_id): - repo = Repository.get(Repository.name == repository_name and + repo = Repository.get(Repository.name == repository_name, Repository.namespace == namespace_name) image = Image.get(Image.image_id == tag_image_id) try: - tag = RepositoryTag.get(RepositoryTag.repository == repo and + tag = RepositoryTag.get(RepositoryTag.repository == repo, RepositoryTag.name == tag_name) tag.image = image tag.save() @@ -146,25 +163,20 @@ def create_or_update_tag(namespace_name, repository_name, tag_name, def delete_tag(namespace_name, repository_name, tag_name): - repo = Repository.get(Repository.name == repository_name and + repo = Repository.get(Repository.name == repository_name, Repository.namespace == namespace_name) - tag = RepositoryTag.get(RepositoryTag.repository == repo and + tag = RepositoryTag.get(RepositoryTag.repository == repo, RepositoryTag.name == tag_name) tag.delete_instance() def delete_all_repository_tags(namespace_name, repository_name): - repo = Repository.get(Repository.name == repository_name and + repo = Repository.get(Repository.name == repository_name, Repository.namespace == namespace_name) RepositoryTag.delete().where(RepositoryTag.repository == repo) -def create_access_token(repository, user): - new_token = AccessToken.create(user=user, repository=repository) - return new_token - - def get_user_repo_permissions(user, repository): select = RepositoryPermission.select() - return select.where(RepositoryPermission.user == user and + return select.where(RepositoryPermission.user == user, RepositoryPermission.repository == repository) diff --git a/endpoints/index.py b/endpoints/index.py index 345cbfe79..8a02c70bd 100644 --- a/endpoints/index.py +++ b/endpoints/index.py @@ -55,9 +55,6 @@ def create_user(): @app.route('/v1/users/', methods=['GET']) @process_auth def get_user(): - if not get_authenticated_user(): - abort(401) - return jsonify({ 'username': get_authenticated_user().username, 'email': get_authenticated_user().email, @@ -101,14 +98,9 @@ def create_repository(namespace, repository): if repo: permission = ModifyRepositoryPermission(namespace, repository) if not permission.can(): - if get_validated_token() or get_authenticated_user(): - abort(403) - else: - abort(401) - else: - if not get_authenticated_user(): - abort(401) + abort(403) + else: if get_authenticated_user().username != namespace: abort(403) @@ -126,8 +118,7 @@ def create_repository(namespace, repository): existing.repositoryimage.delete() for image_description in added_images.values(): - image = model.create_image(image_description['id']) - model.assign_image_repository(repo, image, image_description['Tag']) + image = model.create_image(image_description['id'], repo) response = make_response('Created', 201) return response @@ -141,10 +132,13 @@ def update_images(namespace, repository): permission = ModifyRepositoryPermission(namespace, repository) if permission.can(): + repository = model.get_repository(namespace, repository) image_with_checksums = json.loads(request.data) for image in image_with_checksums: - model.set_image_checksum(image['id'], image['checksum']) + logger.debug('Setting checksum for image id: %s to %s' % + (image['id'], image['checksum'])) + model.set_image_checksum(image['id'], repository, image['checksum']) return make_response('Updated', 204) @@ -165,7 +159,6 @@ def get_repository_images(namespace, repository): for image in model.get_repository_images(namespace, repository): new_image_view = { 'id': image.image_id, - 'tag': image.repositoryimage.tag, 'checksum': image.checksum, } all_images.append(new_image_view) diff --git a/endpoints/registry.py b/endpoints/registry.py index ee3a6193f..0c8ad4c1a 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -14,6 +14,7 @@ from auth.auth import process_auth, extract_namespace_repo_from_session from util import checksums from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission) +from data import model store = storage.load() @@ -286,6 +287,9 @@ def put_image_json(namespace, repository, image_id): abort(409) #'Image already exists', 409) # If we reach that point, it means that this is a new image or a retry # on a failed push + # save the metadata + model.set_image_metadata(image_id, namespace, repository, + data.get('created'), data.get('comment')) store.put_content(mark_path, 'true') store.put_content(json_path, request.data) generate_ancestry(namespace, repository, image_id, parent_id) diff --git a/test.db b/test.db index dd3835cc2307e5b1d4bb82cec04f336605b0d385..9c0dcf2878a2ce518dcb67eef3c8b04b58aaf31d 100644 GIT binary patch literal 29696 zcmeHQTWlOx8J?N5>-8pe(l&9nX^EIR73mUob1rjc&a9}YaW-}m$4Ts%T!gUB+>&i< zui0Iv4JtyLprStVP>Ba#ka&a_o)C{bBdQ9i6;D(k5C{-0w5d>d;GdaY@6KlJq;*ko zG9&NV*>nB>@7%w0@xuApTFcjidSgYlbf#n!RaIWpbwyE5z~>Nry3YZ4$#j3fx7zo< z+3N|V^!p=Oa7F0-&`$x~MR(%Y&4Q%%DFP`1|4Rg(cmiVI{TLYe+!&Y${Xv0R`8W7V zpA>-K0)Od~B9J2Re~-Ydno|z7 zvwQQNf1@RxW&b%UpCk0S#_&$+8jraC-&N4vM?mpZ#T0=Q zfi40Wv^Ouq@xO-tqM#4KNS_pe6oH2mfn6~9e`c*wyCGYC9;W{X*Vdionj6>uHx=|( z^yb5eJZ$(_uL}OaQv^K4`KWdM*5@( zqzF9x2xO6}YFY-Vc`fY!flmxq`d?SSq5fPO)m}rdbs%_2%@xMS)wUJ!jE(xTAAYo{ z`Re3_s(xYetFu*oBD9*&pP#6C6Z*-y3)SiBygqmKfrKBAeo!*8(CPsb3%zcUAc(Ujo6VQ& z4R7h1Y+l<$F7W2B$lCJe&NuuRI)H9H3ixcIV>>&V zD|`joVQa7*4m+=?_gD(wdM)a%p?z)7J@VcQIIKb5h-|J(^4zbVW0eeKZ?rqT-Mx&7Xc66by{w}Uh^ZSPF`R$Lvg`Fy`a5Qxs z#+VmQo~iamQ`dIo3Z=tpdo0SGK82S;Rr-F94b7E44^ePoBY7#F;sJ@g&8Bm9Sl+ zrcQW;^9m+q%fgnyNj40jPR?AqG<{|MBAq+IE?+!hGi@M z@~Ooum#<7u&&(|h24NM(%ZANN%n9lQIX82fUYxx+yLkSzIDSzK1|bzj%LXAF+Zf2a zP!+LoR!kkAS+vRdg~hqkbU-drp{y({ViSUa;tNAeV*9KhXXegXlaq6l%ngLVpnuu0 zY}2OU_C$&xyND)aUAaod90txNvD=YqL>mI*74IiN7 zZfM=n0m^T@{{K-l{{Q%mNg5(WAVpvYB9NH>YbdS%J1`GZky8XdDFOra|4!Kd-}$7G zrQuQpQUrDq0`UQWkpB#vN2RJ>m89RMXut5?ZJVfmM1~VJ7xJB?KpUPD&n8Z@)0G<^%xs zG>0DtlZ<@A?E#tjEe;~~%e3|CQzZHZjSk^@l6*2EetkSwu%YA?hLpT6PQ5e$iKmm< zpK#mAxJ}AiH^TV*->3@r|NR*KLj6Gd75cjNW$n9Y|0CQ8JoKthXWl{V*7#RFgtYTk~7nFg)eL}+KI>FoJ!lL4i`cY4+}F0 zuuUx3k1m`5hu;u_Tf}f>;4$C8f(8}~tbj6R3YqJ~V@sbHocfq^*MgALH5>_B^gS$T zU^5Y5Vp8EtlS&5+95~drZO<@F>I6n$jdbGS)N@?yGuPwN5ybYmjU^|9y2Nq=#}}Su zGY(2&$FPmSGZ_<>=h}u8HYNehcI9z>W|>U**u>OzG4UC-EsJ}mV+!B$C^bAOe83G0 zEQx@V0jA|U6uTCqo*QI3@t9IL4D|At%?;Pa-0(#ZxYY5a!z>7kg#+M8ZZe+-o?{aS zvH-v`7!BY~Pc)v%msAR}Tf!iQg>8t3Yt?MNJ(hPYjsxGascC!EcWudBZZpu0I|gIa zAs&Hz1GpRmP7pcNrNraH4gwnR0Hh4p{}FmeLH|O3g0J*R5l9i(`3Q``KAwZBx(r7t zo2{(28#eunt7>@QcSPF_JAV%5^RZhv|3~P(aQ^>Zx&pBCGd|TIMPMrgLP8*mLUUwe z|9_|c-+^WzeNqHc1Ri1pB0?~#|5{Z4!|{Jc`M#ojUp);rM?Z7b+m@PF+GFkF8D-m( zo0e?A{>r1xYm#!aVh7xYQ`Np>bLJY{g6R=l2W6PthB+O~NwFu0Wtpz&!bBIQPL^Q{ z7iNZmlrZhWJU&FtN|fo8R;W=SqAZ9os9C~R1>?4%jVdJ&yp|uFtR)2@0jHKv91muU zuE35Zq)mL+F&yp)hZ``#gPB?&9aB19;7ZSD4uuIcGi5*}%uu7F?2#aZRyeB=Qidrz zqc|4i1H?q2sByBF5FmA zBSf5t3NtI%D#J;7%j_t+;J4P-^tC$NV|%mIaLbGVRPb1AS)=4D2A{wkSE-8P1A+!1&RU+C9xTB0Sr?g z;Ke@nJ0+8s7}1GYf$I&zl8G5-JT4G1PBMX1d=*)A|6fLdtH0h>f21uV^E0pipNatO znQSMkO6qp$pk!8YS5VWCz&pX0Fdg7;Ooly)h0SaR>#pJ?O6sDCa_Tg zh&m5@44NKp0vTn12}~E3z-Re{dXxpk^}=E&Ifa5sHfpD#6VQWII4yI_1Rdj2cdshb zxsEDF)$NjoNmUpSZ^HB}Ash);q+s3vBEG9Pv z5OAFvHM+Ti?FtiRi(qb9!&=mm)}9{S&}6$bYElzFaN&v(6Eg~AC*05hoSkH*0p$lW z!@(Yv+z1GTZVGbBb{S?s;DVMML)T#e)fe>*Mv36CDKMa4GRn}+5TnycCHsbw_FpSl B(T4y4 delta 1982 zcmb7FZERCj7{2Ge_xA2~?P#}lx80cQIs@yvcD?<&-3YRNu(6GejlsHg!Yr)J;>KiM z5sYSBFviFq<}qS6+$e{Q`Z*ou1 zec$Ij&-yeFiaPimeu1Ch`jVM@ z0qSg4BCD(=3uI~(T(R?`)sCXI6INc`@-It?%6&2rkDOI z=M6eUVxbWjE`3P_EDc3qPBP3%)P~+-fxQTRY-(DpQT0fW zBnhcG4V;}$PiC}*#r5&oM%UAP%f zj;7*fJUiiZ?FK=zT9K+~WV16<8TxWK=tx#`n(DBisq(qZEPZ+WuH$3G<7XP`kd888 z+_0)vr;{42D6hFhQRA8EbasmF1Pybo!GQ#wIR=Z!L`ydIlet;k3SlBgUn zx}2y2pK;wHm{ZNG(F%?D$L1D-u(_L&3CG>*PMiJ!BNAC!3kYs9aC6CQx(xW{>S`nC z82Aj{f!z=RT|vFW&ii=cqbNoPI=cp=-KlW0FE-RU6d&~at5;BsUBtx&ywg%7*oc&O{(kV&QL>Bi=|@_9+1$p`{#K`V-;a-c(;#a%8A; zQ%{^&%s)&bxBF$Uq9~C_*vA%4y0`WQLdi&|KR(biIyf>~LgH%=hP**JB8UAx(jSy0 zKHN95F*w{77*RGPwuDoUS;%3pU-pN>0eV3dJS>tLhOb}|&cg?!XI_PQ+B2p1!Bb&r z$Q%`xMg>$@>NiQhM=W)XI>5tK2JXSHa0_mbEq#`%4Hp1^zlOut6&rEzVE}2iFE!mN zo&ynabOE}s0HR1sj%Q9V%n8)Qwz8*S?Si_ zTWDeT{9D(S03YpN)nFE+E