Upgrade xiloader to use json, add SHA-1 OTP support, upgrade CPM, mbedtls by WinterSolstice8 · Pull Request #39 · LandSandBoat/xiloader

WinterSolstice8 · 2025-10-27T02:37:54Z

I think this is good enough to test now

Switch to sending json instead of bitbanging
Add cool menuing to xiloader for input selection
Add SHA-1 OTP support (open in browser, scan QR code, verify, then login)
Support loading in args from a json file that then gets deleted (potential security improvement, as args to a program can be seen by other programs)
Some other misc stuff along the way to make deps build or improvements

zach2good

I haven't run this, but I want to make sure the addition of OTP stuff and the json file loading isn't so front-and-center that it seems like it's a requirement? Is this at a stage where the server informs xiloader that it wants OTP stuff, so then those options appear, otherwise it's slim like it always was? That would be what I'd expect, rather than you fire it up and it is showing you lots of options that aren't relevant for your server

zach2good · 2025-10-27T09:57:02Z

+    PRIVATE ftxui::screen
+    PRIVATE ftxui::dom
+    PRIVATE ftxui::component


We're not going on to link or expose xiloader, so these don't need to be marked as private. If anything this block could be: target_link_libraries(xiloader PRIVATE

zach2good · 2025-10-27T09:57:27Z

+    "JSON_ImplicitConversions OFF" #Recommended by the author
+)
+
+set(JSON_BuildTests OFF CACHE INTERNAL "")


Can this not be a string in the options? Like JSON_ImplicitConversions is?

I can't remember, but I think that was the recommended way by the developer.
I tried adding it via options as "JSON_BuildTests OFF" and it worked.

Done.

zach2good · 2025-10-27T09:58:17Z

+  NAME qr-code-generator
+  GITHUB_REPOSITORY nayuki/QR-Code-generator
+  GIT_TAG 720f62bddb7226106071d4728c292cb1df519ceb
+  LANGUAGES CXX


Even if you're not going to build this, or it doesn't come with a cmake file, you should still mark this as DOWNLOAD_ONLY

zach2good · 2025-10-27T10:00:54Z

+            xiloader::console::output(xiloader::color::success, "Successfully logged in as %s!", globals::g_Username.c_str());
+
+            shutdown(sock, SD_BOTH);
+            retval = true;


Rather than setting retval and break, and then falling through to thre return, you can just return here. If anything "default return false" by setting retval at the top and eventually returning the unchanged value of it makes all of this harder to reason about. I'd recommend this whole function only be comprised of the switch, and each block handles it's own return (or is explicitly marked as fallthrough to the next block with [[fallthrough]].

This should be done

zach2good · 2025-10-27T10:02:04Z

+static std::string qrToSvgString(const qrcodegen::QrCode& qr, int border)
+{
+    if (border < 0)
+        throw std::domain_error("Border must be non-negative");


allman bracing in this func, even if it's example code from elsewhere

zach2good · 2025-10-27T10:03:13Z

+            return std::nullopt;
+        }
+    }
+    else if constexpr (std::is_same_v<T, bool> || std::is_same_v<T, boolean>)


These are equivalent?

They weren't - I'm not sure why they were. I upgraded cmake to c++20 and the boolean type disappeared so it's gone.

zach2good · 2025-10-27T10:05:13Z

+    return jsonInput[key].get<T>();
+}
+
+// Supposedly, there is template magic to do this inside the template above, but VC++ doesn't support it yet?


Send me whatever error you're facing, this is all absolutely possible and above board

We talked about this in DMs, because of the need of the size the partial specialization was needed.

zach2good · 2025-10-27T10:05:24Z

 ===========================================================================
 */

+#define NOMINMAX 1 // Interferes with std::numeric_limits


God I hate windows

zach2good · 2025-10-27T10:07:33Z

        .help("(optional) Determines whether or not to hide the console window after FFXI starts.")
        .append();

+    args.add_argument("--json", "--json-file")


Is this a thing to support external launchers, etc? I don't think xiloader should be deleting something that another process creates. If an external launcher wants to poop out a JSON file for consumption by xiloader (this sounds funky as I type it out, as a concept) then it should be up to that launcher to clean it up once it sees that xiloader has succeeded or failed.

I modified it so that xiloader only modifies the write time of the file. This way if the launcher or whatever is keeping track of that attribute it can delete the file for us.

But why does it need to modify anything? If an external process is providing this file and managing the lifetime of xiloader, it should be up to that process to determine if "the thing happened or the thing failed", xiloader doesn't need to know or provide things for external callers

zach2good · 2025-10-27T10:10:35Z

+            /*
+            * 0 = Login
+            * 2 = Change Account Password
+            * 3 = Register OTP
+            * 4 = Remove OTP
+            * 5 = Regenerate OTP removal code
+            * 6 = Verify (enable) OTP code
+            */
+            if (selected == 0 || selected == 2 || selected == 3 || selected == 4 || selected == 5 || selected == 6)


Given that this is repeated here and in menus.h, this screams out that it wants to be an enum somewhere, with a toString helper, and/or a toEventCharacter helper. The big if/else chains could be switches, or dispatch to functions, etc.

I completely rewrote the menus and consumption of menus with selected/selection. It should be more satisfactory now.

WinterSolstice8 · 2025-10-27T14:12:13Z

I haven't run this, but I want to make sure the addition of OTP stuff and the json file loading isn't so front-and-center that it seems like it's a requirement? Is this at a stage where the server informs xiloader that it wants OTP stuff, so then those options appear, otherwise it's slim like it always was? That would be what I'd expect, rather than you fire it up and it is showing you lots of options that aren't relevant for your server

Interrogating the server can give attackers information about your account or requirements. Being ambiguous is intentional here. SE themselves requires the user to input an OTP by choice, and not after communicating to the server. All servers shall support OTP, but don't have to enforce it. If the user wants to not use OTP for one reason or another that's on them

As far as the options, I can hide it in a submenu so you enter from a list of

login
change pass
create account
2FA settings

I do also agree the current menu is a bit bloated. 2FA settings will relatively rarely used once configured, though arguably less common than create account

Additionally, json file loading states that the arg is optional in the help text, was that not sufficient?

WinterSolstice8 · 2025-10-28T02:06:38Z

~~I added a "message of the day" that prints when you connect to xi_connect~~
this was true, but I decided to remove it

zach2good

Code all looks good, I've messaged you in Discord about the fate of the JSON login token file

zach2good · 2025-11-05T12:06:55Z

+#include "ftxui/component/component_options.hpp"  // for MenuOption
+#include "ftxui/component/screen_interactive.hpp" // for ScreenInteractive
+
+enum class menuSelection : uint8_t


MenuSelection

zach2good · 2025-11-05T12:08:55Z

+    menuSelection twoFactorSubMenu()
+    {
+        auto screen   = ScreenInteractive::TerminalOutput();
+        int  selected = 0; // Container::Vertical requires `int` input


const auto selected = []() -> int { // all the menu code below }(); switch (selected) ...

Might not work, but worth a look 👍

It will end up looking like this which i think sort of defeats the point. selected is passed by ref into Container::Vertical

const auto selectedOption = []() -> int { int selected = 0; // Container::Vertical requires `int` input // clang-format off auto menu = Container::Vertical( { MenuEntry("1) Register 2FA OTP"), MenuEntry("2) Remove 2FA OTP"), MenuEntry("3) Regenerate 2FA OTP removal code"), MenuEntry("4) Validate 2FA OTP"), MenuEntry("5) Exit Menu"), }, &selected); ... return selected; }();

zach2good · 2025-11-05T12:09:11Z

+    menuSelection mainMenu()
+    {
+        auto screen   = ScreenInteractive::TerminalOutput();
+        int  selected = 0; // Container::Vertical requires `int` input


Try the same pattern I just mentioned here too if you can

Error Logging fixes, upgrade CPM and mbedtls

5591c03

zach2good reviewed Oct 27, 2025

View reviewed changes

WinterSolstice8 force-pushed the json_junk branch from 5e02fe0 to 9d53468 Compare October 28, 2025 01:50

WinterSolstice8 mentioned this pull request Oct 28, 2025

[login] Support json-based xiloader LandSandBoat/server#7980

Merged

4 tasks

WinterSolstice8 force-pushed the json_junk branch 2 times, most recently from b98fffe to beb169f Compare October 30, 2025 17:16

zach2good reviewed Nov 5, 2025

View reviewed changes

WinterSolstice8 force-pushed the json_junk branch from beb169f to 77c339f Compare November 5, 2025 13:38

JSON based rewrite, also add SHA-1 TOTP support

5e3d329

WinterSolstice8 force-pushed the json_junk branch from 77c339f to 5e3d329 Compare November 5, 2025 14:00

zach2good approved these changes Nov 5, 2025

View reviewed changes

zach2good merged commit 9679ac4 into main Nov 5, 2025
1 check passed

zach2good deleted the json_junk branch November 5, 2025 14:05

Conversation

WinterSolstice8 commented Oct 27, 2025

Uh oh!

zach2good left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WinterSolstice8 commented Oct 27, 2025

Uh oh!

WinterSolstice8 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zach2good left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WinterSolstice8 Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

WinterSolstice8 commented Oct 28, 2025 •

edited

Loading

WinterSolstice8 Nov 5, 2025 •

edited

Loading