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

Inject protocol informations into the RequestContext object #1155

Merged
merged 7 commits into from Jul 18, 2018

Conversation

Projects
None yet
5 participants
@scottinet
Member

scottinet commented Jul 17, 2018

What does this PR do ?

Fill the updated version of the RequestContext object (see kuzzle-common-objects@3.0.8) with additional information, either from a direct connection to Kuzzle, or from one established to a Kuzzle Proxy.

Internal backlog task: KZL-49
Fixes: #1106

Other changes

  • Update all references to RequestContext, preventing the use of deprecated properties
  • Fix direct websocket connections IP address retrieval (uws has a different internal structure than ws)
  • There is no need to manually copy protocol headers into Request.input.headers anymore, the 3.0.9 version of the kuzzle-common-objects module takes care of that now. It's that module responsability to handle its own properties
  • Remove a bluebird warning (occuring only in dev. mode) generated by the router and the funnel controllers

scottinet added some commits Jul 17, 2018

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Jul 17, 2018

Codecov Report

Merging #1155 into 1.x will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.x    #1155      +/-   ##
==========================================
- Coverage   93.97%   93.96%   -0.02%     
==========================================
  Files          91       91              
  Lines        6394     6411      +17     
==========================================
+ Hits         6009     6024      +15     
- Misses        385      387       +2
Impacted Files Coverage Δ
lib/api/core/entrypoints/proxy/index.js 100% <ø> (ø) ⬆️
.../api/core/models/notifications/UserNotification.js 100% <100%> (ø) ⬆️
lib/api/core/entrypoints/embedded/index.js 97.67% <100%> (-0.02%) ⬇️
...ib/api/core/models/repositories/tokenRepository.js 91.2% <100%> (ø) ⬆️
lib/api/core/httpRouter/routeHandler.js 100% <100%> (ø) ⬆️
...i/core/entrypoints/embedded/protocols/websocket.js 92.03% <100%> (ø) ⬆️
lib/api/controllers/routerController.js 95.23% <100%> (+0.11%) ⬆️
lib/api/core/hotelClerk.js 91.62% <100%> (ø) ⬆️
.../core/models/notifications/DocumentNotification.js 100% <100%> (ø) ⬆️
lib/api/core/httpRouter/routePart.js 100% <100%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46325fe...e17e336. Read the comment docs.

codecov-io commented Jul 17, 2018

Codecov Report

Merging #1155 into 1.x will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.x    #1155      +/-   ##
==========================================
- Coverage   93.97%   93.96%   -0.02%     
==========================================
  Files          91       91              
  Lines        6394     6411      +17     
==========================================
+ Hits         6009     6024      +15     
- Misses        385      387       +2
Impacted Files Coverage Δ
lib/api/core/entrypoints/proxy/index.js 100% <ø> (ø) ⬆️
.../api/core/models/notifications/UserNotification.js 100% <100%> (ø) ⬆️
lib/api/core/entrypoints/embedded/index.js 97.67% <100%> (-0.02%) ⬇️
...ib/api/core/models/repositories/tokenRepository.js 91.2% <100%> (ø) ⬆️
lib/api/core/httpRouter/routeHandler.js 100% <100%> (ø) ⬆️
...i/core/entrypoints/embedded/protocols/websocket.js 92.03% <100%> (ø) ⬆️
lib/api/controllers/routerController.js 95.23% <100%> (+0.11%) ⬆️
lib/api/core/hotelClerk.js 91.62% <100%> (ø) ⬆️
.../core/models/notifications/DocumentNotification.js 100% <100%> (ø) ⬆️
lib/api/core/httpRouter/routePart.js 100% <100%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46325fe...e17e336. Read the comment docs.

@@ -74,30 +73,32 @@ class HttpProtocol extends Protocol {
this.decoders = this._setDecoders();
this.server.on('request', (request, response) => {
const ips = [request.socket.remoteAddress];
if (request.headers['x-forwarded-for']) {

This comment has been minimized.

@Aschen

Aschen Jul 17, 2018

Contributor

Nitpicking:
If I understand well, the RFC allow another header to indicate the source ip https://tools.ietf.org/html/rfc7239#section-4
eg: Forwarded: for=192.0.0.2, for=192.0.0.3

@Aschen

Aschen Jul 17, 2018

Contributor

Nitpicking:
If I understand well, the RFC allow another header to indicate the source ip https://tools.ietf.org/html/rfc7239#section-4
eg: Forwarded: for=192.0.0.2, for=192.0.0.3

This comment has been minimized.

@scottinet

scottinet Jul 18, 2018

Member

Hmm.
It seems that it's the other way around: Forwarded is the standard header, and x-forwarded-for is a simplified version of it (much simpler to parse), and it's not part of any official specification.
Since I lack expertise with the HTTP protocol: @ballinette, @benoitvidis > do you have any advice?

@scottinet

scottinet Jul 18, 2018

Member

Hmm.
It seems that it's the other way around: Forwarded is the standard header, and x-forwarded-for is a simplified version of it (much simpler to parse), and it's not part of any official specification.
Since I lack expertise with the HTTP protocol: @ballinette, @benoitvidis > do you have any advice?

This comment has been minimized.

@ballinette

ballinette Jul 18, 2018

Member

I did not know about Forwarded header, and as far as I know, x-forwarder-for is more commonly used.
But as Forwarded is the standard specs now, we should support it as well.

@ballinette

ballinette Jul 18, 2018

Member

I did not know about Forwarded header, and as far as I know, x-forwarder-for is more commonly used.
But as Forwarded is the standard specs now, we should support it as well.

@Aschen

Aschen approved these changes Jul 18, 2018

@Aschen Aschen merged commit 199e2b7 into 1.x Jul 18, 2018

4 checks passed

codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +6.02% compared to 46325fe
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
sonarqube SonarQube reported no issues

@Aschen Aschen deleted the KZL-153/inject-protocol-infos branch Jul 18, 2018

@scottinet scottinet referenced this pull request Jul 27, 2018

Merged

Release 1.4.2 #1166

scottinet added a commit that referenced this pull request Jul 27, 2018

Release 1.4.2 (#1166)
* fixes #1125 - aggs & sort on user search (#1144)

* Add AdminController (#1129)

* KZL-143 KZL-145 add admin controller and reset cache action

* Boyscout: fix crash because kuzzle try to load protocols in simplefiles

* Add tests for adminController

* Add resetKuzzleData and resetSecurity

* fix features for admin controller

* Add resetDatabase

* fix tests

* fix tests

* fix tests

* Add default rights restrictions for admin controller

* Fix test

* Fix test

* Use API for all actions except dump

* Add generateDump

* Fix tests and remove old cli tests

* Remove reset security scenario

* Make sonarqube happy

* Remove references to cliController

* Fix features

* fix internal error

* remove internal broker

* re-add shutdown

* fix test

* Put shutdown in adminController

* Reset defaults roles and profiles with resetSecurity

* replace ws by uws

* Nitpicking

* increase coverage

* increase coverage

* increase coverage

* fix linter

* increase coverage

* put back proxyBroker

* fix tests

* remove tests

* fix tests

* nitpicking

* Updates for PR

* Put back defaut config

* Disable dump if it is specified in config

* fix message in cli

* add truncate() method on repositories + refactor admin controller to use truncate()

* fix admin controller

* fix test

* Add Janitor core component

* return number of deleted objects in truncate

* fix linter

* catch TypeError for scanStream

* fix linter

* Please sonarqube

* fix linter

* Nitpicking

* Updates for PR

* update headers

* fix linter

* fix tests

* deprecate "constructor" auth option and replace it with "authenticator" (#1145)

* deprecate "constructor" auth option and replace it with "authenticator"

* fix sonarqube issues

* Add action and controller in request input for http events (#1143)

* Add action and controller in request input for http events

* put back some code

* nitpicking

* Add forgotten files --'

* Test Kuzzle against different node LTS versions (#1147)

* set ulimits for elasticsearch (#1148)

* Update kuzzle-common-objects module (#1150)

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* update to plugin auth local 5.1.3 (#1149)

* update to plugin auth local 5.1.3

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* prevent a breaking change when upgrade from a pre-1.4.0 version

* Revert "prevent a breaking change when upgrade from a pre-1.4.0 version"

This reverts commit 8328f50.

* prevent a breaking change occuring when upgrading Kuzzle with old config files (#1154)

* Inject protocol informations into the RequestContext object (#1155)

* inject protocol info into requestcontext

* fix incoming context infos coming from the proxy

* revert debug modification

* no need to manually copy incoming headers

* update unit tests

* fix sonarqube issu

* jsdoc fix

* fixes #1146, passport login request missing headers (#1153)

* fixes #1146, passport login request missing headers

* oauth - add some comments on arbritraty method & url

* Add script to build and push docker images (#1152)

* Add script to build and push docker images

* Add script to build and push docker images

* Fix build script

* add comment to secret env

* Use kuzzleteam account for dockerhub

* Update subnmodule

* Dont build and push on hotfix merge

* fix #1161 (#1162)

* fix startup message (#1163)

* Fix #1131: listCollections limited to the provided collection name, when specified (#1157)

* fixes #1156 - missing version in swagger output (#1164)

* Release 1.4.2

* dependencies update

* bump kuzzle-common-objects to 3.0.13

* fix outdate unit test

@scottinet scottinet referenced this pull request Sep 13, 2018

Merged

Release 1.5.0 #1194

scottinet added a commit that referenced this pull request Sep 13, 2018

Release 1.5.0 (#1194)
* fixes #1125 - aggs & sort on user search (#1144)

* Add AdminController (#1129)

* KZL-143 KZL-145 add admin controller and reset cache action

* Boyscout: fix crash because kuzzle try to load protocols in simplefiles

* Add tests for adminController

* Add resetKuzzleData and resetSecurity

* fix features for admin controller

* Add resetDatabase

* fix tests

* fix tests

* fix tests

* Add default rights restrictions for admin controller

* Fix test

* Fix test

* Use API for all actions except dump

* Add generateDump

* Fix tests and remove old cli tests

* Remove reset security scenario

* Make sonarqube happy

* Remove references to cliController

* Fix features

* fix internal error

* remove internal broker

* re-add shutdown

* fix test

* Put shutdown in adminController

* Reset defaults roles and profiles with resetSecurity

* replace ws by uws

* Nitpicking

* increase coverage

* increase coverage

* increase coverage

* fix linter

* increase coverage

* put back proxyBroker

* fix tests

* remove tests

* fix tests

* nitpicking

* Updates for PR

* Put back defaut config

* Disable dump if it is specified in config

* fix message in cli

* add truncate() method on repositories + refactor admin controller to use truncate()

* fix admin controller

* fix test

* Add Janitor core component

* return number of deleted objects in truncate

* fix linter

* catch TypeError for scanStream

* fix linter

* Please sonarqube

* fix linter

* Nitpicking

* Updates for PR

* update headers

* fix linter

* fix tests

* deprecate "constructor" auth option and replace it with "authenticator" (#1145)

* deprecate "constructor" auth option and replace it with "authenticator"

* fix sonarqube issues

* Add action and controller in request input for http events (#1143)

* Add action and controller in request input for http events

* put back some code

* nitpicking

* Add forgotten files --'

* Test Kuzzle against different node LTS versions (#1147)

* set ulimits for elasticsearch (#1148)

* Update kuzzle-common-objects module (#1150)

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* update to plugin auth local 5.1.3 (#1149)

* update to plugin auth local 5.1.3

* adapt tests to new common objects version

* dependencies update

* npm audit fix

* prevent a breaking change when upgrade from a pre-1.4.0 version

* Revert "prevent a breaking change when upgrade from a pre-1.4.0 version"

This reverts commit 8328f50.

* prevent a breaking change occuring when upgrading Kuzzle with old config files (#1154)

* Inject protocol informations into the RequestContext object (#1155)

* inject protocol info into requestcontext

* fix incoming context infos coming from the proxy

* revert debug modification

* no need to manually copy incoming headers

* update unit tests

* fix sonarqube issu

* jsdoc fix

* fixes #1146, passport login request missing headers (#1153)

* fixes #1146, passport login request missing headers

* oauth - add some comments on arbritraty method & url

* Add script to build and push docker images (#1152)

* Add script to build and push docker images

* Add script to build and push docker images

* Fix build script

* add comment to secret env

* Use kuzzleteam account for dockerhub

* Update subnmodule

* Dont build and push on hotfix merge

* fix #1161 (#1162)

* fix startup message (#1163)

* Fix #1131: listCollections limited to the provided collection name, when specified (#1157)

* fixes #1156 - missing version in swagger output (#1164)

* add token infos to login response (#1171)

* disable dump by default (#1172)

* disable dump by default

* fix tests:

* fix #447: allow to search document with empty query through a HTTP GET request

* fix unit test

* Replace ParseError errors with BadRequestError objects (#1158)

* replace ParseError (deprecated) with BadRequestError

* fix #1168: remove useless CLI --port option  (#1177)

* fix #1168

* remove useless --port option in CLI start command

* fix #1174 (#1176)

* Fix #1115: Standardize the scrollId return value (#1173)

* Standardize the scrollId return value

* typo

* typo

* Initialize repositories before plugins (#1167)

* Initialize repositories before plugins

* remove one then() level

* Use local multi-stage build for kuzzleio/plugin-dev and kuzzleio/kuzzle (#1178)

* Update Dockerfile and build-docker-images script\n Now with multi-stage build and support for 1.x and 2.x development branch
* Remove pm2 from prod image
* Use latest node 8 version

* Add metadata to published documents (#1186)

* KZL 454 - Allow subscribe filters on metadata (#1185)

* Do not cache requests with publish(), call koncorde two times instead

* Add functionnal test for subscription filter on metadata

* Add LGTM.com code quality badges (#1187)

* KZL 290 - Add fixtures for securities (#1182)

* Move mappings and fixtures loading to Janitor

* Fix bug in repository#delete when object is not present

* Add ability to load roles, profiles and users to Janitor and CLI

* Add test for Janitor#loadSecurities

* remove .only from test

* Fix #1184: Better error message when object not found in repositories
Refactor repository loadOneFromDatabase method to reject with an explicit 404 when not found
Allow the old api usage which return a null object when not found for SecurityController#persistUser and PluginRepository#load

* Fix test

* rename getUserId

* Update for PR

* refactor janitor _create* method

* Please sonarqube

* typo

* Plugins/Protocols manifest.json overhaul (#1181)

* wip

* change reporter to dot

* fix polluting console outputs

* upgrade unit tests to make them handle the new plugin structure

* new manifest.json handling tests+fixes

* fix sonarqube issue

* x_x

* implement @benoitvidis asked changes

* fix circular json in manifest objects

* add control to manifest stringification

* remove privileged mode for protocols

* Update docker build script for new branch name 1-dev (#1190)

* Update images build script

* push only if we are on the major build

* remove test

* update default plugins (#1193)

* KZL-174 Expose to plugins a SDK instance directly mapped to the funnel (#1179)

* wip

* KZL-177: custom "funnel" network wrapper for SDK

* KZL-178: Export javascript SDK to plugins

* refactor executePluginRequest: now returns a promise

* KZL-177: revert funnelcallController => now sdk wrapper calls directly funnel.executePluginRequest

* mark `pluginContext.accessor.execute` as deprecated

* clean code

* update package-lock

* fix comment

* tix typo

* dependencies update

* remove debug config

* Release 1.5.0

* dependencies update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment