Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

datastore: get user by numeric identity attribute - #633

Open
jirikuncar wants to merge 1 commit into
pallets-eco:developfrom
jirikuncar:608-get_user
Open

datastore: get user by numeric identity attribute#633
jirikuncar wants to merge 1 commit into
pallets-eco:developfrom
jirikuncar:608-get_user

Conversation

@jirikuncar

Copy link
Copy Markdown
Contributor

* BETTER Supports login by a numeric identity attribute (e.g. phone
  number, social security number).

Signed-off-by: Jiri Kuncar <jiri.kuncar@gmail.com>
@mattupstate

Copy link
Copy Markdown
Member

This feature seems to fill a very narrow use case. Can anyone provide a reasonable explanation to justify adding this feature?

@jirikuncar

Copy link
Copy Markdown
Contributor Author

There are 2 use-cases, I have seen when browsing through the issues:

  1. Alternative numeric attribute can not be used as it will be always considered a PK.
  2. Non-numeric PK on user table (needs tests).

@kitotik

kitotik commented Jul 14, 2017

Copy link
Copy Markdown

Just bumping this as logging in with mobile number is a fairly common use-case in my experience.

@fmorato

fmorato commented May 7, 2019

Copy link
Copy Markdown

The implementation does seem to exclude some use cases where codes are not only numeric or did I understand the code wrongly? ie. such as social security code in Finland: http://id-check.artega.biz/info-fi.php

@jwag956

jwag956 commented May 8, 2019

Copy link
Copy Markdown
Collaborator

@fmorato I think this works albeit a bit non-optimal. It first does a .get() which will look at the primary key. If it finds it (i.e. you are looking up by user.id or have set your UserModel up to have a different PK) - then all is good. If not, it will go through each of the 'identity_attributes' and try.

Note that as currently written this PR has a bug I recently found - various DB drivers throw errors if the types don't match (such as passing a string to an integer field). In the mongo and peewee and pony - those types of errors are caught - but not std sqlachemy - I am using psycopg2 and just got the:

sqlalchemy.exc.DataError: (psycopg2.DataError) invalid input syntax for integer: "test@sywphoto.com"
LINE 3: WHERE "user".id = 'test@sywphoto.com'

More testing - there are numerous issues here, as well as performance issues.
First the issue - using psycoppg2 - if the first 'get' fails - the transaction is marked as aborted - so further queries all fail (thus the fall through doesn't work in this case).

I am getting concerned that the get_user() that tries to find users based on a series of queries is fraught with issues.

@fmorato can you give a bit more info - did you replace the user_model ID with your finland ID ? or did you just add a new field?

@jwag956

jwag956 commented May 11, 2019

Copy link
Copy Markdown
Collaborator

I have a PR in my fork that I think addresses the various issues. I have tested it for all 3 ORMs (sqlachemy, pony, peewee) and against sqlite, postgres, mysql.

pallets-eco/flask-security#73

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

login with numeric field

5 participants