u

ory · aeneasr · Mar 14, 2020 · Mar 13, 2020 · Mar 13, 2020 · Mar 13, 2020
commit cfc5db373da1c54ddf42eb7954e85290bee6d353
@@ -408,6 +408,8 @@ func (h *Handler) RejectLoginRequest(w http.ResponseWriter, r *http.Request, ps
 		return
 	}
 
+	p.valid = true
+	p.SetDefaults(loginRequestDeniedErrorName)
 	ar, err := h.r.ConsentManager().GetLoginRequest(r.Context(), challenge)
 	if err != nil {
 		h.r.Writer().WriteError(w, r, err)
@@ -625,6 +627,8 @@ func (h *Handler) RejectConsentRequest(w http.ResponseWriter, r *http.Request, p
 		return
 	}
 
+	p.valid = true
+	p.SetDefaults(consentRequestDeniedErrorName)
 	hr, err := h.r.ConsentManager().GetConsentRequest(r.Context(), challenge)
 	if err != nil {
 		h.r.Writer().WriteError(w, r, errors.WithStack(err))

@@ -244,7 +244,7 @@ func (m *MemoryManager) FindGrantedAndRememberedConsentRequests(ctx context.Cont
 			continue
 		}
 
-		if c.Error != nil {
+		if c.HasError() {
 			continue
 		}
 
@@ -289,7 +289,7 @@ func (m *MemoryManager) FindSubjectsGrantedConsentRequests(ctx context.Context,
 			continue
 		}
 
-		if c.Error != nil {
+		if c.HasError() {
 			continue
 		}
 
@@ -331,7 +331,7 @@ func (m *MemoryManager) CountSubjectsGrantedConsentRequests(ctx context.Context,
 			continue
 		}
 
-		if c.Error != nil {
+		if c.HasError() {
 			continue
 		}
 

@@ -347,7 +347,6 @@ func (m *SQLManager) VerifyAndInvalidateConsentRequest(ctx context.Context, veri
 }
 
 func (m *SQLManager) HandleLoginRequest(ctx context.Context, challenge string, r *HandledLoginRequest) (*LoginRequest, error) {
-
 	/* #nosec G201 - sqlParamsAuthenticationRequestHandled is a "constant" array */
 	if _, err := m.DB.NamedExecContext(ctx, fmt.Sprintf(
 		"INSERT INTO hydra_oauth2_authentication_request_handled (%s) VALUES (%s)",

@@ -72,6 +72,7 @@ func MockConsentRequest(key string, remember bool, rememberFor int, hasError boo
 			Hint:        "error_hint,omitempty" + key,
 			Code:        100,
 			Debug:       "error_debug,omitempty" + key,
+			valid:       true,
 		}
 	}
 
@@ -143,6 +144,7 @@ func MockAuthRequest(key string, authAt bool) (c *LoginRequest, h *HandledLoginR
 		Hint:        "error_hint,omitempty" + key,
 		Code:        100,
 		Debug:       "error_debug,omitempty" + key,
+		valid:       true,
 	}
 
 	var authenticatedAt time.Time
@@ -176,6 +178,7 @@ func SaneMockHandleConsentRequest(t *testing.T, m Manager, c *ConsentRequest, au
 			Hint:        "error_hint",
 			Code:        100,
 			Debug:       "error_debug",
+			valid:       true,
 		}
 	}
 

@@ -340,7 +340,8 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
 		return nil, err
 	}
 
-	if session.Error != nil {
+	if session.HasError() {
+		session.Error.SetDefaults(loginRequestDeniedErrorName)
 		return nil, errors.WithStack(session.Error.toRFCError())
 	}
 
@@ -578,7 +579,8 @@ func (s *DefaultStrategy) verifyConsent(w http.ResponseWriter, r *http.Request,
 		return nil, errors.WithStack(fosite.ErrRequestUnauthorized.WithDebug("The consent request has expired, please try again."))
 	}
 
-	if session.Error != nil {
+	if session.HasError() {
+		session.Error.SetDefaults(consentRequestDeniedErrorName)
 		return nil, errors.WithStack(session.Error.toRFCError())
 	}
 

@@ -25,6 +25,7 @@ import (
 	"database/sql/driver"
 	"encoding/json"
 	"fmt"
+	"net/http"
 	"time"
 
 	"github.com/pkg/errors"
@@ -36,7 +37,8 @@ import (
 )
 
 const (
-	requestDeniedErrorName = "consent request denied"
+	consentRequestDeniedErrorName = "consent request denied"
+	loginRequestDeniedErrorName   = "login request denied"
 )
 
 // The response payload sent when accepting or rejecting a login or consent request.
@@ -64,11 +66,27 @@ type RequestDeniedError struct {
 	Hint        string `json:"error_hint,omitempty"`
 	Code        int    `json:"status_code,omitempty"`
 	Debug       string `json:"error_debug,omitempty"`
+
+	valid bool
+}
+
+func (e *RequestDeniedError) IsError() bool {
+	return e == nil || e.valid
+}
+
+func (e *RequestDeniedError) SetDefaults(name string) {
+	if e.Name == "" {
+		e.Name = name
+	}
+
+	if e.Code == 0 {
+		e.Code = http.StatusBadRequest
+	}
 }
 
 func (e *RequestDeniedError) toRFCError() *fosite.RFC6749Error {
 	if e.Name == "" {
-		e.Name = requestDeniedErrorName
+		e.Name = "request was denied"
 	}
 
 	if e.Code == 0 {
@@ -86,17 +104,22 @@ func (e *RequestDeniedError) toRFCError() *fosite.RFC6749Error {
 
 func (e *RequestDeniedError) Scan(value interface{}) error {
 	v := fmt.Sprintf("%s", value)
-	if len(v) == 0 {
+	if len(v) == 0 || v == "{}" {
 		return nil
 	}
 	return errors.WithStack(json.Unmarshal([]byte(v), e))
 }
 
 func (e *RequestDeniedError) Value() (driver.Value, error) {
+	if e == nil {
+		return "{}", nil
+	}
+
 	value, err := json.Marshal(e)
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
+
 	return string(value), nil
 }
 
@@ -135,10 +158,11 @@ type HandledConsentRequest struct {
 	SessionAccessToken sqlxx.MapStringInterface `db:"session_access_token" json:"-"`
 }
 
+func (r *HandledConsentRequest) HasError() bool {
+	return r.Error.IsError()
+}
+
 func (r *HandledConsentRequest) prepareSQL() *HandledConsentRequest {
-	if r.Error == nil {
-		r.Error = new(RequestDeniedError)
-	}
 	return r
 }
 
@@ -241,15 +265,16 @@ type HandledLoginRequest struct {
 	WasUsed         bool                `json:"-" db:"was_used"`
 }
 
+func (r *HandledLoginRequest) HasError() bool {
+	return r.Error.IsError()
+}
+
 func (r *HandledLoginRequest) postSQL(lr *LoginRequest) *HandledLoginRequest {
 	r.LoginRequest = lr
 	return r
 }
 
 func (r *HandledLoginRequest) prepareSQL() *HandledLoginRequest {
-	if r.Error == nil {
-		r.Error = new(RequestDeniedError)
-	}
 	if string(r.Context) == "" {
 		r.Context = sqlxx.JSONRawMessage("{}")
 	}

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ory/fosite"
@@ -16,7 +17,8 @@ func TestToRFCError(t *testing.T) {
 	}{
 		{
 			input: &RequestDeniedError{
-				Name: "not empty",
+				Name:  "not empty",
+				valid: true,
 			},
 			expect: &fosite.RFC6749Error{
 				Name:        "not empty",
@@ -29,18 +31,19 @@ func TestToRFCError(t *testing.T) {
 			input: &RequestDeniedError{
 				Name:        "",
 				Description: "not empty",
+				valid:       true,
 			},
 			expect: &fosite.RFC6749Error{
-				Name:        requestDeniedErrorName,
+				Name:        "request was denied",
 				Description: "not empty",
 				Code:        fosite.ErrInvalidRequest.Code,
 				Debug:       "",
 			},
 		},
 		{
-			input: &RequestDeniedError{},
+			input: &RequestDeniedError{valid: true},
 			expect: &fosite.RFC6749Error{
-				Name:        requestDeniedErrorName,
+				Name:        consentRequestDeniedErrorName,
 				Description: "",
 				Hint:        "",
 				Code:        fosite.ErrInvalidRequest.Code,
@@ -53,3 +56,10 @@ func TestToRFCError(t *testing.T) {
 		})
 	}
 }
+
+func TestRequestDeniedError(t *testing.T) {
+	var e *RequestDeniedError
+	v, err := e.Value()
+	require.NoError(t, err)
+	assert.EqualValues(t, "{}", fmt.Sprintf("%v", v))
+}