Skip to content
This repository has been archived by the owner on Mar 11, 2019. It is now read-only.

Authentication Enhancement #20

Open
NilsG-S opened this issue Jan 9, 2017 · 14 comments
Open

Authentication Enhancement #20

NilsG-S opened this issue Jan 9, 2017 · 14 comments

Comments

@NilsG-S
Copy link
Contributor

NilsG-S commented Jan 9, 2017

In Progress:

  • Clarify instructions for passwords
  • Have password confirmation check onSubmit? (one doesn't have to click out of the password confirmation box to hit submit)
  • Switch to server API
  • Put server error in Tip component (Login.js and Register.js)
  • Put field instructions in Tip component
    • Student ID
    • Password
    • Change inputChecking to handleBlur

Completed/Tabled:

  • Input checking and highlighting for incorrect fields
    • Email - done
    • Password - done
    • Student ID - done
    • Prevent user from submitting if there are errors (disable button by not passing handleSubmit?) - done
    • Move input checking to onBlur event handler from componentDidUpdate to prevent unnecessary checks - done
    • Clear confirm password when new password entered - done
    • Clear student id field/error when role changed - done
    • Prevent empty fields - done
    • Clear errors onFocus - done
  • Confirm password field - done
  • Snackbars/notifications for errors - done
  • Split Auth.js into Login.js and Register.js for clarity purposes - done
  • Implement tests
    • Unit tests - noted
    • Pure exports - noted
    • Auth.js - done
    • AuthContainer.js - what needs to be tested?
    • Login.js - done
    • Register.js - done
    • RequireAuth.js - I couldn't get the HOC to test properly
  • Add propTypes - noted
  • Prevent multiple submits on login/register - done
  • Switch to new logging framework - done

Input Checking:

  • Email: has to end in @ttu.edu
  • Password: has to contain at least 8 ASCII characters, with at least 1 number, 1 uppercase, 1 lowercase, and 1 special character (not in {A-Z, a-z, 0-9}). There is no maximum password length.
  • Password confirmation: must match password
  • Student ID: has to contain exactly 8 numbers
@asclines
Copy link
Collaborator

asclines commented Jan 9, 2017

Perhaps we could even do some stuff client-side? Like checking if the email is a valid email before even sending a request to the backend?

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Jan 9, 2017

That's sort of what I was thinking. It might actually be easier than parsing the error messages.

@asclines
Copy link
Collaborator

asclines commented Jan 9, 2017

Maybe, the errors received from Firebase are designed to be parsed so it might not be too bad.

@NilsG-S NilsG-S self-assigned this Feb 28, 2017
@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

We should discuss what input values will be considered valid for email, password, and student ID.

@asclines
Copy link
Collaborator

@NilsG-S what do you mean?

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

@asclines
I haven't found any clean ways of checking whether emails are valid, so I'm not sure what criteria we will be using. Should we just check that it ends in @ttu.edu?

We're no longer using Firebase, so our password standards aren't already determined for us. We'll need to decide what passwords are acceptable.

Student IDs should be easier, aren't they all the same length?

@asclines
Copy link
Collaborator

@NilsG-S
For the emails we could just have an automated email verification? Or some parser that checks for valid email.

Student IDs will be easier unless we want to check that it is a legit Student ID but that might be better left to the admins to check.

As for passwords, good point does @the-pat have an opinion?

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

@asclines Yeah, the link I put in my comment has instructions for how to use regular expressions for email verification. I'm just not sure what emails we will be accepting. If we just use TTU email addresses it'll be relatively easy to make a regex that verifies them.

I agree on letting the admins check whether student IDs are legit.

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

Okay, so @mariots used his contacts to get some information on how TTU email addresses are structured. For the most part it's as easy as I thought. Basically they're dot separated names, which can include hyphens. If there is a name collision the middle initial is added. If there is a middle initial collision, the whole middle name is added. If there is a collision after that, we're not really sure what happens.

Luckily the ending will always be @ttu.edu for students and administrators.

The more serious issue is that there is a way for people to make a custom email address. If we want to handle all cases, we might have to use a general regex.

@asclines
Copy link
Collaborator

would we really need much more validation than checking @ttu.edu?

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

That would work for me

@the-pat
Copy link

the-pat commented Feb 28, 2017

@asclines and @NilsG-S

Some general password policies are:

  • minimum length of 8 characters
  • allow all valid ASCII characters

How do y'all feel about enforcing a password with at least one upper-case, lower-case, number, and special character?

Also, I don't like the idea of a maximum password length, but the maximum BSON size is 16 megabytes. If we want to impose a maximum password length, I'd want to lower bound to be at least 64 characters.

Thoughts?

@asclines
Copy link
Collaborator

I have no real preference on this so I'll defer my opinion to @NilsG-S

@NilsG-S
Copy link
Contributor Author

NilsG-S commented Feb 28, 2017

@the-pat What do you mean by special characters? Anything that isn't a word character (A-Z, a-z, 0-9, and underscore)?

UPDATE
I defined special character as anything that isn't in the set of {A-Z, a-z, 0-9}

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

No branches or pull requests

3 participants