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

Upgrade botched user accounts #797

Closed
geoffb opened this issue Jan 18, 2014 · 29 comments
Closed

Upgrade botched user accounts #797

geoffb opened this issue Jan 18, 2014 · 29 comments

Comments

@geoffb
Copy link

geoffb commented Jan 18, 2014

I upgrade our install of NodeBB tonight and it created a couple problems with our user data.

  1. All user accounts became flagged as admins. This is obviously a pretty serious bug, but we were able to revert it by removing admin access via the admin panel.
  2. The user id seed was reset somehow. All newly created accounts are overwriting older accounts starting at the first created.

I've taken our forum offline while we address these issues.

Original commit: fcda27e
Upgraded commit: 324bec4

Something else odd was that after running the upgrade script, it ran me through the initial config again, but the defaults were our existing config values.

Anyway, a couple actionable things here:

  1. You may want to take a look at the upgrade script and see how it's possible that these 2 issues can happen. Both are pretty serious. Granted we were on an old-ish version of NodeBB before the upgrade, but if this can be prevented so much the better.
  2. How can I return the user id seed to the correct value? I had a backup script in place which stopped functioning about a month ago it turns out (doh!) so I'm going to need to work with the existing Redis dump file.

Thanks for your help!

@julianlam
Copy link
Member

Hi Geoff,

The upgrade guide can be found in our wiki. For admins following the latest edge builds, it is paramount that they stay up to date as the upgrade script is _reset after every minor revision_. For those who upgrade less frequently, it is advised that they do not stay on the master branch, and instead drop down to a stable branch such as v0.1.x or v0.2.0.

The source of your problems seems to be that you may have upgraded from 0

First off, back up your Redis database.

We'll try to re-run the update.

Secondly, drop down to the v0.2.x branch: git checkout v0.2.x

node app --upgrade

git checkout master

./nodebb upgrade (this command may fail because the 0.2.x upgrades were skipped. Let me know if this happens).

Once you get this far, you'll have to dive into Redis (possibly using redis commander) to restore your original users' accounts that got overwritten.

@julianlam
Copy link
Member

When it comes to why this happened -- the upgrade script was cleared at some point, although I don't recall why, since there have been no minor revisions since your original commit.

Alternatively, you can email us your database, and we can see if we can get it sorted out...

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

Hey Julian,

Thanks for the info. I wasn't aware of the upgrading policies (my fault) and performed the upgrade as I usually do.

Previous to the upgrade, my admin panel listed our NodeBB version as 1.1, so we shouldn't have been upgrading from 0. Not sure if that makes any difference.

Thanks for your help, too. I realize this is my doing and I made the classic mistake of not monitoring my backup script, so I hope my original post didn't come across otherwise.

I'll try to drop down to 0.2 and re-run the upgrade and see how it goes. /crosses fingers

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

I dropped down to v0.2.x and ran node app --upgrade. Everything looks fine.

Checked out master and ran ./nodebb upgrade and got the error (after npm deps update):

info: NodeBB v0.2.1 Copyright (C) 2013 DesignCreatePlay Inc.
info: This program comes with ABSOLUTELY NO WARRANTY.
info: This is free software, and you are welcome to redistribute it under certain conditions.
info: 
info: Beginning database schema update
info: [2013/12/31] Re-slugify Topics and Users skipped
info: [2013/12/31] maximumTitleLength skipped
info: [2014/1/3] categories.class, categories.link fields skipped
info: [2014/1/4] categories.numRecentReplies fields skipped
error: [upgrade] Errors were encountered while updating the NodeBB schema: Error: ERR Operation against a key holding the wrong kind of value

@barisusakli
Copy link
Member

How can I return the user id seed to the correct value? I had a backup script in place which stopped functioning about a month ago it turns out (doh!) so I'm going to need to work with the existing Redis dump file.

The next user id used to be a global key called global:next_user_id but it moved into a hash field. The hash name is global and the field is nextUid. There are bunch of keys like that that needs to be updated. https://github.com/designcreateplay/NodeBB/blob/v0.2.x/src/upgrade.js#L246.

The upgrade script tries to read the old key and set the new key here https://github.com/designcreateplay/NodeBB/blob/v0.2.x/src/upgrade.js#L236.

If it fails ie cant find the old key it will use an initial value.

You can check if the upgrade successfully created those keys through redis-cli. You should get something similar to below after the upgrade.

$ redis-cli
127.0.0.1:6379> hgetall global
 1) "nextUid"
 2) "572"
 3) "nextTid"
 4) "487"
 5) "nextGid"
 6) "8"
 7) "nextNid"
 8) "6208"
 9) "nextCid"
10) "13"
11) "nextMid"
12) "1723"
13) "nextPid"
14) "2889"
15) "userCount"
16) "573"
17) "topicCount"
18) "469"
19) "postCount"
20) "2826"

@barisusakli
Copy link
Member

@geoffb can you go into redis-cli and run this type cid:1:active_users. What is the output?

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

So, here's what I have after the upgrades:

redis 127.0.0.1:6379> hgetall global
1) "nextGid"
2) "1"
3) "nextUid"
4) "2"
5) "userCount"
6) "2"

And the result from the next command:

redis 127.0.0.1:6379> type cid:1:active_users
zset 

@barisusakli
Copy link
Member

Hmm looks like the global object is missing some fields.

Can you run get global:next_user_id and see the result? That is the old user seed value.

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

Yep:

redis 127.0.0.1:6379> get global:next_user_id
"36"

That looks about right.

@barisusakli
Copy link
Member

You can run these commands to fix the global object. Make sure the values are what you expect before running them. And make a backup of your db.

get global:next_user_id
hset global nextUid <valueFromPreviousCommand>
get next_topic_id
hset global nextTid <valueFromPreviousCommand>
get next_gid
hset global nextGid <valueFromPreviousCommand>
get notifications:next_nid
hset global nextNid <valueFromPreviousCommand>
get global:next_category_id
hset global nextCid <valueFromPreviousCommand>
get global:next_message_id
hset global nextMid <valueFromPreviousCommand>
get global:next_post_id
hset global nextPid <valueFromPreviousCommand>
get usercount
hset global userCount <valueFromPreviousCommand>
get totaltopiccount
hset global topicCount <valueFromPreviousCommand>
get totalpostcount
hset global postCount <valueFromPreviousCommand>

@julianlam
Copy link
Member

As those keys are already zsets, you can safely (liberally throwing that word out there) ignore that upgrade directive.

Change this line in upgrade.js to if(false) {

... then re-run ./nodebb upgrade

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

Should I perform both of these fixes? (The posts from both @barisusakli and @julianlam)

@barisusakli
Copy link
Member

Try @julianlam's first then check if hgetall global looks like the one I posted if not perform mine.

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

Follow @julianlam's steps with the following error:

info: [2014/1/5] Re-slugify usernames (again)
error: undefined
error: [upgrade] Errors were encountered while updating the NodeBB schema: Error: ERR Operation against a key holding the wrong kind of value

@barisusakli
Copy link
Member

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

@barisusakli Ran the upgrade script after making your edits and it completed with no errors.

The results of hgetall global doesn't return all the fields you've included so I'll go ahead and run your redis commands from the earlier comment.

@barisusakli
Copy link
Member

Ok check the values you get from the get commands and make sure they are reasonable. They should all be positive values.

@julianlam
Copy link
Member

Hopefully that should be the only things you need to do... barring fixing the overwritten user accounts, that is.

I've done upgrades from v0.1.x to latest with no errors, although you do need to make a pit stop at v0.2.x as well. Once this is all said and done, would it be possible for you to send us a copy of your last known good copy of the db, so we can try running some upgrades on it for post mortem purposes?

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

@barisusakli Here's what I've got:

redis 127.0.0.1:6379> hgetall global
 1) "nextGid"
 2) "1"
 3) "nextUid"
 4) "36"
 5) "userCount"
 6) "36"
 7) "nextTid"
 8) "109"
 9) "nextNid"
10) "2687"
11) "nextCid"
12) "13"
13) "nextMid"
14) "40"
15) "nextPid"
16) "1339"
17) "topicCount"
18) "109"
19) "postCount"
20) "1336"

They all look reasonable. I'm not sure what nextGid is, though. Should it be "1"?

@julianlam There's only one overwritten account (which is mine, as I was the first user). The posts seem to be intact, but the user details are incorrect. How would I go about fixing that?

I'll definitely send along a copy of our last known good data. Looking at my backups, it stopped updating on Nov 19 of last year. I don't think I've done any upgrades since then, but I'm not certain. Either way, I'll send along a copy of that data once I've got this all taken care of.

Many, many thanks to both of you for spending time on your Friday night to help me out. :)

@barisusakli
Copy link
Member

nextGid is the the next group id, if you try to create a new group it will increment that value and use 2 as the next groups id.

To fix your user data since you are user id 1. You can do :

//see user data
hgetall user:1
hset user:1 username geoffb
hset user:1 email geoff@gmail.com
hset user:1 <fieldname> <fieldvalue>

Just fix all the fields you need like that.

@geoffb
Copy link
Author

geoffb commented Jan 18, 2014

So, I think we've got things mostly in working order. Thanks! I do have a few more issues, one of which is fairly critical:

  1. All newly created users are automatically granted administrator status.
  2. No groups are listed in the ACP and new groups cannot be created. Possibly related to the issue above? See below for log warnings.
  3. The "Settings" section of the ACP does not display any data, however the basic settings like name, MOTD, etc are in-use on the forum itself. Additionally, changing the values and clicking Save does nothing besides create log warnings. Again, see below.
  4. Minor issue: My overwritten user account has 0 post count and reputation. Is there a way to recalculate these values?

Log warnings:

warn: [socket.io] Unrecognized message: api:get_all_rooms
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:config.get
warn: [socket.io] Unrecognized message: api:updateHeader
warn: [socket.io] Unrecognized message: api:updateHeader
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:get_all_rooms
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:get_all_rooms
warn: [socket.io] Unrecognized message: event:enter_room
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: api:user.get_online_users
warn: [socket.io] Unrecognized message: api:meta.buildTitle
warn: [socket.io] Unrecognized message: api:groups.create
warn: [socket.io] Unrecognized message: api:groups.create

@barisusakli
Copy link
Member

Make sure you clear the browser cache that might cause those warnings.

If there are no groups you might have to create the admin group manually.

// see if admin group exists
hgetall gid:1
{
"gid":"1",
"name":"Administrators",
"description":"Forum Administrators",
"deleted":"0"
}

If it doesnt exist create it from redis-cli.

hset gid:1 gid 1
hset gid:1 name Administrators
hset gid:1 description "Forum Administrators"
hset gid:1 delete 0

Then do a smembers gid:1:members to see the members of that group. If its empty or non existant create and fill it with admin user ids like so :

sadd gid:1:members 1
sadd gid:1:members <uid of another admin>

@julianlam
Copy link
Member

Before running Baris' commands, maybe give this a try?

Looks like the "registered-users" and "admin" groups have been "merged".

Give this a look: hgetall group:gid. My guess is it will report that both "Administrators" and "registered-users" is gid 1. This explains why all new users are becoming admins. Let's fix this: hdel group:gid registered-users.

hgetall gid:1 should show group details for "Administrators". It probably shows stuff about registered users, right? If so, run the hset gid:1 commands that baris listed above to fix this group's details.

The registered-users group is not of importance, and it should be re-created automatically by NodeBB

@geoffb
Copy link
Author

geoffb commented Jan 19, 2014

Excellent! @julianlam was correct, the registered users group and administrators group had been merged into one. I fixed up the data in Redis as instructed and everything looks good on that front.

I cleared my cache as @barisusakli suggested and that fixes my ACP issues as well. I can now update settings, etc.

I think we're in good shape now. Thanks so much for helping me work through this. I'll be more careful next time around. :)

@geoffb geoffb closed this as completed Jan 19, 2014
@barisusakli
Copy link
Member

Glad to hear everything is resolved 😄

@julianlam
Copy link
Member

Two items to address:

  • Admin scripts seem to be missing a cache buster
  • Upgrade scripts that were removed need to be added to upgrade.js before we launch 0.3.0

@psychobunny
Copy link
Contributor

@julianlam @barisusakli imo I agree with the original decision to keep flushing out older upgrade paths (can you imagine how big that file will be a year from now?)

instead, add a check to make sure we're not upgrading too far so something like a minver check for the last upgradable version would be best imo

I'm re-opening this so we don't forget about it for 0.3.0

@psychobunny psychobunny reopened this Jan 19, 2014
@julianlam
Copy link
Member

Agreed. I was thinking about how we could prevent this from happening
again. Basically, every time we "flush" the upgrade.js, we have to add a
minimum/initial schemaDate to the file. If they're going into upgrade.js
with a schemaDate that's too low, then abort.
On Jan 19, 2014 2:34 AM, "psychobunny" notifications@github.com wrote:

@julianlam https://github.com/julianlam @barisusaklihttps://github.com/barisusakliimo I agree with the original decision to keep flushing out older upgrade
paths (can you imagine how big that file will be a year from now?)

instead, add a check to make sure we're not upgrading too far so something
like a minver check for the last upgradable version would be best imo

I'm re-opening this so we don't forget about it for 0.3.0


Reply to this email directly or view it on GitHubhttps://github.com//issues/797#issuecomment-32702923
.

@barisusakli
Copy link
Member

#808

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

No branches or pull requests

4 participants