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

Allow the cookie to be set if action is not `remove` #474

Merged
merged 1 commit into from Apr 7, 2017

Conversation

Projects
None yet
2 participants
@marshallswain
Copy link
Member

marshallswain commented Apr 6, 2017

This removes the service requirement for setting the cookie. If it did run after remove it won’t set the cookie, though.

Closes feathersjs/authentication-oauth2#12

Allow the cookie to be set if action is not `remove`
This removes the service requirement for setting the cookie.  If it did run after `remove` it won’t set the cookie, though.

@marshallswain marshallswain requested a review from ekryski Apr 6, 2017

// we weren't removing the token and we have a JWT access token.
if (res.hook && res.hook.method !== 'remove' && res.data && res.data.accessToken) {
// Don't set the cookie if this was called after removing the token.
if (res.hook && res.hook.method === 'remove') {

This comment has been minimized.

@ekryski

ekryski Apr 7, 2017

Member

Yup I think this should be fine. Good catch on that issue. Funny how simple logic can have such implications.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Apr 7, 2017

:shipit:

@marshallswain marshallswain merged commit ed4fc3b into master Apr 7, 2017

4 checks passed

codeclimate no new or fixed issues
Details
codeclimate/coverage 84.5% (+0.1%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@marshallswain marshallswain deleted the fix-oauth2-cookie branch Apr 7, 2017

@marshallswain marshallswain referenced this pull request Apr 7, 2017

Closed

Cookie not getting set #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.