Skip to content

Enable submitting cookies in CORS requests#1189

Merged
shockey merged 4 commits intoswagger-api:masterfrom
renuka-fernando:issue#1186
Jan 25, 2018
Merged

Enable submitting cookies in CORS requests#1189
shockey merged 4 commits intoswagger-api:masterfrom
renuka-fernando:issue#1186

Conversation

@renuka-fernando
Copy link
Copy Markdown
Contributor

@renuka-fernando renuka-fernando commented Nov 27, 2017

Changes

  • Change the value of credentials according to the value of http.withCredentials as in the README.md.

Description

  • Change the value of credentials if http.withCredentials value is true.

Motivation and Context

How Has This Been Tested?

  • Tested with two origins.
  • UI running on localhost:9292 sends requests to the server on localhost:9293.

Screenshots (if appropriate):

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@webron webron requested a review from shockey November 27, 2017 20:02
@shockey shockey modified the milestones: November 24, December 1, 2017 Nov 27, 2017
@tmkasun tmkasun mentioned this pull request Nov 28, 2017
Merged
@shockey
Copy link
Copy Markdown
Contributor

shockey commented Nov 30, 2017

@RenukaFernando, can you add some tests to cover this new functionality?

@shockey shockey mentioned this pull request Nov 30, 2017
Closed
@tmkasun tmkasun mentioned this pull request Dec 19, 2017
Merged
@renuka-fernando renuka-fernando force-pushed the issue#1186 branch 2 times, most recently from f71fb67 to 194f80c Compare December 19, 2017 09:06
@HunDunDM HunDunDM mentioned this pull request Dec 20, 2017
Closed
@shockey shockey removed this from the December 1, 2017 milestone Dec 28, 2017
@renuka-fernando
Copy link
Copy Markdown
Contributor Author

renuka-fernando commented Jan 22, 2018

Hi @shockey,
As per our understanding, writing unit tests for this improvement is troublesome. Can you please help writing integration tests on this?
So then this can be merged.

@shockey
Copy link
Copy Markdown
Contributor

shockey commented Jan 22, 2018

@RenukaFernando can do!

@shockey shockey added this to the January 26, 2018 milestone Jan 22, 2018
@shockey shockey merged commit b5ec24e into swagger-api:master Jan 25, 2018
@shockey
Copy link
Copy Markdown
Contributor

shockey commented Jan 25, 2018

thanks, @RenukaFernando!

@renuka-fernando
Copy link
Copy Markdown
Contributor Author

renuka-fernando commented Jan 26, 2018

Thank you! @shockey for you help

@renuka-fernando
Copy link
Copy Markdown
Contributor Author

renuka-fernando commented Jan 30, 2018

Hi @shockey,
We are still facing the issue of handling CORS request. #1236 is fixed the issue and we have tested it with two origins.

Can you please review it?

@renuka-fernando renuka-fernando mentioned this pull request Feb 5, 2018
Merged
@shockey shockey mentioned this pull request Aug 11, 2018
Closed
@dalbani
Copy link
Copy Markdown

dalbani commented Jan 14, 2019

Hi, maybe a silly question, but could someone explain me how to set this http.withCredentials flag in the context of SwaggerUIBundle (i.e. when deploying Swagger UI as shown on https://github.com/swagger-api/swagger-ui/blob/master/dist/index.html#L41)?

@shockey
Copy link
Copy Markdown
Contributor

shockey commented Jan 15, 2019

@dalbani, this is a merged PR - if you need help please open a new support ticket 👍

@dalbani dalbani mentioned this pull request Jan 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shockey shockey Awaiting requested review from shockey

1 more reviewer

@yuval-stratoscale yuval-stratoscale yuval-stratoscale approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

January 26, 2018

Development

Successfully merging this pull request may close these issues.

In CORS requests Cookies are not submitted nor setting correctly

4 participants

@renuka-fernando @shockey @dalbani @yuval-stratoscale