fix: use /token endpoint to get tokens with device flow #2010
Conversation
|
Should we deprecate the |
|
Yes, I think so. I left a to-do but did nothing with deprecation. Maybe adding some sort of warning on endpoint call will be good enough. Something like this. |
|
For me, deprecation would mean
|
Signed-off-by: m.nabokikh <[email protected]>
Signed-off-by: m.nabokikh <[email protected]>
nabokihms
force-pushed
the
switch-device-token-endpoint-to-token
branch
from
ab8b38b to
9ed5cc0
Compare
server/deviceflowhandlers.go
Outdated
| s.logger.Warn(`Request to the deprecated "/device/token" endpoint was received.`) | ||
| s.logger.Warn(`The "/device/token" endpoint will be removed in a future release.`) |
There was a problem hiding this comment.
Can we make this a single log line? And maybe make the message a bit more concise?
server/server_test.go
Outdated
| @@ -1583,7 +1583,7 @@ func TestOAuth2DeviceFlow(t *testing.T) { | |||
|
|
|||
| // Hit the Token Endpoint, and try and get an access token | |||
| tokenURL, _ := url.Parse(issuer.String()) | |||
| tokenURL.Path = path.Join(tokenURL.Path, "/device/token") | |||
| tokenURL.Path = path.Join(tokenURL.Path, "/token") | |||
There was a problem hiding this comment.
Can we add a test with the old path to make sure it's still functional?
server/deviceflowhandlers.go
Outdated
| @@ -151,7 +151,10 @@ func (s *Server) handleDeviceCode(w http.ResponseWriter, r *http.Request) { | |||
| } | |||
| } | |||
|
|
|||
| func (s *Server) handleDeviceToken(w http.ResponseWriter, r *http.Request) { | |||
| func (s *Server) handleDeviceTokenGrant(w http.ResponseWriter, r *http.Request) { | |||
There was a problem hiding this comment.
I think this name is a bit confusing and ambiguous with the handleDeviceToken name. Maybe call it something like deprecatedDeviceTokenHandler or handleDeviceTokenDeprecated.
Signed-off-by: m.nabokikh <[email protected]>
|
@sagikazarmark Anything preventing this from being merged/closed? |
Signed-off-by: m.nabokikh [email protected]
Overview
rfc6749#section-3.2
/tokenendpoint responding to requests with device auth grant/tokenendpointWhat this PR does / why we need it
Fixes #1953
As was mentioned in the issue above, the main problem that the token endpoint is not discoverable.
Special notes for your reviewer
This MR leaves the possibility to get a token from the
/device/tokenendpoint.Does this PR introduce a user-facing change?
There are no breaking changes, but it will be better to avoid using /device/token endpoint in the feature.