Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Resolve several verification problems #253

Merged
merged 3 commits into from
Feb 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions .docker/mailslurper/Dockerfile

This file was deleted.

20 changes: 0 additions & 20 deletions .docker/mailslurper/config.json

This file was deleted.

15 changes: 0 additions & 15 deletions .docker/mailslurper/mailslurper-cert.pem

This file was deleted.

15 changes: 0 additions & 15 deletions .docker/mailslurper/mailslurper-key.pem

This file was deleted.

12 changes: 0 additions & 12 deletions .docker/mailslurper/mailslurper.csr

This file was deleted.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ quickstart:

quickstart-dev:
docker build -f .docker/Dockerfile-build -t oryd/kratos:latest-sqlite .
docker pull oryd/kratos-selfservice-ui-node:latest
# docker pull oryd/kratos-selfservice-ui-node:latest
docker-compose -f quickstart.yml up --build --force-recreate

# Formats the code
Expand Down
4 changes: 2 additions & 2 deletions cmd/daemon/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ func sqa(cmd *cobra.Command, d driver.Driver) *metricsx.Service {
profile.PublicProfileManagementPath,
profile.PublicProfileManagementUpdatePath,
verify.PublicVerificationCompletePath,
strings.ReplaceAll(verify.PublicVerificationConfirmPath, ":code", ""),
strings.ReplaceAll(verify.PublicVerificationInitPath, ":via", ""),
strings.ReplaceAll(strings.ReplaceAll(verify.PublicVerificationConfirmPath, ":via", "email"), ":code", ""),
strings.ReplaceAll(verify.PublicVerificationInitPath, ":via", "email"),
verify.PublicVerificationRequestPath,
errorx.ErrorsPath,
},
Expand Down
1 change: 1 addition & 0 deletions contrib/quickstart/kratos/sqlite/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.sqlite
16 changes: 14 additions & 2 deletions courier/courier.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/pkg/errors"
"gopkg.in/gomail.v2"

"github.com/ory/x/errorsx"

"github.com/ory/kratos/driver/configuration"
"github.com/ory/kratos/x"
)
Expand Down Expand Up @@ -99,11 +101,14 @@ func (m *Courier) watchMessages(ctx context.Context, errChan chan error) {
if err := backoff.Retry(func() error {
messages, err := m.d.CourierPersister().NextMessages(ctx, 10)
if err != nil {
if errorsx.Cause(err) == ErrQueueEmpty {
return nil
}
return err
}

for k := range messages {
var msg Message = messages[k]
var msg = messages[k]

switch msg.Type {
case MessageTypeEmail:
Expand All @@ -120,7 +125,8 @@ func (m *Courier) watchMessages(ctx context.Context, errChan chan error) {
WithError(err).
WithField("smtp_server", fmt.Sprintf("%s:%d", m.dialer.Host, m.dialer.Port)).
WithField("smtp_ssl_enabled", m.dialer.SSL).
WithField("email_to", msg.Recipient).WithField("email_from", from).
// WithField("email_to", msg.Recipient).
WithField("message_from", from).
Error("Unable to send email using SMTP connection.")
continue
}
Expand All @@ -132,6 +138,12 @@ func (m *Courier) watchMessages(ctx context.Context, errChan chan error) {
Error(`Unable to set the message status to "sent".`)
return err
}

m.d.Logger().
WithField("message_id", msg.ID).
WithField("message_type", msg.Type).
WithField("message_subject", msg.Subject).
Debug("Courier sent out message.")
default:
return errors.Errorf("received unexpected message type: %d", msg.Type)
}
Expand Down
33 changes: 20 additions & 13 deletions docs/api.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -705,26 +705,23 @@
}
}
},
"/self-service/browser/flows/verification/confirm/{code}": {
"/self-service/browser/flows/verification/init/{via}": {
"get": {
"description": "This endpoint completes a browser-based verification flow.\n\n\u003e This endpoint is NOT INTENDED for API clients and only works with browsers (Chrome, Firefox, ...) and HTML Forms.\n\nMore information can be found at [ORY Kratos Email and Phone Verification Documentation](https://www.ory.sh/docs/kratos/selfservice/flows/verify-email-account-activation).",
"consumes": [
"application/json",
"application/x-www-form-urlencoded"
],
"description": "This endpoint initializes a browser-based profile management flow. Once initialized, the browser will be redirected to\n`urls.profile_ui` with the request ID set as a query parameter. If no valid user session exists, a login\nflow will be initialized.\n\n\u003e This endpoint is NOT INTENDED for API clients and only works\nwith browsers (Chrome, Firefox, ...).\n\nMore information can be found at [ORY Kratos Email and Phone Verification Documentation](https://www.ory.sh/docs/kratos/selfservice/flows/verify-email-account-activation).",
"schemes": [
"http",
"https"
],
"tags": [
"public"
],
"summary": "Complete the browser-based verification flows",
"operationId": "selfServiceBrowserVerify",
"summary": "Initialize browser-based verification flow",
"operationId": "initializeSelfServiceBrowserVerificationFlow",
"parameters": [
{
"type": "string",
"name": "code",
"description": "What to verify\n\nCurrently only \"email\" is supported.",
"name": "via",
"in": "path",
"required": true
}
Expand All @@ -742,19 +739,29 @@
}
}
},
"/self-service/browser/flows/verification/init/{via}": {
"/self-service/browser/flows/verification/{via}/confirm/{code}": {
"get": {
"description": "This endpoint initializes a browser-based profile management flow. Once initialized, the browser will be redirected to\n`urls.profile_ui` with the request ID set as a query parameter. If no valid user session exists, a login\nflow will be initialized.\n\n\u003e This endpoint is NOT INTENDED for API clients and only works\nwith browsers (Chrome, Firefox, ...).\n\nMore information can be found at [ORY Kratos Email and Phone Verification Documentation](https://www.ory.sh/docs/kratos/selfservice/flows/verify-email-account-activation).",
"description": "This endpoint completes a browser-based verification flow.\n\n\u003e This endpoint is NOT INTENDED for API clients and only works with browsers (Chrome, Firefox, ...) and HTML Forms.\n\nMore information can be found at [ORY Kratos Email and Phone Verification Documentation](https://www.ory.sh/docs/kratos/selfservice/flows/verify-email-account-activation).",
"consumes": [
"application/json",
"application/x-www-form-urlencoded"
],
"schemes": [
"http",
"https"
],
"tags": [
"public"
],
"summary": "Initialize browser-based verification flow",
"operationId": "initializeSelfServiceBrowserVerificationFlow",
"summary": "Complete the browser-based verification flows",
"operationId": "selfServiceBrowserVerify",
"parameters": [
{
"type": "string",
"name": "code",
"in": "path",
"required": true
},
{
"type": "string",
"description": "What to verify\n\nCurrently only \"email\" is supported.",
Expand Down
4 changes: 4 additions & 0 deletions identity/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,9 @@ func TestManager(t *testing.T) {
require.NoError(t, reg.IdentityManager().RefreshVerifyAddress(context.Background(), address))
assert.NotEqual(t, pc, address.Code)
assert.NotEqual(t, ea, address.ExpiresAt)

fromStore, err := reg.IdentityPool().GetIdentity(context.Background(), original.ID)
require.NoError(t, err)
assert.NotEqual(t, pc, fromStore.Addresses[0].Code)
})
}
11 changes: 10 additions & 1 deletion identity/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,17 @@ func TestPool(p PrivilegedPool) func(t *testing.T) {
}
})

t.Run("case=verify expired should not work", func(t *testing.T) {
address := createIdentityWithAddresses(t, -time.Minute, "[email protected]")
require.EqualError(t, errorsx.Cause(p.VerifyAddress(context.Background(), address.Code)), sqlcon.ErrNoRows.Error())
})

t.Run("case=create and verify", func(t *testing.T) {
require.EqualError(t, errorsx.Cause(p.VerifyAddress(context.Background(), "i-do-not-exist")), sqlcon.ErrNoRows.Error())
})

t.Run("case=create and verify", func(t *testing.T) {
address := createIdentityWithAddresses(t, time.Minute, "[email protected]")
address := createIdentityWithAddresses(t, time.Minute, "verify.TestPersister.VerifyAddress.valid@ory.sh")
require.NoError(t, p.VerifyAddress(context.Background(), address.Code))

actual, err := p.FindAddressByValue(context.Background(), address.Via, address.Value)
Expand Down
2 changes: 1 addition & 1 deletion internal/httpclient/client/public/public_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 17 additions & 5 deletions persistence/sql/persister_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ func (p *Persister) ListIdentities(ctx context.Context, limit, offset int) ([]id
is := make([]identity.Identity, 0)

/* #nosec G201 TableName is static */
if err := sqlcon.HandleError(p.GetConnection(ctx).RawQuery(fmt.Sprintf("SELECT * FROM %s LIMIT ? OFFSET ?", new(identity.Identity).TableName()), limit, offset).All(&is)); err != nil {
if err := sqlcon.HandleError(p.GetConnection(ctx).
RawQuery(fmt.Sprintf("SELECT * FROM %s LIMIT ? OFFSET ?", new(identity.Identity).TableName()), limit, offset).
Eager("Addresses").All(&is)); err != nil {
return nil, err
}

Expand Down Expand Up @@ -226,7 +228,7 @@ func (p *Persister) DeleteIdentity(ctx context.Context, id uuid.UUID) error {

func (p *Persister) GetIdentity(ctx context.Context, id uuid.UUID) (*identity.Identity, error) {
var i identity.Identity
if err := p.GetConnection(ctx).Find(&i, id); err != nil {
if err := p.GetConnection(ctx).Eager("Addresses").Find(&i, id); err != nil {
return nil, sqlcon.HandleError(err)
}
i.Credentials = nil
Expand Down Expand Up @@ -299,17 +301,27 @@ func (p *Persister) VerifyAddress(ctx context.Context, code string) error {
return err
}

return sqlcon.HandleError(p.GetConnection(ctx).RawQuery(
count, err := p.GetConnection(ctx).RawQuery(
/* #nosec G201 TableName is static */
fmt.Sprintf(
"UPDATE %s SET status = ?, verified = true, verified_at = ?, code = ? WHERE code = ?",
"UPDATE %s SET status = ?, verified = true, verified_at = ?, code = ? WHERE code = ? AND expires_at > ?",
new(identity.VerifiableAddress).TableName(),
),
identity.VerifiableAddressStatusCompleted,
time.Now().UTC().Round(time.Second),
newCode,
code,
).Exec())
time.Now().UTC(),
).ExecWithCount()
if err != nil {
return sqlcon.HandleError(err)
}

if count == 0 {
return sqlcon.HandleError(sqlcon.ErrNoRows)
}

return nil
}

func (p *Persister) UpdateVerifiableAddress(ctx context.Context, address *identity.VerifiableAddress) error {
Expand Down
6 changes: 4 additions & 2 deletions persistence/sql/persister_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ var _ session.Persister = new(Persister)

func (p *Persister) GetSession(ctx context.Context, sid uuid.UUID) (*session.Session, error) {
var s session.Session
if err := p.GetConnection(ctx).Eager().Find(&s, sid); err != nil {
if err := p.GetConnection(ctx).Find(&s, sid); err != nil {
return nil, sqlcon.HandleError(err)
}
if err := p.injectTraitsSchemaURL(s.Identity); err != nil {
i, err := p.GetIdentity(ctx, s.IdentityID)
if err != nil {
return nil, err
}
s.Identity = i
return &s, nil
}

Expand Down
Loading