Skip to content
This repository has been archived by the owner on Jul 13, 2018. It is now read-only.

Commit

Permalink
Bubble up the OpenID2 dep to this fork
Browse files Browse the repository at this point in the history 
*NOTE(S)*
* Testing this fix for permanence in contrast to alternate fix of `"passport-openid": "^0.4.0",` in liamcurry@4d19567

Applies to OpenUserJS/OpenUserJS.org#1021
  • Loading branch information
User authored and Martii committed Aug 27, 2017
1 parent 559ea32 commit d9198e3
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/passport-steam/strategy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies.
*/
var util = require('util')
, OpenIDStrategy = require('passport-openid').Strategy
, OpenIDStrategy = require('passport-openid-node6support').Strategy
, SteamWebAPI = require('steam-web');

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
"main": "./lib/passport-steam",
"dependencies": {
"passport-openid": "^0.4.0",
"passport-openid-node6support": "git://github.com/OpenUserJs/passport-openid#OpenID2",
"steam-web": "0.4.0"
},
"devDependencies": {
Expand Down

5 comments on commit d9198e3

@Martii
Copy link
Member

@Martii Martii commented on d9198e3 Aug 27, 2017

Choose a reason for hiding this comment

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

Successful login with dev e.g. hash/token returned is same.

@Martii
Copy link
Member

@Martii Martii commented on d9198e3 Aug 27, 2017

Choose a reason for hiding this comment

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

Successful login with local pro e.g. hash/token returned is same.

@Martii
Copy link
Member

@Martii Martii commented on d9198e3 Aug 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@sizzlemctwizzle
Been thinking about this forever this year... the solution passport-steam added later appears to be OpenID 1.0 whereas this continues to use OpenID 2.0. Not entirely sure what repercussions will happen to the hash/token returned if we return to OpenID 1.0. Steam appears to have it mislabeled as well in their current README.md as being 2.0 when it's actually 1.0 from jareds repo at https://github.com/jaredhanson/passport-openid/blob/6221d0e10cd9633f32634c6a2286c22f1e3576a2/package.json#L34

As a result passport-yahoo and passport-aol may have tokens/hashes generated from OpenID2 stored in the DB since I applied that patch to them at the same time as passport-steam was applied with this before a later fix.. I asked on the repo of passport-steam a long time ago if there would be a difference with no response. I'm not sure if I'm willing to gamble with yahoo and aol on the chance that it is different. Somewhere I read from someone that claimed that OpenID2 has issues yet I haven't seen any in the last ~10 months with yahoo and aol.

Long story short... this appears to be a fix that keeps things in sync with OpenID2 and steam is letting me log in gracefully.

I know this is horribly complicated to explain and does produce 4 forks (instead of 2) for us to maintain but I still think it's the best option.

What I could use from you is a 2nd opinion please before I reroute OUJS to use this fork. Thanks in advance.

@sizzlemctwizzle
Copy link
Member

Choose a reason for hiding this comment

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

The openid package supports "OpenID 1.0/1.1/2.0", which is still in the readme text of the last 1.x.x version. Anyway, I really don't think a Openid provider would give us a different identifier (which is what we hash and store) just because a newer or older version of the same protocol is being used. I figure they'd want to return the same value for at least that for backwards compatibility. Now I tried to prove this by tracing from the openid package to passport-openid to passport-steam to openuserjs but I got lost between the layers and different versions, so I can't be certain I'm right. But it just seems silly for different versions of the same protocol to return different user identifiers... but crazier things have happened.

@Martii
Copy link
Member

@Martii Martii commented on d9198e3 Aug 27, 2017

Choose a reason for hiding this comment

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

I got lost between the layers and different versions,

That's easy to do with this.

I really don't think a Openid provider would give us a different identifier

I'm leaning towards that myself but not sure I want to incur the issues on GH dev. ;)

But it just seems silly for different versions of the same protocol to return different user identifiers... but crazier things have happened.

Well the 1.x line of node-openid tree is labeled as maintenance which is why I chose to do this patch on passport-yahoo and passport-aol e.g. get ahead of the eventual deprecation/eol of 1.x of node-openid (aka openid on npmjs.com)

Please sign in to comment.