Skip to content
This repository has been archived by the owner on Jul 17, 2022. It is now read-only.

Commit

Permalink
fix: use email instead of username
Browse files Browse the repository at this point in the history
  • Loading branch information
coderbyheart committed Aug 30, 2021
1 parent e7e3c2a commit 74bd298
Show file tree
Hide file tree
Showing 17 changed files with 65 additions and 59 deletions.
2 changes: 1 addition & 1 deletion db/seeders/20210302050835-demo-groups.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.bulkInsert('UserAccounts', [
{
username: 'seeded-account-id',
email: 'seeded-account-id@example.com',
createdAt: new Date(),
updatedAt: new Date(),
},
Expand Down
4 changes: 2 additions & 2 deletions docs/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

The backend authenticates requests using signed cookies which contains user's id so that it does not have to be fetched for every request.

Cookies are sent [`secure` and `HttpOnly`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies) when users register their account, or when they log in using username and password.
Cookies are sent [`secure` and `HttpOnly`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies) when users register their account, or when they log in using email and password.

Cookies expire after 30 minutes and the client is responsible for renewing cookies by calling the `GET /me/cookie` endpoint before they expire.

When renewing cookies, the server will re-check if the user still exists and if they haven't changed their password. For this a hash of the user's password hash, email, username, and id will be generated and included in the cookie. If any of these properties changes, the cookie cannot be renewed and the user has to log-in again.
When renewing cookies, the server will re-check if the user still exists and if they haven't changed their password. For this a hash of the user's password hash, email, and id will be generated and included in the cookie. If any of these properties changes, the cookie cannot be renewed and the user has to log-in again.

## Admin permissions

Expand Down
9 changes: 6 additions & 3 deletions frontend/src/hooks/useAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,20 @@ export const useAuth = () => {
isLoading,
isAuthenticated,
logout: () => {
// FIXME: clear cookies and reload
// Delete cookies (since the auth cookie is httpOnly we cannot access
// it using JavaScript, e.g. cookie.delete() will not work).
// Therefore we ask the server to send us an invalid cookie.
fetch(`${SERVER_URL}/me/cookie`, { method: 'DELETE' }).then(() => {
setIsAuthenticated(false)
// Reload the page (no need to handle logout in the app)
document.location.reload()
})
},
login: ({ username, password }: { username: string; password: string }) => {
login: ({ email, password }: { email: string; password: string }) => {
setIsLoading(true)
fetch(`${SERVER_URL}/me/login`, {
method: 'POST',
body: JSON.stringify({ username, password }),
body: JSON.stringify({ email, password }),
})
.then(() => {
setIsAuthenticated(true)
Expand Down
36 changes: 18 additions & 18 deletions frontend/src/pages/PublicHome.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ import { useAuth } from '../hooks/useAuth'

const SERVER_URL = process.env.REACT_APP_SERVER_URL

const emailRegEx = /.+@.+\..+/
const passwordRegEx =
/^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{8,}$/

const PublicHomePage: FunctionComponent = () => {
const { login } = useAuth()
const [showRegisterForm, setShowRegisterForm] = useState(false)
const [username, setUsername] = useState('')
const [email, setEmail] = useState('')
const [password, setPassword] = useState('')
const [password2, setPassword2] = useState('')

const loginFormValid =
username.length > 0 &&
/^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9])(?=.*?[#?!@$%^&*-]).{8,}$/.test(
password,
)
const loginFormValid = emailRegEx.test(email) && passwordRegEx.test(password)

const registerFormValid = loginFormValid && password === password2

Expand All @@ -36,11 +36,11 @@ const PublicHomePage: FunctionComponent = () => {
{showLoginForm && (
<form>
<TextField
label="username"
type="text"
name="username"
value={username}
onChange={({ target: { value } }) => setUsername(value)}
label="email"
type="email"
name="email"
value={email}
onChange={({ target: { value } }) => setEmail(value)}
/>
<TextField
label="password"
Expand All @@ -52,7 +52,7 @@ const PublicHomePage: FunctionComponent = () => {
<button
className="bg-navy-800 text-white text-lg px-4 py-2 rounded-sm w-full hover:bg-opacity-90"
type="button"
onClick={() => login({ username, password })}
onClick={() => login({ email, password })}
disabled={!loginFormValid}
>
Log in
Expand All @@ -69,11 +69,11 @@ const PublicHomePage: FunctionComponent = () => {
{showRegisterForm && (
<form>
<TextField
label="username"
type="text"
name="username"
value={username}
onChange={({ target: { value } }) => setUsername(value)}
label="email"
type="email"
name="email"
value={email}
onChange={({ target: { value } }) => setEmail(value)}
/>
<TextField
label="password"
Expand All @@ -97,7 +97,7 @@ const PublicHomePage: FunctionComponent = () => {
onClick={() => {
fetch(`${SERVER_URL}/register`, {
method: 'POST',
body: JSON.stringify({ username, password }),
body: JSON.stringify({ email, password }),
})
}}
disabled={!registerFormValid}
Expand Down
2 changes: 1 addition & 1 deletion src/authenticateRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type AuthContext = {
export const userHash = (user: UserAccount): string =>
crypto
.createHash('sha1')
.update(`${user.id}:${user.username}:${user.passwordHash}`)
.update(`${user.id}:${user.email}:${user.passwordHash}`)
.digest('hex')

export const authCookieName = 'auth'
Expand Down
4 changes: 2 additions & 2 deletions src/models/user_account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import Group from './group'

export interface UserAccountAttributes {
id: number
username: string
email: string
passwordHash: string
isAdmin?: boolean
name: string
Expand All @@ -34,7 +34,7 @@ export default class UserAccount extends Model<

@Unique
@Column
public username!: string
public email!: string

@Column
public passwordHash!: string
Expand Down
6 changes: 3 additions & 3 deletions src/routes/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import { authCookie } from '../authenticateRequest'
import { trimAll } from '../input-validation/trimAll'
import { validateWithJSONSchema } from '../input-validation/validateWithJSONSchema'
import UserAccount from '../models/user_account'
import { passwordInput, usernameInput } from './register'
import { emailInput, passwordInput } from './register'

const loginInput = Type.Object(
{
username: usernameInput,
email: emailInput,
password: passwordInput,
},
{ additionalProperties: false },
Expand All @@ -31,7 +31,7 @@ const login =

const user = await UserAccount.findOne({
where: {
username: valid.value.username,
email: valid.value.email,
},
})
if (user === null) {
Expand Down
2 changes: 1 addition & 1 deletion src/routes/me.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const getProfile = async (request: Request, response: Response) => {
if (user === null) return response.send(404).end()
return response
.json({
username: user.username,
email: user.email,
...(await user.asPublicProfile()),
})
.end()
Expand Down
10 changes: 5 additions & 5 deletions src/routes/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import { trimAll } from '../input-validation/trimAll'
import { validateWithJSONSchema } from '../input-validation/validateWithJSONSchema'
import UserAccount from '../models/user_account'

export const usernameInput = Type.String({
pattern: '^[a-z0-9._-]{1,255}$',
title: 'Username',
export const emailInput = Type.String({
format: 'email',
title: 'Email',
})

export const passwordInput = Type.String({
Expand All @@ -19,7 +19,7 @@ export const passwordInput = Type.String({

const registerUserInput = Type.Object(
{
username: usernameInput,
email: emailInput,
name: Type.String({ minLength: 1, maxLength: 255 }),
password: passwordInput,
},
Expand All @@ -43,7 +43,7 @@ const registerUser =

const user = await UserAccount.create({
passwordHash: bcrypt.hashSync(valid.value.password, saltRounds),
username: valid.value.username,
email: valid.value.email,
name: valid.value.name,
})

Expand Down
4 changes: 2 additions & 2 deletions src/testServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const makeTestServer = async (
const user = await UserAccount.create({
isAdmin: false,
name: 'User',
username: 'user',
email: 'user@example.com',
passwordHash: '',
})

Expand Down Expand Up @@ -63,7 +63,7 @@ export const makeAdminTestServerWithServices = async (
const admin = await UserAccount.create({
isAdmin: true,
name: 'Admin',
username: 'admin',
email: 'admin@example.com',
passwordHash: '',
})

Expand Down
14 changes: 7 additions & 7 deletions src/tests/authentication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ describe('User account API', () => {
let app: Express
let httpServer: Server
let r: SuperTest<Test>
let username: string
let email: string
let password: string
let authCookie: string
beforeAll(async () => {
username = v4()
email = `${v4()}@example.com`
password = 'y{uugBmw"9,?=L_'
app = express()
app.use(cookieParser(process.env.COOKIE_SECRET ?? 'cookie-secret'))
Expand All @@ -74,7 +74,7 @@ describe('User account API', () => {
.set('Accept', 'application/json')
.set('Content-type', 'application/json; charset=utf-8')
.send({
username,
email,
password,
name: 'Alex',
})
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('User account API', () => {
.expect(200)
expect(res.body).toMatchObject({
id: /[0-9]+/,
username,
email,
isAdmin: false,
})
})
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('User account API', () => {
r
.post('/login')
.send({
username,
email,
password,
})
.expect(204)
Expand All @@ -146,15 +146,15 @@ describe('User account API', () => {
r
.post('/login')
.send({
username,
email,
password: "Y<N-'#sQ2/RCrN_c",
})
.expect(401))
it('should fail with user not found', () =>
r
.post('/login')
.send({
username: 'foo',
email: 'foo',
password: "Y<N-'#sQ2/RCrN_c",
})
.expect(401))
Expand Down
19 changes: 11 additions & 8 deletions src/tests/groups_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ describe('Groups API', () => {

// Create test servers
captain = await UserAccount.create({
username: 'captain',
email: 'captain@example.com',
passwordHash: '',
name: 'Captain A',
})
newCaptain = await UserAccount.create({
username: 'new-captain',
email: 'new-captain@example.com',
passwordHash: '',
name: 'New Captain',
})
Expand Down Expand Up @@ -278,17 +278,17 @@ describe('Groups API', () => {
*/
beforeAll(async () => {
captain1 = await UserAccount.create({
username: captain1Name,
email: `${captain1Name}@example.com`,
name: captain1Name,
passwordHash: '',
})
captain2 = await UserAccount.create({
username: captain2Name,
email: `${captain2Name}@example.com`,
name: captain2Name,
passwordHash: '',
})
daCaptain = await UserAccount.create({
username: daCaptainName,
email: `${daCaptainName}@example.com`,
name: daCaptainName,
passwordHash: '',
})
Expand Down Expand Up @@ -493,7 +493,7 @@ describe('Groups API', () => {
variables: {
captainId: (
await UserAccount.findOne({
where: { username: captainName },
where: { email: `${captainName}@example.com` },
})
)?.id,
},
Expand Down Expand Up @@ -533,7 +533,7 @@ describe('Groups API', () => {
variables: {
captainId: (
await UserAccount.findOne({
where: { username: captainName },
where: { email: `${captainName}@example.com` },
})
)?.id,
groupType: groupTypes,
Expand All @@ -547,7 +547,10 @@ describe('Groups API', () => {
})
)
.filter(({ name }) => expectedGroupNames.includes(name))
.filter(({ captain }) => captain.username === captainName)
.filter(
({ captain }) =>
captain.email === `${captainName}@example.com`,
)
.map(toGroupData),
)
},
Expand Down
4 changes: 2 additions & 2 deletions src/tests/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import {
ShipmentCreateInput,
} from '../../server-internal-types'

let fakeusername = 1
let fakeUserCounter = 1

async function createGroup(
input: GroupCreateInput,
captainId: Maybe<number> = null,
): Promise<Group> {
if (!captainId) {
const groupCaptain = await UserAccount.create({
username: `fake-auth-id-${fakeusername++}`,
email: `fake-auth-id-${fakeUserCounter++}@example.com`,
passwordHash: '',
name: 'Captain',
})
Expand Down
2 changes: 1 addition & 1 deletion src/tests/line_items_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('LineItems API', () => {
await sequelize.sync({ force: true })

captain = await UserAccount.create({
username: 'captain',
email: 'captain@example.com',
passwordHash: '',
name: 'Captain',
})
Expand Down
2 changes: 1 addition & 1 deletion src/tests/offers_api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Offers API', () => {
await Shipment.truncate({ cascade: true, force: true })

captain = await UserAccount.create({
username: 'captain',
email: 'captain@example.com',
passwordHash: '',
name: 'Captain',
})
Expand Down
Loading

0 comments on commit 74bd298

Please sign in to comment.