paradimdata / project_chameleon

Repository for Project Chameleon conversion scripts
MIT License
4 stars 1 forks source link

common_handler_access_token TODO #18

Open pcauchy1 opened 5 days ago

pcauchy1 commented 5 days ago

From line 94 in api.py

TODO: Maybe add a flag in the request JSON and only do this if requested to do so?

opener = urllib.request.build_opener() opener.addheaders = [('X-Auth-Access-Token', str(access_token))]

TODO: does this play well with async?

What is the benefit of adding a flag here? Also what does "play well with async" mean in this context?

tmcqueen-materials commented 4 days ago

@pcauchy1 : Without the flag, access tokens are visible/collectible by the upstream url. This is OK if you trust the upstream URI to not do anything naughty with them (which would be true for all URL sources within the PARADIM data infrastructure), but may not be desired if the upstream location is an "untrusted" source (aka an upstream server not under the PARADIM umbrella).

So I suggest adding another optional JSON parameter, input_uri_access_token_header, which controls this. If set to a non-empty value (e.g. if set to "X-Auth-Access-Token"), then add a header to the input_uri request with header name as specified by input_uri_access_token_header. If set to "", then do not include the access token when communicating upstream. The right default value is probably "", i.e. in order to pass the access token upstream, a value for this parameter needs to be provided.

Regarding "play well with async": urllib.request.install_opener(opener) installs the opener process-wide. But the function that handle incoming requests are async (i.e. their execution can be interleaved). So what happens if, say, there are two simultaneous calls to brukerrawconvert, and call 1 updates the opener, but is then paused before requesting the input uri, and then call 2 updates the opener before execution returns to call 1. Does call 1 now use the access token from call 2 when requesting the input URI?

My reading of the python documentation is that this is "not a problem right now," because the common handlers are not declared async, and in none of the async functions are their yield statements in between the call setting the opener and the call downloading the input uri. But I have not tested that to confirm. And if you convert some of the common functions to async in the future, then you may have to adjust how this works.

pcauchy1 commented 1 day ago

Adjusted communication upstream and added input_url_access_token_header in api_helpers.py starting on line 90. Please double check this. I think what I did is accomplishing what you asked for but I am definitely not sure.

tmcqueen-materials commented 12 minutes ago

@pcauchy1 : Looks good to me!