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

Discord Provider #13

Merged
merged 5 commits into from Jan 10, 2019

Conversation

2 participants
@l1n
Copy link

l1n commented Jan 9, 2019

@ploxiln

This comment has been minimized.

Copy link
Owner

ploxiln commented Jan 9, 2019

I'd rather the import path not be renamed from github.com/bitly/oauth2_proxy to github.com/ploxiln/oauth2_proxyin this PR. It may be a bit awkward, but you can clone my repo to$GOPATH/src/github.com/bitly/oauth2_proxy`, or if you already have the bitly repo there you can add my repo as another remote and check out my master branch, etc. I'll probably do the renaming soon - but separately.

Other than that, there's just some minor whitespace/formatting issues.

Show resolved Hide resolved README.md Outdated
Novalinium
@l1n

This comment has been minimized.

Copy link

l1n commented Jan 10, 2019

Fair enough, reverting that commit 👍 I didn't mean to add it in this pull request since it wasn't part of the Discord provider.

@l1n l1n force-pushed the l1n:master branch from 9583d75 to 0a98172 Jan 10, 2019

Novalinium

@l1n l1n force-pushed the l1n:master branch from 0a98172 to 46021c3 Jan 10, 2019

gofmt DiscordUserInfo struct
gofmt is indeed picky
@ploxiln

This comment has been minimized.

Copy link
Owner

ploxiln commented Jan 10, 2019

Thanks!

(I pushed up one last gofmt tweak ... the trick is gofmt -w providers/*.go)

@l1n

This comment has been minimized.

Copy link

l1n commented Jan 10, 2019

I apologize for not speaking Go too natively, my last Go project was a year or so ago 😅 thank you for your help

p.ValidateURL = p.ProfileURL
}
if p.Scope == "" {
p.Scope = "identify email connections"

This comment has been minimized.

@ploxiln

ploxiln Jan 10, 2019

Owner

looks like "connections" is not needed

This comment has been minimized.

@ploxiln

ploxiln Jan 10, 2019

Owner

this and some other little refinements could be in a follow-up pull request if someone cares enough :)

(e.g. removing parts of DiscordUserInfo which are unused, maybe checking Verified property ...)

@ploxiln ploxiln merged commit 89eeb57 into ploxiln:master Jan 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@l1n

This comment has been minimized.

Copy link

l1n commented Jan 10, 2019

It isn't needed right now, but I'd like to extend from the user and email that are by default provided to the proxied host at some point: similar to the way that mod_shib provides displayName https://tcg.stanford.edu/?p=131

brianv0 pushed a commit to brianv0/oauth2_proxy_old that referenced this pull request Jan 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment