Merge pull request #130 from kharidiron/master

Hot fix for players escaping wrapper, party chat feature, fixed login error in webgui, bugfixes in database, more logging and debugging, better regular expression for matching color codes in names, and minor fixes in packet definitions.

Author: kamilion

README.Protocol668.md

@@ -0,0 +1,110 @@
+# Changes regarding Protocol 668
+
+With the release of Starbound protocol version 668, you may have noticed that 
+the account/password system is no longer working. This is resulting from 
+Chucklefish updating their authentication system.
+
+While this does break some of StarryPy's functionality, the solution is not to
+fix StarryPy, but instead to change how you think about 'authentication' in
+Starbound.
+
+For the impatient, please scroll down to the **Fixing the Problem** section for
+the cut-and dry solution.
+
+
+## Changes to **starbound.config**
+
+As @alex-lawson (aka - metadept) pointed out in the news post
+http://playstarbound.com/february-17-server-configuration-changes/ 
+there were some changes in how **starbound.config** is structured. As a callout
+here, the changes are:
+
+```
+  "allowAnonymousConnections" : false,
+  "allowAdminCommands" : true,
+  "allowAdminCommandsFromAnyone" : false,
+  "bannedIPs" : [  ],
+  "bannedUuids" : [  ],
+
+...
+
+  "serverUsers" : {
+    "fred" : {
+      "admin" : true,
+      "password" : "hunter2"
+    },
+    "george" : {
+      "admin" : false,
+      "password" : "swordfish"
+    }
+  },
+```
+
+In order to adapt this to StarryPy, we need to change the way we think about
+authentication. Previously, most servers would use a shared, public or shared,
+private password. This, combined with a UUID and a name would uniquely identify
+a character. The flaw with this system, however, was the assumption that a
+character's UUID would remain obfuscated from other users, ensuring uniqueness.
+
+This however, is by far, no longer the case as UUID numbers are now quite easy 
+to collect, and thus to reuse and *'spoof'* other characters. Particularly for 
+character's with administrative privileges, this was a concern that needed to be
+addressed.
+
+Enter git commit https://github.com/kharidiron/StarryPy/commit/c371ade0301be369c8f4c9baedcc5e9685fc8633
+where I added an additional variable called `admin_ss` for tracking if an
+authenticated user also provided an additional *shared secret* password for
+accessing privileged functions. It was then, up to the server administrators to
+make sure their admins were informed of the shared secret. This would not
+prevent UUID spoofers from doing their spoofing, but it *WOULD* prevent them 
+from being able to run admin commands. This sort of system is termed a 'dead
+man's switch'.
+
+Fast-forward to release of protocol 668, and now people entering the shared
+secret password are being greeted with 'No such account or incorrect password.'
+
+Now what were we to do?
+
+Originally I was starting to work out how to re-write the code to account for
+new user accounts, and access levels, and such... a minor headache, and some
+time debt to say the least. But then a user in the IRC channel 
+(gandalfthecolorb) pointed out that no changes were actually needed. Instead,
+we need to just add an account to the Starbound server configuration to act as
+the collective 'rolls' for all the admin levels. An easy, and elegant solution
+that requires no changing of code on our end, and still maintains the same level
+of security for the servers.
+
+So, now on to fixing the problem.
+
+
+## Fixing the Problem
+
+#### tl;dr
+
+Using the same `admin_ss` password that users set before, along with whatever
+server password StarryPy owners want to ship, we simply need to update the
+starbound.config file to match:
+
+```
+  "serverUsers" : {
+    "<admin_ss goes here>" : {
+      "admin" : <can be true or false, per your needs>,
+      "password" : "<either continue using your old password, or set a new one>"
+    }
+  }
+```
+
+And that is it. If you choose to continue using a shared public password, you
+would need to add an additional section for this, and then provide all of your
+users with a generic 'account' to log into (from metadept's example, this would
+be *'guest'*). You would also need to be sure to set `allowAnonymousConnections`
+to `false` as well.
+
+
+#### An additional note regarding commands
+
+StarryPy can be configured to either block, or allow vanilla server commands,
+by changing the option `command_prefix` to something other than `/`. Some
+suggestions have been for `!` instead. This, on its own, does not enable the
+Starbound `/admin` commands, but conversely, can prevent you from using them if
+you leave the prefix in its default state.

config.py

@@ -19,6 +19,11 @@ def __call__(cls, *args, **kwargs):
 class ConfigurationManager(object):
     __metaclass__ = Singleton
     logger = logging.getLogger("starrypy.config.ConfigurationManager")
+    log_format = logging.Formatter('%(asctime)s - %(levelname)s - %(name)s # %(message)s')
+    logfile_handle = logging.FileHandler("config.log")
+    logfile_handle.setLevel(9)
+    logger.addHandler(logfile_handle)
+    logfile_handle.setFormatter(log_format)
 
     def __init__(self):
         default_config_path = path.preauthChild("config/config.json.default")
@@ -27,7 +32,8 @@ def __init__(self):
             try:
                 with default_config_path.open() as default_config:
                     default = json.load(default_config)
-            except ValueError:
+            except ValueError as e:
+                print "Error: %s" % e
                 self.logger.critical("The configuration defaults file (config.json.default) contains invalid JSON. Please run it against a JSON linter, such as http://jsonlint.com. Shutting down." )
                 sys.exit()
         else:
@@ -39,7 +45,8 @@ def __init__(self):
                 with self.config_path.open() as c:
                     config = json.load(c)
                     self.config = recursive_dictionary_update(default, config)
-            except ValueError:
+            except ValueError as e:
+                print "Error: %s" % e
                 self.logger.critical("The configuration file (config.json) contains invalid JSON. Please run it against a JSON linter, such as http://jsonlint.com. Shutting down.")
                 sys.exit()
         else:
@@ -98,4 +105,4 @@ def __setattr__(self, key, value):
             self.save
         else:
             self.config[key] = value
-            self.save()
+            self.save()

config/config.json.default

@@ -191,8 +191,8 @@
             "admin_ss": "tester",
             "auto_activate": true,
             "name_removal_regexes": [
-                "\\^#[\\w]+;",
-                "[^ \\w]+"
+                "\\^\\w+;|\\^#\\w+;|\\W",
+                "\\s\\s+"
             ]
         },
         "players_plugin": {

packets/packet_types.py

@@ -257,12 +257,25 @@ def _decode(self, obj, context):
                                                             GreedyRange(star_string("requests")))
 
 # (14) - ClientContextUpdate
+#client_context_update = lambda name="client_context": Struct(name,
+#                                                             VLQ("length"),
+#                                                             Byte("arguments"),
+#                                                             Array(lambda ctx: ctx.arguments,
+#                                                                   Struct("key",
+#                                                                   Variant("value"))))
 client_context_update = lambda name="client_context": Struct(name,
                                                              VLQ("length"),
-                                                             Byte("arguments"),
-                                                             Array(lambda ctx: ctx.arguments,
-                                                                   Struct("key",
-                                                                   Variant("value"))))
+                                                             Peek(Byte("a")),
+                                                             If(lambda ctx: ctx["a"] == 0,
+                                                                Struct("junk",
+                                                                       Padding(1),
+                                                                       VLQ("extra_length"))),
+                                                             If(lambda ctx: ctx["a"] > 8,
+                                                                Struct("junk2",
+                                                                       VLQ("extra_length"))),
+                                                             VLQ("subpackets"),
+                                                             Array(lambda ctx: ctx.subpackets,
+                                                                   (Variant("subpacket"))))
 
 # (15) - WorldStart
 world_start = lambda name="world_start": Struct(name,
@@ -409,5 +422,5 @@ def _decode(self, obj, context):
         properties=[Container(key=k, value=Container(type="SVLQ", data=v)) for k, v in dictionary.items()]))
 
 # (53) - Heartbeat
-heartbeat = lambda name="heartbeat": Structure(name,
-                                               UBInt64("remote_step"))
+heartbeat = lambda name="heartbeat": Struct(name,
+                                            VLQ("remote_step"))

plugins/core/admin_commands_plugin/admin_command_plugin.py

@@ -281,14 +281,14 @@ def item(self, data):
                         item_count = item[1]
                     else:
                         item_count = 1
-                    give_item_to_player(target_protocol, item_name, item_count)
+                    given = give_item_to_player(target_protocol, item_name, item_count)
                     target_protocol.send_chat_message(
                         "%s^green; has given you: ^yellow;%s^green; (count: ^cyan;%s^green;)" % (
-                            self.protocol.player.colored_name(self.config.colors), item_name, item_count))
+                            self.protocol.player.colored_name(self.config.colors), item_name, given))
                     self.protocol.send_chat_message("Sent ^yellow;%s^green; (count: ^cyan;%s^green;) to %s" % (
-                        item_name, item_count, target_player.colored_name(self.config.colors)))
+                        item_name, given, target_player.colored_name(self.config.colors)))
                     self.logger.info("%s gave %s %s (count: %s)", self.protocol.player.name, name, item_name,
-                                     item_count)
+                                     given)
                 else:
                     self.protocol.send_chat_message("You have to give an item name.")
             else:

plugins/core/player_manager/manager.py

@@ -45,23 +45,44 @@ def migrate_db(config):
     dbcon = sqlite3.connect(path.preauthChild(config.player_db).path)
     dbcur = dbcon.cursor()
 
+    res = dbcur.execute("PRAGMA user_version;")
+    db_version = res.fetchone()[0]
+    try:
+        if db_version is 0:
+            dbcur.execute('DROP TABLE `ips`;')
+            dbcur.execute("PRAGMA user_version = 1;")
+            logger.info("Migrating DB from version 0 to version 1.")
+    except sqlite3.OperationalError, e:
+        logger.info("No DB exists. Will create a new one.")
+
     try:
         dbcur.execute('SELECT org_name FROM players;')
     except sqlite3.OperationalError, e:
         if "column" in str(e):
+            logger.info("Updating DB to include org_name column.")
             dbcur.execute('ALTER TABLE `players` ADD COLUMN `org_name`;')
             dbcur.execute('UPDATE `players` SET `org_name`=`name`;')
             dbcon.commit()
 
+    try:
+        dbcur.execute('SELECT party_id FROM players;')
+    except sqlite3.OperationalError, e:
+        if "column" in str(e):
+            logger.info("Updating DB to include party_id column.")
+            dbcur.execute('ALTER TABLE `players` ADD COLUMN `party_id`;')
+            dbcur.execute('UPDATE `players` SET `party_id`="";')
+            dbcon.commit()
+
     try:
         dbcur.execute('SELECT admin_logged_in FROM players;')
     except sqlite3.OperationalError, e:
         if "column" in str(e):
+            logger.info("Updating DB to include admin_logged_in column.")
             dbcur.execute('ALTER TABLE `players` ADD COLUMN `admin_logged_in`;')
             dbcur.execute('UPDATE `players` SET `admin_logged_in`=0;')
             dbcon.commit()
-    dbcon.close()
 
+    dbcon.close()
 
 logger = logging.getLogger("starrypy.player_manager.manager")
 
@@ -175,6 +196,7 @@ class Player(Base):
     admin_logged_in = Column(Boolean)
     protocol = Column(String)
     client_id = Column(Integer)
+    party_id = Column(String)
     ip = Column(String)
     plugin_storage = Column(JSONEncodedDict, default=dict())
     planet = Column(String)
@@ -233,6 +255,7 @@ class Ban(Base):
 class PlayerManager(object):
     def __init__(self, config):
         self.config = config
+        migrate_db(self.config)
         logger.info("Loading player database.")
         try:
             self.engine = create_engine('sqlite:///%s' % path.preauthChild(self.config.player_db).path)
@@ -244,6 +267,7 @@ def __init__(self, config):
             for player in session.query(Player).filter_by(logged_in=True).all():
                 player.logged_in = False
                 player.admin_logged_in = False
+                player.party_id = ""
                 player.protocol = None
                 session.commit()
 
@@ -280,7 +304,8 @@ def fetch_or_create(self, uuid, name, org_name, admin_logged_in, ip, protocol=No
                 if player.name != name:
                     logger.info("Detected username change.")
                     player.name = name
-                if ip not in player.ips:
+                if not session.query(IPAddress).filter_by(uuid=uuid, ip=ip).first():
+                    logger.debug("New ip address detected for user. Adding to database.")
                     player.ips.append(IPAddress(ip=ip))
                     player.ip = ip
                 player.protocol = protocol
@@ -295,6 +320,7 @@ def fetch_or_create(self, uuid, name, org_name, admin_logged_in, ip, protocol=No
                                 admin_logged_in=False,
                                 protocol=protocol,
                                 client_id=-1,
+                                party_id="",
                                 ip=ip,
                                 planet="",
                                 on_ship=True)

plugins/core/player_manager/plugin.py

@@ -32,6 +32,7 @@ def check_logged_in(self):
             if player.protocol not in self.factory.protocols.keys():
                 player.logged_in = False
                 player.admin_logged_in = False
+                player.party_id = ""
 
     def on_client_connect(self, data):
         client_data = client_connect().parse(data.data)
@@ -99,7 +100,9 @@ def reject_with_reason(self, reason):
                 Container(
                     success=False,
                     client_id=0,
-                    reject_reason=reason
+                    reject_reason=reason,
+                    celestial_info_exists=False,
+                    celestial_data=None
                 )
             ) + unlocked_sector_magic
         )
@@ -114,6 +117,7 @@ def on_connect_response(self, data):
             else:
                 self.protocol.player.client_id = connection_parameters.client_id
                 self.protocol.player.logged_in = True
+                self.protocol.player.party_id = ""
                 self.logger.info("Player %s (UUID: %s, IP: %s) logged in" % (
                     self.protocol.player.name, self.protocol.player.uuid,
                     self.protocol.transport.getPeer().host))
@@ -256,4 +260,4 @@ def format_player_response(self, players):
                      players[:25]]))
             self.protocol.send_chat_message(
                 "And %d more. Narrow it down with SQL like syntax. Feel free to use a *, it will be replaced appropriately." % (
-                    len(players) - 25))
+                    len(players) - 25))

plugins/fuelgiver/fuelgiver_plugin.py

@@ -28,9 +28,9 @@ def fuel(self, data):
             return
         if not 'last_given_fuel' in my_storage or float(my_storage['last_given_fuel']) <= float(time()) - 86400:
             my_storage['last_given_fuel'] = str(time())
-            give_item_to_player(self.protocol, "fillerup", 1)
+            given = give_item_to_player(self.protocol, "fillerup", 1)
             self.protocol.player.storage = my_storage
             self.protocol.send_chat_message("You were given a daily fuel supply! Now go explore ;)")
             self.logger.info("Gave fuel to %s.", self.protocol.player.name)
         else:
-            self.protocol.send_chat_message("^red;No... -.- Go mining!")
+            self.protocol.send_chat_message("^red;No... -.- Go mining!")

plugins/new_player_greeter_plugin/new_player_greeter_plugin.py

@@ -25,7 +25,7 @@ def after_world_start(self, data):
 
     def give_items(self):
         for item in self.config.plugin_config["items"]:
-            give_item_to_player(self.protocol, item[0], item[1])
+            given = give_item_to_player(self.protocol, item[0], item[1])
 
     def send_greetings(self):
-        self.protocol.send_chat_message(self.config.plugin_config["message"])
+        self.protocol.send_chat_message(self.config.plugin_config["message"])

plugins/partychat_plugin/__init__.py

@@ -0,0 +1 @@
+from partychat_plugin import PartyChatPlugin