From 84de08083bcf60a458ceec8180390ddfcce7e705 Mon Sep 17 00:00:00 2001 From: Iain Collins Date: Fri, 17 Mar 2017 05:38:23 +0000 Subject: [PATCH] Fixes #10 (Session handling bug) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Session state now updates correctly after sign in on all browsers, including Interent Explorer. * The most important element of the fix was adding componentDidMount() call to update session data in auth/signin page (as it’s the oauth callback page). Changes to simply the flow in recent refactoring made this easier to fix. --- README.md | 2 ++ components/session.js | 10 +++++---- package.json | 2 +- pages/auth/signin.js | 42 +++++++++++++++++++++++------------ routes/passport-strategies.js | 2 +- 5 files changed, 38 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index b80f784..9486cdd 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,8 @@ It includes session support (with CSRF and XSS protection), email based sign-in All functionality works both client and server side - including without JavaScript support in the browser. +*Important!* Please upgrade to version 2.8 or newer as this resolves all known cross browser compatibility issues, including session behaviour with Internet Explorer. + ## Demo You can try it out at https://nextjs-starter.now.sh diff --git a/components/session.js b/components/session.js index c370b15..39bbb8b 100644 --- a/components/session.js +++ b/components/session.js @@ -1,4 +1,5 @@ /* global window */ +/* global localStorage */ /* global XMLHttpRequest */ /** * A class to handle signing in and out and caching session data in sessionStore @@ -71,6 +72,7 @@ export default class Session { // If force update is set, clear data from store if (forceUpdate === true) { + this._session = {} this._removeLocalStore('session') } @@ -132,7 +134,7 @@ export default class Session { let xhr = new XMLHttpRequest() xhr.open('POST', '/auth/email/signin', true) xhr.setRequestHeader('Content-type', 'application/x-www-form-urlencoded') - xhr.onreadystatechange = () => { + xhr.onreadystatechange = async () => { if (xhr.readyState === 4) { if (xhr.status !== 200) { return reject(Error('XMLHttpRequest error: Error while attempting to signin')) @@ -181,14 +183,14 @@ export default class Session { // We handle that silently by just returning null here. _getLocalStore(name) { try { - return JSON.parse(window.localStorage.getItem(name)) + return JSON.parse(localStorage.getItem(name)) } catch (err) { return null } } _saveLocalStore(name, data) { try { - window.localStorage.setItem(name, JSON.stringify(data)) + localStorage.setItem(name, JSON.stringify(data)) return true } catch (err) { return false @@ -196,7 +198,7 @@ export default class Session { } _removeLocalStore(name) { try { - window.localStorage.removeItem(name) + localStorage.removeItem(name) return true } catch (err) { return false diff --git a/package.json b/package.json index 642fca6..dbbebcc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "nextjs-starter", - "version": "2.7.0", + "version": "2.8.0", "description": "A starter Next.js project with email and oAuth authentication", "author": "Iain Collins ", "license": "ISC", diff --git a/pages/auth/signin.js b/pages/auth/signin.js index 0a6003a..1ab11ce 100644 --- a/pages/auth/signin.js +++ b/pages/auth/signin.js @@ -14,17 +14,31 @@ export default class extends Page { return {session: await session.getSession(true)} } + async componentDidMount() { + // Get latest session data after rendering on client + // Any page that is specified as the oauth callback should do this + const session = new Session() + this.state = { + email: this.state.email, + session: await session.getSession(true) + } + } + constructor(props) { super(props) this.state = { - email: '' + email: '', + session: this.props.session } this.handleSubmit = this.handleSubmit.bind(this) this.handleEmailChange = this.handleEmailChange.bind(this) } handleEmailChange(event) { - this.setState({email: event.target.value.trim()}) + this.setState({ + email: event.target.value.trim(), + session: this.state.session + }) } async handleSubmit(event) { @@ -43,28 +57,28 @@ export default class extends Page { render() { let signinForm =
- if (this.props.session.user) { + if (this.state.session.user) { let linkWithFacebook =

Link with Facebook

- if (this.props.session.user.facebook) { - linkWithFacebook =
+ if (this.state.session.user.facebook) { + linkWithFacebook =
} let linkWithGoogle =

Link with Google

- if (this.props.session.user.google) { - linkWithGoogle =
+ if (this.state.session.user.google) { + linkWithGoogle =
} let linkWithTwitter =

Link with Twitter

- if (this.props.session.user.twitter) { - linkWithTwitter =
+ if (this.state.session.user.twitter) { + linkWithTwitter =
} signinForm = (

You are signed in

-

Name: {this.props.session.user.name}

-

Email address: {this.props.session.user.email}

-

Email verified: {(this.props.session.user.verified) ? 'Yes' : 'No'}

+

Name: {this.state.session.user.name}

+

Email address: {this.state.session.user.email}

+

Email verified: {(this.state.session.user.verified) ? 'Yes' : 'No'}

You can link your account to your other accounts so you can sign in with them too.

{linkWithFacebook} {linkWithGoogle} @@ -84,7 +98,7 @@ export default class extends Page { signinForm = (
- +

Sign in with email


@@ -105,7 +119,7 @@ export default class extends Page { } return ( - +

Authentication

{signinForm}

How it works

diff --git a/routes/passport-strategies.js b/routes/passport-strategies.js index b41cf57..b42f433 100644 --- a/routes/passport-strategies.js +++ b/routes/passport-strategies.js @@ -31,7 +31,7 @@ exports.configure = ({ passport.deserializeUser(function (id, next) { User.get(id, function (err, user) { // Note: We don't return all user profile fields to the client, just ones - // that are whitelisted here to limit the amount of users' data we expose. + // that are whitelisted here to limit the amount of user data we expose. next(err, { id: user.id, name: user.name,