From 1ea6b0b75b0c259dc7f1eac06957750b7dbd4b03 Mon Sep 17 00:00:00 2001
From: Tom Russell <tomalrussell@gmail.com>
Date: Thu, 13 Sep 2018 16:58:05 +0100
Subject: [PATCH] Handle and show login/signup errors

---
 app/src/frontend/app.js           |  2 ++
 app/src/frontend/error-box.js     | 20 +++++++++++++++++
 app/src/frontend/login.js         | 32 ++++++++++++++-------------
 app/src/frontend/signup.js        | 36 +++++++++++++++++++------------
 app/src/frontend/styles/forms.css |  4 ++--
 app/src/server.js                 |  4 +++-
 app/src/user.js                   | 12 ++++++++---
 7 files changed, 75 insertions(+), 35 deletions(-)
 create mode 100644 app/src/frontend/error-box.js

diff --git a/app/src/frontend/app.js b/app/src/frontend/app.js
index 80ecac94..0f8336fa 100644
--- a/app/src/frontend/app.js
+++ b/app/src/frontend/app.js
@@ -30,6 +30,8 @@ class App extends React.Component {
     }
 
     login(user) {
+        console.log("Logging in")
+        console.log(user)
         this.setState({user: user});
     }
 
diff --git a/app/src/frontend/error-box.js b/app/src/frontend/error-box.js
new file mode 100644
index 00000000..c533016c
--- /dev/null
+++ b/app/src/frontend/error-box.js
@@ -0,0 +1,20 @@
+import React, { Fragment } from 'react';
+
+const ErrorBox = (props) => (
+    <Fragment>
+        {
+            (props.msg)?
+            (
+                <div className="alert alert-danger" role="alert">
+                    {
+                        (typeof props.msg === 'string' || props.msg instanceof String)?
+                        props.msg
+                        : 'Unexpected error'
+                    }
+                </div>
+            ) : null
+        }
+    </Fragment>
+);
+
+export default ErrorBox;
diff --git a/app/src/frontend/login.js b/app/src/frontend/login.js
index 9b961eab..2529e4a0 100644
--- a/app/src/frontend/login.js
+++ b/app/src/frontend/login.js
@@ -1,13 +1,16 @@
 import React, { Component } from 'react';
 import { Redirect, Link } from 'react-router-dom';
 
+import ErrorBox from './error-box';
+
 class Login extends Component {
     constructor(props) {
         super(props);
         this.state = {
             username: '',
             password: '',
-            show_password: ''
+            show_password: '',
+            error: undefined
         };
 
         this.handleChange = this.handleChange.bind(this);
@@ -26,7 +29,6 @@ class Login extends Component {
 
     handleSubmit(event) {
         event.preventDefault();
-        const login = this.props.login
         fetch('/login', {
             method: 'POST',
             body: JSON.stringify(this.state),
@@ -37,20 +39,19 @@ class Login extends Component {
             res => res.json()
         ).then(function(res){
             if (res.error) {
-                console.error(res.error);  // tell user
+                this.setState({error: res.error})
             } else {
-                console.log(res);  // redirect back
+                this.setState({error: undefined})
                 fetch('/users/me').then(
                     (res) => res.json()
-                ).then(function(user){
-                    console.log(user)
-                    login(user);
-                }).catch(function(err){
-                    console.error(err);
-                })
+                ).then(
+                    (user) => this.props.login(user)
+                ).catch(
+                    (err) => this.setState({error: err})
+                )
             }
-        }).catch(
-            err => console.error(err)
+        }.bind(this)).catch(
+            (err) => this.setState({error: err})
         );
     }
 
@@ -62,6 +63,7 @@ class Login extends Component {
             <article>
                 <section className="main-col">
                     <h1 className="h2">Log in</h1>
+                    <ErrorBox msg={this.state.error} />
                     <form onSubmit={this.handleSubmit}>
                         <label htmlFor="username">Username*</label>
                         <input name="username" id="username"
@@ -80,18 +82,18 @@ class Login extends Component {
 
                         <div className="form-check">
                             <input id="show_password" name="show_password"
-                                   className="position-static" type="checkbox"
+                                   className="form-check-input" type="checkbox"
                                    checked={this.state.show_password}
                                    onChange={this.handleChange}
                                    />
-                            <label htmlFor="show_password">Show password?</label>
+                            <label htmlFor="show_password" className="form-check-label">Show password?</label>
                         </div>
 
                         <div className="buttons-container">
                             <input type="submit" value="Log In" className="btn btn-primary" />
                         </div>
 
-                        Would you like to create an account?
+                        Would you like to create an account instead?
 
                         <div className="buttons-container">
                             <Link to="sign-up.html" className="btn btn-outline-dark">Sign Up</Link>
diff --git a/app/src/frontend/signup.js b/app/src/frontend/signup.js
index 03477e91..7ff27f39 100644
--- a/app/src/frontend/signup.js
+++ b/app/src/frontend/signup.js
@@ -1,6 +1,8 @@
 import React, { Component } from 'react';
 import { Redirect, Link } from 'react-router-dom';
 
+import ErrorBox from './error-box';
+
 class SignUp extends Component {
     constructor(props) {
         super(props);
@@ -10,7 +12,8 @@ class SignUp extends Component {
             confirm_email: '',
             password: '',
             show_password: '',
-            confirm_conditions: false
+            confirm_conditions: false,
+            error: undefined
         };
 
         this.handleChange = this.handleChange.bind(this);
@@ -39,17 +42,19 @@ class SignUp extends Component {
             res => res.json()
         ).then(function(res){
             if (res.error) {
-                console.error(res.error);  // tell user
+                this.setState({error: res.error})
             } else {
-                console.log(res);  // redirect back
-                fetch('/users/me').then(function(user){
-                    this.props.login(user);
-                }).catch(function(err){
-                    console.error(err);
-                })
+                this.setState({error: undefined})
+                fetch('/users/me').then(
+                    (res) => res.json()
+                ).then(
+                    (user) => this.props.login(user)
+                ).catch(
+                    (err) => this.setState({error: err})
+                )
             }
-        }).catch(
-            err => console.error(err)
+        }.bind(this)).catch(
+            (err) => this.setState({error: err})
         );
     }
 
@@ -64,6 +69,7 @@ class SignUp extends Component {
                     <p>
                         Create an account to start colouring in.
                     </p>
+                    <ErrorBox msg={this.state.error} />
                     <form onSubmit={this.handleSubmit}>
                         <label htmlFor="username">Username*</label>
                         <input name="username" id="username"
@@ -95,20 +101,22 @@ class SignUp extends Component {
 
                         <div className="form-check">
                             <input id="show_password" name="show_password"
-                                   className="position-static" type="checkbox"
+                                   className="form-check-input" type="checkbox"
                                    checked={this.state.show_password}
                                    onChange={this.handleChange}
                                    />
-                            <label htmlFor="show_password">Show password?</label>
+                            <label className="form-check-label" htmlFor="show_password">
+                                Show password?
+                            </label>
                         </div>
 
                         <div className="form-check">
                             <input id="confirm_conditions" name="confirm_conditions"
-                                   className="position-static" type="checkbox"
+                                   className="form-check-input" type="checkbox"
                                    checked={this.state.confirm_conditions}
                                    onChange={this.handleChange}
                                    required />
-                            <label htmlFor="confirm_conditions">
+                            <label className="form-check-label" htmlFor="confirm_conditions">
                                 I confirm that I have read and agree to the <a
                                 href="/privacy-policy">privacy policy</a> and <a
                                 href="/user-agreement">contributor agreement</a>.
diff --git a/app/src/frontend/styles/forms.css b/app/src/frontend/styles/forms.css
index b4debbdd..53e2d2c8 100644
--- a/app/src/frontend/styles/forms.css
+++ b/app/src/frontend/styles/forms.css
@@ -12,8 +12,8 @@ input[type="email"] {
 input[type="number"] {
     padding-right: 0.25rem;
 }
-.form-check {
-    padding-left: 0;
+.form-check-input {
+    margin-top: 0.6rem;
 }
 label {
     margin: 0.5em 0 0;
diff --git a/app/src/server.js b/app/src/server.js
index aae26e97..bfe785b3 100644
--- a/app/src/server.js
+++ b/app/src/server.js
@@ -178,11 +178,13 @@ server.post('/users', function(req, res){
     const user = req.body;
     if (req.session.user_id) {
         res.send({error: 'Already signed in'});
+        return
     }
 
     if (user.email){
         if (user.email != user.confirm_email) {
           res.send({error: "Email did not match confirmation."});
+          return
         }
     } else {
         user.email = null;
@@ -198,7 +200,7 @@ server.post('/users', function(req, res){
         }
     }).catch(function(err){
         console.error(err);
-        res.send({error: 'Server error'})
+        res.send(err)
     });
 });
 
diff --git a/app/src/user.js b/app/src/user.js
index ed89f680..3a58224f 100644
--- a/app/src/user.js
+++ b/app/src/user.js
@@ -2,6 +2,12 @@ import { query } from './db';
 
 
 function createUser(user) {
+    if (!user.password || user.password.length < 8) {
+        return Promise.reject({error: 'Password must be at least 8 characters'})
+    }
+    if (user.password.length > 70) {
+        return Promise.reject({error: 'Password must be at most 70 characters'})
+    }
     return query(
         `INSERT
         INTO users (
@@ -52,14 +58,14 @@ function authUser(username, password) {
         ]
     ).then(function(data){
         const user = data.rows[0];
-        if (user.auth_ok) {
+        if (user && user.auth_ok) {
             return {user_id: user.user_id}
         } else {
-            return {error: 'Authentication failed'}
+            return {error: 'Username or password not recognised'}
         }
     }).catch(function(err){
         console.error(err);
-        return {error: 'Database error'};
+        return {error: 'Username or password not recognised'};
     })
 }