mozilla / openbadges-backpack

Mozilla Open Badges Backpack
https://backpack.openbadges.org/
Other
862 stars 263 forks source link

Backpack email address letter casing #1130

Closed mattdigitalme closed 7 years ago

mattdigitalme commented 7 years ago

User created account on Backpack with capitalisation - and can log in fine using this capitalised email address. However when he tries to push a badge to the BP from an issuer it automatically converts his email to lower case (is this expected behaviour?)

Also is then unable to push - receives the 'You did not add any badges' messages

@auralon the casing issue, flagged a number of times on here should be up on our list, as and when we can!

Further testing needed of pushing to BP using this email address including on OBA - @omidmufeeddm and John be good to catch up re: this

auralon commented 7 years ago

@mattdigitalme it's an interesting issue. If we're lowercasing the email on push to Backpack, then we probably want to lowercase it when we store it in the database and process login. Really need to investigate and test though.

mattdigitalme commented 7 years ago

@auralon Indeed - first time I've seen it change how the email was presented!

ottonomy commented 7 years ago

Badgr has helped users with this problem by storing all email records lowercase and creating a complementary model that relates "known case variants" to those email addresses. At various times, we can discover a known case variant for a user, and then when that user attempts to upload badges.

Being able to interface with the email addresses for users as other systems know them is one of the most prominent feature requests for openbadges-backpack over the years (case sensitivity issues & multiple email addresses for accounts).

ahripak commented 7 years ago

Thanks @auralon for pointing me here—didn't think to search the issues for email casing.

Similar to Badgr, Credly is downcasing email addresses before hashing against them and in the instances where we auto share Credly badges into the Backpack we store the exact version of the email address returned with the /displayer/convert/email response.

It's great to see that you're considering lowercase by default, perhaps this recommendation might make its way into the spec (https://openbadgespec.org/#IdentityObject) and into the wiki that describes how to hash in various languages (https://github.com/mozilla/openbadges-backpack/wiki/How-to-hash-&-salt-in-various-languages.).

As I wrote in #1126, it would be neat to see error messaging when the hashes don't match, which would make triaging users facing this issue a bit easier. Happy to lend a hand on a pull request unless this is being addressed in the near term.

auralon commented 7 years ago

@ahripak it would be good to see this in the spec, for sure! Feel free to send a pull request with a patch, no-one is working on this issue just yet. Did you find that php wrapper I sent over useful? I'd like to go over the Credly Backpack implementation with you at some point, if possible?

ahripak commented 7 years ago

@auralon yes, quite helpful to see those endpoints used in a PHP context. I think you have my email, so we can schedule some time to chat there if you'd like.

mattdigitalme commented 7 years ago

We've built in an email letter casing variant 'checker' during the latest build and update (4.7.17): https://github.com/mozilla/openbadges-backpack/commit/bca2242c25b4933329a07609d90e63d45974c27b