Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support email usernames in API access validation #63

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

flowln
Copy link
Contributor

@flowln flowln commented Feb 16, 2024

Change the JSON schema to accept ., @, -, and +, in usernames.

Description

In our LDAP implementation, our email address can be used in the place of the username, and that includes both . and @ in them. Of course, an email can have other symbols still, but those two cover the vast majority of cases already.

So, to use DictionaryAPIAccessControl authorization together with our LDAP-based authentication, this change is welcome. Also, I can't see how accepting those characters could be an issue, though I know next to nothing about that stuff, so please boop me if I'm wrong 🙂

Summary of Changes for Release Notes

Added

Added ., @, -, and +, to the allowed character list of BasicAPIAccessControl and DictionaryAPIAccessControl username validations.

How Has This Been Tested?

We tested this change in our local test deploy of HTTP server, and after those changes, we managed to successfully authenticate a user using their email address.

In our LDAP implementation, the username is our email address,
which includes both '.' and '@' in them. Of course, an email can
have other symbols still, but those two cover the vast majority
of cases already.

Signed-off-by: Thiago Donato Ferreira <thiago.ferreira@lnls.br>
bluesky_httpserver/authorization/api_access.py Outdated Show resolved Hide resolved
bluesky_httpserver/authorization/api_access.py Outdated Show resolved Hide resolved
@flowln flowln force-pushed the email_username_support branch 2 times, most recently from 6666370 to feb124b Compare February 19, 2024 15:06
@dmgav
Copy link
Contributor

dmgav commented Feb 19, 2024

I think it should be \\\\-\\\\+.

@flowln flowln force-pushed the email_username_support branch from feb124b to 3310eec Compare February 19, 2024 17:44
These characters is also allowed in emails.

Co-authored-by: Dmitri Gavrilov <46980826+dmgav@users.noreply.github.com>
Signed-off-by: Thiago Donato Ferreira <thiago.ferreira@lnls.br>
@flowln flowln force-pushed the email_username_support branch from 3310eec to 276d6da Compare February 19, 2024 17:52
@dmgav dmgav merged commit 39dc3c8 into bluesky:master Feb 19, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants