Added discord provider #2113
Added discord provider #2113
Conversation
|
+1 |
|
Hi @draeron, I'll have to read some of the discord documentation first before i review this. I'll try to do it next week. |
|
i have updated my fork with the latest master |
|
Hi @draeron, sorry for the delay. Due to the latest release and some bugfixes with higher priority I wasn't able to look into your implementation yet. Please do another update / rebase of your PR and add a CHANGELOG entry :) I'll do my best to test your provider implementation this week and give it a first code review. |
| } | ||
|
|
||
| for _, guild := range guilds { | ||
| s.Groups = append(s.Groups, guild.ID) |
There was a problem hiding this comment.
guild ID are of type snowflake. I would suggest we add another flag for the discord provider to allow groups to be returned either as snowflake or the guild name
There was a problem hiding this comment.
| // Avatar string `json:"avatar"` | ||
| // Flags int `json:"flags"` |
| Do(). | ||
| UnmarshalInto(&guilds) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("error getting user's guilds info: %v", err) |
There was a problem hiding this comment.
| return nil, fmt.Errorf("error getting user's guilds info: %v", err) | |
| return nil, fmt.Errorf("error getting user's guilds info: %w", err) |
| validateURL: discordDefaultValidateURL, | ||
| scope: discordDefaultScope, | ||
| }) | ||
| p.setAllowedGroups(opts.Guilds) |
There was a problem hiding this comment.
This line is a problem with the legacy --allowed-group flag or allowed_groups config field as it will always overwrite the set values with an empty map map[string]struct {} []
There was a problem hiding this comment.
unfortunately, you will have to add legacy support
There was a problem hiding this comment.
Can confirm that legacy support is broken right now. --allowed-group doesn't work
|
Hi @draeron, if you want you can continue working on this and/or you give me collaborator access on your repo and i can work on it as well. Following major todos:
|
|
I'd also like to contribute. I got it running on my end and would be happy to share what I did plus some additional features like optional email scope (so people don't get scared that an app asks for their email even if it doesn't need it) and Discord role check on a server. If you could add me as a collaborator, I'd love to push my changes to a feature-branch for review @draeron. |
|
@draeron any chance to update this PR so it meets all the requirements? I built a custom docker image for this now, would be fantastic if this can be merged eventually. |
|
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
|
Damn I totally forgot extending and testing this one |
|
i think this pull request has drifted too much from the master branch, imho it would need to be redone completely |
Enable discord provider with filtering based on guilds membership.
You can use both structured Alpha configuration or
--allowed-groupoptions.