Skip to content

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.

@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 dariusk force-pushed the dariusk/inspectuser branch from 52da76d to b262fa1 Compare October 22, 2020 20:11
@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.

2 participants