Landing page enhancements #43
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Give tasks padding to not overlap on mobile, keep email input and button bigger, and update image sizes
This was
linked to
issues
Feb 4, 2020
|
[diff-counting] Significant lines: 80. |
SamChou19815
suggested changes
Feb 4, 2020
src/components/Login.vue
Outdated
Comment on lines
178
to
193
| checkEmailAccess(snapshot, user) { | ||
| let isAlphaEmail = false; | ||
| snapshot.forEach(doc => { | ||
| if (doc.data().email === user.user.email) { | ||
| isAlphaEmail = true; | ||
| } | ||
| }); | ||
| this.performingRequest = false; | ||
| if (!isAlphaEmail) { | ||
| fb.auth.signOut(); | ||
| alert('Sorry, but you do not have alpha access.\nPlease sign up below for email updates on when the platform is available and for a chance to test the platform early.'); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| }, |
There was a problem hiding this comment.
This looks bad. You are implicitly exposing the alpha whitelist email to the entire world, since you need to loop over the entire snapshot.
There was a problem hiding this comment.
My suggestion:
- Write firebase security rules so that user can only access his/her own whitelist document
- Try to access that document
- If caught insufficient permission error, then we know that the user is not whitelisted.
With this approach, attacker
- cannot fetch the entire email list
- cannot know whether someone with a specific email is whitelisted, because the security rules match document against auth information, not query parameter.
handotdev
requested changes
Feb 9, 2020
Change whitelist to only allow a user to read their own document
SamChou19815
approved these changes
Feb 13, 2020
…dti/course-plan into landingPageEnhancements
handotdev
approved these changes
Feb 13, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request deals with landing page issues, mainly #39 to alert users if they do not have alpha access, and #37 to fix mobile styling of the task images overlapping with text.
Test Plan
Look over the landing page on a variety of breakpoints to ensure everything looks fine. Also ensure emails without access are given an alert after attempting to login.