Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add api - added ApiKeyAuthentication and DjangoAuthorization #115

Merged
merged 28 commits into from Jun 24, 2012

Conversation

Projects
None yet
2 participants
Contributor

a8 commented Jun 5, 2012

Hi,

Sorry that it took so long. Still not finished, but let's call it a milestone.

I changed the authentication for all resources in the REST API to ApiKeyAuthentication for everything but GET. Thus, fetching data doesn't require any auth. The api key is generated when the user is created. It is shown in the /admin.

The authorization is changed to DjangoAuthorization. Thus, all access rights can be configured in the /admin.

It's mainly testing code and a very few changes on api.py. Nothing else is touched.

The next step will be some refactoring of the ResultBundle to make it more like Tastypie works.

Anyhow, now all resources do have authentication and authorization. I've been using the API for some month now.

a8

a8 added some commits Apr 27, 2012

@a8 a8 added little example on how to create an environment from python 8ec39fe
@a8 a8 Added tests for Environment() to be changed to ApiKeyAuthentication a…
…nd DjangoAuthorization

    - So far the API did not require any authentication and did not support any
      authorization. Starting with the simple Environment() model that will be
      changed by the next commit. Here are the supporting tests. They can be
      cleaned up in the future a little.
    - Some tests were copied from the django-tastypie test suite. That is not
      to check upon tastypie but to verify that tastypie is used correctly in
      codespeed.

Change-Id: I70f083e1aa723b69c32a05ba2b8187fed2b5ebb8
73dcdb0
@a8 a8 Changed UserResource and EnvironmentResource to ApiAuthtentication an…
…d DjangoAuthorisation

    - The UserResource and EnvironmentResource are authenticated by the api key
      generated at user creation. Key handling is described in the Tastypie docu.
    - Both Resources are authorized via Django (E. g. changeable in the Admin).
    - For now even GET requests to the API have to be authenticated. That could
      be relaxed in the future.

Change-Id: Ica28762aeff9cdde3a8099bdd6098ccd191c416c
6f8f018
@a8 a8 added a few more Middleware classes
    - Even if not required some additional Middleware classes are added to
      ensure compatibility.

Change-Id: I91119d5e6e011f351ffde7bd1380e4e36af5ee98
78df42a
@a8 a8 add tests for Project() authentication
    - Project() will use MultiAuthentication(ApiKeyAuthentication(), Authentication())
      Thus, only GET requests will not need authentication.
    - Move user setup to generic FixtureTestCase class. That is common to
      all TestCases

Change-Id: I5893faa293f32ab1c07a6523a951879933a60560
021941a
@a8 a8 add MultiAuthentication(ApiKeyAuthentication(), Authentication()) to …
…Project()

    - now the ProjectResource is authenticated by an api key for everything but
      GET requests.

Change-Id: Ie238744bee0e44780d2f9dacc72e5fe0f708b7bc
7e625b3
@a8 a8 added tests for BranchResource authtentication and authorization
Change-Id: I7b286fc4dc3b0a6e80b441cd085b15daf79893a0
829eb22
@a8 a8 added BranchResource authentication and authorization
Change-Id: I94242079ce6f140d6b02684cd4150b35acaec06e
9f8c249
@a8 a8 added tests for RevisionResource authentication and authorization
Change-Id: I95110d4e7f613d5eebc0d1ee072fdd585936bd21
bfbbdfd
@a8 a8 added RevisionResource authentication and authorization
Change-Id: I693923c1ed5305b20075aa1591938c5cc0cf41cc
b5ed4fc
@a8 a8 added tests for ExecutableResource authentication and authorization
Change-Id: I8c870d82fba103506a212bab013910d19e30f4b9
a7e8856
@a8 a8 added tests for BenchmarkResource authentication and authorization
Change-Id: I2c63ed76070431b6737ff55aaafdc9bfbab6669d
995d557
@a8 a8 added BenchmarkResource authentication and authorization
Change-Id: I7174ede3ee51270f9b1b75b8b9fde9431c3e6657
2c1f84a
@a8 a8 added tests for ReportResource authentication and authorization
Change-Id: Iefc4c8b344ff984f19fd4479776aaaa6f7ae0291
2fc6c54
@a8 a8 added ReportResource authentication and authorization
Change-Id: I897e7db9e0ad283cf7b93a68ed8f5b6ef8e26d93
5d1d65c
@a8 a8 changed to Multiauth for Environment, changed authentication/authoris…
…ation for ResultBundleResource

    - Environment now has the same Authentication/Authorization as all other ModelResources
    - Added same Authentication and Authorization to ResultBundleResource. Warning:
      Authorisation is not fully implemented yet! The old test will still pass.

Change-Id: I2eb663296be172924c354061e80ac24841f716b2
19fea95
@a8 a8 few clean ups
Change-Id: I46fd006147f45f9ba30b116cfaae60339a43ed31
5441385
@a8 a8 Changed ResultBundleResult Djangoauthorization to models.Result, some…
… clean ups

    - for ResultBundle add, update, delete the Django access rights of the
      Result model is used
    - commented out some stuff that will be required maybe in future features

Change-Id: If38a5230bf04137f823e8d7884fa29d41ea202ae
5480f20
@a8 a8 Fixed tests to check if authentication and authorization for ResultBu…
…ndleResource are correct

    - Note, the ResultBundle and ResultBundleResource will be re-written
      to better match tastypie soon. For now it is possible to upload
      data via authentication and authorization.

Change-Id: If112476f4311358b56e3c7378565e910b9304db2
e7ba52f

@tobami tobami commented on an outdated diff Jun 8, 2012

codespeed/api.py
@@ -43,7 +43,8 @@
from tastypie.resources import ModelResource, Resource
from tastypie import fields
from tastypie.authorization import Authorization, DjangoAuthorization
-from tastypie.authentication import ApiKeyAuthentication
+#from tastypie.authentication import ApiKeyAuthentication, Authentication, MultiAuthentication
@tobami

tobami Jun 8, 2012

Owner

You forgot this commented out line, which is a duplicate of the one below

@tobami tobami and 1 other commented on an outdated diff Jun 8, 2012

codespeed/api.py
#setattr(result, 'notify', None)
return result
def obj_create(self, bundle, request=None, **kwargs):
- # FIXME (a8): Make full_hydrate work
+ # not calling hydrate here since bundle.save() has that functionality
#bundle = self.full_hydrate(bundle)
@tobami

tobami Jun 8, 2012

Owner

Can we remove this line then?

@a8

a8 Jun 11, 2012

Contributor

I removed that FIXME line in a later commit already. But, I'd like to use full_hydrate() and I am working on that already.

@tobami tobami commented on an outdated diff Jun 8, 2012

codespeed/tests/tests_api.py
+ self.auth.is_authenticated(self.request), HttpUnauthorized), True)
+
+ def test_is_authenticated(self):
+ """Should correctly authenticate when using an existing user and key"""
+ # Correct user/api_key.
+ user = User.objects.get(username='apiuser')
+ self.request.GET['username'] = 'apiuser'
+ self.request.GET['api_key'] = user.api_key.key
+ self.assertEqual(self.auth.is_authenticated(self.request), True)
+
+
+class UserTest(FixtureTestCase):
+ """Test api user related stuff"""
+
+ def test_has_apikey(self):
+ self.assertTrue(hasattr(self.api_user, 'api_key'))
@tobami

tobami Jun 8, 2012

Owner

Missing a docstring

@tobami tobami commented on an outdated diff Jun 8, 2012

codespeed/tests/tests_api.py
+ """Should correctly authenticate when using an existing user and key"""
+ # Correct user/api_key.
+ user = User.objects.get(username='apiuser')
+ self.request.GET['username'] = 'apiuser'
+ self.request.GET['api_key'] = user.api_key.key
+ self.assertEqual(self.auth.is_authenticated(self.request), True)
+
+
+class UserTest(FixtureTestCase):
+ """Test api user related stuff"""
+
+ def test_has_apikey(self):
+ self.assertTrue(hasattr(self.api_user, 'api_key'))
+
+ def test_len_apikey(self):
+ """Test the key has a length"""
@tobami

tobami Jun 8, 2012

Owner

Would be nice to also match the test docstring style: """Should have api user key with a non-zoro length""". Same for some other tests below

@tobami tobami commented on an outdated diff Jun 8, 2012

codespeed/tests/tests_api.py
+ # Much better.
+ request.user.user_permissions.add(self.delete)
+ # Nuke the perm cache. :/
+ del request.user._perm_cache
+ self.assertTrue(
+ EnvironmentResource()._meta.authorization.is_authorized(request))
+
+ def test_unrecognized_method(self):
+ request = HttpRequest()
+ self.api_user.user_permissions.clear()
+ request.user = self.api_user
+
+ # Check a non-existent HTTP method.
+ request.method = 'EXPLODE'
+ self.assertFalse(
+ EnvironmentResource()._meta.authorization.is_authorized(request))
def test_get_environment(self):
"""Should get an existing environment"""
@tobami

tobami Jun 8, 2012

Owner

Better """Should get an environment when given an existing ID""".

And next you could add a test for """Should return 404 when given a non existing environment ID"""

a8 added some commits Jun 11, 2012

@a8 a8 Improved docstrings and minor code cleanups
Change-Id: I4268567cf26ff62e5659950485ff883b1dcc1f84
c2ea6b2
@a8 a8 Changed django-tastypie to install via git from known working commit
    - The upcomming version of django-tastypie will have some
      features already used by the RESTful API. Thus, we have
      to install it from github. Once, django-tastypie is
      released it will changed back to the official stable
      release.

Change-Id: Id3b361e93d61d711c7f0a3496a2ebabc19846e38
2a8689e
Contributor

a8 commented Jun 11, 2012

OK, I hope the last 2 commits address your comments. If not, please let me know.

a8 added some commits Jun 19, 2012

@a8 a8 Removed unused code from api
    - Removed a few lines that were thought to be required for future
      features. One step at a time.

Change-Id: I0ae88d6144dacec717eac97a8f28ae17d2acad06
5ac8138
@a8 a8 Changed test to use proper resource uris for ResultBundle and ResultB…
…undleResource

    - previously data was provided similar to the tools/save_single_result.py
      Now it uses resource uris

Change-Id: Icc85ec3d3882e3b6f0cde592a591a9832daea881
5ff5c48
@a8 a8 Changed ResultBundle and ResultBundleResource to use uris
    - make the tests pass

Change-Id: Ib846cfd469fec9f17c5e6822ed428192082fa39b
676ef78
@a8 a8 Added an example to save a single result
    - Using requests it saves a single result. Note,
      models like benchmark, environment etc. are not created
      automatically. That has to be done via the api beforehand.

Change-Id: I07e57d03e91fa206a1bf0667c8c43e54c8a7e258
032c627
Contributor

a8 commented Jun 21, 2012

The last 4 commits make the ResultBundle to use proper URIs. I am looking forward to get some feedback and would like you to pull. I consider the 1st version of the API finished. I will provide more examples/documentation later on.

The body of this method has wrong indentation

Owner

a8 replied Jun 23, 2012

sorry. It's fixed.

Owner

tobami commented Jun 23, 2012

Apart from what I just commented we can merge.

Before Codespeed 1.0 also we need:

  • Documentation of course would be very nice for such a thing as an API.
  • We also need to wait for tastypie's release.
  • Last but not least there are a couple of critical issues that
    prevent most newcomers from getting the project up and running at
    all: #91, #114 and #117

a8 added some commits Jun 23, 2012

@a8 a8 fixed identation level
Change-Id: I35616c76a237786075e4e7d70a780c4696ae1602
65a2ede
@a8 a8 changed code formatting for switch, hopefully it's more readable
Change-Id: If79c2cc98e642b1f6e9704d029d0ac986cac36a8
8f72b1d
@a8 a8 fixed wrong identation
Change-Id: Ibe81d4f88836f182c84d1baf458585a465d04d6b
e413dab
Contributor

a8 commented Jun 23, 2012

I will document the API in the next 2 to 3 weeks. The other points are very important too.

@tobami tobami added a commit that referenced this pull request Jun 24, 2012

@tobami tobami Merge pull request #115 from a8/add_api
Add api - added ApiKeyAuthentication and DjangoAuthorization
5b73c97

@tobami tobami merged commit 5b73c97 into tobami:master Jun 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment