mirror of
https://github.com/MeshTender/MeshTender.git
synced 2026-10-23 05:32:05 +00:00
Raise password floor to 12 characters
This commit is contained in:
1 parent
4232ff7385
commit
23405c6d14
10 files changed
+242
-25
No files matched your search
@@ -3,6 +3,7 @@ package auth
|
||||
import (
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/mail"
|
||||
"strconv"
|
||||
@@ -87,12 +88,15 @@ func (s *Handlers) renderAccount(w http.ResponseWriter, r *http.Request, uid int
|
||||
"Platforms": store.UserLinkPlatforms(),
|
||||
"PlatformsJS": web.LinkPlatformsJS(store.UserLinkPlatforms()),
|
||||
"HasPassword": u.PasswordHash != nil,
|
||||
"Passkeys": views,
|
||||
"NextRename": nextRename, // nil when a rename is allowed now
|
||||
"Error": r.URL.Query().Get("error"),
|
||||
"OK": r.URL.Query().Get("ok"),
|
||||
"PKMsg": r.URL.Query().Get("pk"),
|
||||
"PKErr": r.URL.Query().Get("pkerr"),
|
||||
// Stated in the change-password form and used as minlength, so the client
|
||||
// hint can't drift from the server's floor.
|
||||
"MinPasswordLen": MinPasswordLen,
|
||||
"Passkeys": views,
|
||||
"NextRename": nextRename, // nil when a rename is allowed now
|
||||
"Error": r.URL.Query().Get("error"),
|
||||
"OK": r.URL.Query().Get("ok"),
|
||||
"PKMsg": r.URL.Query().Get("pk"),
|
||||
"PKErr": r.URL.Query().Get("pkerr"),
|
||||
}
|
||||
for k, v := range overrides {
|
||||
data[k] = v
|
||||
@@ -421,7 +425,8 @@ func (s *Handlers) handleChangePassword(w http.ResponseWriter, r *http.Request)
|
||||
|
||||
newPassword := r.FormValue("new_password")
|
||||
if !ValidPassword(newPassword) {
|
||||
accountRedirect(w, r, "error", "New password must be at least 8 characters.")
|
||||
accountRedirect(w, r, "error", fmt.Sprintf(
|
||||
"New password must be at least %d characters.", MinPasswordLen))
|
||||
return
|
||||
}
|
||||
// When a password already exists, require the current one to change it.
|
||||
|
||||
@@ -411,7 +411,17 @@ func NormalizePasskeyName(s string) string {
|
||||
// MinPasswordLen is a basic strength floor. There is deliberately no maximum:
|
||||
// passwords are pre-hashed before bcrypt (see hashPassword), so bcrypt's 72-byte
|
||||
// input limit never reaches the user.
|
||||
const MinPasswordLen = 8
|
||||
//
|
||||
// It is enforced only where a password is *set* (signup and the account page's
|
||||
// change-password form), never on the verify path — so raising it doesn't lock out
|
||||
// accounts whose existing password predates the increase. They keep signing in, and
|
||||
// only have to meet the current floor if they choose a new password.
|
||||
//
|
||||
// 12 rather than 8 because length is the only strength signal here (no complexity
|
||||
// rules, no breach-list check) and there is no account recovery to fall back on if a
|
||||
// weak password is guessed — see the audit's B1. Passkeys remain the preferred path
|
||||
// and are offered first on both the sign-in and sign-up forms.
|
||||
const MinPasswordLen = 12
|
||||
|
||||
// ValidPassword reports whether p meets the minimum length.
|
||||
func ValidPassword(p string) bool {
|
||||
|
||||
@@ -2,6 +2,7 @@ package auth
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
|
||||
@@ -14,7 +15,9 @@ func (s *Service) SignupPassword(w http.ResponseWriter, r *http.Request) {
|
||||
displayName := NormalizeDisplayName(r.FormValue("display_name"))
|
||||
password := r.FormValue("password")
|
||||
if !ValidUsername(username) || !ValidPassword(password) {
|
||||
redirectErr(w, r, "/signup", "Choose a username (3–32 chars: letters, digits, _ . -) and a password of at least 8 characters.")
|
||||
redirectErr(w, r, "/signup", fmt.Sprintf(
|
||||
"Choose a username (3–32 chars: letters, digits, _ . -) and a password of at least %d characters.",
|
||||
MinPasswordLen))
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -7,21 +7,39 @@ import (
|
||||
"golang.org/x/crypto/bcrypt"
|
||||
)
|
||||
|
||||
// TestValidPassword pins the 8-char floor and confirms there is no upper bound:
|
||||
// pre-hashing removes bcrypt's 72-byte input limit, so long passwords are valid.
|
||||
// TestValidPassword pins the floor at MinPasswordLen and confirms there is no upper
|
||||
// bound: pre-hashing removes bcrypt's 72-byte input limit, so long passwords are
|
||||
// valid.
|
||||
//
|
||||
// The boundary cases are expressed relative to the constant rather than hardcoded, so
|
||||
// changing the floor doesn't require editing the expectations — only the explicit
|
||||
// "old floor" case below, which is deliberate.
|
||||
func TestValidPassword(t *testing.T) {
|
||||
t.Parallel()
|
||||
cases := []struct {
|
||||
n int
|
||||
want bool
|
||||
}{
|
||||
{7, false}, {8, true}, {72, true}, {73, true}, {5000, true},
|
||||
{MinPasswordLen - 1, false},
|
||||
{MinPasswordLen, true},
|
||||
{72, true}, // bcrypt's raw input limit — pre-hashing means it isn't ours
|
||||
{73, true}, // past it
|
||||
{5000, true}, // no maximum at all
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := ValidPassword(strings.Repeat("a", c.n)); got != c.want {
|
||||
t.Errorf("ValidPassword(len=%d) = %v, want %v", c.n, got, c.want)
|
||||
}
|
||||
}
|
||||
// The floor was raised from 8 to 12 (audit S7); 8 must now be rejected when a
|
||||
// password is *set*. Existing shorter passwords keep working because the floor is
|
||||
// never applied on the verify path — see TestFloorNotAppliedToExistingPasswords.
|
||||
if ValidPassword("12345678") {
|
||||
t.Error("an 8-character password is still accepted; the floor didn't move")
|
||||
}
|
||||
if MinPasswordLen < 12 {
|
||||
t.Errorf("MinPasswordLen = %d, want at least 12", MinPasswordLen)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPasswordHashRoundTrip: a password far past bcrypt's 72-byte input limit
|
||||
|
||||
@@ -236,9 +236,9 @@
|
||||
</div>
|
||||
{{end}}
|
||||
<div class="col-md">
|
||||
<label class="form-label" for="new_pw">{{if .HasPassword}}New password{{else}}Password{{end}} <span class="form-label-description">min 8 characters</span></label>
|
||||
<label class="form-label" for="new_pw">{{if .HasPassword}}New password{{else}}Password{{end}} <span class="form-label-description">min {{.MinPasswordLen}} characters</span></label>
|
||||
<div class="input-group input-group-flat">
|
||||
<input type="password" class="form-control" id="new_pw" name="new_password" autocomplete="new-password" minlength="8" required>
|
||||
<input type="password" class="form-control" id="new_pw" name="new_password" autocomplete="new-password" minlength="{{.MinPasswordLen}}" required>
|
||||
<span class="input-group-text"><a href="#" class="link-secondary" role="button" aria-pressed="false" title="Show password" data-pwtoggle="new_pw" aria-label="Show password">{{template "icon-eye" ""}}</a></span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -25,9 +25,9 @@
|
||||
<div class="hr-text">or use a password</div>
|
||||
|
||||
<div class="mb-3">
|
||||
<label class="form-label" for="password">Password <span class="form-label-description">min 8 characters</span></label>
|
||||
<label class="form-label" for="password">Password <span class="form-label-description">min {{.MinPasswordLen}} characters</span></label>
|
||||
<div class="input-group input-group-flat">
|
||||
<input type="password" class="form-control" id="password" name="password" autocomplete="new-password" minlength="8" placeholder="Choose a password" required>
|
||||
<input type="password" class="form-control" id="password" name="password" autocomplete="new-password" minlength="{{.MinPasswordLen}}" placeholder="Choose a password" required>
|
||||
<span class="input-group-text">
|
||||
<a href="#" class="link-secondary" role="button" aria-pressed="false" title="Show password" data-pwtoggle="password" aria-label="Show password">{{template "icon-eye" ""}}</a>
|
||||
</span>
|
||||
|
||||
@@ -149,6 +149,9 @@ func (s *Handlers) pageSignup(w http.ResponseWriter, r *http.Request) {
|
||||
"Layout": "authbase",
|
||||
"Error": r.URL.Query().Get("error"),
|
||||
"Next": r.URL.Query().Get("next"),
|
||||
// Stated in the form and used as minlength, so the client hint can't drift
|
||||
// from the server's floor.
|
||||
"MinPasswordLen": MinPasswordLen,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ import (
|
||||
// session cookie — the automatable way to mint an authenticated auth-host session.
|
||||
func authSSO(t *testing.T, ts *httptest.Server, h hostEnv, username string) *http.Cookie {
|
||||
t.Helper()
|
||||
resp := post(t, ts, h.auth, "/signup/password", url.Values{"username": {username}, "password": {"supersecret"}})
|
||||
resp := post(t, ts, h.auth, "/signup/password", url.Values{"username": {username}, "password": {testPassword}})
|
||||
resp.Body.Close()
|
||||
c := cookieByName(resp, "meshtender_session")
|
||||
if c == nil {
|
||||
@@ -43,7 +43,7 @@ func TestPasswordSignupAndLogin(t *testing.T) {
|
||||
}
|
||||
|
||||
// #19 signup, valid → creates the account + an SSO session and hands off.
|
||||
ok := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"pwuser"}, "password": {"supersecret"}})
|
||||
ok := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"pwuser"}, "password": {testPassword}})
|
||||
ok.Body.Close()
|
||||
if ok.StatusCode != http.StatusSeeOther || cookieByName(ok, "meshtender_session") == nil {
|
||||
t.Fatalf("valid signup = %d, session=%v; want 303 + session cookie", ok.StatusCode, cookieByName(ok, "meshtender_session"))
|
||||
@@ -60,7 +60,7 @@ func TestPasswordSignupAndLogin(t *testing.T) {
|
||||
}
|
||||
|
||||
// #18 login, correct credentials → session + handoff.
|
||||
good := post(t, ts, h.auth, "/login/password", url.Values{"username": {"pwuser"}, "password": {"supersecret"}})
|
||||
good := post(t, ts, h.auth, "/login/password", url.Values{"username": {"pwuser"}, "password": {testPassword}})
|
||||
good.Body.Close()
|
||||
if good.StatusCode != http.StatusSeeOther || cookieByName(good, "meshtender_session") == nil {
|
||||
t.Fatalf("good login = %d, session=%v; want 303 + session cookie", good.StatusCode, cookieByName(good, "meshtender_session"))
|
||||
@@ -260,7 +260,7 @@ func TestAccountProfilePosts(t *testing.T) {
|
||||
assertAccountOK(t, links, "links")
|
||||
|
||||
pw := post(t, ts, h.auth, "/account/password",
|
||||
url.Values{"new_password": {"anothersecret"}, "current_password": {"supersecret"}}, sso)
|
||||
url.Values{"new_password": {"anothersecret"}, "current_password": {testPassword}}, sso)
|
||||
pw.Body.Close()
|
||||
assertAccountOK(t, pw, "password")
|
||||
|
||||
|
||||
@@ -27,6 +27,20 @@ type hostEnv struct{ auth, app, root, www string }
|
||||
|
||||
// splitServer stands up a server in full split-host mode (root/www/auth/app)
|
||||
// against the test database, returning the store and the per-surface host:port.
|
||||
// testPassword is the fixture password for sign-up/sign-in tests. It must satisfy
|
||||
// auth.MinPasswordLen — asserted below — so raising the floor doesn't quietly break
|
||||
// every credential test with a confusing "password too short" redirect.
|
||||
const testPassword = "correct-horse-battery-staple"
|
||||
|
||||
// TestFixturePasswordMeetsFloor keeps the fixture honest if the floor moves again.
|
||||
func TestFixturePasswordMeetsFloor(t *testing.T) {
|
||||
t.Parallel()
|
||||
if len(testPassword) < auth.MinPasswordLen {
|
||||
t.Fatalf("testPassword is %d chars, below the %d-char floor — credential tests "+
|
||||
"would fail with a misleading validation error", len(testPassword), auth.MinPasswordLen)
|
||||
}
|
||||
}
|
||||
|
||||
func splitServer(t *testing.T) (*store.Store, context.Context, *httptest.Server, hostEnv) {
|
||||
t.Helper()
|
||||
st, ctx := coreStore(t)
|
||||
@@ -251,7 +265,7 @@ func TestSingleLogout(t *testing.T) {
|
||||
|
||||
// Sign up on the auth host: creates login row L and the auth SSO session, then
|
||||
// hands off to the app with a single-use code carrying L.
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"sharuser"}, "password": {"supersecret"}})
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"sharuser"}, "password": {testPassword}})
|
||||
su.Body.Close()
|
||||
sso := cookieByName(su, "meshtender_session")
|
||||
if sso == nil {
|
||||
@@ -301,7 +315,7 @@ func TestAuthLogoutClearsSSO(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, _, ts, h := splitServer(t)
|
||||
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"ssoonly"}, "password": {"supersecret"}})
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"ssoonly"}, "password": {testPassword}})
|
||||
su.Body.Close()
|
||||
sso := cookieByName(su, "meshtender_session")
|
||||
if sso == nil {
|
||||
@@ -329,7 +343,7 @@ func TestLogoutRejectsGet(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, _, ts, h := splitServer(t)
|
||||
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"getlogout"}, "password": {"supersecret"}})
|
||||
su := post(t, ts, h.auth, "/signup/password", url.Values{"username": {"getlogout"}, "password": {testPassword}})
|
||||
su.Body.Close()
|
||||
sso := cookieByName(su, "meshtender_session")
|
||||
if sso == nil {
|
||||
@@ -382,7 +396,7 @@ func TestAccountOnAuthHost(t *testing.T) {
|
||||
t.Fatal("expected a session cookie carrying the auth-local flag")
|
||||
}
|
||||
fin := post(t, ts, h.auth, "/signup/password",
|
||||
url.Values{"username": {"acctlocal"}, "password": {"supersecret"}}, sess)
|
||||
url.Values{"username": {"acctlocal"}, "password": {testPassword}}, sess)
|
||||
fin.Body.Close()
|
||||
if got := fin.Header.Get("Location"); got != "/account" {
|
||||
t.Fatalf("post-login redirect = %q, want local /account (no handoff)", got)
|
||||
@@ -391,7 +405,7 @@ func TestAccountOnAuthHost(t *testing.T) {
|
||||
|
||||
t.Run("auth /account with SSO session renders", func(t *testing.T) {
|
||||
su := post(t, ts, h.auth, "/signup/password",
|
||||
url.Values{"username": {"acctsso"}, "password": {"supersecret"}})
|
||||
url.Values{"username": {"acctsso"}, "password": {testPassword}})
|
||||
su.Body.Close()
|
||||
sso := cookieByName(su, "meshtender_session")
|
||||
if sso == nil {
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
package core
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/cookiejar"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/jleight/meshtender/internal/auth"
|
||||
)
|
||||
|
||||
// TestPasswordFloorAppliesOnlyWhenSetting is the guarantee behind raising the
|
||||
// minimum from 8 to 12 (audit S7): the floor is checked where a password is *set*,
|
||||
// never where one is verified. An account whose password predates the increase keeps
|
||||
// signing in; it only has to meet the current floor if the owner chooses a new
|
||||
// password.
|
||||
//
|
||||
// This is the test that would catch someone "helpfully" adding ValidPassword to the
|
||||
// login path, which would lock out every account with an older, shorter password.
|
||||
func TestPasswordFloorAppliesOnlyWhenSetting(t *testing.T) {
|
||||
t.Parallel()
|
||||
st, ctx, ts, h := splitServer(t)
|
||||
|
||||
// A password that was fine under the old floor and is too short under the new one.
|
||||
const legacyPassword = "old8char" // 8 characters
|
||||
if len(legacyPassword) >= auth.MinPasswordLen {
|
||||
t.Fatalf("fixture password must be shorter than the floor (%d)", auth.MinPasswordLen)
|
||||
}
|
||||
|
||||
// Establish it the way an existing account's hash exists today: through the auth
|
||||
// service's own setter, bypassing the form-level length check.
|
||||
svc, err := auth.New(st, st.Pool(), auth.Config{
|
||||
RPID: "localhost", RPDisplayName: "test",
|
||||
RPOrigins: []string{"http://auth.localhost"},
|
||||
AppHost: testAppHost, AuthHost: testAuthHost, RootHost: testRootHost,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("auth service: %v", err)
|
||||
}
|
||||
legacyUser, err := st.CreateUser(ctx, "legacypw", "")
|
||||
if err != nil {
|
||||
t.Fatalf("create user: %v", err)
|
||||
}
|
||||
if err := svc.SetPassword(ctx, legacyUser.ID, legacyPassword); err != nil {
|
||||
t.Fatalf("set legacy password: %v", err)
|
||||
}
|
||||
|
||||
t.Run("existing short password still signs in", func(t *testing.T) {
|
||||
resp := post(t, ts, h.auth, "/login/password", url.Values{
|
||||
"username": {"legacypw"},
|
||||
"password": {legacyPassword},
|
||||
})
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusSeeOther {
|
||||
t.Fatalf("login = %d, want 303", resp.StatusCode)
|
||||
}
|
||||
loc := resp.Header.Get("Location")
|
||||
if strings.Contains(loc, "error=") {
|
||||
t.Fatalf("login failed for a pre-existing short password: %s", loc)
|
||||
}
|
||||
// A successful sign-in hands off with a code rather than bouncing back to /login.
|
||||
if !strings.Contains(loc, "/session/callback") {
|
||||
t.Fatalf("login didn't hand off; Location = %q", loc)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("verification is unchanged for a wrong password", func(t *testing.T) {
|
||||
resp := post(t, ts, h.auth, "/login/password", url.Values{
|
||||
"username": {"legacypw"},
|
||||
"password": {"wrongwrongwrong"},
|
||||
})
|
||||
defer resp.Body.Close()
|
||||
if loc := resp.Header.Get("Location"); !strings.Contains(loc, "error=") {
|
||||
t.Fatalf("wrong password was accepted; Location = %q", loc)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("signup rejects a password under the floor", func(t *testing.T) {
|
||||
resp := post(t, ts, h.auth, "/signup/password", url.Values{
|
||||
"username": {"tooshortpw"},
|
||||
"password": {strings.Repeat("a", auth.MinPasswordLen-1)},
|
||||
})
|
||||
defer resp.Body.Close()
|
||||
loc := resp.Header.Get("Location")
|
||||
if !strings.Contains(loc, "/signup?error=") {
|
||||
t.Fatalf("short signup password was accepted; Location = %q", loc)
|
||||
}
|
||||
if !strings.Contains(loc, strconv.Itoa(auth.MinPasswordLen)) {
|
||||
t.Errorf("the error should tell the user the required length (%d); Location = %q",
|
||||
auth.MinPasswordLen, loc)
|
||||
}
|
||||
// And no account may have been created.
|
||||
if _, err := st.GetUserByUsername(ctx, "tooshortpw"); err == nil {
|
||||
t.Error("a user was created despite the rejected password")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("signup accepts a password at the floor", func(t *testing.T) {
|
||||
resp := post(t, ts, h.auth, "/signup/password", url.Values{
|
||||
"username": {"longenoughpw"},
|
||||
"password": {strings.Repeat("a", auth.MinPasswordLen)},
|
||||
})
|
||||
defer resp.Body.Close()
|
||||
if loc := resp.Header.Get("Location"); strings.Contains(loc, "error=") {
|
||||
t.Fatalf("a password at exactly the floor was rejected: %s", loc)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestPasswordFormsStateTheFloor checks the signup and account forms advertise the
|
||||
// same minimum the server enforces — both in the human-readable hint and in the
|
||||
// client-side minlength, which is what stops the browser submitting a doomed form.
|
||||
func TestPasswordFormsStateTheFloor(t *testing.T) {
|
||||
t.Parallel()
|
||||
st, ctx, ts, h := splitServer(t)
|
||||
|
||||
want := strconv.Itoa(auth.MinPasswordLen)
|
||||
|
||||
t.Run("signup form", func(t *testing.T) {
|
||||
resp := do(t, ts, h.auth, "/signup")
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("read body: %v", err)
|
||||
}
|
||||
page := string(body)
|
||||
if !strings.Contains(page, "min "+want+" characters") {
|
||||
t.Errorf("signup form doesn't state a %s-character minimum", want)
|
||||
}
|
||||
if !strings.Contains(page, `minlength="`+want+`"`) {
|
||||
t.Errorf("signup password field lacks minlength=%q", want)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("account change-password form", func(t *testing.T) {
|
||||
jar, err := cookiejar.New(nil)
|
||||
if err != nil {
|
||||
t.Fatalf("cookiejar: %v", err)
|
||||
}
|
||||
seedSession(t, ts, st, ctx, jar, "pwformuser")
|
||||
// The account page lives on the auth host and needs that host's own session,
|
||||
// so drive it through the same handoff the app uses.
|
||||
resp := do(t, ts, h.auth, "/account", jar.Cookies(mustURL(t, ts.URL))...)
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if err != nil {
|
||||
t.Fatalf("read body: %v", err)
|
||||
}
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("account page = %d, want 200 — the handoff cookie should carry the "+
|
||||
"auth-host session", resp.StatusCode)
|
||||
}
|
||||
page := string(body)
|
||||
if !strings.Contains(page, "min "+want+" characters") {
|
||||
t.Errorf("account form doesn't state a %s-character minimum", want)
|
||||
}
|
||||
if !strings.Contains(page, `minlength="`+want+`"`) {
|
||||
t.Errorf("account password field lacks minlength=%q", want)
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in new issue
Block a user