Redirect after login loses query params

Dev
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 Likes
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).

3 Likes
4

Do we want URL params? That seems highly unorthodox.

2 Likes
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.

3 Likes
8

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

https://github.com/discourse/discourse/blob/102be5a9e3a063bebe6a62927a102f84904a9bdf/app/assets/javascripts/discourse/routes/new-topic.js.es6#L58-L60

@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: https://github.com/etamponi/discourse-keybase-proofs-plugin

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.

1 Like
10

Looking at:

https://github.com/discourse/discourse/blob/863d8014d06a931b862ebe527e912483bee01786/app/controllers/static_controller.rb#L105

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.

3 Likes
12

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

4 Likes
13

This issue is now fixed via:

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

7 Likes