Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
386 changes: 386 additions & 0 deletions internal/verifier/apiv1/direct_post_decline_test.go

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions internal/verifier/apiv1/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,22 @@ var (
HTTPStatus: http.StatusBadRequest,
}

// ErrSessionAlreadyAnswered is a presentation for a session that
// already reached an outcome - most plainly, one the user declined
// a moment earlier, with the wallet's POST still in flight.
//
// Separate from ErrSessionExpired because it is a different fact
// and the wallet should be able to tell them apart: a deadline
// passed, versus an answer this session already has. Accepting it
// would write a response code onto a session whose Error and
// terminal status stay put, so /ui/completion would report
// "declined" and /ui/resume "complete" for the same session.
ErrSessionAlreadyAnswered = &OAuthError{
ErrorCode: ErrCodeInvalidGrant,
ErrorDescription: "The session has already been answered",
HTTPStatus: http.StatusBadRequest,
}

ErrInvalidVP = &OAuthError{
ErrorCode: ErrCodeInvalidRequest,
ErrorDescription: "Invalid verifiable presentation",
Expand Down
71 changes: 66 additions & 5 deletions internal/verifier/apiv1/handlers_ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@
}

// UIInteraction handles front-end interactions, replying with an Authorization Request that contains a Request URI and DCQL query, the latter for UI to show.
func (c *Client) UIInteraction(ctx context.Context, req *UIInteractionRequest) (*UIInteractionReply, error) {

Check failure on line 462 in internal/verifier/apiv1/handlers_ui.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=SUNET_vc&issues=AaFmQCvi3mVOTdW8O_T2&open=AaFmQCvi3mVOTdW8O_T2&pullRequest=847
c.log.Debug("uiInteraction", "dcql_query", req.DCQLQuery)

// Augment BEFORE the reuse check: the stored request object was
Expand Down Expand Up @@ -807,6 +807,13 @@
if authCtx.VerifierResponseCode != "" {
return false
}
// A refused session has been answered too. Reusing one would hand a
// fresh interaction the state and nonce the user already declined,
// and the decline recorded on it would then describe a request
// nobody made.
if authSessionIsTerminal(authCtx) {
return false
}
if authCtx.ExpiresAt != 0 && authCtx.ExpiresAt <= time.Now().Unix() {
return false
}
Expand Down Expand Up @@ -878,20 +885,55 @@
// client can retry instead of latching onto a bogus "pending" and
// never reconciling the lost completion.
func (c *Client) CompletedResponseCode(ctx context.Context, sessionID string) (string, error) {
outcome, err := c.SessionOutcome(ctx, sessionID)
if err != nil {
return "", err
}
return outcome.ResponseCode, nil
}

// SessionOutcome is what a polling UI needs to stop polling: the
// response code of a completed session, or the error of a declined one.
//
// The two used to be one value, and only one of them existed. A wallet
// that answered with an Authorization Error Response - a user pressing
// "no", which OpenID4VP 1.0 section 8.5 calls access_denied - left the
// session with no response code and no other way to say so, and
// /ui/completion reports a missing response code as "pending". The UI
// then polled until the cache evicted the context, and the relying
// party never learned of a decision the user had already made.
type SessionOutcome struct {
// ResponseCode is set when the wallet presented.
ResponseCode string
// Error and ErrorDescription are set when it refused. The
// description is the wallet's own text and is shown to an operator,
// never acted on.
Error string
ErrorDescription string
}

// Declined reports whether the wallet answered with an error.
func (o SessionOutcome) Declined() bool { return o.Error != "" }

func (c *Client) SessionOutcome(ctx context.Context, sessionID string) (SessionOutcome, error) {
if sessionID == "" {
return "", nil
return SessionOutcome{}, nil
}
authCtx, err := c.cacheService.AuthContext.GetByID(ctx, sessionID)
if err != nil {
if errors.Is(err, cache.ErrNoDocuments) {
return "", nil
return SessionOutcome{}, nil
}
return "", err
return SessionOutcome{}, err
}
if authCtx == nil {
return "", nil
return SessionOutcome{}, nil
}
return authCtx.VerifierResponseCode, nil
return SessionOutcome{
ResponseCode: authCtx.VerifierResponseCode,
Error: authCtx.Error,
ErrorDescription: authCtx.ErrorDescription,
}, nil
Comment thread
leifj marked this conversation as resolved.
}

// UIResumeStatus names the possible outcomes of a /ui/resume call.
Expand All @@ -910,6 +952,9 @@
// UIResumeExpired means the auth context exists but its request object
// has been evicted, so the QR / request_uri is no longer resolvable.
UIResumeExpired UIResumeStatus = "expired"
// UIResumeDeclined means the wallet refused: the user said no, and
// there is nothing left to wait for.
UIResumeDeclined UIResumeStatus = "declined"
)

// UIResumeReply carries one of three alternatives selected by Status:
Expand All @@ -930,6 +975,10 @@

// Complete-only field.
ResponseCode string `json:"response_code,omitempty"`

// Declined-only fields.
Error string `json:"error,omitempty"`
ErrorDescription string `json:"error_description,omitempty"`
}

// UIResume reports whether the wallet interaction named by sessionID is
Expand All @@ -938,7 +987,7 @@
// A fresh DC API request_uri is minted on each pending call for the same
// reason tryReuseInFlightSession does: a reload cannot resume a native
// navigator.credentials.get call, so the existing one is left to time out.
func (c *Client) UIResume(ctx context.Context, sessionID string) (*UIResumeReply, error) {

Check failure on line 990 in internal/verifier/apiv1/handlers_ui.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=SUNET_vc&issues=AaFtwgIr7LMIy_hEeAUX&open=AaFtwgIr7LMIy_hEeAUX&pullRequest=847
if sessionID == "" {
return &UIResumeReply{Status: UIResumeUnknown}, nil
}
Expand All @@ -958,6 +1007,18 @@
ResponseCode: authCtx.VerifierResponseCode,
}, nil
}
// A refusal is an outcome, so a reload must not rebuild the QR for
// it. Without this a reloaded tab resumed "pending" with the
// original QR for a request the user had already declined, and the
// two endpoints disagreed: /ui/completion said declined while
// /ui/resume offered the code again.
if authCtx.Error != "" {
return &UIResumeReply{
Status: UIResumeDeclined,
Error: authCtx.Error,
ErrorDescription: authCtx.ErrorDescription,
Comment on lines +1015 to +1019
}, nil
}
// Past the presentation deadline there is nothing to resume. Checked
// after the completed case, so a finished session still hands back its
// response code, and before the request object, which outlives the
Expand Down
168 changes: 168 additions & 0 deletions internal/verifier/apiv1/handlers_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
"slices"
"strings"

"github.com/SUNET/vc/pkg/cache"
"github.com/SUNET/vc/pkg/helpers"
"github.com/SUNET/vc/pkg/jose"
"github.com/SUNET/vc/pkg/mdoc"
"github.com/SUNET/vc/pkg/model"
Expand Down Expand Up @@ -75,6 +77,29 @@ func (c *Client) VerificationRequestObject(ctx context.Context, req *Verificatio

type VerificationDirectPostRequest struct {
Response string `json:"response" form:"response"`

// Error, ErrorDescription and State carry an Authorization Error
// Response - the Wallet answering the request with a refusal
// instead of a presentation.
//
// OpenID4VP 1.0 section 8.3.1: "If a Wallet is unable to generate an
// encrypted response, it MAY send an error response without
// encryption as per Section 8.2." So a PLAIN error form is
// conformant even when the request asked for direct_post.jwt, and
// section 8.2's own example is exactly these three parameters
// posted as a form.
//
// Section 5.6 says both successful and error responses SHOULD use
// the supplied Response Mode, so an error may also arrive encrypted
// inside Response. Both are accepted; see directPostError.
//
// Until these existed a decline bound to an empty struct and the
// handler went looking for a JWE header in "", which failed as
// "unexpected end of JSON input" - a 500 for the one outcome that
// is not an error at all, with the session left pending forever.
Error string `json:"error,omitempty" form:"error"`
ErrorDescription string `json:"error_description,omitempty" form:"error_description"`
State string `json:"state,omitempty" form:"state"`
// DCAPI marks a response returned inside a navigator.credentials.get
// call and forwarded here by the verifier's own page, rather than POSTed
// by a wallet reached through request_uri. The two bind their mdoc
Expand All @@ -100,9 +125,122 @@ type VerificationDirectPostResponse struct {
RedirectURI string `json:"redirect_uri,omitempty"`
}

// directPostError records an Authorization Error Response and ends the
// session.
//
// The point is the ENDING. A declined request is an outcome the relying
// party is entitled to, and before this the only thing that happened was
// a log line: the session stayed pending until the cache evicted it, so
// a user who pressed "no" left the verifier waiting for an answer they
// had already given.
//
// state is validated against a live session (OpenID4VP 1.0 section
// 14.3.2: the Verifier SHOULD check that state corresponds to a recent
// Authorization Request). An unknown state is refused rather than
// ignored - accepting one would let anyone who can reach response_uri
// end a session they know nothing about, and silently ignoring one
// would leave the real session pending while answering 200.
//
// Nothing of the wallet's text is echoed back to the caller: the reply
// carries only the redirect, and the error reaches the operator through
// the session, not through this response body.
func (c *Client) directPostError(ctx context.Context, code, description, state string) (*VerificationDirectPostResponse, error) {
if state == "" {
return nil, helpers.NewErrorDetailsWithStatus("invalid_request",
"an error response must carry the state of the request it answers", http.StatusBadRequest)
}

authCtx, err := c.cacheService.AuthContext.Get(ctx, &cache.AuthorizationContext{State: state})
switch {
case errors.Is(err, cache.ErrNoDocuments) || (err == nil && authCtx == nil):
// Deliberately the same answer as a state that never existed:
// whether a given state is live is not something this endpoint
// should confirm to an unauthenticated caller.
c.log.Warn("direct_post error response for an unknown state", "error", code)
return nil, helpers.NewErrorDetailsWithStatus("invalid_request",
"state does not correspond to a request this verifier is waiting for", http.StatusBadRequest)
case err != nil:
// A backend that cannot answer is NOT an unknown state. Saying
// 400 here would tell a wallet its own request was malformed
// during a Mongo outage, and the real session would stay
// pending with nobody retrying. 503 is the honest answer and
// names no backend detail.
c.log.Error(err, "could not look up the session for a direct_post error response", "error", code)
return nil, helpers.NewErrorDetailsWithStatus("temporarily_unavailable",
"the session could not be looked up; retry", http.StatusServiceUnavailable)
}

if sessionExpired(authCtx) {
c.log.Info("direct_post error response for an expired session", "state", state, "error", code)
return nil, ErrSessionExpired
}

// First outcome wins. Without this a late error overwrites a
// completed session while its VerifierResponseCode survives, so
// /ui/completion reports "declined" and /ui/resume reports
// "complete" for the same session - two answers, both from stored
// state, with no way to tell which happened.
if authSessionIsTerminal(authCtx) {
c.log.Info("direct_post error response for a session that already reached an outcome",
"state", state, "error", code, "status", authCtx.Status)
return &VerificationDirectPostResponse{}, nil
}

c.log.Warn("the wallet returned an error response instead of a presentation",
"state", state, "error", code, "session_id", authCtx.SessionID)

// Mutate a COPY. MemoryStore.Get hands back the stored pointer
// itself, and Save validates after these assignments - so a wallet
// sending an error_description past the model's 512-byte bound
// would fail the save with the live session already carrying the
// rejected value and a terminal status, while a Mongo-backed
// deployment kept the old one. The description is wallet-controlled
// input, which is exactly the kind that finds that edge.
updated := *authCtx
updated.Status = cache.SessionStatusError
updated.Error = code
updated.ErrorDescription = description
if err := c.cacheService.AuthContext.Save(ctx, &updated); err != nil {
Comment on lines +199 to +203
c.log.Error(err, "failed to record the wallet's error response", "state", state)
return nil, err
}
authCtx = &updated

// The same nudge a presentation sends, so a waiting UI learns now
// rather than on its next poll. The browser is told the outcome, not
// the wallet's prose.
c.notify.Submit(authCtx.SessionID, map[string]string{"error": code})

// OpenID4VP 1.0 section 8.2 allows the Response URI to return
// redirect_uri for an error response, and the engine reads it back
// (go-wallet-backend internal/engine/oid4vp.go submitErrorResponse),
// so a same-device decline can still land the user somewhere.
// Omitted here: where a declined flow should send the browser is a
// verifier UX decision, and inventing one would send users to a page
// nobody designed. 200 with an empty object is what the engine
// already tolerates.
return &VerificationDirectPostResponse{}, nil
}

func (c *Client) VerificationDirectPost(ctx context.Context, req *VerificationDirectPostRequest) (*VerificationDirectPostResponse, error) {
c.log.Debug("Verification direct-post")

// A refusal, before anything looks for a JWE. An error response
// carries no `response`, so every step below it - the KID, the
// ephemeral key, the decryption - is looking for something that was
// never sent.
if req.Error != "" && req.Response == "" {
return c.directPostError(ctx, req.Error, req.ErrorDescription, req.State)
}

// Neither a presentation nor an error is a malformed request, and
// it is the caller's to fix. It used to reach GetKID and come back
// as a 500.
if req.Response == "" {
return nil, helpers.NewErrorDetailsWithStatus("invalid_request",
"the response carries neither a presentation nor an error", http.StatusBadRequest)
}

// Extract KID from JWE header
kid, err := req.GetKID()
if err != nil {
Expand All @@ -126,6 +264,23 @@ func (c *Client) VerificationDirectPost(ctx context.Context, req *VerificationDi
return nil, err
}

// An error response returned the way section 5.6 says it SHOULD be -
// "Both successful and error responses SHOULD be returned using the
// supplied Response Mode" - so for direct_post.jwt it arrives
// encrypted, inside the same envelope a presentation would use.
//
// Checked before the presentation parse rather than after it fails,
// because an error response legitimately has no vp_token and would
// otherwise be reported as a malformed presentation.
var encryptedError struct {
Error string `json:"error"`
ErrorDescription string `json:"error_description"`
State string `json:"state"`
}
if err := json.Unmarshal(decryptedJWE, &encryptedError); err == nil && encryptedError.Error != "" {
return c.directPostError(ctx, encryptedError.Error, encryptedError.ErrorDescription, encryptedError.State)
Comment on lines +280 to +281
}

// Parse response parameters using openid4vp
vpResponse := openid4vp.VPResponse{}
if err := json.Unmarshal(decryptedJWE, &vpResponse); err != nil {
Expand All @@ -151,6 +306,19 @@ func (c *Client) VerificationDirectPost(ctx context.Context, req *VerificationDi
return nil, ErrSessionExpired
}

// The other direction of "first outcome wins". A presentation that
// lands after the session was answered - the user declined and the
// wallet sent anyway, or a response is replayed - would otherwise
// write a VerifierResponseCode while Error and the terminal status
// stay put, and the two UI endpoints would then disagree about what
// happened. Refused rather than ignored: the wallet is handing over
// credentials, and a 200 would say they were accepted.
if authSessionIsTerminal(authCtx) {
c.log.Info("Verification direct post for a session that already reached an outcome",
"state", vpResponse.State, "status", authCtx.Status)
return nil, ErrSessionAlreadyAnswered
}

// Generate response code
responseCode := uuid.NewString()
callbackURL, err := url.JoinPath(c.cfg.Verifier.PublicURL, "/verification/callback")
Expand Down
1 change: 1 addition & 0 deletions internal/verifier/httpserver/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type Apiv1 interface {
UIMetadata(ctx context.Context) (*apiv1.UIMetadataReply, error)
IsActiveAuthSession(ctx context.Context, sessionID string) (active bool, expiresAt int64, err error)
CompletedResponseCode(ctx context.Context, sessionID string) (string, error)
SessionOutcome(ctx context.Context, sessionID string) (apiv1.SessionOutcome, error)
UIResume(ctx context.Context, sessionID string) (*apiv1.UIResumeReply, error)

// OIDC Provider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ func (unimplementedApiv1) UIMetadata(ctx context.Context) (*apiv1.UIMetadataRepl
func (unimplementedApiv1) IsActiveAuthSession(ctx context.Context, sessionID string) (bool, int64, error) {
panic("IsActiveAuthSession not implemented in test")
}
func (unimplementedApiv1) SessionOutcome(ctx context.Context, sessionID string) (apiv1.SessionOutcome, error) {
panic("SessionOutcome not implemented in test")
}

func (unimplementedApiv1) CompletedResponseCode(ctx context.Context, sessionID string) (string, error) {
panic("CompletedResponseCode not implemented in test")
}
Expand Down
Loading
Loading