diff --git a/backend/alembic/versions/h1b2c3d4e5f6_password_optional.py b/backend/alembic/versions/h1b2c3d4e5f6_password_optional.py new file mode 100644 index 0000000..4e2a479 --- /dev/null +++ b/backend/alembic/versions/h1b2c3d4e5f6_password_optional.py @@ -0,0 +1,30 @@ +"""An account may have no password. + +Single sign-on wrote a random string nobody would ever know, which reads as +"has a password" to everything that asks — so Settings demanded a current +password before it would let those accounts set their first, and the only way +through was to click "forgot password" for one they never had. + +Null says the true thing. The random strings already written are left alone: +they are unguessable, so nothing can sign in with them, and clearing them would +mean deciding from outside which accounts were meant to have one. + +Revision ID: h1b2c3d4e5f6 +Revises: g9a1b2c3d4e5 +""" +import sqlalchemy as sa +from alembic import op + +revision = "h1b2c3d4e5f6" +down_revision = "g9a1b2c3d4e5" +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column("users", "hashed_password", existing_type=sa.String(), nullable=True) + + +def downgrade(): + op.execute("UPDATE users SET hashed_password = '' WHERE hashed_password IS NULL") + op.alter_column("users", "hashed_password", existing_type=sa.String(), nullable=False) diff --git a/backend/app/models/user.py b/backend/app/models/user.py index 394f401..036882c 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -13,7 +13,12 @@ class User(Base): id = Column(Integer, primary_key=True, index=True) email = Column(String, unique=True, index=True, nullable=False) - hashed_password = Column(String, nullable=False) + # Null where there is no password at all. Somebody who arrived through + # single sign-on, or who only ever signs in with a code, has never chosen + # one — and storing a random string they can never guess made that + # indistinguishable from having one, so Settings asked them for a current + # password before it would let them set their first. + hashed_password = Column(String, nullable=True) name = Column(String, nullable=False) role = Column(String, default="user") # admin, moderator, user # Which exam the learner is studying for; scopes the bank they see. diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 6f1a362..f67e988 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -183,7 +183,11 @@ async def login(login_data: LoginRequest, db: Session = Depends(get_db), request email_normalized = login_data.email.lower().strip() user = db.query(User).filter(User.email == email_normalized).first() - if not user or not verify_password(login_data.password, user.hashed_password): + # An account with no password is not an account with the wrong password, + # but it is told the same thing: which accounts have one is not a question + # this endpoint answers. + if not user or not user.hashed_password or not verify_password( + login_data.password, user.hashed_password): raise HTTPException(status_code=401, detail="Invalid email or password") # Check email verification — skip for users without any verification record (legacy/seeded) @@ -345,16 +349,22 @@ def save_user_settings( @router.get("/me", response_model=UserResponse) def get_me(current_user: User = Depends(get_current_user)): - return current_user + return UserResponse.of(current_user) @router.put("/me") def update_me(data: UserUpdateMe, db: Session = Depends(get_db), current_user: User = Depends(get_current_user)): if data.new_password: - if not data.current_password: - raise HTTPException(status_code=400, detail="Current password required to set a new one") - if not verify_password(data.current_password, current_user.hashed_password): - raise HTTPException(status_code=400, detail="Current password is incorrect") + # A first password is not a change of one. Somebody who signed in with + # a code or through single sign-on has none to confirm, and asking for + # it locked them out of ever setting one — the only way through was + # "forgot password", which is a strange thing to click when you never + # had one. + if current_user.hashed_password: + if not data.current_password: + raise HTTPException(status_code=400, detail="Current password required to set a new one") + if not verify_password(data.current_password, current_user.hashed_password): + raise HTTPException(status_code=400, detail="Current password is incorrect") if len(data.new_password) < 8: raise HTTPException(status_code=400, detail="New password must be at least 8 characters") current_user.hashed_password = get_password_hash(data.new_password) @@ -457,7 +467,9 @@ async def sso_callback(request: Request, db: Session = Depends(get_db)): if not user: user = User( email=email, - hashed_password=get_password_hash(secrets.token_urlsafe(32)), # random password — SSO users don't use it + # No password rather than one nobody knows. A random string here + # reads as "has a password" everywhere that asks. + hashed_password=None, name=name, role="user", ) diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index 8ca2d1b..f266aa6 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -20,10 +20,21 @@ class UserResponse(BaseModel): role: str is_unthrottled: int = 0 created_at: datetime + # Whether there is one at all, never anything about it. Settings has to say + # "Set a password" or "Change password", and it cannot tell from the outside. + has_password: bool = False class Config: from_attributes = True + @staticmethod + def of(user) -> "UserResponse": + return UserResponse(**{ + "id": user.id, "email": user.email, "name": user.name, "role": user.role, + "is_unthrottled": user.is_unthrottled or 0, "created_at": user.created_at, + "has_password": bool(user.hashed_password), + }) + class Token(BaseModel): access_token: str diff --git a/frontend/src/pages/SettingsPage.jsx b/frontend/src/pages/SettingsPage.jsx index 71b4b29..16c2453 100644 --- a/frontend/src/pages/SettingsPage.jsx +++ b/frontend/src/pages/SettingsPage.jsx @@ -24,6 +24,10 @@ function Section({ title, description, children }) { } function ProfileSection({ user }) { + // Whether there is one at all — never anything about it. Reported by the + // server, because nothing on this side can tell an account with no password + // from one whose password it simply does not know. + const hasPassword = user?.has_password !== false const [name, setName] = useState(user?.name || '') const [currentPassword, setCurrentPassword] = useState('') const [newPassword, setNewPassword] = useState('') @@ -39,7 +43,13 @@ function ProfileSection({ user }) { if (newPassword && newPassword.length < 8) return setError('Password must be at least 8 characters') const payload = {} if (name !== user.name) payload.name = name - if (newPassword) { payload.current_password = currentPassword; payload.new_password = newPassword } + // A first password has none to confirm. Somebody who arrived through + // single sign-on, or who only ever signs in with a code, has never chosen + // one — asking for the current one locked them out of setting their first. + if (newPassword) { + payload.new_password = newPassword + if (hasPassword) payload.current_password = currentPassword + } if (!Object.keys(payload).length) return setError('No changes to save') setLoading(true) try { @@ -71,18 +81,25 @@ function ProfileSection({ user }) {
- Change Password (leave blank to keep current) + {hasPassword ? 'Change password' : 'Set a password'}{' '} + + {hasPassword + ? '(leave blank to keep current)' + : '— optional. You can keep signing in with a code or single sign-on.'} +
+ {hasPassword && ( +