-
Notifications
You must be signed in to change notification settings - Fork 1k
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
add passwordcredentials client like clientcredentials #186
Comments
Using PasswordCredentialsToken requires a TokenSource. This implements a Config similar to oauth2/clientcredentials for the Resource Owner Password Credentials Grant. See https://tools.ietf.org/html/rfc6749#section-4.3 for more info. Fixes golang#186 Change-Id: I3c6032899d6c286b84f8f24e0f6a240004f4f6c0
|
needs tests and doc |
Tests done, doc needs review |
doc should be ready |
Using PasswordCredentialsToken requires a TokenSource. This implements a Config similar to oauth2/clientcredentials for the Resource Owner Password Credentials Grant. See https://tools.ietf.org/html/rfc6749#section-4.3 for more info. Fixes golang#186 Change-Id: I3c6032899d6c286b84f8f24e0f6a240004f4f6c0
reverted by https://go-review.googlesource.com/#/c/23841/, apparently this requires more discussion |
@rakyll I added another comment to the CL, you made a great point. That's how I ended up submitting this. This "reimplements" https://godoc.org/golang.org/x/oauth2#Config.PasswordCredentialsToken using the same implementation pattern from https://godoc.org/golang.org/x/oauth2/clientcredentials, because using PasswordCredentialsToken as is requires implementing a wrapper TokenSource. I'm planning to make a CL reimplementing this using Config.PasswordCredentialsToken. That was how I started in my local fork and didn't like it as much. I don't know if/how to re-open this CL, but I still think it's the most reasonable. |
This CL is still pending, right? https://go-review.googlesource.com/c/23834/ |
Even though we promised not to break any APIs, I am suggesting to remove the Config.PasswordCredentialsToken if we merge the passwordcredentials package. I do not think the current API is widely used and keeping the library orthogonal is more valuable than breaking a few users. |
Yes, I am still using this approach locally. I'm still interested in this CL. |
@adg is there anything I need to do to move this forward? |
@rakyll what do you think? |
I am in favor of adding a deprecation notice to https://godoc.org/golang.org/x/oauth2#Config.PasswordCredentialsToken and go with @josephholsten's package. |
What @rakyll suggests sounds good to me. Whoever wants to do that can do it. |
@josephholsten, I left comments on your change at https://go-review.googlesource.com/c/23834/ a while ago. Can you take a look at those? |
@rakyll sorry, I hadn't pushed my changes when I made that comment. Care to give it another look? |
I would like to use this package, is this every going to move forward? |
For anyone who is interested, a couple of years ago I extracted this PR into a package that you can use to provide the required functionality, pending the merge of this PR: https://github.com/joefitzgerald/passwordcredentials |
https://go-review.googlesource.com/c/23834/
In order to use PasswordCredentialsToken, a TokenSource needs to be provided. It's not complicated to have one of these readily available, in the style of
golang.org/x/oauth2/clientcredentials
.It would also be good to have #146, which would fix #110.
The text was updated successfully, but these errors were encountered: