search

jaredhanson / passport-openidconnect

OpenID Connect authentication strategy for Passport and Node.js.
https://www.passportjs.org/packages/passport-openidconnect/?utm_source=github&utm_medium=referral&utm_campaign=passport-openidconnect&utm_content=about
MIT License
188 stars 173 forks source link

Token validation missing #42

Open pottabathini opened 7 years ago

pottabathini commented 7 years ago

We have seen strategy.js file, in that it showing validation claims part is TODO. When can we expect that to be integrated?

We have couple of other questions

  1. what is the difference between this repository(passport-openidconnect) and passport-openid both are authored by @jaredhanson
fiddur commented 7 years ago

passport-openid is for the older OpenID standards. OpenID Connect is quite different from the older OpenID 1 and 2.

fiddur commented 7 years ago

@pottabathini I don't see the TODO you are referring to, what line are you talking about?

pottabathini commented 7 years ago

I didn't see that TODO: in your current version but in when I install this package from npm (you can see the version which I installed in the attached file).

Please correct me if I am refering any wrong version or repository.

package.zip

pottabathini commented 7 years ago

I am attaching strategy file also for your reference. If I am referring a wrong version please let me know strategy.zip

fiddur commented 7 years ago

Ok I see. The version in npm is terribly outdated. I hope @jaredhanson can release a newer version soon, as I think the strategy works as intended in current master. Some additions still lie as PRs, but current should work better than the old version in npm.

pottabathini commented 7 years ago

Thank you for that. Can I configure the master version in my project? If yes I will try out with my scenarios(I am working on Azure B2C policies now) and bug you if I stuck :-)

fiddur commented 7 years ago

Yeah, you should be able to specify something like git://github.com/jaredhanson/passport-openidconnect.git#1cf968b6eafd11e4ceb153c2ec1e6c38b69d6592 in your package demendencies.

It's good to have the commit-ish there for now, since master is under development and the API might change.

pottabathini commented 7 years ago

The commit-ish you given giving many erros and it not allow me to login atleast :-( Can you guys try to push code at the earliest.

Below are the few problems we identified

  1. Hardcoded value for response_type to "code" (params['response_type'] in strategy.js file)
  2. When data returning back signin callback firing endlessly and ended up with too many redirects error.
  3. when validating tokens we require [jwks_uri]/ [jwks keys] also for manual settings I didn't see that option.

The current master code is working with out any flaw excpet it does not have a token vlidation part in it.

pottabathini commented 7 years ago

Is there any plans to push code to master in near future?

fiddur commented 7 years ago

@pottabathini I think @jaredhanson might be too busy I'm afraid... There are a lot of forks to this repo though that are being used.

prashanthc commented 4 years ago

Any update on this? It seems like we're still missing the id_token validation and also the verification of access-token.

jgf5013 commented 1 year ago

I'm super confused about this. Why would we want to use the library if it doesn't verify the token? I see there are lots of checks on the id token here, but nothing to verify the token itself hasn't been tampered with. And no references to the access token. Am I missing something?