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

request.credentials configuration is ignored #791

Closed
3 of 7 tasks
gajus opened this issue Jul 17, 2018 · 20 comments · Fixed by #836
Closed
3 of 7 tasks

request.credentials configuration is ignored #791

gajus opened this issue Jul 17, 2018 · 20 comments · Fixed by #836

Comments

@gajus
Copy link

gajus commented Jul 17, 2018

This issue pertains to the following package(s):

  • GraphQL Playground - Electron App
  • GraphQL Playground HTML
  • GraphQL Playground
  • GraphQL Playground Express Middleware
  • GraphQL Playground Hapi Middleware
  • GraphQL Playground Koa Middleware
  • GraphQL Playground Lambda Middleware

Setup:

app.use('/', expressPlayground({
  endpoint: '/',
  settings: {
    'request.credentials': 'include'
  }
}));

The generated code confirms that the option is being passed:

GraphQLPlayground.init(root, {
  "endpoint": "/",
  "settings": {
    "request.credentials": "include"
  },
  "version": "1.7.2",
  "canSaveConfig": false
})

However, viewing the configuration in the application (browser) shows that the setting did not have effect:

{
  "general.betaUpdates": false,
  "editor.cursorShape": "line",
  "editor.fontSize": 14,
  "editor.fontFamily": "'Source Code Pro', 'Consolas', 'Inconsolata', 'Droid Sans Mono', 'Monaco', monospace",
  "editor.theme": "dark",
  "editor.reuseHeaders": true,
  "prettier.printWidth": 80,
  "request.credentials": "omit",
  "tracing.hideTracingResponse": true
}

I have tried reseting the local storage values with no effect.

Changing the configuration using the application itself makes it work as expected for that local session.

The issue appears to be that GraphQLPlayground.init ignores request.credentials setting.

Related issues:

@mwilc0x
Copy link

mwilc0x commented Jul 19, 2018

Thank you! I am noticing same thing where the setting change works for local session and then does not work on reload.

@mmun
Copy link

mmun commented Aug 1, 2018

I ran into this as well using Apollo Server. I hope it can be fixed.

@twelve17
Copy link

twelve17 commented Aug 3, 2018

I ran into this with the koa playground. One twist: if I set "editor.fontSize": 10 on the server side, it is clearly affecting the UI, as the font is visibly smaller, but looking at the settings on the client still specifies the font as being "editor.fontSize": 14. So there might be a mix of "the setting is not sent to the client" as well as "the client is not displaying the applied settings".

@tlgimenes
Copy link

tlgimenes commented Aug 22, 2018

I'm having the same behavior here. Some settings are used, while some are ignored in 1.7.2 with GraphQL Playground HTML

@curry684
Copy link

The server should just be able to push defaults over the defined defaults, which should then be overrideable by the settings screen in localStorage.

So if I define serverside that const defaults = {a: 1, b: 2}; and Playground prescribes global defaults {a: 3, c: 2} the UI should show {a: 3, b: 2, c: 2} and allow the end user to override any of them locally.

It would also in this case be nice if there was a "Reset to defaults" button in the UI which deleted any local overrides and fall back to Playground defaults + server defined defaults.

@vincenzo
Copy link
Contributor

Same here, with graphql-playground@1.7.3 on apollo-server-express@2.0.4

@vincenzo
Copy link
Contributor

Also seeing what @curry684 is describing.
For example, if I set the theme to light in code, the UI will still show that it's set to dark.

@vincenzo
Copy link
Contributor

I've been following the code round and round, I can't for the life of me understand where it is that these settings are finally (supposed to be) merge together between the local storage, the server init, etc.

Anyone could steer me in the right direction?

@vincenzo
Copy link
Contributor

This issue #796 is also related.

@vincenzo
Copy link
Contributor

I am trying to collect all these 'settings' issues in one place; I think it's beneficial that all of them are taken into consideration at once, lest we end up with partial fixes or fixes that break other stuff.

Here's the place: #826

Could you all have a look and confirm the problem in this present issue has been captured in the one above?

@cyberdude
Copy link

Why is the default setting omit?!

@vincenzo
Copy link
Contributor

@cyberdude See #470

@boennemann
Copy link

I'm still running into the very same issue and it seems this was only fixed for graphql-playground-react, while graphql-playground-html has the same issue.

Running this via apollo-server-koa @ 2.1.0, which requires @apollographql/graphql-playground-html @ ^1.6.0, which resolves to 1.6.4 and I'm seeing the exact same behavior of ignored settings.

Would appreciate some help here. If you need any more pointers please let me know 🙏

@huv1k
Copy link
Collaborator

huv1k commented Nov 23, 2018

@boennemann Can you please provide minimal repo example? So i can look into that?

@Binarytales
Copy link

@HuVik I have just got started with GraphQL recently and am experience the problem as @boennemann but with apollo-server-express@2.3.1 which requires apollographql/graphql-playground-html@^1.6.6

I have put together a minimal repo (https://github.com/Binarytales/jubilant-octo-telegram) showing that passing these playground settings

  playground: {
    'request.credentials': 'include'
  }

is not reflected when visiting http://localhost:3000/graphql

@huv1k
Copy link
Collaborator

huv1k commented Jan 17, 2019

@Binarytales they are using for of playground, so the best way is to contact them so they update it to use the latest version of graphql-playground. This is already fixed in it.

@Binarytales
Copy link

Binarytales commented Jan 17, 2019

Thank you for the quick response @HuVik I will raise an issue against apollo-server-express.

However it does look like apollo-server-express is using the latest version of graphql-playground-html

Latest version graphql-playground defined here: https://github.com/prisma/graphql-playground/blob/master/packages/graphql-playground-html/package.json#L3

Version of graphql-playground used by apollo-server-express here: https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-express/package.json#L29

Sorry if I've misunderstood anything about the relationship between these packages and their versions.

When running my reduced test case if I inspect the initial index.html page I can see the correct settings being passed to GraphQLPlayground.init - is this still within the realms of apollo-server-express at this point?

        GraphQLPlayground.init(root, {
  "endpoint": "/graphql",
  "subscriptionEndpoint": "/graphql",
  "version": "1.7.10",
  "request.credentials": "include",
  "canSaveConfig": false
})``` 

@huv1k
Copy link
Collaborator

huv1k commented Jan 17, 2019

@Binarytales can you click on cogwheel and check number in the right bottom corner? That is a version of the playground included.

@Binarytales
Copy link

Binarytales commented Jan 18, 2019

@HuVik Ah yes I see now. It's 1.7.10. So does that relate to the overall project version? The latest in that case being 1.8.5.

It looks like this is indeed an issue with apollo-server in that case. While it is using the latest version of graphql-playground-html it's specifying the overall playground version here: https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/playground.ts#L12

Thank you for your help. I'll see if I can raise a PR to the other project.

UPDATE: For the benefit of future travellers stumbling across this thread I eventually discovered this post which was most illuminating: apollographql/apollo-server#1855 (comment)

@huv1k
Copy link
Collaborator

huv1k commented Jan 18, 2019

@Binarytales Yea, we already fixed one issue, why apollo did a fork of the playground. So maybe they could start using prisma version again.

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

Successfully merging a pull request may close this issue.