Skip to content
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

Device grant flow (migrate to master) #701

Open
wants to merge 67 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
ce02ff3
Device grant flow (migrate to master)
BuzzBumbleBee Sep 4, 2022
9976360
Add own flow to the Device Grant
supercairos Sep 7, 2022
ae0144c
Generate Mocks & MemoryStore
supercairos Sep 7, 2022
60e109b
Fix HMACSHA Test
supercairos Sep 7, 2022
7b5a9ba
Fix fosite tests
supercairos Sep 7, 2022
b3e10dd
Merge pull request #3 from BuzzBumbleBee/rcaire/retry-pr
BuzzBumbleBee Sep 7, 2022
e216bb5
Merge remote-tracking branch 'github/master' into feat_dev_grants_2x
supercairos Sep 8, 2022
0ff2699
Fix styling
supercairos Sep 8, 2022
3665fc6
Fix merge errors
supercairos Sep 8, 2022
d710a40
Remove debug logs
supercairos Sep 9, 2022
eefdf61
Fix test following fix merge error
supercairos Sep 9, 2022
57fb5ba
Remove "device_code" auth flow
supercairos Sep 9, 2022
74ef995
Rename to follow Ory's file naming pattern
supercairos Sep 9, 2022
873f022
Add some tests & documentations
supercairos Sep 12, 2022
17b8665
Remove unused files & add some more tests
supercairos Sep 12, 2022
375d89e
Use same method do generate authorization code & device code
supercairos Sep 12, 2022
dbd7860
Fix comment formating
supercairos Sep 12, 2022
8585c75
Add test to device pkce
supercairos Sep 12, 2022
4f7eb2b
rcaire/create-device-code-at-start
supercairos Sep 13, 2022
eea03e0
Split & Move files
supercairos Sep 14, 2022
341d411
fix: edit comments
supercairos Sep 14, 2022
0f0668b
Merge branch 'master' into feat_dev_grants_2x
supercairos Sep 15, 2022
a9aaaba
Merge branch 'feat_dev_grants_2x' into device-code-rework
supercairos Sep 15, 2022
fef42ec
Add missing handler in composer
supercairos Sep 16, 2022
b085d07
Merge branch 'master' into feat_dev_grants_2x
supercairos Sep 29, 2022
3106872
Merge branch 'feat_dev_grants_2x' into device-code-rework
supercairos Sep 29, 2022
9a30db2
Retry CI
supercairos Sep 29, 2022
7bbeb02
Don't use `url.Values.Has()` as it doesn't exist in Golang 1.16...
supercairos Sep 29, 2022
289f9a2
Merge branch 'feat_dev_grants_2x' into device-code-rework
supercairos Sep 29, 2022
142b7ff
Change from Forbiden to BadRequest for authorization_pending to bette…
supercairos Oct 5, 2022
6f66f32
Merge branch 'master' into feat_dev_grants_2x
supercairos Oct 20, 2022
5722afa
Update go/x/text module to 0.4.0
supercairos Oct 20, 2022
a243b93
Merge branch 'master' into feat_dev_grants_2x
supercairos Nov 18, 2022
46cdb1e
Merge branch 'master' into feat_dev_grants_2x
supercairos Dec 12, 2022
3483d6e
Update following merge
supercairos Dec 12, 2022
86c4f95
Initial refactoring following @hackerman recommandations
supercairos Dec 13, 2022
e23a26a
Update following integration test with Hydra
supercairos Dec 14, 2022
a99ed89
Fix build error
supercairos Dec 14, 2022
88abd8d
Fix test
supercairos Dec 14, 2022
1b29706
Merge branch 'master' into feat_dev_grants_2x
supercairos Jan 9, 2023
3313b71
Use HTTP redirect instead of raw http header redirection
supercairos Jan 9, 2023
e99e2bc
Refactor the DeviceResponse struct to only have one definition of it.
supercairos Jan 9, 2023
9279963
Use RandX instead of custom code to generate the Random UserCode
supercairos Jan 9, 2023
e60949d
Remove padding from HMAC generation to avoid confusion in URLs
supercairos Jan 9, 2023
ad37f5d
Use GrantType const string instead of raw string
supercairos Jan 9, 2023
ea146be
Split DeviceAuthorizeHandler and CodeAuthorizeHandler
supercairos Jan 19, 2023
9fcb161
Merge branch 'master' into feat_dev_grants_2x
supercairos Feb 1, 2023
1b306bb
Update Copyright header
supercairos Feb 1, 2023
98709f3
Merge branch 'master' into feat_dev_grants_2x
supercairos Mar 14, 2023
709a443
Merge branch 'master' into feat_dev_grants_2x
supercairos Mar 20, 2023
8a2cf5c
Update fosite refactoring
supercairos Apr 18, 2023
0395b8e
Merge branch 'master' into feat_dev_grants_2x
supercairos Jul 18, 2023
d2316f4
Fix some typo in config storage
supercairos Oct 10, 2023
0e6f5fa
Merge branch 'master' into feat_dev_grants_2x
supercairos Oct 10, 2023
b1fbd36
Merge branch 'master' into feat_dev_grants_2x
supercairos Nov 24, 2023
0847036
As per spec, set polling to 5s (from 10s)
supercairos Nov 27, 2023
5b41e88
Fix typo in comment
supercairos Nov 27, 2023
5f9b0b1
Add tracer as per AuthorizeRequest
supercairos Nov 27, 2023
bc70138
Fix comment in typo
supercairos Nov 27, 2023
c4b608a
use randx runes defined in ory/x
supercairos Nov 29, 2023
6cded09
Remove PKCE
supercairos Nov 30, 2023
7ad3851
Rename DeviceAuthorizeXXX to DeviceUserXXX
supercairos Dec 5, 2023
10419a9
Refactor the way we query the URL Query params
supercairos Dec 5, 2023
5bc8783
Don't send stacktrace when err is AuthorizationPending
supercairos Dec 7, 2023
e8b1654
Add RateLimit to Device Polling endpoint
supercairos Jan 4, 2024
193f7d7
Bump ory/x version to latest
supercairos Jan 4, 2024
eee739a
Merge branch 'master' into feat_dev_grants_2x
supercairos Jan 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions authorize_request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ func TestNewAuthorizeRequest(t *testing.T) {
},
expectedError: ErrInvalidScope,
},
/* fails because scope not given */
/* fails because audience not given */
{
desc: "should fail because client does not have scope baz",
desc: "should fail because client does not have audience https://www.ory.sh/api",
conf: &Fosite{Store: store, Config: &Config{ScopeStrategy: ExactScopeStrategy, AudienceMatchingStrategy: DefaultAudienceMatchingStrategy}},
query: url.Values{
"redirect_uri": {"https://foo.bar/cb"},
Expand Down
9 changes: 9 additions & 0 deletions compose/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ func Compose(config *fosite.Config, storage interface{}, strategy interface{}, f
if ph, ok := res.(fosite.PushedAuthorizeEndpointHandler); ok {
config.PushedAuthorizeEndpointHandlers.Append(ph)
}
if dh, ok := res.(fosite.DeviceEndpointHandler); ok {
config.DeviceEndpointHandlers.Append(dh)
}
if dah, ok := res.(fosite.DeviceAuthorizeEndpointHandler); ok {
config.DeviceAuthorizeEndpointHandlers.Append(dah)
}
}

return f
Expand All @@ -68,6 +74,7 @@ func ComposeAllEnabled(config *fosite.Config, storage interface{}, key interface
storage,
&CommonStrategy{
CoreStrategy: NewOAuth2HMACStrategy(config),
RFC8628CodeStrategy: NewDeviceStrategy(config),
OpenIDConnectTokenStrategy: NewOpenIDConnectStrategy(keyGetter, config),
Signer: &jwt.DefaultSigner{GetPrivateKey: keyGetter},
},
Expand All @@ -77,11 +84,13 @@ func ComposeAllEnabled(config *fosite.Config, storage interface{}, key interface
OAuth2RefreshTokenGrantFactory,
OAuth2ResourceOwnerPasswordCredentialsFactory,
RFC7523AssertionGrantFactory,
RFC8628DeviceFactory,

OpenIDConnectExplicitFactory,
OpenIDConnectImplicitFactory,
OpenIDConnectHybridFactory,
OpenIDConnectRefreshFactory,
OpenIDConnectDeviceFactory,

OAuth2TokenIntrospectionFactory,
OAuth2TokenRevocationFactory,
Expand Down
3 changes: 3 additions & 0 deletions compose/compose_oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package compose
import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/rfc8628"
"github.com/ory/fosite/token/jwt"
)

Expand All @@ -16,8 +17,10 @@ func OAuth2AuthorizeExplicitFactory(config fosite.Configurator, storage interfac
AccessTokenStrategy: strategy.(oauth2.AccessTokenStrategy),
RefreshTokenStrategy: strategy.(oauth2.RefreshTokenStrategy),
AuthorizeCodeStrategy: strategy.(oauth2.AuthorizeCodeStrategy),
DeviceStrategy: strategy.(rfc8628.RFC8628CodeStrategy),
CoreStorage: storage.(oauth2.CoreStorage),
TokenRevocationStorage: storage.(oauth2.TokenRevocationStorage),
DeviceStorage: storage.(rfc8628.RFC8628CodeStorage),
Config: config,
}
}
Expand Down
14 changes: 14 additions & 0 deletions compose/compose_openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,17 @@ func OpenIDConnectHybridFactory(config fosite.Configurator, storage interface{},
OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(strategy.(jwt.Signer), config),
}
}

// OpenIDConnectDeviceFactory creates an OpenID Connect device ("device code flow") grant handler.
//
// **Important note:** You must add this handler *after* you have added an OAuth2 authorize code handler!
func OpenIDConnectDeviceFactory(config fosite.Configurator, storage interface{}, strategy interface{}) interface{} {
return &openid.OpenIDConnectDeviceHandler{
OpenIDConnectRequestStorage: storage.(openid.OpenIDConnectRequestStorage),
IDTokenHandleHelper: &openid.IDTokenHandleHelper{
IDTokenStrategy: strategy.(openid.OpenIDConnectTokenStrategy),
},
OpenIDConnectRequestValidator: openid.NewOpenIDConnectRequestValidator(strategy.(jwt.Signer), config),
Config: config,
}
}
2 changes: 2 additions & 0 deletions compose/compose_pkce.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/pkce"
"github.com/ory/fosite/handler/rfc8628"
)

// OAuth2PKCEFactory creates a PKCE handler.
func OAuth2PKCEFactory(config fosite.Configurator, storage interface{}, strategy interface{}) interface{} {
return &pkce.Handler{
AuthorizeCodeStrategy: strategy.(oauth2.AuthorizeCodeStrategy),
DeviceCodeStrategy: strategy.(rfc8628.DeviceCodeStrategy),
Storage: storage.(pkce.PKCERequestStorage),
Config: config,
}
Expand Down
19 changes: 19 additions & 0 deletions compose/compose_rfc8628.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright © 2022 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package compose

import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/rfc8628"
)

// RFC8628DeviceFactory creates an OAuth2 device code grant ("Device Authorization Grant") handler and registers
// an user code, device code, access token and a refresh token validator.
func RFC8628DeviceFactory(config fosite.Configurator, storage interface{}, strategy interface{}) interface{} {
return &rfc8628.DeviceAuthHandler{
Strategy: strategy.(rfc8628.RFC8628CodeStrategy),
Storage: storage.(rfc8628.RFC8628CodeStorage),
Config: config,
}
}
10 changes: 10 additions & 0 deletions compose/compose_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import (
"github.com/ory/fosite"
"github.com/ory/fosite/handler/oauth2"
"github.com/ory/fosite/handler/openid"
"github.com/ory/fosite/handler/rfc8628"
"github.com/ory/fosite/token/hmac"
"github.com/ory/fosite/token/jwt"
)

type CommonStrategy struct {
oauth2.CoreStrategy
rfc8628.RFC8628CodeStrategy
openid.OpenIDConnectTokenStrategy
jwt.Signer
}
Expand All @@ -27,6 +29,7 @@ type HMACSHAStrategyConfigurator interface {
fosite.GlobalSecretProvider
fosite.RotatedGlobalSecretsProvider
fosite.HMACHashingProvider
fosite.DeviceAndUserCodeLifespanProvider
}

func NewOAuth2HMACStrategy(config HMACSHAStrategyConfigurator) *oauth2.HMACSHAStrategy {
Expand All @@ -50,3 +53,10 @@ func NewOpenIDConnectStrategy(keyGetter func(context.Context) (interface{}, erro
Config: config,
}
}

func NewDeviceStrategy(config fosite.Configurator) *rfc8628.DefaultDeviceStrategy {
return &rfc8628.DefaultDeviceStrategy{
Enigma: &hmac.HMACStrategy{Config: config},
Config: config,
}
}
25 changes: 25 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ type AuthorizeCodeLifespanProvider interface {
GetAuthorizeCodeLifespan(ctx context.Context) time.Duration
}

type DeviceAndUserCodeLifespanProvider interface {
GetDeviceAndUserCodeLifespan(ctx context.Context) time.Duration
}

type DeviceAuthorizeProvider interface {
GetDeviceDone(ctx context.Context) string
}

type DeviceProvider interface {
GetDeviceVerificationURL(ctx context.Context) string
GetDeviceAuthTokenPollingInterval(ctx context.Context) time.Duration
}

// RefreshTokenLifespanProvider returns the provider for configuring the refresh token lifespan.
type RefreshTokenLifespanProvider interface {
// GetRefreshTokenLifespan returns the refresh token lifespan.
Expand Down Expand Up @@ -275,6 +288,18 @@ type PushedAuthorizeRequestHandlersProvider interface {
GetPushedAuthorizeEndpointHandlers(ctx context.Context) PushedAuthorizeEndpointHandlers
}

// DeviceEndpointHandlersProvider returns the provider for setting up the Device handlers.
type DeviceEndpointHandlersProvider interface {
// GetDeviceEndpointHandlers returns the handlers.
GetDeviceEndpointHandlers(ctx context.Context) DeviceEndpointHandlers
}

// DeviceAuthorizeEndpointHandlersProvider returns the provider for setting up the Device Authorize handlers.
type DeviceAuthorizeEndpointHandlersProvider interface {
// GetDeviceAuthorizeEndpointHandlers returns the handlers.
GetDeviceAuthorizeEndpointHandlers(ctx context.Context) DeviceAuthorizeEndpointHandlers
}

// UseLegacyErrorFormatProvider returns the provider for configuring whether to use the legacy error format.
//
// DEPRECATED: Do not use this flag anymore.
Expand Down
49 changes: 49 additions & 0 deletions config_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ var (
_ RevocationHandlersProvider = (*Config)(nil)
_ PushedAuthorizeRequestHandlersProvider = (*Config)(nil)
_ PushedAuthorizeRequestConfigProvider = (*Config)(nil)
_ DeviceEndpointHandlersProvider = (*Config)(nil)
)

type Config struct {
Expand All @@ -75,6 +76,18 @@ type Config struct {
// AuthorizeCodeLifespan sets how long an authorize code is going to be valid. Defaults to fifteen minutes.
AuthorizeCodeLifespan time.Duration

// Sets how long a device user/device code pair is valid for
DeviceAndUserCodeLifespan time.Duration

// DeviceAuthTokenPollingInterval sets the interval that clients should check for device code grants
DeviceAuthTokenPollingInterval time.Duration

// DeviceVerificationURL is the URL of the device verification endpoint, this is is included with the device code request responses
DeviceVerificationURL string

// DeviceDoneURL is the URL of the user is redirected to once the verification is completed
DeviceDoneURL string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this always the same URL or would it differ on a per client basis?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's configured by the 'client'
This URL is used once the user has validated his login on his computer to display that he logged-in successfully and he can (physically) go back to his device.

It will be usally implemented by the ui-side of hydra but can bu actually any website.


// IDTokenLifespan sets the default id token lifetime. Defaults to one hour.
IDTokenLifespan time.Duration

Expand Down Expand Up @@ -194,6 +207,12 @@ type Config struct {
// PushedAuthorizeEndpointHandlers is a list of handlers that are called before the PAR endpoint is served.
PushedAuthorizeEndpointHandlers PushedAuthorizeEndpointHandlers

// DeviceEndpointHandlers is a list of handlers that are called before the device endpoint is served.
DeviceEndpointHandlers DeviceEndpointHandlers

// DeviceAuthorizeEndpointHandlers is a list of handlers that are called before the device authorize endpoint is served.
DeviceAuthorizeEndpointHandlers DeviceAuthorizeEndpointHandlers

// GlobalSecret is the global secret used to sign and verify signatures.
GlobalSecret []byte

Expand Down Expand Up @@ -242,6 +261,14 @@ func (c *Config) GetTokenIntrospectionHandlers(ctx context.Context) TokenIntrosp
return c.TokenIntrospectionHandlers
}

func (c *Config) GetDeviceEndpointHandlers(ctx context.Context) DeviceEndpointHandlers {
return c.DeviceEndpointHandlers
}

func (c *Config) GetDeviceAuthorizeEndpointHandlers(ctx context.Context) DeviceAuthorizeEndpointHandlers {
return c.DeviceAuthorizeEndpointHandlers
}

func (c *Config) GetRevocationHandlers(ctx context.Context) RevocationHandlers {
return c.RevocationHandlers
}
Expand Down Expand Up @@ -360,6 +387,13 @@ func (c *Config) GetAuthorizeCodeLifespan(_ context.Context) time.Duration {
return c.AuthorizeCodeLifespan
}

func (c *Config) GetDeviceAndUserCodeLifespan(_ context.Context) time.Duration {
if c.AuthorizeCodeLifespan == 0 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be DeviceAndUserCodeLifespan?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch :)

return time.Minute * 10
}
return c.DeviceAndUserCodeLifespan
}

// GeIDTokenLifespan returns how long an id token should be valid. Defaults to one hour.
func (c *Config) GetIDTokenLifespan(_ context.Context) time.Duration {
if c.IDTokenLifespan == 0 {
Expand Down Expand Up @@ -488,3 +522,18 @@ func (c *Config) GetPushedAuthorizeContextLifespan(ctx context.Context) time.Dur
func (c *Config) EnforcePushedAuthorize(ctx context.Context) bool {
return c.IsPushedAuthorizeEnforced
}

func (c *Config) GetDeviceDone(ctx context.Context) string {
return c.DeviceDoneURL
}

func (c *Config) GetDeviceVerificationURL(ctx context.Context) string {
return c.DeviceVerificationURL
}

func (c *Config) GetDeviceAuthTokenPollingInterval(ctx context.Context) time.Duration {
if c.DeviceAuthTokenPollingInterval == 0 {
return time.Second * 10
hperl marked this conversation as resolved.
Show resolved Hide resolved
}
return c.DeviceAuthTokenPollingInterval
}
24 changes: 24 additions & 0 deletions device_authorize_request.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright © 2022 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package fosite

// DeviceAuthorizeRequest is an implementation of DeviceAuthorizeRequester
type DeviceAuthorizeRequest struct {
signature string
Request
}

func (d *DeviceAuthorizeRequest) GetDeviceCodeSignature() string {
return d.signature
}

func (d *DeviceAuthorizeRequest) SetDeviceCodeSignature(signature string) {
d.signature = signature
}

func NewDeviceAuthorizeRequest() *DeviceAuthorizeRequest {
return &DeviceAuthorizeRequest{
Request: *NewRequest(),
}
}
37 changes: 37 additions & 0 deletions device_authorize_request_handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright © 2022 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package fosite

import (
"context"
"net/http"

"github.com/ory/fosite/i18n"
"github.com/ory/x/errorsx"
)

func (f *Fosite) NewDeviceAuthorizeRequest(ctx context.Context, r *http.Request) (DeviceAuthorizeRequester, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the URL the user calls, right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, all the DeviceAuthorizeXXX are on the interactive side (aka mobile, PC, ...), all the DeviceXXX are on the non interactive side (aka TV, Fridge, VR headsets ...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a nice mnemonic! How about renaming it to DeviceUserXXX? This would make it clearer to readers to know who is calling what.

Copy link

@supercairos supercairos Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed all the files/objects from DeviceAuthorizeXXX to DeviceUserXXX

Done ✅

request := NewDeviceAuthorizeRequest()
request.Lang = i18n.GetLangFromRequest(f.Config.GetMessageCatalog(ctx), r)

if err := r.ParseForm(); err != nil {
return nil, errorsx.WithStack(ErrInvalidRequest.WithHint("Unable to parse HTTP body, make sure to send a properly formatted form request body.").WithWrap(err).WithDebug(err.Error()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hydra registers this as GET, so the reference to the request body is wrong, right?

Copy link

@supercairos supercairos Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AuthorizeRequestHandler is doing the same:
https://github.com/ory/fosite/blob/master/authorize_request_handler.go#L325

It actually parse the query string into the r.Form field:
https://pkg.go.dev/net/http#Request.ParseForm (since it's a GET it doesn't parse the body)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that for authorization code, this was because of PAR (https://datatracker.ietf.org/doc/html/rfc9126)? So that endpoint actually supports POST. Right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it only support GET. Do you want me to change to way I parse the query params?

Copy link

@supercairos supercairos Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've refactored the way I'm querying the QueryParams. I now use r.URL.Query()

}
request.Form = r.Form
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be

Suggested change
request.Form = r.Form
request.Form = r.PostForm

or do we expect URL query parameters as well?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember clearly, but I will try to look a bit deeper once I hook the new refactoring with Ory/Hydra

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I've fixed this one also, It should be good


verifier := request.GetRequestForm().Get("device_verifier")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, may I know what device_verifier is? I cannot find it in https://www.rfc-editor.org/rfc/rfc8628

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember what this code is for ? Maybe @BuzzBumbleBee knows ?

BTW, I've wrote some sample clients to test it here:
https://github.com/supercairos/oauth-device-flow-client-sample

hperl marked this conversation as resolved.
Show resolved Hide resolved
if verifier != "" {
client, err := f.Store.GetClient(ctx, request.GetRequestForm().Get("client_id"))
hperl marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errorsx.WithStack(ErrInvalidClient.WithHint("The requested OAuth 2.0 Client does not exist.").WithWrap(err).WithDebug(err.Error()))
}
request.Client = client

if !client.GetGrantTypes().Has("urn:ietf:params:oauth:grant-type:device_code") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make this string a const

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still a string though?

Copy link

@supercairos supercairos Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a const string in a global define now that's used everywhere :)

if !client.GetGrantTypes().Has(string(GrantTypeDeviceCode)) {

return nil, errorsx.WithStack(ErrInvalidGrant.WithHint("The requested OAuth 2.0 Client does not have the 'urn:ietf:params:oauth:grant-type:device_code' grant."))
}
}

return request, nil
}
Loading