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

Change sentence to ensure app password name letters are English #973

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xnuk
Copy link

@xnuk xnuk commented Jul 5, 2023

No description provided.

@pfrazee
Copy link
Collaborator

pfrazee commented Jul 5, 2023

Is this the case? I just glanced through the server code and didnt see this constraint

@pfrazee pfrazee added the x:discussing We've seen the request and we're talking about it! label Jul 5, 2023
@xnuk
Copy link
Author

xnuk commented Jul 6, 2023

Is this the case? I just glanced through the server code and didnt see this constraint

You mean this client-side constraint right?

// sanitize input
// we only all alphanumeric characters, spaces, dashes, and underscores
// if the user enters anything else, we ignore it and shake the input container
// also, it cannot start with a space
if (text.match(/^[a-zA-Z0-9-_ ]*$/)) {

@pfrazee
Copy link
Collaborator

pfrazee commented Jul 6, 2023

Oh interesting, right. @ansh do you remember why we have those rules?

@ansh
Copy link
Contributor

ansh commented Jul 6, 2023

Yeah, I put that in there just so to prevent users from shooting themselves in the foot since we had no server-side validation as you noted. @pfrazee

@pfrazee
Copy link
Collaborator

pfrazee commented Jul 7, 2023

Huh -- is there a reason we have that constraint though?

@ansh
Copy link
Contributor

ansh commented Jul 18, 2023

@pfrazee No reason other than people not putting weird app password names and breaking things. We can revert it.

@pfrazee
Copy link
Collaborator

pfrazee commented Jul 19, 2023

@ansh yeah if we have any specific ways we know it'll break things, we should keep it. I just can't think of any

@ansh
Copy link
Contributor

ansh commented Jul 19, 2023

@pfrazee Nothing specific. I was just being overly cautious. Should we remove it, then?

@pfrazee
Copy link
Collaborator

pfrazee commented Jul 24, 2023

Yeah I think so?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:discussing We've seen the request and we're talking about it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants