search

Gizra / elm-restful

Elm utilities useful for working with a RESTful backend (especially Drupal)
MIT License
3 stars 0 forks source link

Don't hit user/logout on Drupal config #19

Open amitaibu opened 6 years ago

amitaibu commented 6 years ago

As we have an access token, it is rare we'll need to hit that endpoint.

rgrempel commented 6 years ago

Just to give some context, the reason this exists at all is a scenario in which Drupal, at least in some circumstances, also sets an HTTP-only cookie when you successfully obtain an access token. You can't get rid of the cookie via Javascript (since it's HTTP-only). So, even if you forget your access token, you are still effectively logged-in because of the cookie -- until you actually hit the Drupal endpoint that logs you out (and removes the cookie).

If I remember correctly, this was only a problem if the app is being served from the same domain as the Drupal backend. Otherwise, perhaps the scope of the cookie is wrong, so it doesn't get used. (That is, if I'm serving the app from localhost:3000 and the Drupal backend is localhost:8888, then no problem. If I'm serving the app from localhost:8888/app, then I could see the problem).

There's probably a way of avoiding that scenario -- in fact, you might already know how! It is something I'll need to figure out eventually for one of our projects.

amitaibu commented 6 years ago

Yes, I understand, but I still think it's wrong:

If you are served the Elm app from the site via Drupal, you don't need an access token at-all, since you'll have the cookies).

And if you use an access token, then you don't have the cookie for the site..

rgrempel commented 6 years ago

Ah, yes, I see your point.

So, perhaps when we're serving the app via Drupal, we don't need to use the access token at all? Since the cookie will authenticate us? Though, in that case, we do still need to contact the backend to effectively logout, since we can't clear the cookie otherwise.

Now, our Config doesn't know whether we're going to use an access token or not. So, we need the option to contact the backend -- this PR is just about the default setting. So, I guess it's fine to merge it. Though, it is a kind of stealthy breaking change -- it changes the behaviour of drupalConfig without any change to the types. (That is, I don't think elm-package will see this as a breaking change, but in reality it is).

I'll give a little thought to whether we could do this in a way that's not a breaking change. I suppose that would mean keeping drupalConfig as it is and come up with a new function with a new name for this variant.

amitaibu commented 6 years ago

So, perhaps when we're serving the app via Drupal, we don't need to use the access token at all?

If it's under /app - we should use an access token, as it doesn't pass through Drupal menu system

If it's an embed inside Drupal, we can rely on the cookies.

So, I believe setting this to Nothing will not break any existing installation (and I doubt that there are may if any non-Gizra projects that use druapalConfig)

rgrempel commented 6 years ago

If it's under /app - we should use an access token, as it doesn't pass through Drupal menu system

Well, I was running into trouble in that scenario -- somehow, I was getting the cookie even though I was only wanting an access token. I'll see if I can figure out the exact steps which led to that problem.