From 075c75d0311a4f7a8622332f2ff2589a19071ba9 Mon Sep 17 00:00:00 2001
From: Joseph Schorr <joseph.schorr@coreos.com>
Date: Tue, 2 Jun 2015 15:16:22 -0400
Subject: [PATCH] Change to always granting a signed token if there is a valid
 user OR if there is valid permissions on a repository

This fixes the issue whereby attempting to pull a public repository as an authenticated user with anonymous access disabled caused an unexpected 401. This change also adds tests for a few other use cases to verify we haven't broken anything.
---
 endpoints/index.py     | 23 +++++++++++------------
 test/registry_tests.py | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/endpoints/index.py b/endpoints/index.py
index bb3413a76..e84a93719 100644
--- a/endpoints/index.py
+++ b/endpoints/index.py
@@ -43,6 +43,7 @@ def generate_headers(scope=GrantType.READ_REPOSITORY):
       # Setting session namespace and repository
       session['namespace'] = namespace
       session['repository'] = repository
+
       # We run our index and registry on the same hosts for now
       registry_server = urlparse.urlparse(request.url).netloc
       response.headers['X-Docker-Endpoints'] = registry_server
@@ -50,24 +51,22 @@ def generate_headers(scope=GrantType.READ_REPOSITORY):
       has_token_request = request.headers.get('X-Docker-Token', '')
 
       if has_token_request:
-        permission = AlwaysFailPermission()
         grants = []
-        if scope == GrantType.READ_REPOSITORY:
-          permission = ReadRepositoryPermission(namespace, repository)
-          grants.append(repository_read_grant(namespace, repository))
-        elif scope == GrantType.WRITE_REPOSITORY:
-          permission = ModifyRepositoryPermission(namespace, repository)
-          grants.append(repository_write_grant(namespace, repository))
 
-        if permission.can():
-          # Generate a signed grant which expires here
+        if scope == GrantType.READ_REPOSITORY:
+          if ReadRepositoryPermission(namespace, repository).can():
+            grants.append(repository_read_grant(namespace, repository))
+        elif scope == GrantType.WRITE_REPOSITORY:
+          if ModifyRepositoryPermission(namespace, repository).can():
+            grants.append(repository_write_grant(namespace, repository))
+
+        # Generate a signed token for the user (if any) and the grants (if any)
+        if grants or get_authenticated_user():
           user_context = get_authenticated_user() and get_authenticated_user().username
           signature = generate_signed_token(grants, user_context)
           response.headers['WWW-Authenticate'] = signature
           response.headers['X-Docker-Token'] = signature
-        else:
-          logger.warning('Registry request with invalid credentials on repository: %s/%s',
-                         namespace, repository)
+
       return response
     return wrapper
   return decorator_method
diff --git a/test/registry_tests.py b/test/registry_tests.py
index 06c7c68b7..a2f98c067 100644
--- a/test/registry_tests.py
+++ b/test/registry_tests.py
@@ -276,7 +276,7 @@ class RegistryTests(RegistryTestCase):
     self.do_pull('devtable', 'newrepo', 'devtable', 'password')
 
 
-  def test_public_no_anonymous_access(self):
+  def test_public_no_anonymous_access_with_auth(self):
     # Turn off anonymous access.
     with TestFeature(self, 'ANONYMOUS_ACCESS', False):
       # Add a new repository under the public user, so we have a real repository to pull.
@@ -317,5 +317,34 @@ class RegistryTests(RegistryTestCase):
       self.do_pull('public', 'newrepo', 'public', 'password')
 
 
+  def test_public_no_anonymous_access_no_auth(self):
+    # Turn off anonymous access.
+    with TestFeature(self, 'ANONYMOUS_ACCESS', False):
+      # Add a new repository under the public user, so we have a real repository to pull.
+      images = [{
+        'id': 'onlyimagehere'
+      }]
+      self.do_push('public', 'newrepo', 'public', 'password', images)
+      self.clearSession()
+
+      # First try to pull the (currently private) repo as anonymous, which should fail as it
+      # is private.
+      self.do_pull('public', 'newrepo', expected_code=401)
+
+      # Make the repository public.
+      self.conduct_api_login('public', 'password')
+      self.change_repo_visibility('public', 'newrepo', 'public')
+      self.clearSession()
+
+      # Try again to pull the (currently public) repo as anonymous, which should fail as
+      # anonymous access is disabled.
+      self.do_pull('public', 'newrepo', expected_code=401)
+
+      # Pull the repository as public, which should succeed because the repository is owned by public.
+      self.do_pull('public', 'newrepo', 'public', 'password')
+
+      # Pull the repository as devtable, which should succeed because the repository is public.
+      self.do_pull('public', 'newrepo', 'devtable', 'password')
+
 if __name__ == '__main__':
   unittest.main()