Bump go-acc and resolve test issues (#154)

Due to a bug in `go-acc`, tests would not run if `-tags sqlite` was supplied as a go tool argument to `go-acc`. This patch resolves that issue and also includes several test patches from previous community PRs and some internal test issues.

Closes #152
Closes #151

.circleci/config.yml

@@ -35,13 +35,13 @@ jobs:
         environment:
           - GO111MODULE=on
           - TEST_MAILHOG_SMTP=smtp://test:test@127.0.0.1:1025
-          - TEST_MAILHOG_API=smtp://127.0.0.1:8025
+          - TEST_MAILHOG_API=http://127.0.0.1:8025
           - TEST_SELFSERVICE_OIDC_HYDRA_ADMIN=http://127.0.0.1:4445
           - TEST_SELFSERVICE_OIDC_HYDRA_PUBLIC=http://127.0.0.1:4444
           - TEST_SELFSERVICE_OIDC_HYDRA_INTEGRATION_ADDR=127.0.0.1:4499
           - TEST_DATABASE_POSTGRESQL=postgres://test:test@localhost:5432/postgres?sslmode=disable
-          - TEST_DATABASE_MYSQL=mysql://root:test@(localhost:3306)/mysql?parseTime=true
-#          - TEST_DATABASE_COCKROACHDB=cockroach://root@localhost:26257/defaultdb?sslmode=disable
+          - TEST_DATABASE_MYSQL=mysql://root:test@(localhost:3306)/mysql?parseTime=true&multiStatements=true
+          - TEST_DATABASE_COCKROACHDB=cockroach://root@localhost:26257/defaultdb?sslmode=disable
       -
         image: mailhog/mailhog:v1.0.0
         command: MailHog -invite-jim -jim-linkspeed-affect=0.25 -jim-reject-auth=0.25 -jim-reject-recipient=0.25 -jim-reject-sender=0.25 -jim-disconnect=0.25 -jim-linkspeed-min=1250 -jim-linkspeed-max=12500
@@ -55,9 +55,9 @@ jobs:
         image: mysql:5.7
         environment:
           - MYSQL_ROOT_PASSWORD=test
-#      -
-#        image: cockroachdb/cockroach:v19.2.0
-#        command: start --insecure
+      -
+        image: cockroachdb/cockroach:v19.2.0
+        command: start --insecure
       -
         image: oryd/hydra:v1.0.0
         environment:

.gitignore

@@ -3,3 +3,4 @@ cover.out
 tmp/
 .DS_Store
 ./kratos
+coverage.txt

.golangci.yml

@@ -19,3 +19,5 @@ linters:
 run:
   skip-dirs:
     - sdk/
+  skip-files:
+    - ".+_test.go"

Makefile

@@ -22,7 +22,7 @@ init:
 
 .PHONY: lint
 lint:
-		GO111MODULE=on golangci-lint run
+		GO111MODULE=on golangci-lint run -v ./...
 
 .PHONY: format
 format:

courier/courier_test.go

@@ -17,20 +17,35 @@ import (
 	"github.com/tidwall/gjson"
 
 	"github.com/ory/dockertest"
+
 	"github.com/ory/viper"
+	dhelper "github.com/ory/x/sqlcon/dockertest"
 
 	templates "github.com/ory/kratos/courier/template"
 	"github.com/ory/kratos/driver/configuration"
 	"github.com/ory/kratos/internal"
 )
 
-func runTestSMTP(t *testing.T) (smtp, api string, r *dockertest.Resource) {
+var resources []*dockertest.Resource
+
+// nolint:staticcheck
+func TestMain(m *testing.M) {
+	atexit := dhelper.NewOnExit()
+	atexit.Add(func() {
+		for _, resource := range resources {
+			resource.Close()
+		}
+	})
+	atexit.Exit(m.Run())
+}
+
+func runTestSMTP(t *testing.T) (smtp, api string) {
 	if smtp, api := os.Getenv("TEST_MAILHOG_SMTP"), os.Getenv("TEST_MAILHOG_API"); smtp != "" && api != "" {
 		t.Logf("Skipping Docker setup because environment variables TEST_MAILHOG_SMTP and TEST_MAILHOG_API are both set.")
-		return smtp, api, nil
+		return smtp, api
 	} else if len(smtp)+len(api) > 0 {
 		t.Fatal("Environment variables TEST_MAILHOG_SMTP, TEST_MAILHOG_API must both be set!")
-		return "", "", nil
+		return "", ""
 	}
 
 	pool, err := dockertest.NewPool("")
@@ -52,6 +67,7 @@ func runTestSMTP(t *testing.T) (smtp, api string, r *dockertest.Resource) {
 			},
 		})
 	require.NoError(t, err)
+	resources = append(resources, resource)
 
 	smtp = fmt.Sprintf("smtp://test:test@127.0.0.1:%s", resource.GetPort("1025/tcp"))
 	api = fmt.Sprintf("http://127.0.0.1:%s", resource.GetPort("8025/tcp"))
@@ -70,16 +86,15 @@ func runTestSMTP(t *testing.T) (smtp, api string, r *dockertest.Resource) {
 		return nil
 	}, backoff.WithMaxRetries(backoff.NewConstantBackOff(time.Second), 15)))
 
-	return smtp, api, resource
+	return smtp, api
 }
 
 func TestSMTP(t *testing.T) {
 	if testing.Short() {
 		t.SkipNow()
 	}
 
-	smtp, api, resource := runTestSMTP(t)
-	defer resource.Close()
+	smtp, api := runTestSMTP(t)
 
 	conf, reg := internal.NewRegistryDefault(t)
 	viper.Set(configuration.ViperKeyCourierSMTPURL, smtp)

courier/persistence.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"testing"
+	"time"
 
 	"github.com/bxcodec/faker"
 	"github.com/pkg/errors"
@@ -47,6 +48,7 @@ func TestPersister(p interface {
 				require.NoError(t, faker.FakeData(&m))
 				require.NoError(t, p.AddMessage(context.Background(), &m))
 				messages[k] = m
+				time.Sleep(time.Second) // wait a bit so that the timestamp ordering works in MySQL.
 			}
 		})
 
@@ -69,22 +71,20 @@ func TestPersister(p interface {
 				})
 			}
 
-			ms, err := p.NextMessages(context.Background(), 10)
-			require.NoError(t, err)
-			assert.Len(t, ms, 0)
+			_, err := p.NextMessages(context.Background(), 10)
+			require.EqualError(t, err, ErrQueueEmpty.Error())
 		})
 
 		t.Run("case=setting message status", func(t *testing.T) {
 			require.NoError(t, p.SetMessageStatus(context.Background(), messages[0].ID, MessageStatusQueued))
 			ms, err := p.NextMessages(context.Background(), 1)
 			require.NoError(t, err)
 			require.Len(t, ms, 1)
-			assert.Equal(t, messages[0].ID, ms[1].ID)
+			assert.Equal(t, messages[0].ID, ms[0].ID)
 
 			require.NoError(t, p.SetMessageStatus(context.Background(), messages[0].ID, MessageStatusSent))
-			ms, err = p.NextMessages(context.Background(), 1)
+			_, err = p.NextMessages(context.Background(), 1)
 			require.EqualError(t, err, ErrQueueEmpty.Error())
-			assert.Len(t, ms, 0)
 		})
 	}
 }

docs/.kratos.yaml

@@ -12,6 +12,10 @@ dsn: memory
 log:
   level: debug
 
+courier:
+  smtp:
+    connection_uri: smtp://foo:bar@baz/
+
 urls:
   default_return_to: http://return-to-3-test.ory.sh/
   mfa_ui: http://test.kratos.ory.sh/mfa
@@ -67,15 +71,15 @@ selfservice:
           allow_user_defined_redirect: false
     after:
       password:
-        - run: session
         - run: revoke_active_sessions
+        - run: session
         - run: redirect
           config:
             default_redirect_url: http://test.kratos.ory.sh:4000/
             allow_user_defined_redirect: true
       oidc:
-        - run: session
         - run: revoke_active_sessions
+        - run: session
         - run: redirect
           config:
             default_redirect_url: http://test.kratos.ory.sh:4000/

docs/config.schema.json

@@ -33,6 +33,18 @@
         "config"
       ]
     },
+    "selfServiceSessionRevokerHook": {
+      "type": "object",
+      "properties": {
+        "run": {
+          "const": "revoke_active_sessions"
+        }
+      },
+      "additionalItems": false,
+      "required": [
+        "run"
+      ]
+    },
     "selfServiceSessionIssuerHook": {
       "type": "object",
       "properties": {
@@ -94,7 +106,7 @@
         "schema_url"
       ]
     },
-    "selfServiceAfterHooks": {
+    "selfServiceAfterLoginHooks": {
       "type": "array",
       "items": {
         "anyOf": [
@@ -103,19 +115,48 @@
           },
           {
             "$ref": "#/definitions/selfServiceSessionIssuerHook"
+          },
+          {
+            "$ref": "#/definitions/selfServiceSessionRevokerHook"
           }
         ]
       },
       "uniqueItems": true
     },
-    "selfServiceAfter": {
+    "selfServiceAfterRegistrationHooks": {
+      "type": "array",
+      "items": {
+        "anyOf": [
+          {
+            "$ref": "#/definitions/selfServiceRedirectHook"
+          },
+          {
+            "$ref": "#/definitions/selfServiceSessionIssuerHook"
+          }
+        ]
+      },
+      "uniqueItems": true
+    },
+    "selfServiceAfterLogin": {
+      "type": "object",
+      "properties": {
+        "password": {
+          "$ref": "#/definitions/selfServiceAfterLoginHooks"
+        },
+        "oidc": {
+          "$ref": "#/definitions/selfServiceAfterLoginHooks"
+        }
+      },
+      "additionalItems": false
+    },
+    "selfServiceAfterRegistration": {
       "type": "object",
       "properties": {
         "password": {
-          "$ref": "#/definitions/selfServiceAfterHooks"
+          "$ref": "#/definitions/selfServiceAfterRegistrationHooks"
         },
         "oidc": {
-          "$ref": "#/definitions/selfServiceAfterHooks"
+          "$ref": "#/definitions/selfServiceAfterRegistrationHooks"
         }
       },
       "additionalItems": false
@@ -207,14 +248,14 @@
               "$ref": "#/definitions/selfServiceBefore"
             },
             "after": {
-              "$ref": "#/definitions/selfServiceAfter"
+              "$ref": "#/definitions/selfServiceAfterLogin"
             }
           },
           "additionalItems": false
         },
         "registration": {
           "type": "object",
-          "additionalItems": false,
+          "additionalProperties": false,
           "properties": {
             "request_lifespan": {
               "type": "string",
@@ -227,6 +268,9 @@
                 "$ref": "#/definitions/selfServiceRedirectHook"
               },
               "uniqueItems": true
+            },
+            "after": {
+              "$ref": "#/definitions/selfServiceAfterRegistration"
             }
           }
         }
@@ -253,7 +297,9 @@
             "connection_uri": {
               "title": "SMTP connection string",
               "description": "This URI will be used to connect to the SMTP server.",
-              "examples": ["smtps://foo:bar@my-mailserver:1234/"],
+              "examples": [
+                "smtps://foo:bar@my-mailserver:1234/"
+              ],
               "type": "string",
               "format": "uri"
             },
@@ -265,11 +311,15 @@
               "default": "no-reply@ory.kratos.sh"
             }
           },
-          "required": ["connection_uri"],
+          "required": [
+            "connection_uri"
+          ],
           "additionalProperties": false
         }
       },
-      "required": ["smtp"],
+      "required": [
+        "smtp"
+      ],
       "additionalProperties": false
     },
     "serve": {

driver/configuration/provider_viper_test.go

@@ -112,7 +112,7 @@ func TestViperProvider(t *testing.T) {
 				},
 			} {
 				t.Run("hook=after/strategy="+tc.strategy, func(t *testing.T) {
-					hooks := p.SelfServiceLoginAfterHooks(tc.strategy)
+					hooks := p.SelfServiceRegistrationAfterHooks(tc.strategy)
 
 					hook := hooks[0]
 					assert.EqualValues(t, "session", hook.Run)
@@ -151,9 +151,12 @@ func TestViperProvider(t *testing.T) {
 					hooks := p.SelfServiceLoginAfterHooks(tc.strategy)
 
 					hook := hooks[0]
-					assert.EqualValues(t, "session", hook.Run)
+					assert.EqualValues(t, "revoke_active_sessions", hook.Run)
 
 					hook = hooks[1]
+					assert.EqualValues(t, "session", hook.Run)
+
+					hook = hooks[2]
 					assert.EqualValues(t, "redirect", hook.Run)
 					assert.JSONEq(t, tc.redirectConfig, string(hook.Config))
 				})

driver/registry_default.go

@@ -41,7 +41,9 @@ import (
 var _ Registry = new(RegistryDefault)
 
 func init() {
-	dbal.RegisterDriver(NewRegistryDefault())
+	dbal.RegisterDriver(func() dbal.Driver {
+		return NewRegistryDefault()
+	})
 }
 
 type RegistryDefault struct {
@@ -303,7 +305,7 @@ func (m *RegistryDefault) CanHandle(dsn string) bool {
 
 func (m *RegistryDefault) Init() error {
 	if m.persister != nil {
-		return nil
+		panic("RegistryDefault.Init() must not be called more than once.")
 	}
 
 	bc := backoff.NewExponentialBackOff()