Improve: add functions for handling game character details#3952
Improve: add functions for handling game character details#3952SlySven wants to merge 34 commits intoMudlet:developmentfrom
Conversation
`getPlayerName` takes no arguments and returns the "Character name" from
the connection screen. There is no provision to change this stored setting.
`sendPlayerName` takes no arguments and causes the "Character name" from
the connection screen to be sent without echoing to the game server, it
will return a nil plus an error message should the name not be set (is
empty string).
`sendPlayerPassword` takes no arguments and causes the "Password" from
the connection screen to be sent without echoing to the game server, it
will return a nil plus an error message should no password be set (is empty
string). There is no provision to find out this stored setting.
`sendCustomLoginText` takes no arguments and causes the custom login text
to be sent provided it is not disabled (set to the 0th one).
`getCustomLoginTextId` gets the index (from zero upwards) of the custom
login text - so at this point will return `0` {disabled} or `1`.
Remove saving and restoring login name and password either side of
package/module loading - we do not need them to protect the details from
being changed in a loaded XML file because those pieces of information are
not stored in there.
Move the existing timed sending of the user name and password to a new
`doLogin()` Lua function in the external `LuaGlobal.lua` file which
effectively does the same thing but can be replaced by something more
sophisticated if the user needs it. It also checks whether the option to
use a custom login text has been set and will send the chosen string two
seconds after a successful connection instead of the character name and
password after two and three seconds respectively.
Alternatively the user can replace it with a dummy (empty) function with
the same name and use a pair of triggers (which I have tested) or an
`sysConnectionEvent` event handler.
The custom login currently only offers one text - of the form:
"connect {character name} {password}"
which can be selected from a new comboBox on the "Special options" tab of
the preferences. It is saved with the profile data.
The custom strings could also be appended to if a compelling case is made
to the Mudlet Makers though for backwards compatibility it would not be
recommended to remove a string previously added.
For user display purposes the string shown in the preferences is expected
to be translated for the tokens "{character name}" and "{password}" however
the system is designed to not have the actual string changed on a per
locale basis. The UI texts are generated by a method that detects whether
it is being used for the first time on dialogue creation (and so considers
the existing profile's setting) or after a GUI language change - which
keeps the same setting but shows it in the new language. It is my intend
to refactor out the population of some of the other combo-boxes in that
dialogue as well so that the user will not have to restart Mudlet just to
get other important settings in a language that they can comprehend.
Also:
* demote dlgConnectionProfiles::slot_connectToServer() from a slot to a
normal method - as it is not connected up to the signal/slot system.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
That is not a practical problem facing Mudlet players today - let's avoid complicating the code further to address a non-issue. There are other real issues we could fix instead. |
There was a problem hiding this comment.
This a great start for custom login texts, but we need to streamline the design to be simpler to understand for everyone involved.
I don't understand how can I make a custom login setup for a game that doesn't use "connect username password" when the options in the UI fit neither.
I had a much simpler design in my head: no UI changes, just a doLogin() function that you can overwrite. People then can create a package that'll handle logging in appropriately. For example we could make a "connect username password" package - install it and you're good to go. Combined with the fact that we're working on the package manager, having more extensibility and less things hardcoded in Mudlet is better this way.
The whole point is to avoid the possibility of a script/package being put in that could send a random (i.e. the contents not reviewed/approve by us) string containing the password - and for the use of that to be clearly specified/chosen/visible/controllable to/by the user via the GUI. |
Vadim pointed out how to use std::chrono_literals - which I did not previous understand... Signed-off-by: Stephen Lyons <slysven@virginmedia.com> Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com> Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>
|
We can solve that just by disabling the send password function after X seconds within connecting. |
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Using `auto` doesn't seem to work in this situation but it isn't the end of the work to have to give the type in the definition. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux
|
That last commit was needed to resolve a conflict caused by another (just merged by me) PR adding something else in the same place in a header file... |
|
#3952 (review) is still outstanding :) I'd like to simplify the design to be clearer for everyone involved. |
This is one portion of the stalled PR Mudlet#3952 that can be lifted from there and be implemented without raising any eyebrows (IMHO). It returns the string that the user entered into the "Character name:" field on the "Connection profile" dialogue and is needed to allow a shared module to customise its behaviour depending on the particular profile that it is being used with on the basis of the identifier being sent for login purposes - *c.f.* [`getProfileName()`](https://wiki.mudlet.org/w/Manual:Lua_Functions#getProfileName) which instead identifies the profile name which is very likely not the same text. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
This is one portion of the stalled PR #3952 that can be lifted from there and be implemented without raising any eyebrows (IMHO). It returns the string that the user entered into the "Character name:" field on the "Connection profile" dialogue and is needed to allow a shared module to customise its behaviour depending on the particular profile that it is being used with on the basis of the identifier being sent for login purposes - c.f. https://wiki.mudlet.org/w/Manual:Lua_Functions#getProfileName which instead identifies the profile name which is very likely not the same text. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux Needed to remove getCharacterName() function which has been extracted from this PR and done separately - and now merged into the main developement branch. Signed-off by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux This is so as to try and fix some CI issues that I believe have been solved in the main development branch. Signed-off by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux To bring in changes from main development branch. Signed-off by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux
Did you have the call? |
…aracterDetails_redux
…aracterDetails_redux
…aracterDetails_redux
|
We did not... and even with my simpler plan of implementing it, it would still have required the player to fobble with some knobs in order to get it to work. That's not how we want Mudlet to be though, right? We want things to just work out of the box. So I put together a spec instead to address this: https://wiki.mudlet.org/w/Standards:GMCP_Authentication, and I implemented it in Mudlet: #7152. There's also a little mock server validating the implementation that can be used as inspiration for server developers. If a server implements GMCP Auth, it will both avoid the players having to configure how authentication should work and it will avoid the security issues we've been running into as we try to implement this. |
…aracterDetails_redux
Also update some things that had been changed elsewhere in the code-base during the period this PR has been open. (Replacement of `QStringLiteral` by `qsl`, addition of `(QString) utils::richText(const QString)` wrapper. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey, appreciate the effort here. This might be more complex than we need at the moment 1, so I’m closing the PR. Let’s keep the ideas coming! 1 (in comparison, making a trigger is much simpler, although not as as secure as a password in a keychain) |
…aracterDetails_redux Conflicts resolved in: * src/dlgConnectionProfiles.cpp Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux
Extended the functions disabled by the login timeout to include the `sendCharacterName()` function - and allow the timer that provides that timeout to be stopped by the GMCP authenticator that Mudlet#7152 introduced. Such a stop also immediately disables those functions so that it will neutralise any scripts or triggers that would be using them. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…aracterDetails_redux
|
I use multiple characters on one profile so don't use autologin. My password is shown in my logs and command line history. It could also be sniffed with sysDataSendRequest. It is sent plain text down the wire all the way to the game server. Does my game encrypt it? Who knows?! There are other methods to get my password which could be used. Spending a lot of effort here on security when there are alternatives to snatch it, just saying. Just make it easier and customisable for the end user. And for me this would mean triggers and scripts and |
|
@ZookaOnGit doing that is your choice 😲 - but given the other steps we have taken over the years to store the passwords securely within each OSes "secure storage" systems if that is the (less portable, admittedly) choice they take and to provided means to use an encrypted transport to the Game Server if it is available then we don't want to make it trivial for them to be sniffed out by a "bad" package/script... |
|
Seems discussion on the best way to move forward has stalled. Closing this one for now. |
getCharacterNametakes no arguments and returns the "Character name" from the connection screen. There is no provision to change this stored setting.sendCharacterNametakes no arguments and causes the "Character name" from the connection screen to be sent without echoing to the game server, it will return a nil plus an error message should the name not be set (is empty string).sendCharacterPasswordtakes no arguments and causes the "Password" from the connection screen to be sent without echoing to the game server, it will return a nil plus an error message should no password be set (is empty string). There is no provision to find out this stored setting.sendCustomLoginTexttakes no arguments and causes the custom login text to be sent provided it is not disabled (set to the 0th one).getCustomLoginTextIdgets the index (from zero upwards) of the custom login text - so at this point will return0{disabled} or1.Remove saving and restoring login name and password either side of package/module loading - we do not need them to protect the details from being changed in a loaded XML file because those pieces of information are not stored in there.
Move the existing timed sending of the user name and password to a new
doLogin()Lua function in the externalLuaGlobal.luafile which effectively does the same thing but can be replaced by something more sophisticated if the user needs it. It also checks whether the option to use a custom login text has been set and will send the chosen string two seconds after a successful connection instead of the character name and password after two and three seconds respectively.Alternatively the user can replace it with a dummy (empty) function with the same name and use a pair of triggers (which I have tested) or an
sysConnectionEventevent handler.The custom login currently only offers one text - of the form:
"connect {character name} {password}"
which can be selected from a new comboBox on the "Special options" tab of the preferences. It is saved with the profile data.
The custom strings could also be appended to if a compelling case is made to the Mudlet Makers though for backwards compatibility it would not be recommended to remove a string previously added.
For user display purposes the string shown in the preferences is expected to be translated for the tokens "{character name}" and "{password}" however the system is designed to not have the actual string changed on a per locale basis. The UI texts are generated by a method that detects whether it is being used for the first time on dialogue creation (and so considers the existing profile's setting) or after a GUI language change - which keeps the same setting but shows it in the new language. It is my intention to refactor out the population of some of the other combo-boxes in that dialogue as well so that the user will not have to restart Mudlet just to get other important settings in a language that they can comprehend on the first time they run Mudlet.
Also:
This is a cherry-pick and edit of code originally promulgated as #2252 with the extraneous bits picked out...! 😀
Closes #2961, #3920 and would also have closed #3562 if it were still open.
Signed-off-by: Stephen Lyons slysven@virginmedia.com