Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect url should be urlencoded #2307

Closed
ambrt opened this issue May 6, 2016 · 8 comments
Closed

Redirect url should be urlencoded #2307

ambrt opened this issue May 6, 2016 · 8 comments
Assignees
Labels

Comments

@ambrt
Copy link
Contributor

ambrt commented May 6, 2016

The user verify link is made in such way that it provides redirect path in the middle of whole link.

The problem is that when one would use redirect=/#/my-angular-path then the token is treated as hash valu and isn't read by api (to verify) and api throws error.
Redirect is added on line 407 and token on 431.

I would just remove:

   '&redirect=' +
      options.redirect;

from line 407 and put it behind
options.verifyHref += '&token=' + user.verificationToken;
on line 431

@superkhau
Copy link
Contributor

Can you provide a sample repo for us to reproduce your issue?

@ambrt
Copy link
Contributor Author

ambrt commented May 8, 2016

@superkhau Here's a sample of it https://github.com/ambrt/loopback-sandbox

@superkhau
Copy link
Contributor

@davidcheung Reassigning to you as this is your FA.

@davidcheung
Copy link
Contributor

@ambrt if we also url encode the redirectUrl would this already not be a problem? it is altering the url?

@ambrt
Copy link
Contributor Author

ambrt commented Nov 13, 2016

Hi @davidcheung
Yes, I think that url encode here and url decode here would solve the problem.

@davidcheung
Copy link
Contributor

actually i believe that would also mean if the url has parameters it will also be lost, we should fix this

@davidcheung davidcheung changed the title Redirecting user using verify link to a AngularJS with #path Redirect url should be urlencoded Nov 17, 2016
ambrt added a commit to ambrt/loopback that referenced this issue Dec 8, 2016
@bajtos
Copy link
Member

bajtos commented Feb 6, 2017

Closing in favour of #3185.

@bajtos
Copy link
Member

bajtos commented Feb 9, 2017

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants