Add Hardware ID support on Licenses#766
Conversation
fuzziqersoftware
left a comment
fuzziqersoftware
left a comment
There was a problem hiding this comment.
Overall this looks good; I have a couple of style comments. Thanks for working on this.
I didn't review until now because this PR is in the draft state, but today I noticed that the PR description asks for feedback. In the future you can ping me (on Discord, or here with an @-mention) if I don't notice and you want a more timely review.
| struct BBLicense { | ||
| std::string username; | ||
| std::string password; | ||
| std::string hardware_id; |
There was a problem hiding this comment.
std::optional<uint64_t> would make more sense here. We could serialize it as an integer or null in JSON.
There was a problem hiding this comment.
I originally tried to serialize it as a uint64_t, but either I screwed something up or it was a serialization issue that it ended up being as signed in the json file and when read back, it was doing it wrong, that's why I went for a string. I'll change it to the std::optional<uint64_t>, though.
There was a problem hiding this comment.
The negative value in the JSON file should still work as long as its 64-bit representation matches the unsigned value that was originally serialized. If that feels a bit weird (as it probably would to me, to be honest), you could also change all the places where hardware_id is used to le_int64_t/be_int64_t/int64_t for consistency. The treatment of hardware ID as a 64-bit integer is nonstandard anyway with regard to vanilla PSO, so it's not important that the value is unsigned.
|
|
||
| shared_ptr<Login> AccountIndex::from_bb_credentials( | ||
| const string& username, const string* password, bool allow_create) { | ||
| const string& username, const string* password, const uint64_t* hardware_id, bool allow_create, bool bind_hardware_id) { |
There was a problem hiding this comment.
It would be cleaner to not have bool bind_hardware_id, and if the hardware_id pointer is null, it means the hardware ID shouldn't be checked or bound. Then the callsite in on_93_BB would just pass null here if s->bind_hardware_ids_on_login is false.
There was a problem hiding this comment.
I don't remember exactly why I did it that way, I think it was to make it work with other usages like $bbsave (I think it was that one), I'll change it to be that way, though.
2 participants
This PR adds Hardware ID support to newserv by using the id sent during the login sequence.
This has changes to the license formats for the affected platforms, but don't require making a new format for them. Included is a fallback so previous licenses can be loaded without the need of editing the files manually.
I'm including small checklist to track progress
This PR is a draft until the checklist is completed, but wanted to post the work done so far on this for feedback.