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

Authentication component that tracks user authentication status #45

Merged
merged 21 commits into from
Jul 20, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
41 changes: 32 additions & 9 deletions frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"@testing-library/user-event": "^7.2.1",
"bootstrap": "^4.5.0",
"firebase": "^7.15.5",
"history": "^5.0.0",
"react": "^16.13.1",
"react-bootstrap": "1.0.1",
"react-dom": "^16.13.1",
Expand Down
19 changes: 11 additions & 8 deletions frontend/src/components/App/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import {BrowserRouter as Router, Route} from 'react-router-dom';
import { AuthProvider, PrivateRoute } from '../Auth';

import LandingPage from '../Landing';
import SignInPage from '../SignIn'
Expand All @@ -14,14 +15,16 @@ import * as ROUTES from '../../constants/routes';
class App extends React.Component {
render() {
return (
<Router>
<div>
<Route exact path={ROUTES.LANDING} component={LandingPage} />
<Route path={ROUTES.SIGN_IN} component={SignInPage} />
<Route path={ROUTES.VIEW_TRIPS} component={ViewTripsPage} />
<Route path={ROUTES.VIEW_ACTIVITIES + '/:tripId'} component={ViewActivitiesPage} />
</div>
</Router>
<AuthProvider>
<Router>
<div>
<Route exact path={ROUTES.LANDING} component={LandingPage} />
<Route path={ROUTES.SIGN_IN} component={SignInPage} />
<PrivateRoute path={ROUTES.VIEW_TRIPS} component={ViewTripsPage} />
<PrivateRoute path={ROUTES.VIEW_ACTIVITIES + '/:tripId'} component={ViewActivitiesPage} />
</div>
</Router>
</AuthProvider>
);
}
};
Expand Down
36 changes: 36 additions & 0 deletions frontend/src/components/Auth/AuthContext.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import React, { useState, useEffect } from 'react';
import app from '../Firebase';

const AuthContext = React.createContext();

/**
* AuthProvider component that keeps track of the authentication status of the
* current user. Allows global use of this status using React Context and should
* be wrapped around the contents of the App component.
*/
const AuthProvider = (props) => {
const [currentUser, setCurrentUser] = useState(null);
const [isLoading, setIsLoading] = useState(true);
keiffer01 marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
// Set up listener that changes user status whenever it is updated.
app.auth().onAuthStateChanged((user) => {
setCurrentUser(user);
setIsLoading(false);
});
}, []);

if (isLoading) {
// TODO (Issue #25): Page initially displays "Loading..." for testing
// purposes, make this blank in the deployed build.
return ( <h1>Loading...</h1> );
}

return (
<AuthContext.Provider value={currentUser}>
{props.children}
</AuthContext.Provider>
);
}

export { AuthContext, AuthProvider };
80 changes: 80 additions & 0 deletions frontend/src/components/Auth/AuthContext.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import React, { useContext } from 'react';
import { render, act, screen, cleanup } from '@testing-library/react';
import { AuthContext, AuthProvider } from './AuthContext.js';

jest.useFakeTimers();

// All times are in milliseconds.
const TIME_BEFORE_USER_IS_LOADED = 500;
const TIME_WHEN_USER_IS_LOADED = 1000;
const TIME_AFTER_USER_IS_LOADED = 2000;

// Mock the the Firebase Auth onAuthStateChanged function, which pauses for the
// time given by TIME_WHEN_USER_IS_LOADED, then returns a fake user with only
// the property `name: 'Keiffer'`.
const mockOnAuthStateChanged = jest.fn(callback => {
setTimeout(() => {
callback({ name: 'Keiffer' })
}, TIME_WHEN_USER_IS_LOADED);
});
jest.mock('firebase/app', () => {
return {
initializeApp: () => {
return {
auth: () => {
return {
onAuthStateChanged: mockOnAuthStateChanged
}
}
}
}
}
});

afterEach(cleanup);

describe('AuthProvider component', () => {
beforeEach(() => { render(<AuthProvider />) });

it('initially displays "Loading"', () => {
act(() => jest.advanceTimersByTime(TIME_BEFORE_USER_IS_LOADED));
expect(screen.getByText('Loading...')).toBeInTheDocument();
});

it('returns a provider when onAuthStateChanged is called', () => {
act(() => jest.advanceTimersByTime(TIME_AFTER_USER_IS_LOADED));
expect(screen.queryByText('Loading...')).not.toBeInTheDocument();
});
});
keiffer01 marked this conversation as resolved.
Show resolved Hide resolved

describe('AuthContext Consumer component', () => {
// A Consumer component for AuthContext that just displays the current
// user.
const TestAuthContextConsumerComponent = () => {
const currentUser = useContext(AuthContext);

return (
<div>
<div>{currentUser.name}</div>
</div>
);
};

beforeEach(() => {
render(
<AuthProvider>
<TestAuthContextConsumerComponent />
</AuthProvider>
);
});
keiffer01 marked this conversation as resolved.
Show resolved Hide resolved

it('initially displays "Loading"', () => {
act(() => jest.advanceTimersByTime(TIME_BEFORE_USER_IS_LOADED));
expect(screen.getByText('Loading...')).toBeInTheDocument();
});

it('displays the current user when they are authenticated', () => {
act(() => jest.advanceTimersByTime(TIME_AFTER_USER_IS_LOADED));
expect(screen.getByText('Keiffer')).toBeInTheDocument();
});
});
39 changes: 39 additions & 0 deletions frontend/src/components/Auth/PrivateRoute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React, { useContext } from 'react';
import { Route, Redirect } from 'react-router-dom';
import { AuthContext } from '../Auth';

import { SIGN_IN } from '../../constants/routes.js';

/**
* PrivateRoute component that functions similarly to the `Route` component,
* with the added check that determines if the user is currently signed in.
*
* This component takes the authentication status of the current user from
* AuthContext. If they are authenticated, they will be allowed to view the
* contents of the Route component. If they are not authenticated, they will be
* redirected to the SIGN_IN page.
*
* @param {Object} props The following props are expected:
* - component {React.Component} The component that `PrivateRoute` should render
* if the user is currently authenticated.
*/
const PrivateRoute = ({ component: RouteComponent, ...rest }) => {
const currentUser = useContext(AuthContext);

// The rest of the props passed into `PrivateRoute` are given as props to the
// `Route` component as normal. The render prop is used to specify that, if
// the user is signed in, the given component to render should be rendered
// along with all the standard Route paths (URL path, etc.), and if the user
// is not signed in, a `Redirect` prop should be rendered instead.
return (
<Route
{...rest}
render={routeProps =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I understand correctly, ...rest are all the props passed into this component besides RouteComponent so justpath. So then where does routeProps come from? Also why is it render instead of component? Should it have the same props as one like this:

<Route path={ROUTES.SIGN_IN} component={SignInPage} />

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Essentially I just want PrivateRoute to act exactly as a Route component, with the added authentication check. So yeah you have the right idea that ...rest are just all the other props passed into PrivateRoute. I then used a render prop to specify that the component to be rendered is either the RouteComponent, if the user is logged in, or a Redirect component if the user is not logged in. The routeProps are the props that are usually passed into the RouteComponent, such as URL params, etc.

If you think it would be beneficial to comment my code on some of these I can definitely do that!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a brief comment mentioning this would be good. +1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, will do that before merging.

currentUser ? <RouteComponent {...routeProps} />
: <Redirect to={SIGN_IN} />
}
/>
);
}

export default PrivateRoute;
50 changes: 50 additions & 0 deletions frontend/src/components/Auth/PrivateRoute.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from 'react';
import { Router, Route } from 'react-router-dom';
import { render, screen, cleanup } from '@testing-library/react';
import { createMemoryHistory } from 'history';
import PrivateRoute from './PrivateRoute.js';

import { SIGN_IN } from '../../constants/routes.js';

const history = createMemoryHistory();

// Mock the useContext function so that, when called in the PrivateRoute
// component, returns null the first time and a fake user the second time.
jest.mock('react', () => (
{
...(jest.requireActual('react')),
useContext: jest
.fn()
.mockReturnValueOnce(null)
.mockReturnValueOnce({ name: 'Keiffer' })
}
));

describe('PrivateRoute component', () => {
const TestComponent = () => {
return (
<div>Hello, World!</div>
);
};

beforeEach(() => {
render(
<Router history={history}>
<PrivateRoute component={TestComponent} />
<Route path={SIGN_IN} />
</Router>
);
});

afterEach(cleanup);

// mockOnAuthStateChanged called first time, so user should be null.
it('redirects to SIGN_IN when the user is not authenticated', () => {
expect(history.location.pathname).toEqual(SIGN_IN);
});

// mockOnAuthStateChanged called second time, so user should not be null.
it('renders the given component when the user is authenticated', () => {
expect(screen.getByText('Hello, World!')).toBeInTheDocument();
});
});
4 changes: 4 additions & 0 deletions frontend/src/components/Auth/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { AuthContext, AuthProvider } from './AuthContext.js';
import PrivateRoute from './PrivateRoute.js';

export { AuthContext, AuthProvider, PrivateRoute };
keiffer01 marked this conversation as resolved.
Show resolved Hide resolved