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

Logout on expired sessions #325

Merged
merged 4 commits into from
Nov 21, 2017

Conversation

wacii
Copy link
Contributor

@wacii wacii commented Nov 20, 2017

If the user continues using the app after having timed out, all requests will fail, some silently. This PR instead clears the client session when the server signals a session timeout.

The main change was simple. In the api middleware, if a response has status 401, clear the user from client state. b1cbc57

To support this change the server needed to return 401 responses consistently when the user was not authenticated. 7262004

On logging out an admin, the formatting was off; there was an empty space where the sidebar used to be. This is because of DOM manipulation being done in <Application /> using jQuery instead of React. c0c9105.

The last change wasn't strictly necessary, it was a simple refactor I did while reviewing how the client handled authorization. I thought it might be useful in better enforcing guest only pages, but I didn't find anywhere else that I was certain needed it. Other than maybe <ResetPassword />. 3b48bc3

Closes #296.

@wacii
Copy link
Contributor Author

wacii commented Nov 20, 2017

A question. Some of the express routes were exported as factory methods. So...

import buildDonorRouter from "server/routes/donor";
const donorRouter = buildDonorRouter();

// as opposed to just...

import packingRouter from "server/routes/packing";

It seemed connected to whether or not the router was pulling methods off of userRouter, but I couldn't figure out why this was done.

}

componentWillReceiveProps(nextProps) {
if (this.props.user !== nextProps.user)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@jspaine
Copy link
Contributor

jspaine commented Nov 21, 2017

Thanks, it looks good!

The route factories were for testing, didn't get round to doing more than the api router though.

@wacii
Copy link
Contributor Author

wacii commented Nov 21, 2017

I removed if (this.props.user !== nextProps.user).

@@ -1,5 +1,8 @@
import Router from 'express-promise-router'
import donationController from '../controllers/donation'
import usersController from '../controllers/users'

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants