Skip to content

Commit

Permalink
chore: annotate strategy spans with abort reasons
Browse files Browse the repository at this point in the history
  • Loading branch information
zepatrik committed Sep 2, 2024
1 parent 32737dc commit 9bfd131
Show file tree
Hide file tree
Showing 27 changed files with 139 additions and 75 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ require (
github.com/ory/jsonschema/v3 v3.0.8
github.com/ory/mail/v3 v3.0.0
github.com/ory/nosurf v1.2.7
github.com/ory/x v0.0.647
github.com/ory/x v0.0.655
github.com/peterhellberg/link v1.2.0
github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5
github.com/pkg/errors v0.9.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,8 @@ github.com/ory/nosurf v1.2.7 h1:YrHrbSensQyU6r6HT/V5+HPdVEgrOTMJiLoJABSBOp4=
github.com/ory/nosurf v1.2.7/go.mod h1:d4L3ZBa7Amv55bqxCBtCs63wSlyaiCkWVl4vKf3OUxA=
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpixwHiuAwpp0Ock6khSVHkrv6lQQU=
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
github.com/ory/x v0.0.647 h1:DVKgA3ykMB9qXuMdSl5C8SFWr3yw7Xe8jpSm0+iqGeU=
github.com/ory/x v0.0.647/go.mod h1:M+0EAXo7DT7Z2/Yrzvh4mgxOoV1fGI1jOKyAJ72d4Qs=
github.com/ory/x v0.0.655 h1:P+uwq8GE2YoB9sEyo/8nxuPwdHzBvXE/Xnkyujl7HeQ=
github.com/ory/x v0.0.655/go.mod h1:M+0EAXo7DT7Z2/Yrzvh4mgxOoV1fGI1jOKyAJ72d4Qs=
github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8=
github.com/pelletier/go-toml v1.9.5/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c=
github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6Wq+LM=
Expand Down
6 changes: 5 additions & 1 deletion selfservice/flow/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"net/http"
"strings"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/selfservice/strategy"
"github.com/ory/x/decoderx"
Expand Down Expand Up @@ -100,8 +103,9 @@ func MethodEnabledAndAllowedFromRequest(r *http.Request, flow FlowName, expected
return MethodEnabledAndAllowed(r.Context(), flow, expected, method.Method, d)
}

func MethodEnabledAndAllowed(ctx context.Context, flowName FlowName, expected, actual string, d config.Provider) error {
func MethodEnabledAndAllowed(ctx context.Context, _ FlowName, expected, actual string, d config.Provider) error {
if actual != expected {
trace.SpanFromContext(ctx).SetAttributes(attribute.String("not_responsible_reason", "method mismatch"))
return errors.WithStack(ErrStrategyNotResponsible)
}

Expand Down
4 changes: 3 additions & 1 deletion selfservice/strategy/code/strategy_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (s *Strategy) methodEnabledAndAllowedFromRequest(r *http.Request, f *login.
}

func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (_ *identity.Identity, err error) {
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.strategy.Login")
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.Strategy.Login")
defer otelx.End(span, &err)

if s.deps.Config().SelfServiceCodeStrategy(ctx).PasswordlessEnabled {
Expand All @@ -193,10 +193,12 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,

if p, err := s.methodEnabledAndAllowedFromRequest(r, f); errors.Is(err, flow.ErrStrategyNotResponsible) {
if !s.deps.Config().SelfServiceCodeStrategy(ctx).MFAEnabled {
span.SetAttributes(attribute.String("not_responsible_reason", "MFA is not enabled"))
return nil, err
}

if p == nil || len(p.Address) == 0 {
span.SetAttributes(attribute.String("not_responsible_reason", "method not set or address not set"))
return nil, err
}

Expand Down
5 changes: 3 additions & 2 deletions selfservice/strategy/code/strategy_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type updateRecoveryFlowWithCodeMethod struct {
TransientPayload json.RawMessage `json:"transient_payload,omitempty" form:"transient_payload"`
}

func (s Strategy) isCodeFlow(f *recovery.Flow) bool {
func (s *Strategy) isCodeFlow(f *recovery.Flow) bool {
value, err := f.Active.Value()
if err != nil {
return false
Expand All @@ -100,11 +100,12 @@ func (s Strategy) isCodeFlow(f *recovery.Flow) bool {
}

func (s *Strategy) Recover(w http.ResponseWriter, r *http.Request, f *recovery.Flow) (err error) {
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.strategy.Recover")
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.Strategy.Recover")
span.SetAttributes(attribute.String("selfservice_flows_recovery_use", s.deps.Config().SelfServiceFlowRecoveryUse(ctx)))
defer otelx.End(span, &err)

if !s.isCodeFlow(f) {
span.SetAttributes(attribute.String("not_responsible_reason", "not code flow"))
return errors.WithStack(flow.ErrStrategyNotResponsible)
}

Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/code/strategy_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (s *Strategy) validateAndGetCredentialsFromTraits(ctx context.Context, i *i
}

func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registration.Flow, i *identity.Identity) (err error) {
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.strategy.Register")
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.Strategy.Register")
defer otelx.End(span, &err)

if err := flow.MethodEnabledAndAllowedFromRequest(r, f.GetFlowName(), s.ID().String(), s.deps); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/code/strategy_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (body *updateVerificationFlowWithCodeMethod) getMethod() verification.Verif
}

func (s *Strategy) Verify(w http.ResponseWriter, r *http.Request, f *verification.Flow) (err error) {
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.strategy.Verify")
ctx, span := s.deps.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.code.Strategy.Verify")
span.SetAttributes(attribute.String("selfservice_flows_verification_use", s.deps.Config().SelfServiceFlowVerificationUse(ctx)))
defer otelx.End(span, &err)

Expand Down
4 changes: 4 additions & 0 deletions selfservice/strategy/idfirst/strategy_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package idfirst
import (
"net/http"

"go.opentelemetry.io/otel/attribute"

"github.com/ory/x/otelx"

"github.com/ory/kratos/schema"
Expand Down Expand Up @@ -45,10 +47,12 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
defer otelx.End(span, &err)

if !s.d.Config().SelfServiceLoginFlowIdentifierFirstEnabled(ctx) {
span.SetAttributes(attribute.String("not_responsible_reason", "strategy is not enabled"))
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel1); err != nil {
span.SetAttributes(attribute.String("not_responsible_reason", "requested AAL is not sufficient"))
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/link/strategy_recovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ type updateRecoveryFlowWithLinkMethod struct {
}

func (s *Strategy) Recover(w http.ResponseWriter, r *http.Request, f *recovery.Flow) (err error) {
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.link.strategy.Recover")
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.link.Strategy.Recover")
span.SetAttributes(attribute.String("selfservice_flows_recovery_use", s.d.Config().SelfServiceFlowRecoveryUse(ctx)))
defer otelx.End(span, &err)

Expand Down
6 changes: 3 additions & 3 deletions selfservice/strategy/link/strategy_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ func (s *Strategy) VerificationStrategyID() string {
return string(verification.VerificationStrategyLink)
}

func (s *Strategy) RegisterPublicVerificationRoutes(public *x.RouterPublic) {
func (s *Strategy) RegisterPublicVerificationRoutes(_ *x.RouterPublic) {
}

func (s *Strategy) RegisterAdminVerificationRoutes(admin *x.RouterAdmin) {
func (s *Strategy) RegisterAdminVerificationRoutes(_ *x.RouterAdmin) {
}

func (s *Strategy) PopulateVerificationMethod(r *http.Request, f *verification.Flow) error {
Expand Down Expand Up @@ -129,7 +129,7 @@ type updateVerificationFlowWithLinkMethod struct {
}

func (s *Strategy) Verify(w http.ResponseWriter, r *http.Request, f *verification.Flow) (err error) {
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.link.strategy.Verify")
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.link.Strategy.Verify")
span.SetAttributes(attribute.String("selfservice_flows_verification_use", s.d.Config().SelfServiceFlowVerificationUse(ctx)))
defer otelx.End(span, &err)

Expand Down
5 changes: 4 additions & 1 deletion selfservice/strategy/lookup/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"net/http"
"time"

"go.opentelemetry.io/otel/attribute"

"github.com/ory/x/otelx"

"github.com/ory/x/sqlcon"
Expand Down Expand Up @@ -90,11 +92,12 @@ type updateLoginFlowWithLookupSecretMethod struct {
Code string `json:"lookup_secret"`
}

func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (i *identity.Identity, err error) {
func (s *Strategy) Login(_ http.ResponseWriter, r *http.Request, f *login.Flow, sess *session.Session) (i *identity.Identity, err error) {
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.lookup.strategy.Login")
defer otelx.End(span, &err)

if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel2); err != nil {
span.SetAttributes(attribute.String("not_responsible_reason", "requested AAL is not AAL2"))
return nil, err
}

Expand Down
10 changes: 6 additions & 4 deletions selfservice/strategy/lookup/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"net/http"
"time"

"go.opentelemetry.io/otel/attribute"

"github.com/ory/x/otelx"

"github.com/tidwall/gjson"
Expand Down Expand Up @@ -106,7 +108,7 @@ func (s *Strategy) Settings(w http.ResponseWriter, r *http.Request, f *settings.
var p updateSettingsFlowWithLookupMethod
ctxUpdate, err := settings.PrepareUpdate(s.d, w, r, f, ss, settings.ContinuityKey(s.SettingsStrategyID()), &p)
if errors.Is(err, settings.ErrContinuePreviousAction) {
return ctxUpdate, s.continueSettingsFlow(r, ctxUpdate, p)
return ctxUpdate, s.continueSettingsFlow(ctx, r, ctxUpdate, p)
} else if err != nil {
return ctxUpdate, s.handleSettingsError(w, r, ctxUpdate, p, err)
}
Expand All @@ -122,12 +124,13 @@ func (s *Strategy) Settings(w http.ResponseWriter, r *http.Request, f *settings.
return nil, s.handleSettingsError(w, r, ctxUpdate, p, err)
}
} else {
span.SetAttributes(attribute.String("not_responsible_reason", "neither reveal, regenerate, confirm, nor disable was set"))
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

// This does not come from the payload!
p.Flow = ctxUpdate.Flow.ID.String()
if err := s.continueSettingsFlow(r, ctxUpdate, p); err != nil {
if err := s.continueSettingsFlow(ctx, r, ctxUpdate, p); err != nil {
return ctxUpdate, s.handleSettingsError(w, r, ctxUpdate, p, err)
}

Expand All @@ -146,8 +149,7 @@ func (s *Strategy) decodeSettingsFlow(r *http.Request, dest interface{}) error {
)
}

func (s *Strategy) continueSettingsFlow(r *http.Request, ctxUpdate *settings.UpdateContext, p updateSettingsFlowWithLookupMethod) error {
ctx := r.Context()
func (s *Strategy) continueSettingsFlow(ctx context.Context, r *http.Request, ctxUpdate *settings.UpdateContext, p updateSettingsFlowWithLookupMethod) error {
if p.ConfirmLookup || p.RevealLookup || p.RegenerateLookup || p.DisableLookup {
if err := flow.MethodEnabledAndAllowed(ctx, flow.SettingsFlow, s.SettingsStrategyID(), s.SettingsStrategyID(), s.d); err != nil {
return err
Expand Down
7 changes: 6 additions & 1 deletion selfservice/strategy/oidc/strategy_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"strings"
"time"

"go.opentelemetry.io/otel/attribute"

"github.com/ory/kratos/selfservice/strategy/idfirst"
"github.com/ory/x/stringsx"

Expand Down Expand Up @@ -192,10 +194,11 @@ func (s *Strategy) processLogin(ctx context.Context, w http.ResponseWriter, r *h
}

func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, _ *session.Session) (i *identity.Identity, err error) {
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.oidc.strategy.Login")
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.oidc.Strategy.Login")
defer otelx.End(span, &err)

if err := login.CheckAAL(f, identity.AuthenticatorAssuranceLevel1); err != nil {
span.SetAttributes(attribute.String("not_responsible_reason", "requested AAL is not AAL1"))
return nil, err
}

Expand All @@ -210,6 +213,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,

pid := p.Provider // this can come from both url query and post body
if pid == "" {
span.SetAttributes(attribute.String("not_responsible_reason", "provider ID missing"))
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

Expand All @@ -220,6 +224,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,
WithField("provider", p.Provider).
WithField("method", p.Method).
Warn("The payload includes a `provider` field but is using a method other than `oidc`. Therefore, social sign in will not be executed.")
span.SetAttributes(attribute.String("not_responsible_reason", "method is not oidc"))
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

Expand Down
6 changes: 5 additions & 1 deletion selfservice/strategy/oidc/strategy_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"strings"
"time"

"go.opentelemetry.io/otel/attribute"

"github.com/dgraph-io/ristretto"
"github.com/gofrs/uuid"
"github.com/julienschmidt/httprouter"
Expand Down Expand Up @@ -150,7 +152,7 @@ func (s *Strategy) newLinkDecoder(p interface{}, r *http.Request) error {
}

func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registration.Flow, i *identity.Identity) (err error) {
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.oidc.strategy.Register")
ctx, span := s.d.Tracer(r.Context()).Tracer().Start(r.Context(), "selfservice.strategy.oidc.Strategy.Register")
defer otelx.End(span, &err)

var p UpdateRegistrationFlowWithOidcMethod
Expand All @@ -160,6 +162,7 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat

pid := p.Provider // this can come from both url query and post body
if pid == "" {
span.SetAttributes(attribute.String("not_responsible_reason", "provider ID missing"))
return errors.WithStack(flow.ErrStrategyNotResponsible)
}

Expand All @@ -174,6 +177,7 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat
WithField("provider", p.Provider).
WithField("method", p.Method).
Warn("The payload includes a `provider` field but is using a method other than `oidc`. Therefore, social sign in will not be executed.")
span.SetAttributes(attribute.String("not_responsible_reason", "method is not oidc"))
return errors.WithStack(flow.ErrStrategyNotResponsible)
}

Expand Down
24 changes: 12 additions & 12 deletions selfservice/strategy/oidc/strategy_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"net/http"
"time"

"go.opentelemetry.io/otel/attribute"

"github.com/ory/x/otelx"

"github.com/ory/x/sqlxx"
Expand Down Expand Up @@ -278,13 +280,13 @@ func (s *Strategy) Settings(w http.ResponseWriter, r *http.Request, f *settings.
return nil, errors.WithStack(herodot.ErrNotFound.WithReason(strategy.EndpointDisabledMessage))
}

if l := len(p.Link); l > 0 {
if len(p.Link) > 0 {
if err := s.initLinkProvider(w, r, ctxUpdate, &p); err != nil {
return nil, err
}

return ctxUpdate, nil
} else if u := len(p.Unlink); u > 0 {
} else if len(p.Unlink) > 0 {
if err := s.unlinkProvider(w, r, ctxUpdate, &p); err != nil {
return nil, err
}
Expand All @@ -297,36 +299,34 @@ func (s *Strategy) Settings(w http.ResponseWriter, r *http.Request, f *settings.
return nil, s.handleSettingsError(w, r, ctxUpdate, &p, err)
}

if len(p.Link+p.Unlink) == 0 {
if len(p.Link)+len(p.Unlink) == 0 {
span.SetAttributes(attribute.String("not_responsible_reason", "neither link nor unlink set"))
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

if !s.d.Config().SelfServiceStrategy(ctx, s.SettingsStrategyID()).Enabled {
return nil, errors.WithStack(herodot.ErrNotFound.WithReason(strategy.EndpointDisabledMessage))
}

if l, u := len(p.Link), len(p.Unlink); l > 0 && u > 0 {
switch l, u := len(p.Link), len(p.Unlink); {
case l > 0 && u > 0:
return nil, s.handleSettingsError(w, r, ctxUpdate, &p, errors.WithStack(&jsonschema.ValidationError{
Message: "it is not possible to link and unlink providers in the same request",
InstancePtr: "#/",
}))
} else if l > 0 {
case l > 0:
if err := s.initLinkProvider(w, r, ctxUpdate, &p); err != nil {
return nil, err
}
return ctxUpdate, nil
} else if u > 0 {
case u > 0:
if err := s.unlinkProvider(w, r, ctxUpdate, &p); err != nil {
return nil, err
}

return ctxUpdate, nil
}

return nil, s.handleSettingsError(w, r, ctxUpdate, &p, errors.WithStack(errors.WithStack(&jsonschema.ValidationError{
Message: "missing properties: link, unlink", InstancePtr: "#/",
Context: &jsonschema.ValidationErrorContextRequired{Missing: []string{"link", "unlink"}},
})))
// this case should never be reached as we previously checked whether link and unlink are both empty
return nil, errors.WithStack(flow.ErrStrategyNotResponsible)
}

func (s *Strategy) isLinkable(ctx context.Context, ctxUpdate *settings.UpdateContext, toLink string) (*identity.Identity, error) {
Expand Down
Loading

0 comments on commit 9bfd131

Please sign in to comment.