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

Generic OAuth userinfo properies now configurable #405

Merged
merged 2 commits into from Mar 18, 2021

Conversation

dariusk
Copy link
Contributor

@dariusk dariusk commented Oct 22, 2020

When connecting to a generic OAuth provider, you are never sure what object you'll be receiving from the userinfo endpoint (it isn't actually specified anywhere). So this commit adds mapping values to the generic oauth configuration section of config.ini, allowing the user to specify which keys in the remote endpoint it expects to read the UserID, Username, DisplayName, and Email from. Default values if unspecified remain as they were before this commit.


  • I have signed the CLA

When connecting to a generic OAuth provider, you are never sure what object you'll be receiving from the userinfo endpoint (it isn't actually specified anywhere). So this commit adds mapping values to the generic oauth configuration section of config.ini, allowing the user to specify which keys in the remote endpoint it expects to read the UserID, Username, DisplayName, and Email from. Default values if unspecified remain as they were before this commit.
@dariusk
Copy link
Contributor Author

dariusk commented Oct 22, 2020

I am not a golang programmer and probably made some very basic mistakes here. This in particular, the 4 if statements in a row, seems like it could be improved.

Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

It's all good -- this is almost there! Parsing can be a bit tricky, but there are just a few changes to make here. Let me know if you have any questions about this stuff.

Also, just need to run go fmt on the file so formatting is consistent.

oauth_generic.go Outdated Show resolved Hide resolved
oauth_generic.go Outdated Show resolved Hide resolved
oauth_generic.go Outdated Show resolved Hide resolved
@thebaer thebaer added this to the 0.13 milestone Oct 22, 2020
@dariusk
Copy link
Contributor Author

dariusk commented Oct 22, 2020

Okay, updated. (The more I learn about go, the more I like it.)

@dariusk
Copy link
Contributor Author

dariusk commented Oct 22, 2020

Oops, one more requested change incoming.

@dariusk
Copy link
Contributor Author

dariusk commented Oct 22, 2020

Okay, did a force push so now all requested changes are in the second commit.

Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

Finally getting back to this -- sorry for the delay. This looks good! We'll get it merged.

@thebaer thebaer merged commit 3984042 into writefreely:develop Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants