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.

Sign up for GitHub

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

Jump to bottom

update deprecated dep version #3343

Closed
wants to merge 6 commits into from
Closed

update deprecated dep version #3343

wants to merge 6 commits into from

Conversation

dhmlau
Copy link
Member

@dhmlau dhmlau commented Apr 7, 2017 •
edited
Loading

Description

Some of the dependencies in loopback has deprecated and one of them indicated upgrading to newer version to avoid a RegExp DoS issue.

Related issues

https://github.com/strongloop-internal/scrum-apex/issues/126

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

bajtos and others added 4 commits March 24, 2017 15:01
@bajtos
Forward options in prepareForTokenInvalidation
a5ac150
@bajtos
Merge pull request #3309 from strongloop/fix/options-in-token-invalid…
ac2462e 
…ations

Forward options in prepareForTokenInvalidation
@aaronbuchanan @bajtos
Fix user-literal rewrite for anonymous requests
50e0e48 
Currently any `currentUserLiteral` routes when accessed with a bad
token throw a 500 due to a SQL error that is raised because
`Model.findById` is invoked with `id={currentUserLiteral}`
(`id=me` in our case) when the url rewrite fails.

This commit changes the token middleware to return 401 Not Authorized
when the client is requesting a currentUserLiteral route without
a valid access token.
@bajtos
Merge pull request #3330 from strongloop/fix/unauthorized-current-use…
cf38c62 
…r-literal-2x

Fix user-literal rewrite for anonymous requests
@0candy 0candy added the review label Apr 7, 2017
b-admike
b-admike approved these changes Apr 7, 2017
View reviewed changes
Copy link
Member

@b-admike b-admike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once tests pass

@jannyHou
Copy link
Contributor

jannyHou commented Apr 7, 2017 •
edited
Loading

i searched through the repo, didn't find the file requires supertest-as-promised, but from CI failure log it's still needed, any particular reason why removes it?

>> Mocha exploded!
>> Error: Cannot find module 'supertest-as-promised'
>>     at Function.Module._resolveFilename (module.js:469:15)
>>     at Function.Module._load (module.js:417:25)
>>     at Module.require (module.js:497:17)
>>     at require (internal/module.js:20:19)
>>     at Object.<anonymous> (D:\workspace\loopback\d60c4a15\test\context-options.test.js:10:49)
>>     at Module._compile (module.js:570:32)
>>     at Module.replacementCompile (D:\workspace\loopback\d60c4a15\node_modules\nyc\node_modules\append-transform\index.js:58:13)
>>     at module.exports (D:\workspace\loopback\d60c4a15\node_modules\nyc\node_modules\default-require-extensions\js.js:8:9)
>>     at Object.<anonymous> (D:\workspace\loopback\d60c4a15\node_modules\nyc\node_modules\append-transform\index.js:62:4)
>>     at Module.load (D:\workspace\loopback\d60c4a15\node_modules\coffee-script\lib\coffee-script\register.js:45:36)
>>     at tryModuleLoad (module.js:446:12)
>>     at Function.Module._load (module.js:438:3)
>>     at Module.require (module.js:497:17)
>>     at require (internal/module.js:20:19)
>>     at D:\workspace\loopback\d60c4a15\node_modules\mocha\lib\mocha.js:222:27
>>     at Array.forEach (native)
>>     at Mocha.loadFiles (D:\workspace\loopback\d60c4a15\node_modules\mocha\lib\mocha.js:219:14)
>>     at MochaWrapper.run (D:\workspace\loopback\d60c4a15\node_modules\grunt-mocha-test\tasks\lib\MochaWrapper.js:51:15)
>>     at D:\workspace\loopback\d60c4a15\node_modules\grunt-mocha-test\tasks\mocha-test.js:86:20
>>     at capture (D:\workspace\loopback\d60c4a15\node_modules\grunt-mocha-test\tasks\mocha-test.js:33:5)
>>     at Object.<anonymous> (D:\workspace\loopback\d60c4a15\node_modules\grunt-mocha-test\tasks\mocha-test.js:81:5)
>>     at Object.<anonymous> (D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\grunt\task.js:255:15)
>>     at Object.thisTask.fn (D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\grunt\task.js:73:16)
>>     at Object.<anonymous> (D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\util\task.js:294:30)
>>     at Task.runTaskFn (D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\util\task.js:244:24)
>>     at Task.<anonymous> (D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\util\task.js:293:12)
>>     at D:\workspace\loopback\d60c4a15\node_modules\grunt\lib\util\task.js:220:11
>>     at _combinedTickCallback (internal/process/next_tick.js:67:7)
>>     at process._tickDomainCallback (internal/process/next_tick.js:122:9)
Warning: Task "mochaTest:unit-xml" failed.� Use --force to continue.

jannyHou
jannyHou approved these changes Apr 7, 2017
View reviewed changes
Copy link
Contributor

@jannyHou jannyHou left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

WAIT....why it suddenly goes to 2.x branch lol.

@ssh24
Copy link
Contributor

ssh24 commented Apr 7, 2017

This PR doesn't look correct. Why is there so many changes?

@jannyHou
Copy link
Contributor

jannyHou commented Apr 7, 2017 •
edited
Loading

2.x-latest branch is for release use, to backport commit to LB2.x version, we cherry pick the commit to branch 2.x, not 2.x-latest.
While according to the original change, I think this pr is supposed to check in master branch, not 2.x :)

ssh24
ssh24 suggested changes Apr 7, 2017
View reviewed changes
Copy link
Contributor

@ssh24 ssh24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change it to point to Master branch instead of 2.x

@dhmlau
Copy link
Member Author

dhmlau commented Apr 7, 2017 •
edited
Loading

oops.. sorry... This PR was supposed to be for 2.x. I was trying to fix the master branch as well, but mess up the branch.

@dhmlau dhmlau closed this Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssh24 ssh24 ssh24 requested changes

@jannyHou jannyHou jannyHou approved these changes

@b-admike b-admike b-admike approved these changes

Assignees
No one assigned
Labels
#review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@dhmlau @jannyHou @ssh24 @b-admike @0candy @bajtos @aaronbuchanan