Skip to content
This repository has been archived by the owner on Sep 14, 2022. It is now read-only.

Exposed verify token. Fixes #43. #82

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

YourDeveloperFriend
Copy link

No description provided.

@dougwilson
Copy link
Contributor

Please add some tests and documentation :)

@YourDeveloperFriend
Copy link
Author

Please add some tests and documentation :)

Done!

test/test.js Outdated
@@ -323,6 +323,63 @@ describe('csurf', function () {
})
})

describe.only('req.verifyToken()', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

The .only disabled all other tests. Please just use describe.

Choose a reason for hiding this comment

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

My bad, I had that in there for my own tests. I'll remove it.

@dougwilson dougwilson self-assigned this Sep 18, 2015
@dougwilson dougwilson added the pr label Sep 18, 2015
@dougwilson
Copy link
Contributor

Thanks, looking good! The tests are failing in Node.js 0.12 and up, it looks like. Seems like just an issue in the tests themselves, I would guess.

@YourDeveloperFriend
Copy link
Author

Awesome, I'll take a look at it in 0.12 and see if I can find out what's going on.

@camacho
Copy link

camacho commented Oct 27, 2015

@YourDeveloperFriend @dougwilson Any update on the status of this PR? Tests seem to be passing and code approved - would be great to have access to this functionality without having to use a forked version. Happy to contribute if there's more work to be done.

@dougwilson
Copy link
Contributor

Hi @camacho , sorry, I didn't realize the issue was addressed, as there was no follow-up comment after "I'll take a look" and GitHub provides no notifications for when new commits are pushed to a PR, so it completely dropped off my radar.

@camacho
Copy link

camacho commented Oct 28, 2015

no worries @dougwilson - is there anything additional that needs to be done with this PR?

@mindvox
Copy link

mindvox commented Jul 17, 2016

This would be great to get implemented.. would be nice to base64 encode/decode or encrypt tokens during use.

@JustinLivi
Copy link

Is there anything I could do to help move this along? I was about to fork myself to build this exact feature. I would very much prefer to be able to use the upstream library.

@alvarotrigo
Copy link
Contributor

It was never merged?

@YourDeveloperFriend
Copy link
Author

AFAIK there's nothing on my end that needs to happen. Please let me know if there's something that's missing from my PR.

@iofluxdev1
Copy link

How about getting this merged in. It has been 2 years...

@davidjb
Copy link

davidjb commented Apr 19, 2018

👍 For this feature. My use case is the same as #43 in that I'm validating state within an OAuth callback.

@jamesfiltness
Copy link

👍 for this. I also want to use csurf to validate state in an OAuth context.

@jamesfiltness
Copy link

jamesfiltness commented May 3, 2018

For anyone needing csurf in the context of an OAuth callback you can use the following as a middleware:

const csrfProtection = csrf({
  value: function(req) {
    // grab the csrf token from the query param
    return req.query.state;
  },
  // by default csurf ignores GET requests
  ignoreMethods: ['HEAD', 'OPTIONS'],
});
router.get('/', csrfProtection, require('./kloudless-oauth-callback'));

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet