fix: return proper error code in refresh and code flows

.dockerignore

@@ -10,3 +10,4 @@ sdk/js
 sdk/php
 vendor/
 dist/
+.bin/

.schema/api.swagger.json

@@ -1818,7 +1818,7 @@
       "title": "JSONRawMessage represents a json.RawMessage that works well with JSON, SQL, and Swagger."
     },
     "JSONWebKey": {
-      "description": "JSONWebKey It is important that this model object is named JSONWebKey for\n\"swagger generate spec\" to generate only on definition of a\nJSONWebKey.",
+      "description": "JSONWebKey JSONWebKey It is important that this model object is named JSONWebKey for\n\"swagger generate spec\" to generate only on definition of a\nJSONWebKey.",
       "type": "object",
       "required": [
         "use",
@@ -1917,7 +1917,7 @@
       }
     },
     "JSONWebKeySet": {
-      "description": "It is important that this model object is named JSONWebKeySet for\n\"swagger generate spec\" to generate only on definition of a\nJSONWebKeySet. Since one with the same name is previously defined as\nclient.Client.JSONWebKeys and this one is last, this one will be\neffectively written in the swagger spec.",
+      "description": "JSONWebKeySet It is important that this model object is named JSONWebKeySet for\n\"swagger generate spec\" to generate only on definition of a\nJSONWebKeySet. Since one with the same name is previously defined as\nclient.Client.JSONWebKeys and this one is last, this one will be\neffectively written in the swagger spec.",
       "type": "object",
       "properties": {
         "keys": {
@@ -1938,7 +1938,7 @@
       "title": "NullTime implements sql.NullTime functionality."
     },
     "PreviousConsentSession": {
-      "description": "PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession The response used to return used consent requests\nsame as HandledLoginRequest, just with consent_request exposed as json",
+      "description": "PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession PreviousConsentSession The response used to return used consent requests\nsame as HandledLoginRequest, just with consent_request exposed as json",
       "type": "object",
       "properties": {
         "consent_request": {
@@ -1959,7 +1959,7 @@
           }
         },
         "handled_at": {
-          "description": "handled at\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time",
+          "description": "handled at\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time\nFormat: date-time",
           "type": "string",
           "format": "date-time"
         },
@@ -1986,7 +1986,7 @@
     },
     "acceptConsentRequest": {
       "type": "object",
-      "title": "AcceptConsentRequest AcceptConsentRequest The request payload used to accept a consent request.",
+      "title": "AcceptConsentRequest AcceptConsentRequest AcceptConsentRequest The request payload used to accept a consent request.",
       "properties": {
         "grant_access_token_audience": {
           "type": "array",
@@ -2054,7 +2054,7 @@
     },
     "completedRequest": {
       "type": "object",
-      "title": "CompletedRequest CompletedRequest CompletedRequest The response payload sent when accepting or rejecting a login or consent request.",
+      "title": "The response payload sent when accepting or rejecting a login or consent request.",
       "properties": {
         "redirect_to": {
           "description": "RedirectURL is the URL which you should redirect the user to once the authentication process is completed.",
@@ -2064,7 +2064,7 @@
     },
     "consentRequest": {
       "type": "object",
-      "title": "ConsentRequest Contains information on an ongoing consent request.",
+      "title": "ConsentRequest ConsentRequest Contains information on an ongoing consent request.",
       "properties": {
         "acr": {
           "description": "ACR represents the Authentication AuthorizationContext Class Reference value for this authentication session. You can use it\nto express that, for example, a user authenticated using two factor authentication.",
@@ -2121,7 +2121,7 @@
     },
     "consentRequestSession": {
       "type": "object",
-      "title": "Used to pass session data to a consent request.",
+      "title": "ConsentRequestSession Used to pass session data to a consent request.",
       "properties": {
         "access_token": {
           "description": "AccessToken sets session data for the access and refresh token, as well as any future tokens issued by the\nrefresh grant. Keep in mind that this data will be available to anyone performing OAuth 2.0 Challenge Introspection.\nIf only your services can perform OAuth 2.0 Challenge Introspection, this is usually fine. But if third parties\ncan access that endpoint as well, sensitive data from the session might be exposed to them. Use with care!",
@@ -2140,11 +2140,11 @@
       }
     },
     "flushInactiveOAuth2TokensRequest": {
-      "description": "FlushInactiveOAuth2TokensRequest FlushInactiveOAuth2TokensRequest flush inactive o auth2 tokens request",
+      "description": "FlushInactiveOAuth2TokensRequest FlushInactiveOAuth2TokensRequest FlushInactiveOAuth2TokensRequest flush inactive o auth2 tokens request",
       "type": "object",
       "properties": {
         "notAfter": {
-          "description": "NotAfter sets after which point tokens should not be flushed. This is useful when you want to keep a history\nof recently issued tokens for auditing.\nFormat: date-time\nFormat: date-time",
+          "description": "NotAfter sets after which point tokens should not be flushed. This is useful when you want to keep a history\nof recently issued tokens for auditing.\nFormat: date-time\nFormat: date-time\nFormat: date-time",
           "type": "string",
           "format": "date-time"
         }
@@ -2153,7 +2153,7 @@
     "genericError": {
       "description": "Error responses are sent when an error (e.g. unauthorized, bad request, ...) occurred.",
       "type": "object",
-      "title": "Error response",
+      "title": "GenericError Error response",
       "required": [
         "error"
       ],
@@ -2182,7 +2182,7 @@
       }
     },
     "healthNotReadyStatus": {
-      "description": "HealthNotReadyStatus HealthNotReadyStatus HealthNotReadyStatus HealthNotReadyStatus health not ready status",
+      "description": "HealthNotReadyStatus HealthNotReadyStatus HealthNotReadyStatus HealthNotReadyStatus HealthNotReadyStatus health not ready status",
       "type": "object",
       "properties": {
         "errors": {
@@ -2195,6 +2195,7 @@
       }
     },
     "healthStatus": {
+      "description": "HealthStatus health status",
       "type": "object",
       "properties": {
         "status": {
@@ -2204,12 +2205,11 @@
       }
     },
     "jsonWebKeySetGeneratorRequest": {
-      "description": "JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest JSONWebKeySetGeneratorRequest json web key set generator request",
       "type": "object",
       "required": [
         "alg",
-        "use",
-        "kid"
+        "kid",
+        "use"
       ],
       "properties": {
         "alg": {
@@ -2276,7 +2276,7 @@
     },
     "logoutRequest": {
       "type": "object",
-      "title": "Contains information about an ongoing logout request.",
+      "title": "LogoutRequest Contains information about an ongoing logout request.",
       "properties": {
         "request_url": {
           "description": "RequestURL is the original Logout URL requested.",
@@ -2298,7 +2298,7 @@
     },
     "oAuth2Client": {
       "type": "object",
-      "title": "OAuth2Client Client represents an OAuth 2.0 Client.",
+      "title": "OAuth2Client OAuth2Client Client represents an OAuth 2.0 Client.",
       "properties": {
         "allowed_cors_origins": {
           "$ref": "#/definitions/StringSlicePipeDelimiter"
@@ -2339,7 +2339,7 @@
           "$ref": "#/definitions/StringSlicePipeDelimiter"
         },
         "created_at": {
-          "description": "CreatedAt returns the timestamp of the client's creation.\nFormat: date-time",
+          "description": "CreatedAt returns the timestamp of the client's creation.\nFormat: date-time\nFormat: date-time",
           "type": "string",
           "format": "date-time"
         },
@@ -2414,7 +2414,7 @@
           "type": "string"
         },
         "updated_at": {
-          "description": "UpdatedAt returns the timestamp of the last update.\nFormat: date-time",
+          "description": "UpdatedAt returns the timestamp of the last update.\nFormat: date-time\nFormat: date-time",
           "type": "string",
           "format": "date-time"
         },
@@ -2528,7 +2528,7 @@
     },
     "openIDConnectContext": {
       "type": "object",
-      "title": "Contains optional information about the OpenID Connect request.",
+      "title": "OpenIDConnectContext Contains optional information about the OpenID Connect request.",
       "properties": {
         "acr_values": {
           "description": "ACRValues is the Authentication AuthorizationContext Class Reference requested in the OAuth 2.0 Authorization request.\nIt is a parameter defined by OpenID Connect and expresses which level of authentication (e.g. 2FA) is required.\n\nOpenID Connect defines it as follows:\n\u003e Requested Authentication AuthorizationContext Class Reference values. Space-separated string that specifies the acr values\nthat the Authorization Server is being requested to use for processing this Authentication Request, with the\nvalues appearing in order of preference. The Authentication AuthorizationContext Class satisfied by the authentication\nperformed is returned as the acr Claim Value, as specified in Section 2. The acr Claim is requested as a\nVoluntary Claim by this parameter.",
@@ -2563,7 +2563,7 @@
     },
     "rejectRequest": {
       "type": "object",
-      "title": "RejectRequest The request payload used to accept a login or consent request.",
+      "title": "The request payload used to accept a login or consent request.",
       "properties": {
         "error": {
           "description": "error",
@@ -2589,7 +2589,7 @@
       }
     },
     "userinfoResponse": {
-      "description": "UserinfoResponse UserinfoResponse UserinfoResponse UserinfoResponse The userinfo response",
+      "description": "UserinfoResponse UserinfoResponse UserinfoResponse UserinfoResponse UserinfoResponse The userinfo response",
       "type": "object",
       "properties": {
         "birthdate": {
@@ -2672,7 +2672,6 @@
       }
     },
     "version": {
-      "description": "Version version",
       "type": "object",
       "properties": {
         "version": {
@@ -2684,7 +2683,7 @@
     "wellKnown": {
       "description": "It includes links to several endpoints (e.g. /oauth2/token) and exposes information on supported signature algorithms\namong others.",
       "type": "object",
-      "title": "WellKnown WellKnown WellKnown WellKnown WellKnown WellKnown WellKnown represents important OpenID Connect discovery metadata",
+      "title": "WellKnown WellKnown WellKnown WellKnown WellKnown WellKnown WellKnown WellKnown represents important OpenID Connect discovery metadata",
       "required": [
         "issuer",
         "authorization_endpoint",

Makefile

@@ -35,7 +35,9 @@ test-resetdb:
 .PHONY: docker
 docker:
 		make sqlbin
+		packr2
 		CGO_ENABLED=0 GO111MODULE=on GOOS=linux GOARCH=amd64 go build
+		packr2 clean
 		docker build -t oryd/hydra:latest .
 		rm hydra
 

cmd/server/handler.go

@@ -105,7 +105,7 @@ func RunServeAdmin(version, build, date string) func(cmd *cobra.Command, args []
 		isDSNAllowed(d)
 
 		admin, _, adminmw, _ := setup(d, cmd)
-		cert := getOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
+		cert := GetOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
 
 		var wg sync.WaitGroup
 		wg.Add(1)
@@ -133,7 +133,7 @@ func RunServePublic(version, build, date string) func(cmd *cobra.Command, args [
 		isDSNAllowed(d)
 
 		_, public, _, publicmw := setup(d, cmd)
-		cert := getOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
+		cert := GetOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
 
 		var wg sync.WaitGroup
 		wg.Add(1)
@@ -159,7 +159,7 @@ func RunServeAll(version, build, date string) func(cmd *cobra.Command, args []st
 		).CallRegistry()
 
 		admin, public, adminmw, publicmw := setup(d, cmd)
-		cert := getOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
+		cert := GetOrCreateTLSCertificate(cmd, d) // we do not want to run this concurrently.
 
 		var wg sync.WaitGroup
 		wg.Add(2)

cmd/server/helper_cert.go

@@ -22,26 +22,39 @@ package server
 
 import (
 	"context"
+	"crypto/sha1" // #nosec G505 - This is required for certificate chains alongside sha256
+	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
 
 	"github.com/ory/viper"
+	"gopkg.in/square/go-jose.v2"
 
 	"github.com/ory/hydra/driver"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
-	"github.com/ory/hydra/jwk"
 	"github.com/ory/x/tlsx"
+
+	"github.com/ory/hydra/jwk"
 )
 
 const (
 	tlsKeyName = "hydra.https-tls"
 )
 
-func getOrCreateTLSCertificate(cmd *cobra.Command, d driver.Driver) []tls.Certificate {
+func AttachCertificate(priv *jose.JSONWebKey, cert *x509.Certificate) {
+	priv.Certificates = []*x509.Certificate{cert}
+	sig256 := sha256.Sum256(cert.Raw)
+	// #nosec G401 - This is required for certificate chains alongside sha256
+	sig1 := sha1.Sum(cert.Raw)
+	priv.CertificateThumbprintSHA256 = sig256[:]
+	priv.CertificateThumbprintSHA1 = sig1[:]
+}
+
+func GetOrCreateTLSCertificate(cmd *cobra.Command, d driver.Driver) []tls.Certificate {
 	cert, err := tlsx.Certificate(
 		viper.GetString("serve.tls.cert.base64"),
 		viper.GetString("serve.tls.key.base64"),
@@ -66,13 +79,13 @@ func getOrCreateTLSCertificate(cmd *cobra.Command, d driver.Driver) []tls.Certif
 			d.Registry().Logger().WithError(err).Fatalf(`Could not generate a self signed TLS certificate`)
 		}
 
-		priv.Certificates = []*x509.Certificate{cert}
+		AttachCertificate(priv, cert)
 		if err := d.Registry().KeyManager().DeleteKey(context.TODO(), tlsKeyName, priv.KeyID); err != nil {
 			d.Registry().Logger().WithError(err).Fatal(`Could not update (delete) the self signed TLS certificate`)
 		}
 
 		if err := d.Registry().KeyManager().AddKey(context.TODO(), tlsKeyName, priv); err != nil {
-			d.Registry().Logger().WithError(err).Fatal(`Could not update (add) the self signed TLS certificate`)
+			d.Registry().Logger().WithError(err).Fatalf(`Could not update (add) the self signed TLS certificate: %s %x %d`, cert.SignatureAlgorithm, cert.Signature, len(cert.Signature))
 		}
 	}
 

cmd/server/helper_cert_test.go

@@ -0,0 +1,31 @@
+package server_test
+
+import (
+	"bytes"
+	"encoding/json"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/ory/x/tlsx"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/square/go-jose.v2"
+
+	"github.com/ory/hydra/cmd/server"
+	"github.com/ory/hydra/jwk"
+)
+
+func TestGetOrCreateTLSCertificate(t *testing.T) {
+	keys, err := (&jwk.RS256Generator{KeyLength: 1024}). // this is ok because we're just using it for tests
+		Generate(uuid.New().String(), "sig")
+	require.NoError(t, err)
+
+	private := keys.Keys[0]
+	cert, err := tlsx.CreateSelfSignedCertificate(private.Key)
+	require.NoError(t, err)
+	server.AttachCertificate(&private, cert)
+
+	var actual jose.JSONWebKeySet
+	var b bytes.Buffer
+	require.NoError(t, json.NewEncoder(&b).Encode(keys))
+	require.NoError(t, json.NewDecoder(&b).Decode(&actual))
+}

go.mod

@@ -28,11 +28,10 @@ require (
 	github.com/olekukonko/tablewriter v0.0.1
 	github.com/opentracing/opentracing-go v1.1.1-0.20190913142402-a7454ce5950e
 	github.com/ory/analytics-go/v4 v4.0.1
-	github.com/ory/fosite v0.31.0
+	github.com/ory/fosite v0.31.2
 	github.com/ory/go-acc v0.2.1
 	github.com/ory/graceful v0.1.1
 	github.com/ory/herodot v0.7.0
-	github.com/ory/jsonschema/v3 v3.0.1
 	github.com/ory/sdk/swagutil v0.0.0-20200407150153-53df6d772608
 	github.com/ory/viper v1.7.4
 	github.com/ory/x v0.0.111
@@ -47,6 +46,7 @@ require (
 	github.com/sirupsen/logrus v1.5.0
 	github.com/spf13/cobra v0.0.7
 	github.com/sqs/goreturns v0.0.0-20181028201513-538ac6014518
+	github.com/square/go-jose/v3 v3.0.0-20200415055503-21f2ca25ccce // indirect
 	github.com/stretchr/testify v1.5.1
 	github.com/tidwall/gjson v1.6.0
 	github.com/tidwall/pretty v1.0.1 // indirect
@@ -58,5 +58,5 @@ require (
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
 	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
 	golang.org/x/tools v0.0.0-20200313205530-4303120df7d8
-	gopkg.in/square/go-jose.v2 v2.4.1
+	gopkg.in/square/go-jose.v2 v2.5.0
 )