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

WIP for discussion #46

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

WIP for discussion #46

wants to merge 10 commits into from

Conversation

nrn
Copy link
Member

@nrn nrn commented Nov 6, 2024

Mirrored here for discussion and debugging

@@ -221,10 +226,10 @@ module.exports = exports = (log, loga, argv) ->
displayName: extractUserInfo(argv.oauth2_DisplayNameField, 'params.user_id')
}
}
cb(null, user)
Copy link
Member Author

Choose a reason for hiding this comment

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

This here is the function call that puts the user on req.session.passport.user, where it's later showing up undefined. The change here is a little sus, in that cb is now only getting called in the try block, and will never call back on error. The old way is a little odd too in that it could add a partial user object w/o all of the expected information. This should probably call cb(e) in the catch block.

It seems likely that what's happening is that something is throwing an error in here and the feedback isn't getting to the client, is there any info in the error output from the server?

@@ -185,13 +190,17 @@ module.exports = exports = (log, loga, argv) ->
userInfoURL: argv.oauth2_UserInfoURL
}, (accessToken, refreshToken, params, profile, cb) ->

token = jwtDecode(accessToken)
Copy link
Member Author

Choose a reason for hiding this comment

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

This decode probably needs to be in a try to avoid throwing on bad tokens.

@nrn
Copy link
Member Author

nrn commented Nov 6, 2024

The other thing to look at other than the server error logs would be the oauth2_.*field settings. Either a bad token or one of these settings being off would explain these symptoms also.

@paul90
Copy link
Member

paul90 commented Nov 7, 2024

Looking back for the last time I can say with certainty that claim was working, was mid March which would have been with 0.6.2

@paul90
Copy link
Member

paul90 commented Nov 7, 2024

It was a thought, but the current version works correctly, allowing claim with a github login.

Looks as if the issue is isolated to the oauth2 login.

@Bortseb, @3-w-c If you can identify a know working version of the oauth2 code for which claim works, you should be able to use git bisect to fairly quickly identify which commit broke claim.

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

Successfully merging this pull request may close these issues.

4 participants