Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

Use new OAuth module from PJC #45

Merged
merged 4 commits into from
Feb 22, 2016
Merged

Use new OAuth module from PJC #45

merged 4 commits into from
Feb 22, 2016

Conversation

rogerhutchings
Copy link
Contributor

No description provided.

@rogerhutchings
Copy link
Contributor Author

This should be ready to go. It now resolves any outstanding login / session promises before loading the app.

@simoneduca
Copy link
Contributor

Nice one, thanks! I'll have a look shortly.

@shaunanoordin
Copy link
Member

I've taken a look at the PR, and I found something I'm not sure is intended?

Anomaly: After a user has successfully logged in to the app, if she/he then opens a new window to the app OR refreshes the page, then she/he will see the Login button instead of the (expected) Logout button.
Analysis: At this stage, clicking on the Login button immediately sets the user as logged in (without having to go through the login form). So it appears that some credentials (some sort of authorisaiton token?) are properly cookie-saved after the user logs in, but I can't tell if the app is aware that the user is correctly logged in when the page is loaded again.

@shaunanoordin
Copy link
Member

screen shot 2016-02-22 at 13 55 01

Tested on:

  • FF44/Chrome48 on OSX, with Privacy Mode switched on. Re-confirmed without Privacy Mode.

Update:

  • This issue does not appear on the current master branch; logging in for the first time will replace the Login button with a Logout button, as expected, and opening a new window while logged in will show the Logout button.

@rogerhutchings
Copy link
Contributor Author

You may need to reinstall the panoplies client module, it’s using a custom version which hasn’t been merged yet

On Mon, Feb 22, 2016 at 1:59 PM, Shaun A. Noordin
[email protected] wrote:

screen shot 2016-02-22 at 13 55 01 Tested on: - FF44/Chrome48 on OSX, with Privacy Mode switched on. Re-confirmed without Privacy Mode. Update:

* This issue does not appear on the current master branch; logging in for the first time will replace the Login button with a Logout button, as expected, and opening a new window while logged in will show the Logout button.

Reply to this email directly or view it on GitHub:
#45 (comment)

@simoneduca
Copy link
Contributor

@rogerhutchings LGTM, just left a couple of comments.

@shaunanoordin
Copy link
Member

Alright, Login works as expected!

  • Sanity check - Code LGTM
  • Logging in - OK
  • Logging out - OK
  • State consistency, user remains logged in (or out) when opening a new window/tab - OK

Tested on:

  • FF44/Chrome48 on OSX, with Privacy Mode switched on.
  • Safari9 on OSX, in non-Privacy Mode.

A bit of a note: when in Private Mode, Safari disables certain features. (Such as localstorage, I believe.) This causes wonkiness when testing on Safari Private Mode - after accessing the login form and being redirected back to the app, the app gets stuck in a never-ending load. Since accessing the app with Safari in Private Mode while attempting to log in is such a niche case that it's not considered normal user behaviour, I'm going to mark this as a non-issue.

PR Status: Validated
Action: Merging

shaunanoordin added a commit that referenced this pull request Feb 22, 2016
Use new OAuth module from PJC
@shaunanoordin shaunanoordin merged commit 8fe2fbb into master Feb 22, 2016
@simoneduca simoneduca deleted the login branch February 23, 2016 14:09
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