Panic on recovery for deactivated user

[LeonidChistov]

Describe the bug

When deactivated user opens recovery link from the recovery email, following error message appears in Kratos log and 500 HTTP error code is reported for call to `/self-service/recovery?flow=<..>."

kratos_1            | 2021/09/29 13:47:36 http: panic serving 172.28.0.4:58654: runtime error: invalid memory address or nil pointer dereference
kratos_1            | goroutine 64686 [running]:
kratos_1            | net/http.(*conn).serve.func1(0xc0008459a0)
kratos_1            | 	/usr/local/go/src/net/http/server.go:1824 +0x153
kratos_1            | panic(0x1560460, 0x2317060)
kratos_1            | 	/usr/local/go/src/runtime/panic.go:971 +0x499
kratos_1            | github.com/ory/kratos/persistence/sql.(*Persister).CreateSession(0xc00067c060, 0x19eb518, 0xc001309e60, 0x0, 0xc000cdafd8, 0x4977c6)
kratos_1            | 	/home/ory/persistence/sql/persister_session.go:39 +0x8e
kratos_1            | github.com/ory/kratos/session.(*ManagerHTTP).CreateAndIssueCookie(0xc0003ef3f8, 0x19eb518, 0xc001309e60, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0x0, 0x19c4860, 0xc00041c080)
kratos_1            | 	/home/ory/session/manager_http.go:43 +0x6d
kratos_1            | github.com/ory/kratos/selfservice/strategy/link.(*Strategy).recoveryIssueSession(0xc0000ee3d8, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0xc000265a40, 0xd5410070fe24f9fb, 0xaa1e735a5e5c96bd, 0x0, 0x596)
kratos_1            | 	/home/ory/selfservice/strategy/link/strategy_recovery.go:276 +0x42d
kratos_1            | github.com/ory/kratos/selfservice/strategy/link.(*Strategy).recoveryUseToken(0xc0000ee3d8, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0xc000c51860, 0x4, 0x7f5d65a7c2a0)
kratos_1            | 	/home/ory/selfservice/strategy/link/strategy_recovery.go:330 +0x419
kratos_1            | github.com/ory/kratos/selfservice/strategy/link.(*Strategy).Recover(0xc0000ee3d8, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0xc000264000, 0xc000264000, 0x0)
kratos_1            | 	/home/ory/selfservice/strategy/link/strategy_recovery.go:229 +0x205
kratos_1            | github.com/ory/kratos/selfservice/flow/recovery.(*Handler).submitFlow(0xc0000a0410, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0x0, 0x0, 0x0)
kratos_1            | 	/home/ory/selfservice/flow/recovery/handler.go:362 +0x216
kratos_1            | github.com/ory/kratos/session.(*Handler).IsNotAuthenticated.func1(0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0x0, 0x0, 0x0)
kratos_1            | 	/home/ory/session/handler.go:176 +0x48c
kratos_1            | github.com/ory/kratos/x.NoCacheHandler.func1(0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600, 0x0, 0x0, 0x0)
kratos_1            | 	/home/ory/x/nocache.go:18 +0x83
kratos_1            | github.com/julienschmidt/httprouter.(*Router).ServeHTTP(0xc0003a82a0, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5600)
kratos_1            | 	/go/pkg/mod/github.com/julienschmidt/httprouter@v1.3.0/router.go:387 +0xc7e
kratos_1            | github.com/ory/nosurf.(*CSRFHandler).handleSuccess(...)
kratos_1            | 	/go/pkg/mod/github.com/ory/nosurf@v1.2.5/handler.go:201
kratos_1            | github.com/ory/nosurf.(*CSRFHandler).ServeHTTP(0xc0000ba0b0, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500)
kratos_1            | 	/go/pkg/mod/github.com/ory/nosurf@v1.2.5/handler.go:152 +0x7b0
kratos_1            | github.com/urfave/negroni.Wrap.func1(0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500, 0xc00108a920)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:46 +0x4d
kratos_1            | github.com/urfave/negroni.HandlerFunc.ServeHTTP(0xc000122120, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500, 0xc00108a920)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:29 +0x4e
kratos_1            | github.com/urfave/negroni.middleware.ServeHTTP(0x19c80a0, 0xc000122120, 0xc000122510, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:38 +0x9c
kratos_1            | github.com/ory/kratos/x.glob..func1(0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500, 0xc00108a900)
kratos_1            | 	/home/ory/x/clean_url.go:12 +0x8b
kratos_1            | github.com/urfave/negroni.HandlerFunc.ServeHTTP(0x1843e60, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500, 0xc00108a900)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:29 +0x4e
kratos_1            | github.com/urfave/negroni.middleware.ServeHTTP(0x19c80a0, 0x1843e60, 0xc0001224f8, 0x7f5d65a7d6e0, 0xc0000a3540, 0xc0008a5500)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:38 +0x9c
kratos_1            | github.com/urfave/negroni.(*Negroni).ServeHTTP(0xc0009ccba0, 0x19e4fd8, 0xc000267ce0, 0xc0008a5500)
kratos_1            | 	/go/pkg/mod/github.com/urfave/negroni@v1.0.0/negroni.go:96 +0xf1
kratos_1            | net/http.serverHandler.ServeHTTP(0xc0002660e0, 0x19e4fd8, 0xc000267ce0, 0xc0008a5500)
kratos_1            | 	/usr/local/go/src/net/http/server.go:2887 +0xa3
kratos_1            | net/http.(*conn).serve(0xc0008459a0, 0x19eb518, 0xc001508940)
kratos_1            | 	/usr/local/go/src/net/http/server.go:1952 +0x8cd
kratos_1            | created by net/http.(*Server).Serve
kratos_1            | 	/usr/local/go/src/net/http/server.go:3013 +0x39b

Reproducing the bug

Steps to reproduce the behavior:

1. Create new user and deactivate new user account
2. Request recovery email for deactivated user
3. Open recovery email and click on recovery link

Expected behavior

No panic shall appear in Kratos logs and service shall respond with some 4xx HTTP error code that may be handled by browser or redirect to error page.

Also, recovery email could explicitly say that account is deactivated and contain no recovery link, but it looks more like a separate feature request.

Environment

• Version: v0.7.6-alpha.1
• Environment: Docker
• Database: Postgres:11.6

[aeneasr]

Nice find! Any fixes welcomed!

[lanphan]

Hi @LeonidChistov , @aeneasr ,

I think expected behavior should be "content of email sent to deactivated user should be similar with content of email sent to invalid user", what do you think?

[aeneasr]

Yes, agreed!

Regarding the issue, I think this happens when we send a recovery link, then deactivate the user, then the user opens the link!

[LeonidChistov]

@aeneasr Right now deactivation and sending of the recovery link may happen in any order. But if recovery link sending will be fixed for deactivated user to behave exactly as for invalid user, then only "send recovery link" -> "deactivate" -> "open link" will cause the panic, yes.

[lanphan]

@aeneasr , @LeonidChistov ,
Let I fix this issue

[aeneasr]

Nice :)