Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth support #709

Merged
merged 47 commits into from
Mar 12, 2013
Merged

OAuth support #709

merged 47 commits into from
Mar 12, 2013

Conversation

tomchristie
Copy link
Member

Support for OAuth 1.0a and OAuth 2

swistakm and others added 30 commits February 25, 2013 16:56
…r issues

- alias oauth2 as oauth
- remove rouge print
- remove docstring markups
- OAuthAuthentication.authenticate() now returns (user, token) two-tuple on success
- don't set request.user because it's already set
oauth_provider can be added to INSTALLED_APPS only if these packages are installed
Conflicts:
	rest_framework/tests/authentication.py
to avoid naming collision with `oauth2` used for OAuth 1
for a cross python versions compatibility
with a warning for incompatibility with Python 3
and taking @tomchristie advice into account on how to reformulate some
sentences
@tomchristie tomchristie mentioned this pull request Mar 8, 2013
@tomchristie
Copy link
Member Author

@dulaccc, @swistakm - If you have twitter handles, can you let me know them, so I know who to thank when this gets released? :)

@tomchristie
Copy link
Member Author

Ideally the docs could do with a bit more polishing but otherwise I think this is basically good-to-go.
Thanks so much!

@tomchristie
Copy link
Member Author

Note: if either of you have anything else you think needs adding/atering before this hits master, you can make PRs against the oauth branch in the meantime, and they'll be added to this PR.

@dulacp
Copy link
Contributor

dulacp commented Mar 9, 2013

@tomchristie my twitter handle is the same as my github one : )

Note: if either of you have anything else you think needs adding/atering before this hits master, you can make PRs against the oauth branch in the meantime, and they'll be added to this PR.

Nothing special is coming to my mind, but I'll sure let you know and PR on the oauth branch otherwise.

By the way, it's always a pleasure to give a hand on a great project, and django-rest-framework is definitely one of them!

@noirbizarre
Copy link

It seems really interesting!!! It's a must-have feature.

Do you plan to merge it into the master soon ?
Do you know which release is targeted ?

@tomchristie
Copy link
Member Author

@noirbizarre Yup the plan is for this to make it in soon.

One last thing I would ideally like to see before this gets merged is a permission class that works together with these oauth authentication classes. Something along these lines...

class TokenHasReadWriteScope(BasePermission):
    def has_permissions(self, request, view):
        if not request.token:
            return False

        read_only = request.method in permissions.SAFE_METHODS:
        if hasattr(request.token, 'resource'):
            ... # django-oauth-plus token - Check `request.token.resource.is_readonly` as appropriate
        elif hasattr(request.token, 'scope'):
            ... # django-oauth2-provider token - Check `request.token.scope` is 'read' or 'write' as appropriate
        else:
            # Improperly configured!

@swistakm, @dulaccc, @noirbizarre - Anyone fancy taking that on plus tests and issuing a PR against the oauth branch? ;)

@dulacp
Copy link
Contributor

dulacp commented Mar 10, 2013

I've some time today, so I can try to integrate that.
@tomchristie how many hours do I get ? I like challenges ;)

@dulacp
Copy link
Contributor

dulacp commented Mar 10, 2013

Done. #721

@dulacp
Copy link
Contributor

dulacp commented Mar 11, 2013

@thedrow ^^ thanks!

tomchristie added a commit that referenced this pull request Mar 12, 2013
@tomchristie tomchristie merged commit 40e3fc0 into master Mar 12, 2013
@tomchristie tomchristie deleted the oauth branch March 12, 2013 19:27
@tomchristie
Copy link
Member Author

@thedrow - I've actually no good excuse at all for being slack regarding history, and particularly wrt. to failing tests you make a really good point. Could def be persuaded to clean up my act. :)

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

Successfully merging this pull request may close these issues.

4 participants