Failure on U2F verify

[d3cline]

ERROR 2020-06-13 19:08:23,133 228 log Internal Server Error: /mfa/u2f/verify
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.UndefinedFunction: function json_extract(text, unknown) does not exist
LINE 1: ..."owned_by_enterprise" FROM "mfa_user_keys" WHERE (JSON_EXTRA...
                                                             ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.6/site-packages/mfa/U2F.py", line 107, in verify
    x= validate(request,request.session["base_username"])
  File "/usr/local/lib/python3.6/site-packages/mfa/U2F.py", line 55, in validate
    key=User_Keys.objects.get(username=username,properties__shas="$.device.publicKey=%s"%device["publicKey"])
  File "/usr/local/lib/python3.6/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 411, in get
    num = len(clone)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 258, in __len__
    self._fetch_all()
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 1261, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 57, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/compiler.py", line 1152, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.6/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.6/site-packages/django/db/backends/utils.py", line 86, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: function json_extract(text, unknown) does not exist
LINE 1: ..."owned_by_enterprise" FROM "mfa_user_keys" WHERE (JSON_EXTRA...
                                                             ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

I tried tracking this down, but I am not sure whats the best step. Its this line But I am not sure what about that queryset is failing here exactly. Would it be possible to write this queryset without using this import? Looking up the module it appears to be a MySQL module,

https://pypi.org/project/jsonLookup/

Can this be done database agnostic? (assuming that is the problem)

[d3cline]

P.S. I don't mind attempting to re-write it myself, I am mostly asking about context/intent. If it is possible to rewrite in a database agnostic way would this be alright?

[d3cline]

Been thinking about this today. I think a good solution might be to write it using an if statement for django.db.connection.vendor. If its Mysql we can use the queries in your module, if its postgres we can use the lookup methods in the Django docs. If this is an acceptable solution I can start working on it.

[mkalioby]

Hello John,

Sorry for slow response, but i was switching places.

This type of query exists in most methods and we need to handle it, so what about writinh a function that returns the used key, so we can handle all dbs in one function, using the vendor trick you mentioned

[d3cline]

Sounds good, I will work on this in the next few days and should have something soon. P.S. I have been integrating this module pretty well and I like it a lot, thanks for working on the updates etc. I have some other ideas about JSON integration but will post them once I have everything working and can post example code.

[d3cline]

I spent some more time digging into this and I made the following discoveries,

1. The jsonfield module has its intent documented about these kinds of lookups vs the native jsonfield methods, tldr its more DB agnostic, at the cost of lookup speed. I don't believe for this project and this use case we need the advanced lookups.

2. There are ways to use regex to do the search which are DB agnostic if we keep jsonfield and don't replace with natives and use the if statement.

3. We always could go with ifs at the model level and downstream, but this gets to be kinda crazy.

IMO the most elegant solution is to convert the lookup to use regex,

key = User_Keys.objects.filter(username=username, properties__iregex=rf'{device["publicKey"]}')

By still invoking username these keys should be unique. Some more testing needs to be done, but I was able to get my U2F key to auth in our staging environment tonight with this change. I believe based on the above its the best approach to fix the widest use cases.

[d3cline]

I did a simple grep and there are other places shas is in place, I know its your module and the code looks good, but being locked to MySQL alone IMO we should try to convert all of them to DB agnostic, how do you feel about this?

[mkalioby]

What do you think about extending jsonLookups for other db vendors? so we can fix the module with this trick so it will work here

Thanks

On Tue, Jun 16, 2020, 07:27 John Spounias [email protected] wrote:

I did a simple grep and there are other places shas is in place, I know its your module and the code looks good, but being locked to MySQL alone IMO we should try to convert all of them to DB agnostic, how do you feel about this?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mkalioby/django-mfa2/issues/11#issuecomment-644523645, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPOPRD2HWULL2R4WDGCRJLRW3YDHANCNFSM4N5DPB6A .

[d3cline]

I basically see 2 options, go with the current jsonfield module and use its design methodology (DB agnostic/simple string lookups using regex) or go with the native ones and go with their design methodology, (DB specific code, faster lookups). IMO for this use case option 1 is a lower hanging fruit, and it fits the bill. The lookup in this case is a large string/key which is unique, so the regex will work, and bakes-in the DB agnostic stuff, which 'just works' downstream. I do like the idea of expanding the jsonLookup module, but just not using it here as the use cases diverge a bit. I do see potential use cases for jsonLookup for large json fields, or the requirement of more detailed querysets. Its possible the other code which uses shas requires it, I haven't taken a look.

tldr for this 1 line I feel the regex is better, I wouldn't mind still contributing to the other module if I get the chance, it has merit.

[mkalioby]

Thank you.

There is a third way which is getting all the user keys from the specific type in django and doing loop.on python level as the user won't have alot of keys so it won't be slow.

What do you think?

Thanks Mohamed

On Tue, Jun 16, 2020, 08:38 John Spounias [email protected] wrote:

I basically see 2 options, go with the current jsonfield module and use its design methodology (DB agnostic/simple string lookups using regex) or go with the native ones and go with their design methodology, (DB specific code, faster lookups). IMO for this use case option 1 is a lower hanging fruit, and it fits the bill. The lookup in this case is a large string/key which is unique, so the regex will work, and bakes-in the DB agnostic stuff, which 'just works' downstream. I do like the idea of expanding the jsonLookup module, but just not using it here as the use cases diverge a bit. I do see potential use cases for jsonLookup for large json fields, or the requirement of more detailed querysets. Its possible the other code which uses shas requires it, I haven't taken a look.

tldr for this 1 line I feel the regex is better, I wouldn't mind still contributing to the other module if I get the chance, it has merit.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mkalioby/django-mfa2/issues/11#issuecomment-644542050, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPOPRH62K5DUTDZJI6NJVTRW4ANFANCNFSM4N5DPB6A .

[d3cline]

I thought about that too, loading the entire json string into memory might be a hit, having the DB do it is likely better. With some example code it could be possible to compare it.

[mkalioby]

Ok, let me open a branch and see how will it work

Thanks Mohamed

On Tue, Jun 16, 2020, 08:43 John Spounias [email protected] wrote:

I thought about that too, loading the keys into memory might be a hit, having the DB do it is likely better. With some example code it could be possible to compare it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mkalioby/django-mfa2/issues/11#issuecomment-644543656, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPOPRBDJKR33I6HURXYIKTRW4BBNANCNFSM4N5DPB6A .

[mkalioby]

Lets try the provider trick

[d3cline]

I will get to this in the next day or so, thanks for waiting.

[d3cline]

OK, https://github.com/mkalioby/django-mfa2/pull/15