Skip to content

Commit 3db8fe7

Browse files
aeneasraeneasr
committed
chore: code review
1 parent 1493c05 commit 3db8fe7

20 files changed

+325
-653
lines changed

‎.schema/config.schema.json‎

Lines changed: 59 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,14 @@
619619
"https://my-service.com/oauth2/auth"
620620
]
621621
},
622+
"device_authorization_url": {
623+
"type": "string",
624+
"description": "Overwrites the OAuth2 Device Auth URL",
625+
"format": "uri-reference",
626+
"examples": [
627+
"https://my-service.com/oauth2/device/auth"
628+
]
629+
},
622630
"client_registration_url": {
623631
"description": "Sets the OpenID Connect Dynamic Client Registration Endpoint",
624632
"type": "string",
@@ -645,19 +653,11 @@
645653
},
646654
"userinfo_url": {
647655
"type": "string",
648-
"description": "A URL of the userinfo endpoint to be advertised at the OpenID Connect Discovery endpoint `/.well-known/openid-configuration`. Defaults to Ory Hydra's userinfo endpoint at `/userinfo`. Set this value if you want to handle this endpoint yourself.",
656+
"description": "A URL of the userinfo endpoint to be advertised at the OpenID Connect Discovery endpoint /.well-known/openid-configuration. Defaults to Ory Hydra's userinfo endpoint at /userinfo. Set this value if you want to handle this endpoint yourself.",
649657
"format": "uri-reference",
650658
"examples": [
651659
"https://example.org/my-custom-userinfo-endpoint"
652660
]
653-
},
654-
"device_authorization_url": {
655-
"type": "string",
656-
"description": "A URL of the device authorization endpoint to be advertised at the OpenID Connect Discovery endpoint `/.well-known/openid-configuration`. Defaults to Ory Hydra's device authorization endpoint at `/oauth2/device/auth`. Set this value if you want to handle this endpoint yourself.",
657-
"format": "uri-reference",
658-
"examples": [
659-
"https://example.org/oauth2/device/auth"
660-
]
661661
}
662662
}
663663
}
@@ -816,23 +816,29 @@
816816
"/ui/logout"
817817
]
818818
},
819-
"device_verification": {
820-
"type": "string",
821-
"description": "Sets the device user code verification endpoint. Defaults to an internal fallback URL showing an error.",
822-
"format": "uri-reference",
823-
"examples": [
824-
"https://my-logout.app/device_verification",
825-
"/ui/device_verification"
826-
]
827-
},
828-
"device_verification_success": {
829-
"type": "string",
830-
"description": "Sets the post device authentication endpoint. Defaults to an internal fallback URL showing an error.",
831-
"format": "uri-reference",
832-
"examples": [
833-
"https://my-logout.app/device_done",
834-
"/ui/device_done"
835-
]
819+
"device": {
820+
"type": "object",
821+
"description": "Configure URLs for the OAuth 2.0 Device Code Flow.",
822+
"properties": {
823+
"verification": {
824+
"type": "string",
825+
"description": "Sets the device user code verification endpoint. Defaults to an internal fallback URL showing an error.",
826+
"format": "uri-reference",
827+
"examples": [
828+
"https://my-logout.app/device_verification",
829+
"/ui/device_verification"
830+
]
831+
},
832+
"success": {
833+
"type": "string",
834+
"description": "Sets the post device authentication endpoint. Defaults to an internal fallback URL showing an error.",
835+
"format": "uri-reference",
836+
"examples": [
837+
"https://my-logout.app/device_done",
838+
"/ui/device_done"
839+
]
840+
}
841+
}
836842
},
837843
"error": {
838844
"type": "string",
@@ -980,8 +986,8 @@
980986
]
981987
},
982988
"device_user_code": {
983-
"description": "Configures how long device and user codes are valid.",
984-
"default": "15m",
989+
"description": "Configures how long device & user codes are valid.",
990+
"default": "10m",
985991
"allOf": [
986992
{
987993
"$ref": "#/definitions/duration"
@@ -1104,23 +1110,6 @@
11041110
}
11051111
}
11061112
},
1107-
"device_authorization": {
1108-
"token_polling_interval": {
1109-
"description": "Sets the starting token polling interval.",
1110-
"default": "5s",
1111-
"allOf": [
1112-
{
1113-
"$ref": "#/definitions/duration"
1114-
}
1115-
]
1116-
},
1117-
"user_code_entropy": {
1118-
"type": "string",
1119-
"description": "Sets the entropy for the user codes.",
1120-
"default": "medium",
1121-
"enum": ["high", "medium", "low"]
1122-
}
1123-
},
11241113
"grant": {
11251114
"type": "object",
11261115
"additionalProperties": false,
@@ -1181,6 +1170,28 @@
11811170
}
11821171
]
11831172
},
1173+
"device_authorization": {
1174+
"type": "object",
1175+
"additionalProperties": false,
1176+
"properties": {
1177+
"token_polling_interval": {
1178+
"allOf": [
1179+
{
1180+
"$ref": "#/definitions/duration"
1181+
}
1182+
],
1183+
"default": "5s",
1184+
"description": "configure how often a non-interactive device should poll the device token endpoint",
1185+
"examples": ["5s", "15s", "1m"]
1186+
},
1187+
"user_code_entropy": {
1188+
"type": "string",
1189+
"description": "Sets the entropy for the user codes.",
1190+
"default": "medium",
1191+
"enum": ["high", "medium", "low"]
1192+
}
1193+
}
1194+
},
11841195
"token_hook": {
11851196
"description": "Sets the token hook endpoint for all grant types. If set it will be called while providing token to customize claims.",
11861197
"examples": ["https://my-example.app/token-hook"],
@@ -1194,8 +1205,8 @@
11941205
}
11951206
]
11961207
}
1197-
}
1198-
},
1208+
}
1209+
},
11991210
"secrets": {
12001211
"type": "object",
12011212
"additionalProperties": false,
@@ -1240,7 +1251,7 @@
12401251
"examples": ["cpu"]
12411252
},
12421253
"tracing": {
1243-
"$ref": "https://raw.githubusercontent.com/ory/x/v0.0.675/otelx/config.schema.json"
1254+
"$ref": "ory://tracing-config"
12441255
},
12451256
"sqa": {
12461257
"type": "object",

‎cmd/cmd_perform_device_flow.go‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ import (
2222
func NewPerformDeviceCodeCmd() *cobra.Command {
2323
cmd := &cobra.Command{
2424
Use: "device-code",
25-
Example: "{{ .CommandPath }} --client-id ... --client-secret ...",
25+
Example: "{{ .CommandPath }} --client-id ...",
2626
Short: "An exemplary OAuth 2.0 Client performing the OAuth 2.0 Device Code Flow",
2727
Long: `Performs the device code flow. Useful for getting an access token and an ID token in machines without a browser.
28-
The client that will be used MUST support the "client_secret_post" token-endpoint-auth-method.
29-
`,
28+
The client that will be used MUST use the "none" or "client_secret_post" token-endpoint-auth-method.`,
3029
RunE: func(cmd *cobra.Command, args []string) error {
3130
client, endpoint, err := cliclient.NewClient(cmd)
3231
if err != nil {
@@ -43,8 +42,8 @@ The client that will be used MUST support the "client_secret_post" token-endpoin
4342

4443
clientID := flagx.MustGetString(cmd, "client-id")
4544
if clientID == "" {
46-
_, _ = fmt.Fprint(cmd.OutOrStdout(), cmd.UsageString())
47-
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Please provide a Client ID using --client-id flag, or OAUTH2_CLIENT_ID environment variable.")
45+
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cmd.UsageString())
46+
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Please provide a Client ID using --client-id flag, or OAUTH2_CLIENT_ID environment variable.")
4847
return cmdx.FailSilently(cmd)
4948
}
5049

@@ -79,24 +78,24 @@ The client that will be used MUST support the "client_secret_post" token-endpoin
7978
)
8079
if err != nil {
8180
_, _ = fmt.Fprintf(
82-
cmd.ErrOrStderr(), "Failed to perform the device authorization request: %s", err)
81+
cmd.ErrOrStderr(), "Failed to perform the device authorization request: %s\n", err)
8382
return cmdx.FailSilently(cmd)
8483
}
8584

8685
_, _ = fmt.Fprintln(
87-
cmd.OutOrStdout(),
86+
cmd.ErrOrStderr(),
8887
"To login please go to:\n\t",
8988
deviceAuthResponse.VerificationURIComplete,
9089
)
9190

9291
token, err := conf.DeviceAccessToken(ctx, deviceAuthResponse)
9392
if err != nil {
9493
_, _ = fmt.Fprintf(
95-
cmd.ErrOrStderr(), "Failed to perform the device token request: %s", err)
94+
cmd.ErrOrStderr(), "Failed to perform the device token request: %s\n", err)
9695
return cmdx.FailSilently(cmd)
9796
}
9897

99-
fmt.Println("Successfully signed in!")
98+
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Successfully signed in!")
10099

101100
cmdx.PrintRow(cmd, outputOAuth2Token(*token))
102101
return nil

‎consent/handler.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,6 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request,
11571157
}
11581158

11591159
f.RequestURL = reqURL.String()
1160-
11611160
hr, err := h.r.ConsentManager().HandleDeviceUserAuthRequest(ctx, f, challenge, &p)
11621161
if err != nil {
11631162
h.r.Writer().WriteError(w, r, err)

‎consent/strategy_default.go‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,20 +1191,33 @@ func (s *DefaultStrategy) HandleOAuth2DeviceAuthorizationRequest(
11911191
ctx, span := trace.SpanFromContext(ctx).TracerProvider().Tracer("").Start(ctx, "DefaultStrategy.HandleOAuth2DeviceAuthorizationRequest")
11921192
defer otelx.End(span, &err)
11931193

1194+
// This handler has the following validation states:
1195+
//
1196+
// 1. The flow is initiated (no verifiers) -> we request a device verifier (can only be achieved by solving the device challenge)
1197+
// 2. Device verifier is given -> we request login verifier (can only be achieved by solving the login challenge)
1198+
// 3. Login verifier is given -> we request consent verifier (can only be achieved by solving the consent challenge)
1199+
// 4. Consent verifier is given -> done.
1200+
11941201
deviceVerifier := strings.TrimSpace(r.URL.Query().Get("device_verifier"))
11951202
loginVerifier := strings.TrimSpace(r.URL.Query().Get("login_verifier"))
11961203
consentVerifier := strings.TrimSpace(r.URL.Query().Get("consent_verifier"))
11971204

1205+
ar := fosite.NewAuthorizeRequest()
1206+
11981207
var deviceFlow *flow.Flow
11991208
if deviceVerifier == "" && loginVerifier == "" && consentVerifier == "" {
1200-
// ok, we need to process this request and redirect to device auth endpoint
1209+
// No verifiers are set, let's start by requesting the device verifier first.
12011210
return nil, nil, s.requestDevice(ctx, w, r)
12021211
} else if deviceVerifier != "" && loginVerifier == "" && consentVerifier == "" {
1212+
// Device verifier is set, but login and consent are not. So we need to verify the device.
12031213
var err error
12041214
deviceFlow, err = s.verifyDevice(ctx, w, r, deviceVerifier)
12051215
if err != nil {
12061216
return nil, nil, err
12071217
}
1218+
1219+
ar.RequestedScope = fosite.Arguments(deviceFlow.RequestedScope)
1220+
ar.RequestedAudience = fosite.Arguments(deviceFlow.RequestedAudience)
12081221
}
12091222

12101223
// Validate client_id
@@ -1220,18 +1233,15 @@ func (s *DefaultStrategy) HandleOAuth2DeviceAuthorizationRequest(
12201233
}
12211234

12221235
// Fake an authorization request to instantiate the flow.
1223-
ar := fosite.NewAuthorizeRequest()
12241236
ar.Client = c
12251237
ar.Form = r.Form
1226-
if deviceFlow != nil {
1227-
ar.RequestedScope = fosite.Arguments(deviceFlow.RequestedScope)
1228-
ar.RequestedAudience = fosite.Arguments(deviceFlow.RequestedAudience)
1229-
}
12301238

12311239
if loginVerifier == "" && consentVerifier == "" {
1232-
// ok, we need to process this request and redirect to the authentication endpoint
1240+
// Here we end up if the device has been verified, but login and verification are still missing.
1241+
// Let's request authentication.
12331242
return nil, nil, s.requestAuthentication(ctx, w, r, ar, deviceFlow)
12341243
} else if loginVerifier != "" {
1244+
// Login verification was given, let's verify!
12351245
f, err := s.verifyAuthentication(ctx, w, r, ar, loginVerifier)
12361246
if err != nil {
12371247
return nil, nil, err
@@ -1241,6 +1251,7 @@ func (s *DefaultStrategy) HandleOAuth2DeviceAuthorizationRequest(
12411251
return nil, f, s.requestConsent(ctx, w, r, ar, f)
12421252
}
12431253

1254+
// Here we end up when consent verifier is set, so we verify the consent.
12441255
var consentSession *flow.AcceptOAuth2ConsentRequest
12451256
var f *flow.Flow
12461257

‎contrib/quickstart/5-min/hydra.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ urls:
88
consent: http://127.0.0.1:3000/consent
99
login: http://127.0.0.1:3000/login
1010
logout: http://127.0.0.1:3000/logout
11-
device_verification: http://127.0.0.1:3000/device/code
12-
device_verification_success: http://127.0.0.1:3000/device/complete
11+
device:
12+
verification: http://127.0.0.1:3000/device/verify
13+
success: http://127.0.0.1:3000/device/success
1314

1415
secrets:
1516
system:

‎driver/config/provider.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ const (
8686
KeyLogoutURL = "urls.logout"
8787
KeyConsentURL = "urls.consent"
8888
KeyErrorURL = "urls.error"
89-
KeyDeviceVerificationURL = "urls.device_verification"
90-
KeyDeviceDoneURL = "urls.device_verification_success"
89+
KeyDeviceVerificationURL = "urls.device.verification"
90+
KeyDeviceDoneURL = "urls.device.success"
9191
KeyPublicURL = "urls.self.public"
9292
KeyAdminURL = "urls.self.admin"
9393
KeyIssuerURL = "urls.self.issuer"

‎internal/.hydra.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@ urls:
101101
consent: https://consent
102102
logout: https://logout
103103
error: https://error
104-
device_verification: https://device
105-
device_verification_success: https://device/callback
104+
device:
105+
verification: https://device
106+
success: https://device/callback
106107
post_logout_redirect: https://post_logout
107108

108109
strategies:

0 commit comments

Comments
 (0)