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

1.9.2 authenticated-route-mixin - queryParam regression #1915

Closed
st-h opened this issue Jul 29, 2019 · 7 comments
Closed

1.9.2 authenticated-route-mixin - queryParam regression #1915

st-h opened this issue Jul 29, 2019 · 7 comments

Comments

@st-h
Copy link

st-h commented Jul 29, 2019

Starting ember-simple-auth version 1.9.2 ( I think it's very likely related to 84c076d ) ember shows a pretty strange error when using query params and a route is trying to be accessed that applies the authenticated-route-mixin. As it looks it only shows up when the user is not authenticated and the redirect is triggered. Reverting back to 1.9.1 fixes the issue

Assertion Failed: You passed the `application:referral` query parameter during a transition into application, please update to referral
    at assert (index.js:163)
    at Class._hydrateUnsuppliedQueryParams (router.js:974)
    at Class._prepareQueryParams (router.js:801)
    at Class._doTransition (router.js:749)
    at RouterService.transitionTo (router.js:114)
    at Class.triggerAuthentication (authenticated-route-mixin.js:127)
    at authenticated-route-mixin.js:107
    at runIfUnauthenticated (authenticated-route-mixin.js:30)
    at Class.beforeModel (authenticated-route-mixin.js:106)
    at Class.superWrapper [as beforeModel] (utils.js:366)

Likely related:
When debugging authenticated-route-mixin::triggerAuthentication it looks like

this.get('_authRouter') returns the RouterService

whereas:

this.get('_router') returns a different router (probably the internal ember router)

at least: this.get('_router') != this.get('_authRouter')

Probably in the 1.9.1. implementation the _router property defined in the mixing was overridden with the ember router and has therefore been working (but that is getting a little vague now)

@st-h
Copy link
Author

st-h commented Jul 29, 2019

To confirm my assumptions, this is using 1.9.1 debugging the triggerAuthentication method:

Ember.getOwner(this).lookup('service:router')
> RouterService {_router: Class, __INIT_WAS_CALLED__ember15644177776861236198101007__: true}
this.get('_router')
> Class {currentURL: null, currentRouteName: null, currentPath: null, currentRoute: null, _qpCache: {…}, …}

So, 1.9.2 is using a different router than 1.9.1. However, I am pretty much at a loss what the intended behaviour is/was and what would be the right way to fix this

@marcoow
Copy link
Member

marcoow commented Aug 2, 2019

I think this is a bug in Ember.js actually where the router service is not 100% compatible with calling e.g. transitionTo in a route direclty, see emberjs/ember.js#16594

@st-h
Copy link
Author

st-h commented Aug 9, 2019

@marcoow I think this is correct. I submitted a PR emberjs/ember.js#18244 to fix this, however I think this should be reviewed by someone who is more familiar with the inner workings of the ember routing system.

@marcoow marcoow closed this as completed Sep 13, 2019
@IBue
Copy link

IBue commented Dec 16, 2019

This is related to emberjs/ember.js#17494 and emberjs/ember.js#14875

@st-h
Copy link
Author

st-h commented Dec 16, 2019

@IBue this bug should be fixed in recent ember releases (at least I was never able to reproduce it after my PR was merged). Are you still experiencing the same error with latest versions?

@IBue
Copy link

IBue commented Dec 17, 2019

@st-h: With ember-source 3.12.2 + ember-simple-auth 1.9.2/2.1.0 there is still this error when an unauthenticated redirect (AuthenticatedRouteMixin#triggerAuthentication() ) occurs into a route that is either query-parameterized by itself or by its parent.

@st-h
Copy link
Author

st-h commented Dec 17, 2019

Would be interesting if you are still seeing this with ember >= 3.14.1. (I don't know if the fix has actually been backported yet) However, I am afraid I can not be of more help here right now. I am just a little surprised that I have never seen this issue reappear after upgrading to 3.14.1

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

No branches or pull requests

3 participants