-
Notifications
You must be signed in to change notification settings - Fork 981
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
oauth2: add device flow support #356
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! |
CLAs look good, thanks! |
This PR (HEAD: 7cf8880) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/156537 to see it. Tip: You can toggle comments from me using the |
Message from Gobot Gobot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/156537. |
any chance to have this merged? "device flow" has become quite common. |
@devbycm I believe you should have a |
"client_id": {c.ClientID}, | ||
"grant_type": {"urn:ietf:params:oauth:grant-type:device_code"}, | ||
"device_code": {da.DeviceCode}, | ||
"code": {da.DeviceCode}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe the code
parameter is required in this request - see rfc8628#section-3.4
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/156537. |
Hey, please fix the conflicting file. This feature will be so useful! |
@centimitr Hello, thanks for this PR. Apart from the conflict, is there anything else missing? any chance to have this merged? |
AuthURL string | ||
DeviceAuthURL string | ||
TokenURL string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@centimitr these lines are creating a merge conflict, you'd just need to slightly change this bit to match the new version of the file and it'd be a straight merge.
@centimitr are you able to fix the merge conflicts and get this merged? |
Implemented in e3fb0fb |
Draft: device flow is still a draft, but it is a very handy flow for many practical cases.
This pr includes an early stage implementation of the device flow, according to the draft, and it works with Azure AD. I am going to add tests and doing some refactor, like renaming
AuthCodeOption
with type alias etc. I am looking for the time it can be merged as a standard.