Skip to content

Commit

Permalink
refactor(password): internals and deprecated fields
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Mar 7, 2022
1 parent 77afc6f commit a7784bd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 37 deletions.
3 changes: 2 additions & 1 deletion selfservice/strategy/password/.schema/login.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"type": "string",
"minLength": 1
},
"password_identifier": {
"identifier": {
"type": "string",
"minLength": 1
},
Expand All @@ -25,6 +25,7 @@
"if": {
"properties": {
"password_identifier": {
"type": "string",
"minLength": 1
}
},
Expand Down
22 changes: 5 additions & 17 deletions selfservice/strategy/password/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"bytes"
"context"
"encoding/json"
"github.com/ory/x/stringsx"
"github.com/ory/kratos/selfservice/flowhelpers"
"net/http"
"time"

"github.com/ory/x/stringsx"

"github.com/gofrs/uuid"

"github.com/ory/kratos/session"
Expand All @@ -33,7 +35,7 @@ func (s *Strategy) RegisterLoginRoutes(r *x.RouterPublic) {
func (s *Strategy) handleLoginError(w http.ResponseWriter, r *http.Request, f *login.Flow, payload *submitSelfServiceLoginFlowWithPasswordMethodBody, err error) error {
if f != nil {
f.UI.Nodes.ResetNodes("password")
f.UI.Nodes.SetValueAttribute("identifier", payload.Identifier)
f.UI.Nodes.SetValueAttribute("identifier", stringsx.Coalesce(payload.Identifier, payload.LegacyIdentifier))
if f.Type == flow.TypeBrowser {
f.UI.SetCSRF(s.d.GenerateCSRFToken(r))
}
Expand Down Expand Up @@ -126,21 +128,7 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, requestedAAL identity.Au
return nil
}

// This block adds the identifier to the method when the request is forced - as a hint for the user.
var identifier string
if !sr.IsForced() {
// do nothing
} else if sess, err := s.d.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
// do nothing
} else if id, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), sess.IdentityID); err != nil {
// do nothing
} else if creds, ok := id.GetCredentials(s.ID()); !ok {
// do nothing
} else if len(creds.Identifiers) == 0 {
// do nothing
} else {
identifier = creds.Identifiers[0]
}
identifier := flowhelpers.GuessForcedLoginIdentifier(r, s.d, sr, s.ID())

sr.UI.SetCSRF(s.d.GenerateCSRFToken(r))
sr.UI.SetNode(node.NewInputField("identifier", identifier, node.DefaultGroup, node.InputAttributeTypeText, node.WithRequiredInputAttribute).WithMetaLabel(text.NewInfoNodeLabelID()))
Expand Down
17 changes: 8 additions & 9 deletions selfservice/strategy/password/settings.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package password

import (
"encoding/json"
"net/http"
"time"

Expand All @@ -13,6 +14,7 @@ import (
"github.com/gofrs/uuid"
"github.com/pkg/errors"

"github.com/ory/herodot"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/flow"
Expand Down Expand Up @@ -122,25 +124,22 @@ func (s *Strategy) continueSettingsFlow(
return err
}

i, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), ctxUpdate.Session.Identity.ID)
co, err := json.Marshal(&identity.CredentialsConfig{HashedPassword: string(hpw)})
if err != nil {
return err
}

c, ok := i.GetCredentials(s.ID())
if !ok {
c = &identity.Credentials{Type: s.ID()}
return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to encode password options to JSON: %s", err))
}

if err := i.SetCredentialsWithConfig(s.ID(), *c, &identity.CredentialsPassword{HashedPassword: string(hpw)}); err != nil {
i, err := s.d.PrivilegedIdentityPool().GetIdentityConfidential(r.Context(), ctxUpdate.Session.Identity.ID)
if err != nil {
return err
}

i.UpsertCredentialsConfig(s.ID(), co)
if err := s.validateCredentials(r.Context(), i, p.Password); err != nil {
return err
}

ctxUpdate.UpdateIdentity(i)

return nil
}

Expand Down
22 changes: 12 additions & 10 deletions selfservice/strategy/password/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,13 @@ func TestSettings(t *testing.T) {
}

t.Run("description=should update the password even if no password was set before", func(t *testing.T) {
email := fmt.Sprintf("test+%[email protected]", x.NewUUID())
bi := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
si := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
ai := newIdentityWithoutCredentials(x.NewUUID().String() + "@ory.sh")
browserUser := testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, bi)
spaUser := testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, si)
apiUser := testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, ai)

var check = func(t *testing.T, actual string, id *identity.Identity) {
assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual)
assert.Empty(t, gjson.Get(actual, "ui.nodes.#(name==password).attributes.value").String(), "%s", actual)
Expand All @@ -340,31 +346,27 @@ func TestSettings(t *testing.T) {
cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config)
assert.Contains(t, cfg, "hashed_password", "%+v", actualIdentity.Credentials)
require.Len(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers, 1)
assert.Equal(t, email, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0])
assert.Contains(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0], "-4")
}

var payload = func(v url.Values) {
v.Set("method", "password")
v.Set("password", randx.MustString(16, randx.AlphaNum))
}

id := newIdentityWithoutCredentials(email)
browserUser := testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, id)
apiUser := testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, id)

t.Run("type=api", func(t *testing.T) {
actual := expectSuccess(t, true, false, apiUser, payload)
check(t, actual, id)
check(t, actual, ai)
})

t.Run("type=spa", func(t *testing.T) {
actual := expectSuccess(t, false, true, browserUser, payload)
check(t, actual, id)
actual := expectSuccess(t, false, true, spaUser, payload)
check(t, actual, si)
})

t.Run("type=browser", func(t *testing.T) {
actual := expectSuccess(t, false, false, browserUser, payload)
check(t, actual, id)
check(t, actual, bi)
})
})

Expand Down

0 comments on commit a7784bd

Please sign in to comment.