Skip to content

Commit

Permalink
fix: resolve merge regressions
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Apr 9, 2021
1 parent 84a9315 commit 9862ac7
Show file tree
Hide file tree
Showing 16 changed files with 42 additions and 134 deletions.
2 changes: 1 addition & 1 deletion courier/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestGetTemplateType(t *testing.T) {
}

func TestNewEmailTemplateFromMessage(t *testing.T) {
conf := internal.NewConfigurationWithDefaults()
conf := internal.NewConfigurationWithDefaults(t)
for tmplType, expectedTmpl := range map[courier.TemplateType]courier.EmailTemplate{
courier.TypeRecoveryInvalid: template.NewRecoveryInvalid(conf, &template.RecoveryInvalidModel{To: "foo"}),
courier.TypeRecoveryValid: template.NewRecoveryValid(conf, &template.RecoveryValidModel{To: "bar", RecoveryURL: "http://foo.bar"}),
Expand Down
3 changes: 1 addition & 2 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"time"

"github.com/google/uuid"

"github.com/ory/x/dbal"

"github.com/ory/x/stringsx"
Expand All @@ -25,13 +26,11 @@ import (
kjson "github.com/knadh/koanf/parsers/json"
"github.com/pkg/errors"
"github.com/rs/cors"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"

"github.com/ory/x/configx"
"github.com/ory/x/jsonx"
"github.com/ory/x/logrusx"
"github.com/ory/x/stringsx"
"github.com/ory/x/tracing"
)

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ require (
github.com/ory/go-acc v0.2.6
github.com/ory/go-convenience v0.1.0
github.com/ory/graceful v0.1.1
github.com/ory/herodot v0.9.3
github.com/ory/herodot v0.9.5
github.com/ory/jsonschema/v3 v3.0.3
github.com/ory/kratos-client-go v0.5.4-alpha.1.0.20210308170950-06c2c1c071a8
github.com/ory/kratos/corp v0.0.0-00010101000000-000000000000
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1104,8 +1104,8 @@ github.com/ory/herodot v0.7.0/go.mod h1:YXKOfAXYdQojDP5sD8m0ajowq3+QXNdtxA+QiUXB
github.com/ory/herodot v0.8.3/go.mod h1:rvLjxOAlU5omtmgjCfazQX2N82EpMfl3BytBWc1jjsk=
github.com/ory/herodot v0.9.0/go.mod h1:GYF7mp8/WFRYDYJBR989lipjgx3NTjjdVdUC+hpB8mc=
github.com/ory/herodot v0.9.2/go.mod h1:Da2HXR8mpwPbPrH+Gv9qV8mM5gI3v+PoJ69BA4l2RAk=
github.com/ory/herodot v0.9.3 h1:vfhidpS1fafk8FzQh7lEhImkp72UUU7x0G3gM1Un5CE=
github.com/ory/herodot v0.9.3/go.mod h1:g3yAI/d6wPdGnOt3dbYUj5JGTZBNuUVLuuDqHnfc1lM=
github.com/ory/herodot v0.9.5 h1:CkLzuip0Xc+qqOMYfu6JPEffDWGXDxk71U+/1fcDPiw=
github.com/ory/herodot v0.9.5/go.mod h1:g3yAI/d6wPdGnOt3dbYUj5JGTZBNuUVLuuDqHnfc1lM=
github.com/ory/jsonschema/v3 v3.0.1/go.mod h1:jgLHekkFk0uiGdEWGleC+tOm6JSSP8cbf17PnBuGXlw=
github.com/ory/jsonschema/v3 v3.0.2/go.mod h1:BPH8eu7Ws2kxeu4NRA0Pqrm15+fOJDPfQxb55v2sRXA=
github.com/ory/jsonschema/v3 v3.0.3 h1:Y7KT4n84ROq8pJ3IMf9JDAulXqYKSU5xUtHjdQFbCLI=
Expand Down
2 changes: 1 addition & 1 deletion hash/hash_comparator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func CompareArgon2id(_ context.Context, password []byte, hash []byte) error {
}

// Derive the key from the other password using the same parameters.
otherHash := argon2.IDKey([]byte(password), salt, p.Iterations, p.Memory, p.Parallelism, p.KeyLength)
otherHash := argon2.IDKey([]byte(password), salt, p.Iterations, uint32(p.Memory), p.Parallelism, p.KeyLength)

// Check that the contents of the hashed passwords are identical. Note
// that we are using the subtle.ConstantTimeCompare() function for this
Expand Down
60 changes: 2 additions & 58 deletions hash/hasher_argon2.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ func (h *Argon2) Generate(ctx context.Context, password []byte) ([]byte, error)
// Pass the plaintext password, salt and parameters to the argon2.IDKey
// function. This will generate a hash of the password using the Argon2id
// variant.
hash := argon2.IDKey(password, salt, p.Iterations, toKB(p.Memory), p.Parallelism, p.KeyLength)
hash := argon2.IDKey([]byte(password), salt, p.Iterations, toKB(p.Memory), p.Parallelism, p.KeyLength)

var b bytes.Buffer
if _, err := fmt.Fprintf(
&b,
"$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s",
argon2.Version, p.Memory, p.Iterations, p.Parallelism,
argon2.Version, toKB(p.Memory), p.Iterations, p.Parallelism,
base64.RawStdEncoding.EncodeToString(salt),
base64.RawStdEncoding.EncodeToString(hash),
); err != nil {
Expand All @@ -63,59 +63,3 @@ func (h *Argon2) Generate(ctx context.Context, password []byte) ([]byte, error)

return b.Bytes(), nil
}

func (h *Argon2) Compare(ctx context.Context, password []byte, hash []byte) error {
// Extract the parameters, salt and derived key from the encoded password
// hash.
p, salt, hash, err := decodeHash(string(hash))
if err != nil {
return err
}

// Derive the key from the other password using the same parameters.
otherHash := argon2.IDKey([]byte(password), salt, p.Iterations, toKB(p.Memory), p.Parallelism, p.KeyLength)

// Check that the contents of the hashed passwords are identical. Note
// that we are using the subtle.ConstantTimeCompare() function for this
// to help prevent timing attacks.
if subtle.ConstantTimeCompare(hash, otherHash) == 1 {
return nil
}
return ErrMismatchedHashAndPassword
}

func decodeHash(encodedHash string) (p *config.Argon2, salt, hash []byte, err error) {
parts := strings.Split(encodedHash, "$")
if len(parts) != 6 {
return nil, nil, nil, ErrInvalidHash
}

var version int
_, err = fmt.Sscanf(parts[2], "v=%d", &version)
if err != nil {
return nil, nil, nil, err
}
if version != argon2.Version {
return nil, nil, nil, ErrIncompatibleVersion
}

p = new(config.Argon2)
_, err = fmt.Sscanf(parts[3], "m=%d,t=%d,p=%d", &p.Memory, &p.Iterations, &p.Parallelism)
if err != nil {
return nil, nil, nil, err
}

salt, err = base64.RawStdEncoding.DecodeString(parts[4])
if err != nil {
return nil, nil, nil, err
}
p.SaltLength = uint32(len(salt))

hash, err = base64.RawStdEncoding.DecodeString(parts[5])
if err != nil {
return nil, nil, nil, err
}
p.KeyLength = uint32(len(hash))

return p, salt, hash, nil
}
1 change: 0 additions & 1 deletion selfservice/flow/login/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package login_test

import (
"crypto/tls"
"github.com/ory/kratos/internal"
"fmt"
"net/http"
"testing"
Expand Down
4 changes: 2 additions & 2 deletions selfservice/flow/verification/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ func NewFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Reques
return f, nil
}

func NewPostHookFlow(exp time.Duration, csrf string, r *http.Request, strategies Strategies, original flow.Flow) (*Flow, error) {
f, err := NewFlow(exp, csrf, r, strategies, original.GetType())
func NewPostHookFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Request, strategies Strategies, original flow.Flow) (*Flow, error) {
f, err := NewFlow(conf, exp, csrf, r, strategies, original.GetType())
if err != nil {
return nil, err
}
Expand Down
24 changes: 14 additions & 10 deletions selfservice/flow/verification/flow_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package verification
package verification_test

import (
"context"
Expand All @@ -8,6 +8,9 @@ import (
"testing"
"time"

"github.com/ory/kratos/internal"
"github.com/ory/kratos/selfservice/flow/verification"

"github.com/ory/kratos/driver/config"
"github.com/ory/x/configx"
"github.com/ory/x/logrusx"
Expand All @@ -25,18 +28,18 @@ func TestFlow(t *testing.T) {
conf, err := config.New(context.Background(), logrusx.New("", ""), configx.SkipValidation())
require.NoError(t, err)

must := func(r *Flow, err error) *Flow {
must := func(r *verification.Flow, err error) *verification.Flow {
require.NoError(t, err)
return r
}

u := &http.Request{URL: urlx.ParseOrPanic("http://foo/bar/baz"), Host: "foo"}
for k, tc := range []struct {
r *Flow
r *verification.Flow
expectErr bool
}{
{r: must(NewFlow(conf, time.Hour, "", u, nil, flow.TypeBrowser))},
{r: must(NewFlow(conf, -time.Hour, "", u, nil, flow.TypeBrowser)), expectErr: true},
{r: must(verification.NewFlow(conf, time.Hour, "", u, nil, flow.TypeBrowser))},
{r: must(verification.NewFlow(conf, -time.Hour, "", u, nil, flow.TypeBrowser)), expectErr: true},
} {
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
err := tc.r.Valid()
Expand All @@ -49,8 +52,8 @@ func TestFlow(t *testing.T) {
})
}

assert.EqualValues(t, StateChooseMethod,
must(NewFlow(conf, time.Hour, "", u, nil, flow.TypeBrowser)).State)
assert.EqualValues(t, verification.StateChooseMethod,
must(verification.NewFlow(conf, time.Hour, "", u, nil, flow.TypeBrowser)).State)
}

func TestGetType(t *testing.T) {
Expand All @@ -59,26 +62,27 @@ func TestGetType(t *testing.T) {
flow.TypeBrowser,
} {
t.Run(fmt.Sprintf("case=%s", ft), func(t *testing.T) {
r := &Flow{Type: ft}
r := &verification.Flow{Type: ft}
assert.Equal(t, ft, r.GetType())
})
}
}

func TestGetRequestURL(t *testing.T) {
expectedURL := "http://foo/bar/baz"
f := &Flow{RequestURL: expectedURL}
f := &verification.Flow{RequestURL: expectedURL}
assert.Equal(t, expectedURL, f.GetRequestURL())
}

func TestNewPostHookFlow(t *testing.T) {
conf := internal.NewConfigurationWithDefaults(t)
u := &http.Request{URL: urlx.ParseOrPanic("http://foo/bar/baz"), Host: "foo"}
expectReturnTo := func(t *testing.T, originalFlowRequestQueryParams url.Values, expectedReturnTo string) {
originalFlow := registration.Flow{
RequestURL: "http://foo.com/bar?" + originalFlowRequestQueryParams.Encode(),
}
t.Log(originalFlow.RequestURL)
f, err := NewPostHookFlow(time.Second, "", u, nil, &originalFlow)
f, err := verification.NewPostHookFlow(conf, time.Second, "", u, nil, &originalFlow)
require.NoError(t, err)
url, err := urlx.Parse(f.RequestURL)
require.NoError(t, err)
Expand Down
13 changes: 3 additions & 10 deletions selfservice/hook/verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ package hook
import (
"net/http"

"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/x"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/flow"
Expand Down Expand Up @@ -57,7 +53,9 @@ func (e *Verifier) do(r *http.Request, i *identity.Identity, f flow.Flow) error
continue
}

verificationFlow, err := verification.NewPostHookFlow(e.r.Config(r.Context()).SelfServiceFlowVerificationRequestLifespan(), e.r.GenerateCSRFToken(r), r, e.r.VerificationStrategies(r.Context()), f)
verificationFlow, err := verification.NewPostHookFlow(e.r.Config(r.Context()),
e.r.Config(r.Context()).SelfServiceFlowVerificationRequestLifespan(),
e.r.GenerateCSRFToken(r), r, e.r.VerificationStrategies(r.Context()), f)
if err != nil {
return err
}
Expand All @@ -66,11 +64,6 @@ func (e *Verifier) do(r *http.Request, i *identity.Identity, f flow.Flow) error
return err
}

token := link.NewVerificationToken(address, e.r.Config(r.Context()).SelfServiceFlowVerificationRequestLifespan())
if err := e.r.VerificationTokenPersister().CreateVerificationToken(r.Context(), token); err != nil {
return err
}

token := link.NewSelfServiceVerificationToken(address, verificationFlow)
if err := e.r.VerificationTokenPersister().CreateVerificationToken(r.Context(), token); err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions selfservice/hook/verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

Expand Down Expand Up @@ -78,7 +77,7 @@ func TestVerifier(t *testing.T) {

h := hook.NewVerifier(reg)
require.NoError(t, hf(h, i, originalFlow))
expectedVerificationFlow, err := verification.NewPostHookFlow(conf.SelfServiceFlowVerificationRequestLifespan(), "", u, reg.VerificationStrategies(context.Background()), originalFlow)
expectedVerificationFlow, err := verification.NewPostHookFlow(conf, conf.SelfServiceFlowVerificationRequestLifespan(), "", u, reg.VerificationStrategies(context.Background()), originalFlow)
require.NoError(t, err)

var verificationFlow verification.Flow
Expand Down
21 changes: 8 additions & 13 deletions selfservice/strategy/link/strategy_verification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,20 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/ory/kratos/ui/node"
"net/http"
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/ory/kratos/selfservice/strategy/link"
"github.com/ory/kratos/ui/node"

"github.com/ory/x/pointerx"

"github.com/ory/kratos-client-go"

"github.com/ory/x/ioutilx"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"

"github.com/ory/x/assertx"
"github.com/ory/x/sqlxx"

"github.com/ory/kratos-client-go"
"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/internal"
Expand All @@ -35,6 +27,10 @@ import (
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/text"
"github.com/ory/kratos/x"
"github.com/ory/x/assertx"
"github.com/ory/x/ioutilx"
"github.com/ory/x/pointerx"
"github.com/ory/x/sqlxx"
)

func TestVerification(t *testing.T) {
Expand Down Expand Up @@ -276,7 +272,7 @@ func TestVerification(t *testing.T) {
})

newValidFlow := func(t *testing.T, requestURL string) (*verification.Flow, *link.VerificationToken) {
f, err := verification.NewFlow(time.Hour, x.FakeCSRFToken, httptest.NewRequest("GET", requestURL, nil), nil, flow.TypeBrowser)
f, err := verification.NewFlow(conf, time.Hour, x.FakeCSRFToken, httptest.NewRequest("GET", requestURL, nil), nil, flow.TypeBrowser)
require.NoError(t, err)
f.State = verification.StateEmailSent
require.NoError(t, reg.VerificationFlowPersister().CreateVerificationFlow(context.Background(), f))
Expand All @@ -287,7 +283,6 @@ func TestVerification(t *testing.T) {
token := link.NewSelfServiceVerificationToken(&identityToVerify.VerifiableAddresses[0], f)
require.NoError(t, reg.VerificationTokenPersister().CreateVerificationToken(context.Background(), token))
return f, token

}

t.Run("case=respects return_to URI parameter", func(t *testing.T) {
Expand All @@ -310,7 +305,7 @@ func TestVerification(t *testing.T) {
`{"csrf_token":"%s","email":"%s"}`, flow.CSRFToken, verificationEmail,
)

res, err := client.Post(public.URL+link.RouteVerification+"?"+url.Values{"token": {token.Token}}.Encode(), "application/json", bytes.NewBuffer([]byte(body)))
res, err := client.Post(public.URL+verification.RouteSubmitFlow+"?"+url.Values{"flow": {flow.ID.String()}, "token": {token.Token}}.Encode(), "application/json", bytes.NewBuffer([]byte(body)))
require.NoError(t, err)
assert.Equal(t, http.StatusFound, res.StatusCode)
redirectURL, err := res.Location()
Expand Down
11 changes: 0 additions & 11 deletions selfservice/strategy/link/token_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,6 @@ func NewSelfServiceVerificationToken(address *identity.VerifiableAddress, f *ver
FlowID: uuid.NullUUID{UUID: f.ID, Valid: true}}
}

func NewVerificationToken(address *identity.VerifiableAddress, expiresIn time.Duration) *VerificationToken {
now := time.Now().UTC()
return &VerificationToken{
ID: x.NewUUID(),
Token: randx.MustString(32, randx.AlphaNum),
VerifiableAddress: address,
ExpiresAt: now.Add(expiresIn),
IssuedAt: now,
}
}

func (f *VerificationToken) Valid() error {
if f.ExpiresAt.Before(time.Now()) {
return errors.WithStack(verification.NewFlowExpiredError(f.ExpiresAt))
Expand Down
4 changes: 2 additions & 2 deletions selfservice/strategy/oidc/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (s *Strategy) ID() identity.CredentialsType {
return identity.CredentialsTypeOIDC
}

func (s *Strategy) validateFlow(ctx context.Context, r *http.Request, rid uuid.UUID) (ider, error) {
func (s *Strategy) validateFlow(ctx context.Context, r *http.Request, rid uuid.UUID) (flow.Flow, error) {
if x.IsZeroUUID(rid) {
return nil, errors.WithStack(herodot.ErrBadRequest.WithReason("The session cookie contains invalid values and the flow could not be executed. Please try again."))
}
Expand Down Expand Up @@ -201,7 +201,7 @@ func (s *Strategy) validateFlow(ctx context.Context, r *http.Request, rid uuid.U
return ar, err // this must return the error
}

func (s *Strategy) validateCallback(w http.ResponseWriter, r *http.Request) (ider, *authCodeContainer, error) {
func (s *Strategy) validateCallback(w http.ResponseWriter, r *http.Request) (flow.Flow, *authCodeContainer, error) {
var (
code = r.URL.Query().Get("code")
state = r.URL.Query().Get("state")
Expand Down
Loading

0 comments on commit 9862ac7

Please sign in to comment.