Allow hash path (/#/path ) for redirect Url

[ambrt]

Changed adding '&redirect=' +options.redirect to verifyHref, from inside .verfiy() method to insde of sendEmail() method, to allow hash paths for redirect url. From issue at #2307

connect to #2307

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[richardpringle]

ok to test

[superkhau]

LGTM. Thanks for the contribution! ;) @bajtos For final review and merge.

[bajtos]

I am not very familiar with this part of LoopBack. @ritch could you please review?

However, I would like to see a unit-test covering the change, to demonstrate the new behaviour and prevent regressions in the future.

[ambrt]

Two things to note: the test Should report 302 when redirect url is set fails when the redirect is having hash value like:

 it('Should report 302 when redirect url is set', function(done) {
testConfirm(function(result, done) {
request(app)
.get('/test-users/confirm?uid=' + (result.uid) +
'&token=' + encodeURIComponent(result.token) +
'&redirect=http://foo.com/bar/#/')
.expect(302)
.expect('Location', 'http://foo.com/bar/#/')
.end(done);
}, done);
});

with error:
Error: expected "Location" of "http://foo.com/bar/#/", got "http://foo.com/bar/" which is expected because server can't read hash values.

The second thing is that test "Confirm a user verification" encodes the /# into %2F%23 here
so test passes even when the redirect is set with hash (like redirect: 'http://foo.com/bar/#/foo'),
while the email link will be broken.

My idea to solve it would be to encodeURIComponent when creating verifyHref.
and decode it after verifying email.

I'm just not sure where the redirecting is happening.

[ritch]

test please

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[ritch]

ok to test

[bajtos]

Error: expected "Location" of "http://foo.com/bar/#/", got "http://foo.com/bar/" which is expected because server can't read hash values.
The second thing is that test "Confirm a user verification" encodes the /# into %2F%23 here
so test passes even when the redirect is set with hash (like redirect: 'http://foo.com/bar/#/foo'),
while the email link will be broken
My idea to solve it would be to encodeURIComponent when creating verifyHref.
and decode it after verifying email.

Yes please, we should be applying encodeURIComponent to all query string values.

[bajtos]

@ambrt I see this patch is rather old, sorry for our long silence! Are you still keen to work on this change?

[ambrt]

@bajtos I made new pull request with solution #3035

[bajtos]

@ambrt I made new pull request with solution #3035

I am closing this pull request then.

In the future, please use git commit --amend or git rebase -i together with git push -f and keep the work in the same pull request. That way all discussion stays in one place.