Skip to content

Commit

Permalink
chore: code review
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Jan 16, 2025
1 parent 14cf7cd commit 0a4fa13
Show file tree
Hide file tree
Showing 8 changed files with 368 additions and 18 deletions.
6 changes: 3 additions & 3 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -645,15 +645,15 @@
},
"userinfo_url": {
"type": "string",
"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.",
"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.",
"format": "uri-reference",
"examples": [
"https://example.org/my-custom-userinfo-endpoint"
]
},
"device_authorization_url": {
"type": "string",
"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 authorizatoin endpoint at /oauth2/device/auth. Set this value if you want to handle this endpoint yourself.",
"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.",
"format": "uri-reference",
"examples": [
"https://example.org/oauth2/device/auth"
Expand Down Expand Up @@ -825,7 +825,7 @@
"/ui/device_verification"
]
},
"post_device_done": {
"device_verification_success": {
"type": "string",
"description": "Sets the post device authentication endpoint. Defaults to an internal fallback URL showing an error.",
"format": "uri-reference",
Expand Down
21 changes: 12 additions & 9 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request,
d := json.NewDecoder(r.Body)
d.DisallowUnknownFields()
if err := d.Decode(&reqBody); err != nil {
h.r.Writer().WriteErrorCode(w, r, http.StatusBadRequest, errorsx.WithStack(err))
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrInvalidRequest.WithWrap(err).WithHintf("Unable to decode request body: %s", err.Error())))
return
}

Expand All @@ -1106,7 +1106,7 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request,

cr, err := h.r.ConsentManager().GetDeviceUserAuthRequest(ctx, challenge)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
h.r.Writer().WriteError(w, r, err)
return
}

Expand All @@ -1118,17 +1118,18 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request,

userCodeSignature, err := h.r.RFC8628HMACStrategy().UserCodeSignature(r.Context(), reqBody.UserCode)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithHint(`'user_code' signature could not be computed`)))
h.r.Writer().WriteError(w, r, fosite.ErrServerError.WithWrap(err).WithHint(`The 'user_code' signature could not be computed.`))
return
}

userCodeRequest, err := h.r.OAuth2Storage().GetUserCodeSession(r.Context(), userCodeSignature, nil)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrNotFound.WithWrap(err).WithHint(`'user_code' session not found`)))
h.r.Writer().WriteError(w, r, fosite.ErrInvalidRequest.WithWrap(err).WithHint(`The 'user_code' session could not be found or has expired or is otherwise malformed.`))
return
}
err = h.r.RFC8628HMACStrategy().ValidateUserCode(ctx, userCodeRequest, reqBody.UserCode)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(fosite.ErrTokenExpired.WithWrap(err).WithHint(`'user_code' has expired`)))

if err := h.r.RFC8628HMACStrategy().ValidateUserCode(ctx, userCodeRequest, reqBody.UserCode); err != nil {
h.r.Writer().WriteError(w, r, fosite.ErrInvalidRequest.WithWrap(err).WithHint(`The 'user_code' session could not be found or has expired or is otherwise malformed.`))
return
}

Expand All @@ -1148,22 +1149,24 @@ func (h *Handler) acceptUserCodeRequest(w http.ResponseWriter, r *http.Request,
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
return
}

if reqURL.Query().Get("client_id") == "" {
q := reqURL.Query()
q.Add("client_id", userCodeRequest.GetClient().GetID())
reqURL.RawQuery = q.Encode()
}

f.RequestURL = reqURL.String()

hr, err := h.r.ConsentManager().HandleDeviceUserAuthRequest(ctx, f, challenge, &p)
if err != nil {
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
h.r.Writer().WriteError(w, r, err)
return
}

ru, err := url.Parse(hr.RequestURL)
if err != nil {
h.r.Writer().WriteError(w, r, err)
h.r.Writer().WriteError(w, r, fosite.ErrInvalidRequest.WithWrap(err).WithHint(`Unable to parse the request_url.`))
return
}

Expand Down
4 changes: 2 additions & 2 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -1210,11 +1210,11 @@ func (s *DefaultStrategy) HandleOAuth2DeviceAuthorizationRequest(
// Validate client_id
clientID := r.URL.Query().Get("client_id")
if clientID == "" {
return nil, nil, errorsx.WithStack(fosite.ErrInvalidClient.WithHintf(`client_id query parameter is missing`))
return nil, nil, errorsx.WithStack(fosite.ErrInvalidClient.WithHintf(`Query parameter 'client_id' is missing.`))
}
c, err := s.r.ClientManager().GetConcreteClient(r.Context(), clientID)
if errors.Is(err, x.ErrNotFound) {
return nil, nil, errorsx.WithStack(fosite.ErrInvalidClient.WithHintf(`Unknown client_id %s`, clientID))
return nil, nil, errorsx.WithStack(fosite.ErrInvalidClient.WithWrap(err).WithHintf(`Client does not exist`))
} else if err != nil {
return nil, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion contrib/quickstart/5-min/hydra.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ urls:
login: http://127.0.0.1:3000/login
logout: http://127.0.0.1:3000/logout
device_verification: http://127.0.0.1:3000/device_code
post_device_done: http://127.0.0.1:3000/device_complete
device_verification_success: http://127.0.0.1:3000/device_complete

secrets:
system:
Expand Down
2 changes: 1 addition & 1 deletion driver/config/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const (
KeyConsentURL = "urls.consent"
KeyErrorURL = "urls.error"
KeyDeviceVerificationURL = "urls.device_verification"
KeyDeviceDoneURL = "urls.post_device_done"
KeyDeviceDoneURL = "urls.device_verification_success"
KeyPublicURL = "urls.self.public"
KeyAdminURL = "urls.self.admin"
KeyIssuerURL = "urls.self.issuer"
Expand Down
2 changes: 1 addition & 1 deletion internal/.hydra.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ urls:
logout: https://logout
error: https://error
device_verification: https://device
post_device_done: https://device/callback
device_verification_success: https://device/callback
post_logout_redirect: https://post_logout

strategies:
Expand Down
Loading

0 comments on commit 0a4fa13

Please sign in to comment.