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

Player Password File for "account database" #2917

Merged
merged 58 commits into from
Aug 24, 2021

Conversation

cwisniew
Copy link
Member

@cwisniew cwisniew commented Aug 24, 2021

This PR adds a player account file which contains usernames and passwords
Note this only the password file and authentication pieces (including public / private key).
There is still the user interface and macros to be developed as next step, but its still useable even if it does currently require manually editing of the player password file.

The password file is stored in the passwords.json file in the config subdir of the data directory (e.g. ~/.maptool-rptools/config/passwords.json

An example of this file

{
  "passwords": [
    {
      "username": "player1",
      "password": "/49vX2/i/5YhQZTaalYnoA",
      "salt": "DZAp3sKZSY/vWuLfTG26mhQbOx5PLNT0tRdNvTNMn8KjDlY3jJg180ZBN56ehfkygOk6v0CgySZaufGkOKj7RqOwudzoWZhtNGSabkbI4s85TG7Ecfz8HQQRfIi1XtWrlqkrnuiyD8zQFg9zNr0xvQlUwzL5UOK7emt/xPr2QfU",
      "role": "Player"
    },
    {
      "username": "gm1",
      "password": "9SH7OrqlXyRMrSXy2fQHaw",
      "salt": "LOWXSGZ5tNeCwVpzNFuK/4EC+9jHMPmgpZ3Q05kElZyJKSWk0dGQOuZWADH2ZYAs4UvN3DVczMbAO6Dm5+NdnvsZsb/SnZ/xEM0QZ5yvmOh0bGjWEHwqTwFBbRUrzlpzy1nNKdJU8e7G2qWmaLUCDSV5Ut50l9aBfeL29d5btLA",
      "role": "GM"
    },
    {
        "username": "player2",
        "publicKeys": [
            "player2.key"
        ],
        "role": "Player"
    }
  ]
}

The "password" field is after encryption (using the salt specified), to add a user you can add the plain text password without the "salt" and when the file is read (during start server) it will encrypt the password and rewrite the file with the encrypted password and salt (a backup of the old password file will be created). It's also possible to add new entries without editing the file by creating a passwords_add.json file in the same directory with the same format and on "start server" this will add/replace passwords in passwords.json (this method also will encrypt any password without a salt)

e.g.

{
	"passwords": [
		{
	            "username": "player3",
		    "password": "This is my password",
                    "role": "Player"
		}
	]
}

Will be replaced with

{
	"passwords": [
		{
			"username": "player3",
			"password": "<encrypted password>",
	 		"salt": "<random salt>"
			"role": "Player"
		}
	]
}

public keys are stored in the config/keys subdirectory, a player can have more than one public key which can be stored in multiple files or multiple in a single file. The format of public key is the same as that displayed / copied from MapTool (see below)

When starting the server select the "Use MapTool Password file" to start with the password file (this is also the trigger for reading file and encrypting passwords as described above)

CleanShot 2021-08-24 at 22 29 06@2x

If this checkbox is checked the person starting the server will still connect with the specified name in the dialog which DOES NOT need to be in the password file.

You can display the public key in the preferences dialog, authentication tab...

CleanShot 2021-08-24 at 22 28 29@2x

To connect to a server using public key you can just specify the player name that server expects a public key for and the public key will be used automagically... There needs to be changes to the connect to server dialog so players dont need to enter a dummy password an issued has been created for this (2916)

The value copied for the public key in the password file is exactly the same as copied to the clipboard with the "Copy Public Key" button.

Fixes #2906
Fixes #2915


This change is Reviewable

Copy link
Contributor

@Phergus Phergus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 65 of 65 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @cwisniew)

@Phergus Phergus merged commit be6c5da into RPTools:develop Aug 24, 2021
@Azhrei
Copy link
Member

Azhrei commented Aug 24, 2021

I haven’t looked through the whole thing (on my phone), but I believe there’s a bug in receiveHandshake() where the name string is constructed by String() without passing “UTF-8” as the second parameter. If the receiver’s locale is not already UTF-8, I think this can fail. (The sender forces UTF-8 so it makes sense for the receiver to do so as well.)

@Phergus
Copy link
Contributor

Phergus commented Aug 24, 2021

As the JVM is started with file.encoding set to UTF-8 wouldn't adding it to the constructor call be redundant?

@Azhrei
Copy link
Member

Azhrei commented Aug 24, 2021

Idk. The documentation says it’ll use the “platform default”. I took that to mean, well, the default for the platform, not the value set by a command line parameter.

In any case, if the locale is being forced by the sending function, shouldn’t it be forced on the receiving side, just for the sake of orthogonality if nothing else?

Besides, do you really want to rely on a command line parameter to ensure the code works properly when adding 9 characters to the source code would guarantee it? :)

@thelsing
Copy link
Collaborator

Is the included web endpoint necessary for this feature, or did it get included on mistake?

@Phergus
Copy link
Contributor

Phergus commented Aug 24, 2021

Besides, do you really want to rely on a command line parameter to ensure the code works properly when adding 9 characters to the source code would guarantee it? :)

Well, we do rely upon another dozen or so command line parameters to insure the code works properly so not a huge concern but putting that charset parameter in will make it more consistent and potentially avoid problems in the future.

@cwisniew
Copy link
Member Author

Is the included web endpoint necessary for this feature, or did it get included on mistake?

I will be making use of it in the UI (and potentially bulk updates) for the epic.

@cwisniew cwisniew deleted the feature-player-pw-backend branch August 25, 2021 05:38
@thelsing
Copy link
Collaborator

Another one: You talk about "encrypting" passwords. But you hash them, right?

@cwisniew
Copy link
Member Author

Another one: You talk about "encrypting" passwords. But you hash them, right?

Hashed with PBKDF2WithHmacSHA1
It was almost midnight when I wrote the text above and hashing is a technically a form of encryption... That's my story and I am sticking to it :)

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

Successfully merging this pull request may close these issues.

Add public / private key support to player data Password File Authentication for Player Passwords
4 participants