LTIJS is not compatible with Express 5

[dmolin]

This is only a tentative question:

We're using LTIJS with our MeteorJS application and everything has been working fine so far. We've been using it in Serverless mode, attaching the LTIJS Provider.app instance to the internal Meteor ExpressJS server instance (via MeteorInstance.use("/ltitool", Provider.app))

The most recent version of MeteorJS is now using Express 5. If we upgrade to this version, LTIJS stops working.
The issue we're observing is the following: when LTIJS is redirecting, like here in Provider.js:

            provMainDebug("Redirecting to endpoint with ltik");
            provMainDebug(
              "Redirect to:" +
                req.baseUrl +
                (this.#path || "") +
                req.path +
                "?" +
                urlSearchParams,
            );
            return res.redirect(
              req.baseUrl +
                (this.#path || "") +
                req.path +
                "?" +
                urlSearchParams,
            );

the receiving code (in Server.js) doesn't see anything in req.query... it's just... empty!

    this.app.use(async (req, res, next) => {
      // Return if req.token is already defined
      if (req.token) return next();
      // Attempt to retrieve ltik from query parameters
      if (req.query && req.query.ltik) {
        req.token = req.query.ltik;
        return next();
      }

We see that the redirect is performed with the correct query params:

provider:main Redirect to:/ltitool/launch?ltik=<long token>

But the code receiving the request doesn't see anything in the request query params... I'm not sure yet if the issue is within Express 5 but I wanted to flag this and check if you guys have tried using LTIJS with Express 5 and, if so, if you're aware of any particular change needed to make request processing work as it should

[dmolin]

So, apparently Express doesn't include query parsing by default.
As such, a change is necessary in LTIJS to support Express v5.
Specifically, Server.js needs to include an explicit use of the query parser middleware:

    this.app.use(express.query());

This small change should be enough to make LTIJS compatible with Express 5

[liv1n9]

So, apparently Express doesn't include query parsing by default. As such, a change is necessary in LTIJS to support Express v5. Specifically, Server.js needs to include an explicit use of the query parser middleware:

    this.app.use(express.query());

This small change should be enough to make LTIJS compatible with Express 5

express@5.1 doesn't have express.query anymore. I also tried lti.app.set("query parser", "extended"); but it didn't work (req.query is still undefined).

[dmolin]

You're correct @liv1n9. The problem was not actually the missing query parser!
I made ltijs work with Express 5 in my local fork, but I had to remove this.app.options("*", cors()); and I replaced it with this.app.use(cors()); (I see no downsides in this)
dmolin@b31d978

[liv1n9]

@dmolin I was trying the origin package (ltijs) at the time and didn't realize that it was fixed it in your fork. Changed to your fork and it worked. Thank you

[dmolin]

Glad it's of help @liv1n9
Though, the burden of maintaining a fork is quite a pain when you're working in an already big project; Hopefully, If my PR is merged at some point (#258), we both would be able to then just discontinue working on the fork and go back to the original repo (one less dependency to maintain!) 🤞🏻