expect JSON response from web hook

internal/client-go/go.sum

@@ -4,6 +4,7 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e h1:bRhVy7zSSasaqNksaRZiA5EEI+Ei4I1nO5Jh72wfHlg=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
+golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

selfservice/hook/password_migration_hook.go

@@ -5,16 +5,15 @@ package hook
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 
 	"github.com/pkg/errors"
 	"github.com/tidwall/gjson"
-	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
-	semconv "go.opentelemetry.io/otel/semconv/v1.11.0"
+	"go.opentelemetry.io/otel/semconv/v1.11.0"
 	"go.opentelemetry.io/otel/trace"
 	grpccodes "google.golang.org/grpc/codes"
 
@@ -29,24 +28,20 @@ type (
 		deps webHookDependencies
 		conf json.RawMessage
 	}
-	PasswordMigrationData struct {
+	PasswordMigrationRequest struct {
 		Identifier string `json:"identifier"`
 		Password   string `json:"password"`
 	}
+	PasswordMigrationResponse struct {
+		Status string `json:"status"`
+	}
 )
 
-var passwordMigrationJsonnetTemplate string
-
-func init() {
-	snippet := []byte(`function(ctx) { password: ctx.password, identifier: ctx.identifier }`)
-	passwordMigrationJsonnetTemplate = "base64://" + base64.StdEncoding.EncodeToString(snippet)
-}
-
 func NewPasswordMigrationHook(deps webHookDependencies, conf json.RawMessage) *PasswordMigration {
 	return &PasswordMigration{deps: deps, conf: conf}
 }
 
-func (p *PasswordMigration) Execute(ctx context.Context, data *PasswordMigrationData) (err error) {
+func (p *PasswordMigration) Execute(ctx context.Context, data *PasswordMigrationRequest) (err error) {
 	var (
 		httpClient = p.deps.HTTPClient(ctx)
 		emitEvent  = gjson.GetBytes(p.conf, "emit_analytics_event").Bool() || !gjson.GetBytes(p.conf, "emit_analytics_event").Exists() // default true
@@ -59,19 +54,20 @@ func (p *PasswordMigration) Execute(ctx context.Context, data *PasswordMigration
 	if emitEvent {
 		instrumentHTTPClientForEvents(ctx, httpClient)
 	}
-	builder, err := request.NewBuilder(ctx, p.conf, p.deps, jsonnetCache)
+	builder, err := request.NewBuilder(ctx, p.conf, p.deps, nil)
 	if err != nil {
-		return err
+		return errors.WithStack(err)
 	}
-
-	builder.Config.TemplateURI = passwordMigrationJsonnetTemplate
-
-	req, err := builder.BuildRequest(ctx, data)
-	if errors.Is(err, request.ErrCancel) {
-		span.SetAttributes(attribute.Bool("password_migration.jsonnet.canceled", true))
-		return nil
-	} else if err != nil {
-		return err
+	req, err := builder.BuildRequest(ctx, nil) // passing a nil body here skips Jsonnet
+	if err != nil {
+		return errors.WithStack(err)
+	}
+	rawData, err := json.Marshal(data)
+	if err != nil {
+		return errors.WithStack(err)
+	}
+	if err = req.SetBody(rawData); err != nil {
+		return errors.WithStack(err)
 	}
 
 	p.deps.Logger().WithRequest(req.Request).Info("Dispatching password migration hook")
@@ -84,8 +80,8 @@ func (p *PasswordMigration) Execute(ctx context.Context, data *PasswordMigration
 				CodeField:     http.StatusGatewayTimeout,
 				StatusField:   http.StatusText(http.StatusGatewayTimeout),
 				GRPCCodeField: grpccodes.DeadlineExceeded,
-				ErrorField:    err.Error(),
 				ReasonField:   "A third-party upstream service could not be reached. Please try again later.",
+				ErrorField:    "calling the password migration hook failed",
 			}.WithWrap(errors.WithStack(err))
 		}
 		return herodot.DefaultError{
@@ -101,7 +97,14 @@ func (p *PasswordMigration) Execute(ctx context.Context, data *PasswordMigration
 
 	switch resp.StatusCode {
 	case http.StatusOK:
+		// We now check if the response matches `{"status": "password_match" }`.
+		dec := json.NewDecoder(io.LimitReader(resp.Body, 1024)) // limit the response body to 1KB
+		var response PasswordMigrationResponse
+		if err := dec.Decode(&response); err != nil || response.Status != "password_match" {
+			return errors.WithStack(schema.NewInvalidCredentialsError())
+		}
 		return nil
+
 	case http.StatusForbidden:
 		return errors.WithStack(schema.NewInvalidCredentialsError())
 	default:

selfservice/strategy/password/login.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/ory/kratos/hash"
 	"github.com/ory/kratos/selfservice/flowhelpers"
 	"github.com/ory/kratos/selfservice/hook"
 	"github.com/ory/kratos/session"
@@ -23,7 +24,6 @@ import (
 	"github.com/ory/herodot"
 	"github.com/ory/x/decoderx"
 
-	"github.com/ory/kratos/hash"
 	"github.com/ory/kratos/identity"
 	"github.com/ory/kratos/schema"
 	"github.com/ory/kratos/selfservice/flow"
@@ -84,42 +84,33 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
 	}
 
 	if o.ShouldUsePasswordMigrationHook() {
-		hookConfig := s.d.Config().PasswordMigrationHook(r.Context())
-		if !hookConfig.Enabled {
+		pwHook := s.d.Config().PasswordMigrationHook(r.Context())
+		if !pwHook.Enabled {
 			// TODO: How can we make sure this does not trigger on-call?
 			return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Password migration hook is not enabled but password migration is requested."))
 		}
 
-		migrationHook := hook.NewPasswordMigrationHook(s.d, hookConfig.Config)
-		err = migrationHook.Execute(r.Context(), &hook.PasswordMigrationData{Identifier: identifier, Password: p.Password})
+		migrationHook := hook.NewPasswordMigrationHook(s.d, pwHook.Config)
+		err = migrationHook.Execute(r.Context(), &hook.PasswordMigrationRequest{Identifier: identifier, Password: p.Password})
 		if err != nil {
 			return nil, s.handleLoginError(w, r, f, &p, err)
 		}
 
 		if err := s.migratePasswordHash(r.Context(), i.ID, []byte(p.Password)); err != nil {
 			return nil, s.handleLoginError(w, r, f, &p, err)
 		}
-
-		f.Active = identity.CredentialsTypePassword
-		f.Active = s.ID()
-		if err = s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), f); err != nil {
-			return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReason("Could not update flow").WithDebug(err.Error())))
+	} else {
+		if err := hash.Compare(r.Context(), []byte(p.Password), []byte(o.HashedPassword)); err != nil {
+			return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
 		}
 
-		return i, nil
-	}
-
-	if err := hash.Compare(r.Context(), []byte(p.Password), []byte(o.HashedPassword)); err != nil {
-		return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(schema.NewInvalidCredentialsError()))
-	}
-
-	if !s.d.Hasher(r.Context()).Understands([]byte(o.HashedPassword)) {
-		if err := s.migratePasswordHash(r.Context(), i.ID, []byte(p.Password)); err != nil {
-			return nil, s.handleLoginError(w, r, f, &p, err)
+		if !s.d.Hasher(r.Context()).Understands([]byte(o.HashedPassword)) {
+			if err := s.migratePasswordHash(r.Context(), i.ID, []byte(p.Password)); err != nil {
+				return nil, s.handleLoginError(w, r, f, &p, err)
+			}
 		}
 	}
 
-	f.Active = identity.CredentialsTypePassword
 	f.Active = s.ID()
 	if err = s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), f); err != nil {
 		return nil, s.handleLoginError(w, r, f, &p, errors.WithStack(herodot.ErrInternalServerError.WithReason("Could not update flow").WithDebug(err.Error())))

selfservice/strategy/password/login_test.go

@@ -885,12 +885,15 @@ func TestCompleteLogin(t *testing.T) {
 			switch pw := passwordDB[payload.Identifier]; pw {
 			case payload.Password:
 				w.WriteHeader(http.StatusOK)
+				_, _ = io.WriteString(w, `{"status":"password_match"}`)
 
 			// Set up diverse fake passwords to trigger special response status codes:
 			case "500":
 				w.WriteHeader(http.StatusInternalServerError)
 			case "201":
 				w.WriteHeader(http.StatusCreated)
+			case "200":
+				w.WriteHeader(http.StatusCreated)
 			case "400":
 				w.WriteHeader(http.StatusBadRequest)
 
@@ -920,6 +923,11 @@ func TestCompleteLogin(t *testing.T) {
 			credentialsConfig: `{"use_password_migration_hook": true}`,
 			addPassword:       func(identifier, password string) { passwordDB[identifier] = "wrong" },
 			expectSuccess:     false,
+		}, {
+			name:              "should not update identity when the migration hook returns 200 without JSON",
+			credentialsConfig: `{"use_password_migration_hook": true}`,
+			addPassword:       func(identifier, password string) { passwordDB[identifier] = "200" },
+			expectSuccess:     false,
 		}, {
 			name:              "should not update identity when the migration hook returns 500",
 			credentialsConfig: `{"use_password_migration_hook": true}`,