conwetlab / ckanext-oauth2

OAuth2 support for CKAN
GNU Affero General Public License v3.0
25 stars 56 forks source link

flask and python3 conversion #42

Open FedericOldani opened 3 years ago

FedericOldani commented 3 years ago

Hi there! Since few people asked for python 3 version of this useful ckan extension (issue #39), I implemented it. I converted the code to be compatible with ckan 2.9 which uses python3 and Flask (instead of Pylons). It is now no more compatible with Pylons. I did a dirty job of conversion, the best way would be re-implement the extension from zero but this is a starting point.

frafra commented 2 years ago

It is a pity that such PR is stuck because of minor changes which can be easily applied. I can work on it if needed.

FedericOldani commented 2 years ago

Thank you guys for your interest in this conversion. Actually, the development is gone a little bit further and I solve some issues. Unfortunately, I didn't have time to address the changes requested. Some of them are due to the try&error process I followed to make things work but for sure they can be written better. Hope to address this by the end of March but I cannot promise that

frafra commented 2 years ago

Actually, the development is gone a little bit further and I solve some issues.

Great! :)

Unfortunately, I didn't have time to address the changes requested. Some of them are due to the try&error process I followed to make things work but for sure they can be written better. Hope to address this by the end of March but I cannot promise that

Feel free to share your progress here if you need help.

jeverling commented 2 years ago

Hi all, what is the proper way to setup the DB tables with this approach? If I just try to use the extension based on this code, the tables are not created and I get a psycopg2.errors.UndefinedTable exception.

FedericOldani commented 2 years ago

I quickly revised some of the comments, but the most important change is in the creation of the DB table during the first run. This was a big bug in my first version. Hope you guys can run it, let me know!

jeverling commented 2 years ago

Thanks, I can confirm the table gets created now. :+1:

frafra commented 2 years ago

It works just fine for me. I guess the review should be updated :)

frafra commented 2 years ago

Hi @FedericOldani! I can help with this PR if needed :)

frafra commented 2 years ago

It looks much better to me :) There are a couple of changes related to SSL, to force HTTPS I suppose.

simao-silva commented 2 years ago

When using this version I get the following error:

AttributeError: 'Request' object has no attribute 'GET'

This is the full traceback:

 Traceback (most recent call last):
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/views.py", line 54, in callback
     token = oauth2helper.get_token()
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/oauth2.py", line 116, in get_token
     token = oauth.fetch_token(self.token_endpoint,
   File "/usr/lib/python3.8/site-packages/requests_oauthlib/oauth2_session.py", line 244, in fetch_token
     self._client.parse_request_body_response(r.text, scope=self.scope)
   File "/usr/lib/python3.8/site-packages/oauthlib/oauth2/rfc6749/clients/base.py", line 448, in parse_request_body_response
     self.token = parse_token_response(body, scope=scope)
   File "/usr/lib/python3.8/site-packages/oauthlib/oauth2/rfc6749/parameters.py", line 441, in parse_token_response
     validate_token_parameters(params)
   File "/usr/lib/python3.8/site-packages/oauthlib/oauth2/rfc6749/parameters.py", line 448, in validate_token_parameters
     raise_from_error(params.get('error'), params)
   File "/usr/lib/python3.8/site-packages/oauthlib/oauth2/rfc6749/errors.py", line 399, in raise_from_error
     raise cls(**kwargs)
 oauthlib.oauth2.rfc6749.errors.InvalidGrantError: (invalid_grant) Code not valid
frafra commented 2 years ago

Do you also have the same problem with the previous version?

simao-silva commented 2 years ago

@frafra If by previous version you mean the original 0.7.0 version the answer is no. When using that version I get the error No module named 'oauth2'. Here is the full traceback:

 Traceback (most recent call last):
   File "/usr/bin/ckan", line 33, in <module>
     sys.exit(load_entry_point('ckan', 'console_scripts', 'ckan')())
   File "/usr/lib/python3.8/site-packages/click/core.py", line 829, in __call__
     return self.main(*args, **kwargs)
   File "/usr/lib/python3.8/site-packages/click/core.py", line 781, in main
     with self.make_context(prog_name, args, **extra) as ctx:
   File "/usr/lib/python3.8/site-packages/click/core.py", line 700, in make_context
     self.parse_args(ctx, args)
   File "/srv/app/src/ckan/ckan/cli/cli.py", line 115, in parse_args
     result = super(ExtendableGroup, self).parse_args(ctx, args)
   File "/usr/lib/python3.8/site-packages/click/core.py", line 1212, in parse_args
     rest = Command.parse_args(self, ctx, args)
   File "/usr/lib/python3.8/site-packages/click/core.py", line 1048, in parse_args
     value, args = param.handle_parse_result(ctx, opts, args)
   File "/usr/lib/python3.8/site-packages/click/core.py", line 1630, in handle_parse_result
     value = invoke_param_callback(self.callback, ctx, self, value)
   File "/usr/lib/python3.8/site-packages/click/core.py", line 123, in invoke_param_callback
     return callback(ctx, param, value)
   File "/srv/app/src/ckan/ckan/cli/cli.py", line 125, in _init_ckan_config
     _add_ctx_object(ctx, value)
   File "/srv/app/src/ckan/ckan/cli/cli.py", line 134, in _add_ctx_object
     ctx.obj = CtxObject(path)
   File "/srv/app/src/ckan/ckan/cli/cli.py", line 56, in __init__
     self.app = make_app(self.config)
   File "/srv/app/src/ckan/ckan/config/middleware/__init__.py", line 56, in make_app
     load_environment(conf)
   File "/srv/app/src/ckan/ckan/config/environment.py", line 123, in load_environment
     p.load_all()
   File "/srv/app/src/ckan/ckan/plugins/core.py", line 165, in load_all
     load(*plugins)
   File "/srv/app/src/ckan/ckan/plugins/core.py", line 179, in load
     service = _get_service(plugin)
   File "/srv/app/src/ckan/ckan/plugins/core.py", line 281, in _get_service
     return plugin.load()(name=plugin_name)
   File "/usr/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2443, in load
     return self.resolve()
   File "/usr/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2449, in resolve
     module = __import__(self.module_name, fromlist=['__name__'], level=0)
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/plugin.py", line 24, in <module>
     import oauth2
 ModuleNotFoundError: No module named 'oauth2'
frafra commented 2 years ago

@simao-silva that is a missing dependency that you haven't installed. If you cannot test a previous version, then it is hard to determine if you are hitting a regression.

simao-silva commented 2 years ago

@frafra I believe you are referring to python-oauth2. Still, if installed, I get the same error but if I install the extension using the code from this PR the error No module named 'oauth2' no longer appears. This only happens when using Python3. When using Python2, the version 0.7.0 works fine and does not need python-oauth2.

frafra commented 2 years ago

@simao-silva try https://github.com/frafra/ckanext-oauth2/tree/flask_conversion_and_ckan_auth revision 01da0474c4f3f07edd5fba1a324168864ba4d86c from branch. It contains a previous snapshot of this PR.

frafra commented 2 years ago

@aitormagan any feedback on this PR?

simao-silva commented 2 years ago

@simao-silva try https://github.com/frafra/ckanext-oauth2/tree/flask_conversion_and_ckan_auth revision 01da047 from branch. It contains a previous snapshot of this PR.

@frafra that revisions does not forward to the oauth2 endpoint automatically when clicking on login. Still, if I correct that issue, the response from the authentication provider (Keycloak) throws the following error:

 2022-07-18 19:52:44,112 ERROR [ckan.config.middleware.flask_app] 'Request' object has no attribute 'GET'
 Traceback (most recent call last):
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/views.py", line 58, in callback
     user_name = oauth2helper.identify(token)
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/oauth2.py", line 131, in identify
     access_token = bytes(token['access_token'])
 TypeError: string argument without an encoding

 During handling of the above exception, another exception occurred:

 Traceback (most recent call last):
   File "/usr/lib/python3.8/site-packages/flask/app.py", line 1949, in full_dispatch_request
     rv = self.dispatch_request()
   File "/usr/lib/python3.8/site-packages/flask/app.py", line 1935, in dispatch_request
     return self.view_functions[rule.endpoint](**req.view_args)
   File "/srv/app/src/ckanext-oauth2/ckanext/oauth2/views.py", line 69, in callback
     error_description = toolkit.request.GET.get('error_description')
   File "/usr/lib/python3.8/site-packages/werkzeug/local.py", line 347, in __getattr__
     return getattr(self._get_current_object(), name)
   File "/usr/lib/python3.8/site-packages/werkzeug/local.py", line 347, in __getattr__
     return getattr(self._get_current_object(), name)
 AttributeError: 'Request' object has no attribute 'GET'
frafra commented 2 years ago

@simao-silva I currently use revision 3633240df15112af591fbd7f87739681acf1af5a; you could try that one.

frafra commented 2 years ago

@aitormagan is unresponsive, and this is critical issue for the project, which seems dead to me. If nothing changes, we should just maintain a fork elsewhere. Any thoughts on that?

aitormagan commented 2 years ago

I am not unresponsive... I reviewed the code and highlighted things that must be changed before the PR can be merged... In addition, tests are required to be adapted before the PR can be approved.

BR Aitor

frafra commented 2 years ago

@FedericOldani there are some required small changes that need to be made before having your PR being accepted; do you have the time to make them?