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

Bug: Sessions and authentication #6034

Closed
cobbspur opened this issue Nov 2, 2015 · 11 comments
Closed

Bug: Sessions and authentication #6034

cobbspur opened this issue Nov 2, 2015 · 11 comments
Labels
bug [triage] something behaving unexpectedly P2 - High [triage] High priority for immediate patch release

Comments

@cobbspur
Copy link
Member

cobbspur commented Nov 2, 2015

Issue Summary

I have had difficulties recently setting up ghost/logging in to ghost/creating exports which I think are all inter-related hence raising them in one issue initially.

One issue is setting up a new blog after deleting database results in errors.

The second issue seems to be that if I log out I cannot log back in again - access is denied. Using the password reset can sometimes enable me to log-in but the issue recurs.

The third issue is that the export tool in labs does not work - access in denied (this has been split out to #6040)

Steps to Reproduce

The easiest issue to reproduce is to log into your blog and go to the labs page /ghost/settings/labs and click the export button.

You will see an error in your console.

The other issues appear to be related at least partly to sessions already being in progress which will hopefully become clearer as you read through the steps to reproduce but because of the first steps are:

  1. Log on to existing blog or create a blog as per usual.
  2. Stop Ghost
  3. Delete database
  4. Start ghost - npm start wait for database to be created
  5. Navigate to blog admin e.g localhost:2368/ghost
  6. Attempt to create a blog.

Repeat these steps and try to create a blog in both normal browsing mode and incognito mode. Here is what I see.

Non-incognito mode - npm start redirects to /ghost/setup and I see a 500 error :

screen shot 2015-11-02 at 17 45 11

I get the following error in console: ERROR: Access denied.

Error
    at Error.UnauthorizedError (/Ghost/core/server/errors/unauthorized-error.js:6:18)
    at authenticate (//Ghost/core/server/middleware/auth.js:114:50)
    at allFailed (/Ghost/node_modules/passport/lib/middleware/authenticate.js:87:18)
    at attempt (Ghost/node_modules/passport/lib/middleware/authenticate.js:160:28)
    at Strategy.strategy.fail (/Ghost/node_modules/passport/lib/middleware/authenticate.js:277:9)
    at verified (/Ghost/node_modules/passport-http-bearer/lib/strategy.js:124:19)
    at then (/Ghost/core/server/middleware/auth-strategies.js:55:28)
    at tryCatcher (/Ghost/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/Ghost/node_modules/bluebird/js/main/promise.js:507:31)
    at Promise._settlePromiseAt (/Ghost/node_modules/bluebird/js/main/promise.js:581:18)
    at Async._drainQueue (/Ghost/node_modules/bluebird/js/main/async.js:128:12)
    at Async._drainQueues (/Ghost/node_modules/bluebird/js/main/async.js:133:10)
    at Async.drainQueues (/Ghost/node_modules/bluebird/js/main/async.js:15:14)
    at process._tickDomainCallback (node.js:463:13) 

Incognito mode - correctly redirected to /ghost/setup/one

I can then try to fill out the form but on submission of step 2 I get the forever loading spinner:
screen shot 2015-11-02 at 17 38 57

Error this time is: ERROR: Access denied.

Error
    at Error.UnauthorizedError (/Ghost/core/server/errors/unauthorized-error.js:6:18)
    at authenticate (/Ghost/core/server/middleware/auth.js:95:50)
    at Strategy.strategy.success (/Ghost/node_modules/passport/lib/middleware/authenticate.js:194:18)
    at verified (/Ghost/node_modules/passport-oauth2-client-password/lib/strategy.js:50:10)
    at then (/GHOST/Ghost/core/server/middleware/auth-strategies.js:21:32)
    at tryCatcher (/Ghost/node_modules/bluebird/js/main/util.js:26:23)
    at Promise._settlePromiseFromHandler (/Ghost/node_modules/bluebird/js/main/promise.js:507:31)
    at Promise._settlePromiseAt (/Ghost/node_modules/bluebird/js/main/promise.js:581:18)
    at Async._drainQueue (/Ghost/node_modules/bluebird/js/main/async.js:128:12)
    at Async._drainQueues (/Ghost/node_modules/bluebird/js/main/async.js:133:10)
    at Async.drainQueues (/Ghost/node_modules/bluebird/js/main/async.js:15:14)
    at process._tickDomainCallback (node.js:463:13) 

If I refresh the page I am taken to the sign-in page but attempting to log in results in the same error.

Technical details

  • Ghost Version: master - latest commit: 8db90ba
    Tested on OS X Yosemite and windows7 x64 ultimate
@ErisDS ErisDS added bug [triage] something behaving unexpectedly P2 - High [triage] High priority for immediate patch release labels Nov 2, 2015
@kevinansfield
Copy link
Contributor

I've seen something like this recently when deleting/recreating databases. Deleting the auth info from local storage fixes it so that definitely points to an issue with either the changes in ember-simple-auth (most likely) or changes in how we handle invalid sessions.

I'll have a play and see if I can find anything that may be causing the behaviour in ESA.

@acburdine
Copy link
Member

I think it may have to do with the oauth-prefilter instance initializer. That was the only reasonable way I could think to implement that behavior when I upgraded it, but I think it may cause issues on setup if the blog's database is deleted. I will also look into this.

@ErisDS
Copy link
Member

ErisDS commented Nov 2, 2015

Hitting the export button is the easiest reproduction case atm I think

@acburdine
Copy link
Member

The login/signup error: When Ember is loaded, Ember Simple Auth calls the oauth2 authenticator's restore method in order to verify that the user is still logged in. The issue stems from the fact that the restore method only pings the server if the regular access_token is expired. In the steps above, they are all done rather quickly, which results in the token not being expired, at least on the client side.

What may need to happen to fix this (and this is just a quick idea), is to better handle 401 unauthorized errors on the client. Or, the authenticator needs to actually ping the server each time (regardless of whether or not the refresh_token is expired) and go from there. Will spend some time seeing if one or the other works better.

As to the second issue: The export function differs from the rest of the api calls in that it is a url appended to an iframe. I haven't had much time to research it, but I made sure I fixed that one when I did the upgrade, so it must have broken sometime after that. Because it is appended to the iframe, the access token is passed as a query parameter. The pipeline refactor may have caused this, as that's the only change I think that's happened to the db api, but then again I haven't had a ton of time to research this.

@kevinansfield
Copy link
Contributor

Non-incognito mode - npm start redirects to /ghost/setup and I see a 500 error

The issue here is that your access token is still around in local storage from the previous install so when loading ghost again it thinks you're logged in so tries to load normally but the server returns a 401 as your access_token no longer exists.

One workaround is to always redirect to /signin when we get a 401 error but this opens up edge cases where we may not want to redirect, e.g. You're editing a post but your session has timed out before clicking save - in this instance I think sticking around on the current page and showing an alert is preferable (this may already happen, it may just be on transitions that we throw the 500 screen).

Do we return different messages from the server if an access_token has expired vs the access_token not existing? If there's a difference then we can handle the various cases appropriately in the client.

The above also highlights that our error processing has gone awry somewhere - rather than just showing the 500 page we should also be showing an alert indicating that you are no longer logged in. We should also offer some way of easily getting back to the login screen (essentially quick-access to the "log out" flow - removing the local storage credentials will auto-redirect to the signin screen).

Incognito mode - correctly redirected to /ghost/setup/one (but get infinite spinner and 401)

This is an odd one, I'm not sure why that would happen. @cobbspur Does this happen every time you delete the database and try to setup up again or only in combination with other steps?

@cwonrails
Copy link

I can try to replicate locally. What is your node and npm version?

@bangbang93
Copy link

from #6043
node v4.2.2
npm v2.14.7

same as Incognito mode,forever loading spinner
in users table I got an user record

# id, uuid, name, slug, password, email, image, cover, bio, website, location, accessibility, status, language, meta_title, meta_description, tour, last_login, created_at, created_by, updated_at, updated_by
'1', '9ef6529f-dd59-4a01-83b6-c18eacd8441d', '聚工厂', 'ju', 'passwordHash', 'admin@cofactories.com', NULL, NULL, NULL, NULL, NULL, NULL, 'active', 'en_US', NULL, NULL, NULL, NULL, '2015-11-04 17:50:54', '1', '2015-11-04 18:11:27', '1'

@cobbspur
Copy link
Member Author

cobbspur commented Nov 5, 2015

Okay so this seems to be my mistake. Other than the export issue which is its own separate issue now.

Locally without realising it, I have been using an old shortcut 127.0.0.1:2368/ghost but this is mapped to localhost in my hosts file. Using localhost:2368/ghost I do not experience any of these problems.

@kevinansfield
Copy link
Contributor

Ah okay, I thought it was strange that I couldn't reproduce exactly. You have managed to highlight a number of areas where our handling of authorisation has regressed however so I'm adding tests for those and working on fixes 😄

Are we good to close this issue in favour of the separate export and redirect/re-auth modal issues that I'll be opening?

@cobbspur
Copy link
Member Author

cobbspur commented Nov 5, 2015

Yeah I believe so, the export one has already been raised

@kevinansfield
Copy link
Contributor

Closing in favour of #6040, #6047, and #6048.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly P2 - High [triage] High priority for immediate patch release
Projects
None yet
Development

No branches or pull requests

6 participants