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

OAuth2TokenMiddleware does not work with Django 1.10 #406

Closed
bastbnl opened this issue Aug 9, 2016 · 23 comments
Closed

OAuth2TokenMiddleware does not work with Django 1.10 #406

bastbnl opened this issue Aug 9, 2016 · 23 comments

Comments

@bastbnl
Copy link
Contributor

bastbnl commented Aug 9, 2016

Adding the OAuth2TokenMiddleware to the MIDDLEWARE array will break the application:
Traceback (most recent call last): ... File "/var/www/brownpapersession/dev/env-brownpapersession/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 82, in load_middleware mw_instance = middleware(handler) TypeError: object.__new__() takes no parameters

Mitigation described here: https://docs.djangoproject.com/en/1.10/topics/http/middleware/

jdelight added a commit to jdelight/django-oauth-toolkit that referenced this issue Aug 27, 2016
* Should fix jazzband#406

Should now work with old-style Middleware and the Django 1.10 new style

* Added notification concerning the migrations

This should prevent others from running into jazzband#405 while trying to create both a swappable instance and swap it out at once
@mucyomiller
Copy link

great works!

@niknokseyer
Copy link

I'm experiencing this issue as well.

@caxco93
Copy link

caxco93 commented Sep 27, 2016

not working for me.

when added it shows the following...

File "/home/apeiron/Proyectos/venvs/auth/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 82, in load_middleware
    mw_instance = middleware(handler)
TypeError: object() takes no parameters

@bastbnl
Copy link
Contributor Author

bastbnl commented Sep 27, 2016

@caxco93 Which Django version are you using?

@szepczynski
Copy link

szepczynski commented Oct 26, 2016

@bastbnl I have the same issue (like a @caxco93) with Django 1.10

@bastbnl
Copy link
Contributor Author

bastbnl commented Oct 26, 2016

@szepczynski do you mean it doesn't work for you!

@szepczynski
Copy link

@bastbnl it doesn't work I got also this error (caused by OAuth2TokenMiddleware):

mw_instance = middleware(handler)
TypeError: object() takes no parameters

@bastbnl
Copy link
Contributor Author

bastbnl commented Oct 26, 2016

OK, thanks. I'll look into it and update the pull request

Op wo 26 okt. 2016 17:24 schreef Marcin Szepczyński <
notifications@github.com>:

@bastbnl https://github.com/bastbnl it doesn't work I got also this
error (caused by OAuth2TokenMiddleware):

mw_instance = middleware(handler)
TypeError: object() takes no parameters

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#406 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFoMs8Q3nZuKf9x-3KiKZynPdrTjKhvJks5q33CVgaJpZM4JgbXv
.

@bastbnl
Copy link
Contributor Author

bastbnl commented Oct 26, 2016

@szepczynski Could you tell me what happens when you try to run this import from the Django shell? (python manage.py shell):

from django.utils.deprecation import MiddlewareMixin

@Axenu
Copy link

Axenu commented Oct 31, 2016

There is a simple fix for this! Just edit the file /oauth2_provider/middleware.py
and add import statement:

from django.utils.deprecation import MiddlewareMixin

and change the class inheritance from "object" to "MiddlewareMixin"

For further details see the official Django 1.10 middleware migration page: https://docs.djangoproject.com/el/1.10/topics/http/middleware/#upgrading-pre-django-1-10-style-middleware

@bastbnl
Copy link
Contributor Author

bastbnl commented Oct 31, 2016

Thanks for your reply. I understand that the easy fix will work on Django 1.10. I intended to create something that will work with Django 1.9 as well. Honesty never checked to see if this class exists on pre 1.10

@Axenu
Copy link

Axenu commented Oct 31, 2016

I looked in the code and it seems that the class is present in django 1.9 aswell ;)

@noptongbai
Copy link

@Axenu You save my life. Thank you so much ^3^

@bastbnl
Copy link
Contributor Author

bastbnl commented Oct 31, 2016

@Axenu Just took a look into the 1.9 code on github and didn't see that class on the 1.9 tags. Then again, I don't really know about the deprecation strategy behind this app: should it still work on 1.8 as well? Or even 1.7? It's in the README, btw.

I'll take another look and attempt to improve the PR

@Axenu
Copy link

Axenu commented Nov 3, 2016

I myself just took another look at it and it seems that it is not available in the earlier versions as I previously thought. I suppose another way to work around it would be to rewrite the class with a custom method:
def __call__(self, request): response = None if hasattr(self, 'process_request'): response = self.process_request(request) if not response: response = self.get_response(request) if hasattr(self, 'process_response'): response = self.process_response(request, response) return response
Which would satisfy the needs of the current Django version while still supporting the pervious versions. The code is taken directly from the midvlewaremixin class in Django 10.10.
It could be simplified a bit when added into the project.

@jleclanche
Copy link
Member

Whats the status of this issue and #405? Is this library usable on 1.10 at all?

Seeing as the fix is simple and there hasnt been activity on it since August, I'm considering whether picking it for oauth support is the correct move.

maryokhin pushed a commit to villoid/django-oauth-toolkit that referenced this issue Nov 14, 2016
Should now work with old-style Middleware and the Django 1.10 new style
@bastbnl
Copy link
Contributor Author

bastbnl commented Nov 16, 2016

@jleclanche I'm using this app on Django 1.10 with the patches I provided and I see it working properly. I believe it is a great app, but I feel the same. No maintainer comments on the PR and people are reporting issues on it, which I can't reproduce. Obviously improving it or fixing it gets hard

I say go for it as I haven't found anything that beats it

@niknokseyer
Copy link

Yup. I like how everything works so far. But I agree, no patches or fixes for known Django 1.10 issues. Wonder what happened. @bastbnl @jleclanche

@jleclanche
Copy link
Member

We're going to be using this app in prod pretty soon so we'll be maintaining a fork of this in git with the patch. If the maintainers (@synasius @Girbons) have disappeared, or are no longer maintaining this project, we'll make it more official but that's really something I want to avoid.

Heads up to @evonove: I can help maintain this project if needs be.

@synasius
Copy link
Contributor

@jleclanche @niknokseyer @bastbnl any help is more than welcome! There are a lot of issues that still need to be triaged and PRs to be reviewed. We're trying to work on it on a regular basis, at least every two weeks, but I know it's not enough.

So let me know if any of you guys want to be added as a contributor with write permissions and I'll gladly do that. The only rule is that any "important" change or design choice should be discussed and reviewed together before it can be merged.

@jleclanche
Copy link
Member

@synasius that's great. I'll keep an eye on the issues and see what I can do once I start using the lib in prod :)

Any chance for a release by the way now that it's 1.10 compatible?

@synasius
Copy link
Contributor

@jleclanche sure! we need to address a couple of issues and then we'll release a new version

@jleclanche
Copy link
Member

@synasius I'm now starting to work on the library for our project. I'd like write permissions if your offer still stands. My main goal will be to PR a cleaned up version of #395. I would also like to land a rebased #321 and triage all current issues that I can.

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

No branches or pull requests

9 participants