Redirect after login loses query params

Development

AI-generated summary

The discussion revolves around an issue with redirecting after login, where query parameters are lost. emanuele shared their route code, which redirects to the login page and then to the correct controller, but without query parameters.

emanuele temporarily solved the issue by using URL parameters instead of query parameters. However, codinghorror questioned whether URL parameters are the right approach.

techAPJ believed it was a bug in the core and found a similar fix for SSO. After investigating, techAPJ found that redirects with query parameters survive login flow for certain routes and requested more details from emanuele to reproduce the issue.

techAPJ discovered that Discourse only preserves query parameters for pre-defined paths and proposed supporting query parameters for all paths. sam agreed, as long as it is carefully reviewed.

The issue was eventually fixed with a commit to the Discourse repository, as reported by techAPJ.

1

This is my route code:

import { defaultHomepage } from "discourse/lib/utilities";

export default Discourse.Route.extend({
  beforeModel(transition) {
    if (!Discourse.User.current()) {
      $.cookie("destination_url", window.location.href);
      return this.replaceWith("login");
    }
    this.replaceWith(`/${defaultHomepage()}`).then(e => {
      Ember.run.next(() => {
        this.controllerFor("new-proof").send("openModal", transition.to.queryParams);
      });
    });
  },
});

The redirection to the login page works well, but after login, I get redirected to the correct controller but without the query parameters I had in the original request.

Is there a way around this that doesn’t involve using custom local storage entries/cookies? Am I doing something wrong?

3

I still don’t know how to preserve query params, but I have solved my own issue by using url params instead of query params (so the route is now /keybase-proofs/new-proof/:username/:kb_username/:sig_hash).

4

Do we want URL params? That seems highly unorthodox.

5

I assumed it was the right way to do it since the query params get lost in the transition. Perhaps I am doing something wrong with destination_url or the redirection to login?

7

I think it’s a bug in the core. I see a similar fix for SSO by @sam – FIX: stop removing query params from destination url in sso · discourse/discourse@0b334cd · GitHub

I will investigate and push a fix.

8

Note that redirect with query params survives login flow for http://localhost:9292/new-topic?title=beta. Relevant code can be found here:

@emanuele I will need more details.

  • What login method are you using (email/password, social, sso, etc)?
  • Can you provide a sample URL with query param?
  • Is your plugin live? Can I test it on my local instance?

Feel free to PM me above details.

9

Hi :slight_smile:

  1. I am using email/password
  2. http://localhost:9292/keybase-proofs/new-proof?kb_username=etamponi&username=emanuele.tamponi&sig_hash=puppa&kb_ua=foo
  3. The plugin code is at: GitHub - etamponi/discourse-keybase-proofs-plugin: Discourse Plugin for Keybase Proofs · GitHub

To test it, replace the value of the username query param with your username. The route code is in routes/new-proof.js.es6 and it looks basically the same as the code you posted.

10

Looking at:

It seems like we preserve query params for a few pre-defined paths and this pattern started with this commit (not sure why I did that… :confused:).

@sam should we start supporting query params for all the paths? I don’t see any security implication by allowing query params on random path.

12

As long as this is carefully reviewed I think it is fine to extend support here.

13

This issue is now fixed via:

https://github.com/discourse/discourse/commit/1708be4f27216392d429442524916bf8b6175443