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

Landing page enhancements #43

Merged
merged 7 commits into from
Feb 13, 2020
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 32 additions & 44 deletions src/components/Login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@
<div class="col-12 col-md-6 tasks-wrapper">
<div class="row tasks">
<div class="col-1 tasks"><img src="@/assets/images/Task1.svg" alt = "checklist"/></div>
<div class="col-11"><p class= "sub">Fully personalized to track your requirements</p></div>
<div class="col-11"><p class= "sub sub--task">Fully personalized to track your requirements</p></div>
</div>
<div class="row tasks">
<div class="col-1 tasks"><img src="@/assets/images/Task2.svg" alt = "browser" /></div>
<div class="col-11"><p class= "sub">Customizable interface to view your courses</p> </div>
<div class="col-11"><p class= "sub sub--task">Customizable interface to view your courses</p> </div>
</div>
<div class="row tasks">
<div class="col-1 tasks"><img src="@/assets/images/Task3.svg" alt = "Network" /></div>
<div class="col-11"><p class= "sub">Built-in system to check your progress</p></div>
<div class="col-11"><p class= "sub sub--task">Built-in system to check your progress</p></div>
</div>
<div class="row tasks">
<div class="col-1 tasks"><img src="@/assets/images/Task4.svg" alt = "Starred comment" /></div>
<div class="col-11"><p class= "sub">Recommends courses based on your needs</p> </div>
<div class="col-11"><p class= "sub sub--task">Recommends courses based on your needs</p> </div>
</div>
</div>
<div class="col-6 col-md-6 image-wrapper women-wrapper">
Expand Down Expand Up @@ -152,35 +152,6 @@ export default {
};
},
methods: {
login() {
this.performingRequest = true;
fb.auth
.signInWithEmailAndPassword(this.loginForm.email, this.loginForm.password)
.then(user => {
alphaWhitelistCollection.get().then(querySnapshot => {
let isAlphaEmail = false;
querySnapshot.forEach(doc => {
if (doc.data().email === this.loginForm.email) {
isAlphaEmail = true;
}
});
this.performingRequest = false;
if (!isAlphaEmail) {
fb.auth.signOut();
return;
}

this.$store.commit('setCurrentUser', user);
this.$store.dispatch('fetchUserProfile');
this.$router.push(`${process.env.BASE_URL}/`);
});
})
.catch(err => {
console.log(err);
this.performingRequest = false;
this.errorMsg = err.message;
});
},
socialLogin() {
this.performingRequest = true;
const provider = new firebase.auth.GoogleAuthProvider();
Expand All @@ -189,15 +160,7 @@ export default {
.then(user => {
// Check whitelist emails to ensure user can log in
alphaWhitelistCollection.get().then(querySnapshot => {
let isAlphaEmail = false;
querySnapshot.forEach(doc => {
if (doc.data().email === user.user.email) {
isAlphaEmail = true;
}
});
this.performingRequest = false;
if (!isAlphaEmail) {
fb.auth.signOut();
if (!this.checkEmailAccess(querySnapshot, user)) {
return;
}

Expand All @@ -212,6 +175,22 @@ export default {
this.errorMsg = err.message;
});
},
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;
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks bad. You are implicitly exposing the alpha whitelist email to the entire world, since you need to loop over the entire snapshot.

Copy link
Contributor

@SamChou19815 SamChou19815 Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion:

  1. Write firebase security rules so that user can only access his/her own whitelist document
  2. Try to access that document
  3. 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.

validateEmail(email) {
return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test((email));
},
Expand Down Expand Up @@ -292,6 +271,11 @@ export default {
}
.email{
padding: 20px;
@media (max-width: 767px) {
width: 350px;
max-width: 350px;
flex: unset;
}
}
.email-button{
border: 0;
Expand Down Expand Up @@ -376,6 +360,10 @@ export default {
font-size: 24px;
color: #FFFFFF;
margin: 0;

&--task {
margin-left: 2rem;
}
}
.head{
font-weight: 600;
Expand Down Expand Up @@ -407,15 +395,15 @@ export default {
position: relative;

@media (max-width: 1274px) {
width: 70vw;
width: 62vw;
}
}
.schedule{
margin-top: 16px;
width: 650px;

@media (max-width: 1274px) {
width: 72vw;
width: 50vw;
}
}
.comment{
Expand Down