Skip to content

Commit

Permalink
feat: block linking credentials if they do not match identifier of lo…
Browse files Browse the repository at this point in the history
…gged in identity (CORE-2006)
  • Loading branch information
splaunov committed May 2, 2023
1 parent a38a5b1 commit 4d2551e
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 12 deletions.
1 change: 1 addition & 0 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ func init() {
"NewInfoSelfServiceLoginContinue": text.NewInfoSelfServiceLoginContinue(),
"NewErrorValidationSuchNoWebAuthnUser": text.NewErrorValidationSuchNoWebAuthnUser(),
"NewInfoSelfServiceLoginLinkCredentials": text.NewInfoSelfServiceLoginLinkCredentials(),
"NewErrorValidationLoginLinkedCredentialsDoNotMatch": text.NewErrorValidationLoginLinkedCredentialsDoNotMatch(),
}
}

Expand Down
10 changes: 10 additions & 0 deletions schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,3 +303,13 @@ func NewNoWebAuthnCredentials() error {
Messages: new(text.Messages).Add(text.NewErrorValidationSuchNoWebAuthnUser()),
})
}

func NewLinkedCredentialsDoNotMatch() error {
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Message: `linked credentials do not match`,
InstancePtr: "#/",
},
Messages: new(text.Messages).Add(text.NewErrorValidationLoginLinkedCredentialsDoNotMatch()),
})
}
5 changes: 3 additions & 2 deletions selfservice/flow/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ const InternalContextDuplicateCredentialsPath = "registration_duplicate_credenti
const InternalContextLinkCredentialsPath = "link_credentials"

type RegistrationDuplicateCredentials struct {
CredentialsType identity.CredentialsType
CredentialsConfig sqlxx.JSONRawMessage
CredentialsType identity.CredentialsType
CredentialsConfig sqlxx.JSONRawMessage
DuplicateIdentifier string
}

func AppendFlowTo(src *url.URL, id uuid.UUID) *url.URL {
Expand Down
20 changes: 20 additions & 0 deletions selfservice/flow/login/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package login

import (
"context"
_ "embed"
"encoding/json"
"fmt"
Expand Down Expand Up @@ -67,6 +68,7 @@ type (
x.CSRFProvider
config.Provider
ErrorHandlerProvider
identity.PrivilegedPoolProvider
}
HandlerProvider interface {
LoginHandler() *Handler
Expand Down Expand Up @@ -817,6 +819,9 @@ func (h *Handler) linkCredentials(r *http.Request, s *session.Session, i *identi
}

if lc.CredentialsType != "" {
if err := h.checkDuplecateCredentialsIdentifierMatch(r.Context(), i.ID, lc.DuplicateIdentifier); err != nil {
return err
}
strategy, err := h.d.AllLoginStrategies().Strategy(lc.CredentialsType)
if err != nil {
return err
Expand Down Expand Up @@ -847,3 +852,18 @@ func (h *Handler) getInternalContextLinkCredentials(f *Flow, internalContextPath
}
return nil
}

func (h *Handler) checkDuplecateCredentialsIdentifierMatch(ctx context.Context, identityID uuid.UUID, match string) error {
i, err := h.d.PrivilegedIdentityPool().GetIdentityConfidential(ctx, identityID)
if err != nil {
return err
}
for _, credentials := range i.Credentials {
for _, identifier := range credentials.Identifiers {
if identifier == match {
return nil
}
}
}
return schema.NewLinkedCredentialsDoNotMatch()
}
26 changes: 24 additions & 2 deletions selfservice/flow/registration/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type (
executorDependencies interface {
config.Provider
identity.ManagementProvider
identity.PrivilegedPoolProvider
identity.ValidationProvider
login.FlowPersistenceProvider
login.StrategyProvider
Expand Down Expand Up @@ -151,9 +152,14 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
_, ok := strategy.(login.LinkableStrategy)

if ok {
duplicateIdentifier, err := e.getDuplicateIdentifier(r.Context(), i)
if err != nil {
return err
}
registrationDuplicateCredentials := flow.RegistrationDuplicateCredentials{
CredentialsType: ct,
CredentialsConfig: i.Credentials[ct].Config,
CredentialsType: ct,
CredentialsConfig: i.Credentials[ct].Config,
DuplicateIdentifier: duplicateIdentifier,
}
loginFlowID, err := a.GetOuterLoginFlowID()
if err != nil {
Expand Down Expand Up @@ -290,6 +296,22 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
return nil
}

func (e *HookExecutor) getDuplicateIdentifier(ctx context.Context, i *identity.Identity) (string, error) {
for ct, credentials := range i.Credentials {
for _, identifier := range credentials.Identifiers {
_, _, err := e.d.PrivilegedIdentityPool().FindByCredentialsIdentifier(ctx, ct, identifier)
if err != nil {
if errors.Is(err, sqlcon.ErrNoRows) {
continue
}
return "", err
}
return identifier, nil
}
}
return "", errors.New("Duplicate credential not found")
}

func (e *HookExecutor) PreRegistrationHook(w http.ResponseWriter, r *http.Request, a *Flow) error {
for _, executor := range e.d.PreRegistrationHooks(r.Context()) {
if err := executor.ExecuteRegistrationPreHook(w, r, a); err != nil {
Expand Down
24 changes: 23 additions & 1 deletion selfservice/strategy/oidc/strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"net/http/cookiejar"
"net/http/httptest"
"net/url"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -550,6 +551,7 @@ func TestStrategy(t *testing.T) {

t.Run("case=registration should start new login flow if duplicate credentials detected", func(t *testing.T) {
subject = "[email protected]"
subject2 := "[email protected]"
scope = []string{"openid"}
password := "lwkj52sdkjf"

Expand All @@ -563,8 +565,15 @@ func TestStrategy(t *testing.T) {
Config: sqlxx.JSONRawMessage(`{"hashed_password":"` + string(p) + `"}`),
})
i.Traits = identity.Traits(`{"subject":"` + subject + `"}`)

require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i))

i2 := identity.NewIdentity(config.DefaultIdentityTraitsSchemaID)
i2.SetCredentials(identity.CredentialsTypePassword, identity.Credentials{
Identifiers: []string{subject2},
Config: sqlxx.JSONRawMessage(`{"hashed_password":"` + string(p) + `"}`),
})
i2.Traits = identity.Traits(`{"subject":"` + subject2 + `"}`)
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), i2))
})

c := testhelpers.NewClientWithCookieJar(t, nil, false)
Expand Down Expand Up @@ -594,6 +603,19 @@ func TestStrategy(t *testing.T) {
assert.NotNil(t, loginFlow, "%s", body)
})

t.Run("case=should fail login if existing identity identifier doesn't match", func(t *testing.T) {
res, err := c.PostForm(loginFlow.UI.Action, url.Values{
"csrf_token": {loginFlow.CSRFToken},
"method": {"password"},
"identifier": {subject2},
"password": {password}})
require.NoError(t, err, loginFlow.UI.Action)
body, err := io.ReadAll(res.Body)
require.NoError(t, res.Body.Close())
require.NoError(t, err)
assert.Equal(t, strconv.Itoa(int(text.ErrorValidationLoginLinkedCredentialsDoNotMatch)), gjson.GetBytes(body, "ui.messages.0.id").String(), "%s", body)
})

t.Run("case=should link oidc credentials to existing identity", func(t *testing.T) {
res, err := c.PostForm(loginFlow.UI.Action, url.Values{
"csrf_token": {loginFlow.CSRFToken},
Expand Down
15 changes: 8 additions & 7 deletions text/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,14 @@ const (
)

const (
ErrorValidationLogin ID = 4010000 + iota // 4010000
ErrorValidationLoginFlowExpired // 4010001
ErrorValidationLoginNoStrategyFound // 4010002
ErrorValidationRegistrationNoStrategyFound // 4010003
ErrorValidationSettingsNoStrategyFound // 4010004
ErrorValidationRecoveryNoStrategyFound // 4010005
ErrorValidationVerificationNoStrategyFound // 4010006
ErrorValidationLogin ID = 4010000 + iota // 4010000
ErrorValidationLoginFlowExpired // 4010001
ErrorValidationLoginNoStrategyFound // 4010002
ErrorValidationRegistrationNoStrategyFound // 4010003
ErrorValidationSettingsNoStrategyFound // 4010004
ErrorValidationRecoveryNoStrategyFound // 4010005
ErrorValidationVerificationNoStrategyFound // 4010006
ErrorValidationLoginLinkedCredentialsDoNotMatch // 4010007
)

const (
Expand Down
8 changes: 8 additions & 0 deletions text/message_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,11 @@ func NewInfoSelfServiceLoginLinkCredentials() *Message {
Context: context(nil),
}
}

func NewErrorValidationLoginLinkedCredentialsDoNotMatch() *Message {
return &Message{
ID: ErrorValidationLoginLinkedCredentialsDoNotMatch,
Text: "Linked credentials do not match.",
Type: Error,
}
}

0 comments on commit 4d2551e

Please sign in to comment.