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.