cube icon indicating copy to clipboard operation
cube copied to clipboard

Assigning to security context in check_auth doesn't work in Python

Open igorlukanin opened this issue 1 year ago • 2 comments

Describe the bug Apparently, assigning to security context works in check_auth in JavaScript but doesn't work in Python.

To Reproduce Run the following cube.py snippet:

from cube import config
 
@config('query_rewrite')
def query_rewrite(query: dict, ctx: dict) -> dict:
  print("Query rewrite started")
  print(ctx)
  print("Query rewrite finished")
  return query

@config('check_auth')
def check_auth(ctx: dict, token: str) -> None:

  ctx['securityContext'] = {
    "sub": "1234567890",
    "iat": 1516239022,
    "user_id": 42
  }

  context = ctx['securityContext']

  print(context)
 
  if not context:
    raise Exception('Access denied')
  return ctx

Result: Screenshot 2024-03-13 at 15 41 46

Expected behavior Assigning to security context works in Python, 42 is printed as user_id inside the security context.

Version: 0.35.10

Additional context We can fix this in a backwards-compatible way as follows:

  • Similarly to check_sql_auth, support returning security context from check_auth both in JavaScript and Python.
  • If nothing is returned, fall back to the current behavior.

igorlukanin avatar Apr 11 '24 22:04 igorlukanin

Hey @ovr @igorlukanin Has there been any work on this issue yet? Are there any workarounds?

sabinevidal avatar Jun 26 '24 09:06 sabinevidal

Are there any updates on this bug?

jcarlos92 avatar Jun 26 '24 15:06 jcarlos92

There's no ETA that I can share as of now. I recommend switching your cube.py to cube.js syntax as a workaround; you'll be able to convert it to Python when this issue is resolved.

igorlukanin avatar Jul 04 '24 15:07 igorlukanin

@jcarlos92 @sabinevidal FYI, we have fixed this one in the linked PR and this is going to land in the next release. Please stay tuned!

igorlukanin avatar Aug 29 '24 13:08 igorlukanin

I've just tested this with Cube v0.36.2 and it works as expected. Closing as resolved then.

igorlukanin avatar Sep 25 '24 17:09 igorlukanin