ckanext-oauth2 icon indicating copy to clipboard operation
ckanext-oauth2 copied to clipboard

flask and python3 conversion

Open FedericOldani opened this issue 3 years ago • 21 comments

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.

FedericOldani avatar Jun 10 '21 16:06 FedericOldani

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.

frafra avatar Feb 15 '22 07:02 frafra

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

FedericOldani avatar Feb 16 '22 10:02 FedericOldani

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.

frafra avatar Feb 16 '22 11:02 frafra

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.

jeverling avatar Feb 16 '22 20:02 jeverling

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!

FedericOldani avatar Feb 17 '22 10:02 FedericOldani

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

jeverling avatar Feb 17 '22 11:02 jeverling

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

frafra avatar Apr 01 '22 19:04 frafra

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

frafra avatar Apr 07 '22 11:04 frafra

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

frafra avatar Apr 13 '22 08:04 frafra

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

simao-silva avatar May 27 '22 23:05 simao-silva

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

frafra avatar May 28 '22 20:05 frafra

@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'

simao-silva avatar May 29 '22 00:05 simao-silva

@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.

frafra avatar May 29 '22 10:05 frafra

@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.

simao-silva avatar May 29 '22 14:05 simao-silva

@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 avatar Jul 14 '22 16:07 frafra

@aitormagan any feedback on this PR?

frafra avatar Jul 17 '22 07:07 frafra

@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'

simao-silva avatar Jul 18 '22 20:07 simao-silva

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

frafra avatar Aug 14 '22 15:08 frafra

@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?

frafra avatar Aug 14 '22 15:08 frafra

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

aitormagan avatar Aug 16 '22 10:08 aitormagan

@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?

frafra avatar Oct 10 '22 16:10 frafra