Can't generate a valid `write` token from IResourceController context

[tomeksabala]

How the issue was encountered:

I'm trying to create a wrapper on resource create and update CKAN actions in order to support giftless storage in our CKAN implementation. I would like to keep CKAN api unchanged and hence keep giftless presence transparent to our API users.

The issue

authz_authorize action returns an empty list of granted scopes, despite user passed in the context is an admin of dataset's organization:

# IResourceController
    def before_create(self, context, data_dict):
      # 1. get authz token
      authorize = toolkit.get_action('authz_authorize')
      scope = 'obj:{}/{}/*:write'.format(org_name, dataset_name)
      authz_result = authorize(context, {"scopes": [scope]}) <<< this returns empty list with granted scopes
      # 2. use the token and giftless-client to upload file
      # [...]
      # 3. update resource dict with blob-storage related metadata and proceed with resource create
      # [...]

Debugging summary

After some debugging I found out that the authorize action at some point is calling ckanext.authz_service.authz_binding.common.ckan_auth_check

def ckan_auth_check(permission, data_dict=None):
    try:
        toolkit.check_access(permission, get_user_context(), data_dict=data_dict)
    except (toolkit.NotAuthorized, toolkit.ObjectNotFound):
        return False
    return True

which depends on ckanext.authz_service.authz_binding.common.get_user_context

def get_user_context():
    context = dict(model=model)
    if hasattr(g, 'user'):
        context['user'] = g.user
    if hasattr(g, 'userobj'):
        context['auth_user_obj'] = g.userobj
    return context

get_user_context creates a context object with user information from scratch with help of ckan.common.g - a proxy to flask global namespace.

I think that IResourceController is living outside of flask context (with debugger I've checked that ckan.common.g lacks user and userobj attributes). I also think that authz_authorize action should fully respect the context which is passed to it as a parameter and in should be passed to any subsequent ckan action call (including the toolkit.check_access inside ckanext.authz_service.authz_binding.common.ckan_auth_check.

Do you think it's a bug in authz-service? If not how could I get the token from withing IResourceController.
Thank you in advance for your help.

[tomeksabala]

@shevron or @pdelboca I would really welcome your opinion on that if you have some time to spare for it.

[shevron]

@tomeksabala while I never liked the custom context thing, it was kind of an initial design limitation of authzzie (the underlying permission binding library) as I was trying to make it framework agnostic and not bound to any CKAN APIs. The original thought was that it gets some string representing a scope, and based on (generic) bound callbacks, will tell you if the scope is granted or not. This works well until it fails when there's a need to customize the user context.

I gave this a bit more thought today (sorry but I am working on other projects and have little time to invest here lately...), and came up with this approach to tweak the authzzie API to accept and pass down custom keyword arguments: https://github.com/datopian/ckanext-authz-service/compare/experiment/allow-passing-args-to-authorizers - this is far from a full implementation, but I think this diff will showcase the approach I was thinking on.

A full implementation of this will require that all bound callbacks are aware of all potential keyword arguments that might be passed in when requesting a related scope - e.g. by accepting **kwargs or by accepting specific arguments. In puts more burden on developers, but only in cases where special arguments need to be passed around (like with CKAN) - the default case can remain as simple as it is today.

It also means that changes to other CKAN extensions that rely on ckanext-authz-service, such as ckanext-blob-storage and potentially others, will need to be updated to use this.

Let me know what you think about this approach and if it looks like a reasonable direction, I will spend a few more hours in the next days implementing it.

I'm wondering if this can also simplify the work on supporting older revisions / activity IDs. @pdelboca would love your input on this as well, as I know you might have encountered similar limitations.

[tomeksabala]

Thank you very much @shevron I do not have a full understanding of the big picture of this extension. Your comment was very detailed and helped me a lot. My opinion, however, should be taken with a grain of salt :)
As I understand the context dict in CKAN, it is supposed to carry db context (model & session) and user context between different pieces of the code. As I've learned while debugging this very issue it's especially needed with some interfaces implementations (like mentioned IResourceController) which are existing outside of Flask context (so without the access to global flask namespace g).
ckanext-authz-service provides authorize action which accepts context dict as many other CKAN actions. It should be respected all the way down with any other internal CKAN actions call and passed as a param.

The original thought was that it gets some string representing a scope, and based on (generic) bound callbacks, will tell you if the scope is granted or not. This works well until it fails when there's a need to customize the user context.
I understand the desire to keep authzzie a CKAN agnostic library. However the "generic bound callback" in this case is depends on ckan_auth_check which requires "the infamous context dict CKAN flow".

I have reviewed your experimental branch and it looks like a decent solution to the problem. I would love to see this implemented in ckanext-authz-service. Additional support of kwargs in authzzie looks a little fuzzy but I have no idea on how this could be done better.
Let me know if I could be of any help in getting this change ready. Lack of it is a blocker for my work so in the meantime I was thinking to call the authorize action from within IResourceController via HTTP (so via flask) with proper user context. Do you have any other ideas for a better temporary fix of the issue?

[shevron]

@tomeksabala I've created a PR that fully implements this fix: #25 - it is not merged yet as I am waiting for a little bit of feedback but I think it will be merged in the next couple of days.

I think this should solve the context problems, as it means context will be passed down from CKAN actions all the way to authorization callbacks.

If you've overwritten any auth callbacks (e.g. in ckanext-blob-storage or any extension that implements IAuthorizationBindings) you may need to update it to ensure any callbacks accept the context keyword argument so it can be provided. Note that if you have any extensions that override bindings and have not pinned the version of ckanext-authz-service you use to 0.1.x, you may see some breakage once this PR is merged.

[shevron]

@tomeksabala that PR is now merged, would appreciate it if you can verify that it addresses the problem and if so close this issue.

[tomeksabala]

@shevron Finally I've managed to get back to this. I've checked out your changes and it worked like a charm. Thanks!
As you suggested I needed to put a small update to ckanext-blob-storage so it supports new kwargs context. I've created a draft PR to blob-storage but I'm not sure if you guys would like to get it merged: datopian/ckanext-blob-storage#59

Blob storage is still configured to user authz-service v1.2.0 which is quite old. Would upgrading the version to latest be a breaking change for a lot of your stuff?