fix: no password sign-up offered on a site that signs in through a provider
/register drew the whole form under SSO-only — name, email, password twice, invite code, Sign Up — and only the POST refused it. That is asking somebody for four fields and a password they will never use before telling them the door does not exist. /auth/signup-policy is the question the form already asks, so it answers it: sso_only, the provider's name, and registration_open false whenever single sign-on is the only way in. The page says one line instead. The Register tab on the landing modal and the Register button in the header go with it — and the modal falls back to Sign in if it was opened straight into Register. The sign-in page needed nothing: it already hid its "Sign up" link on registration_open false, which is now also true under SSO. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
c9e0655d6e
commit
282b6a25f7
5 changed files with 119 additions and 7 deletions
|
|
@ -121,9 +121,21 @@ def signup_policy(db: Session = Depends(get_db)):
|
||||||
# door that is locked. It was offering one: registration can be turned off
|
# door that is locked. It was offering one: registration can be turned off
|
||||||
# site-wide, and the only way to find out was to fill the form in and be
|
# site-wide, and the only way to find out was to fill the form in and be
|
||||||
# refused.
|
# refused.
|
||||||
|
# Single sign-on closes the password door, and this is the one question
|
||||||
|
# the sign-up form already asks. Saying so here means the form can decline
|
||||||
|
# to draw itself rather than collect a name, an email, a password twice and
|
||||||
|
# an invite code, and then be refused by the POST.
|
||||||
|
from app.config import settings as cfg
|
||||||
|
sso_only = _get_sso_settings()["sso_only"]
|
||||||
return {"invite_required": site_settings.get_flag("invite_only"),
|
return {"invite_required": site_settings.get_flag("invite_only"),
|
||||||
"first_user": False,
|
"first_user": False,
|
||||||
"registration_open": site_settings.get_flag("registration_enabled")}
|
"sso_only": sso_only,
|
||||||
|
# So the page can name the provider rather than say "single
|
||||||
|
# sign-on" at somebody who only knows it as PedsHub SSO.
|
||||||
|
"provider_name": (cfg.OIDC_PROVIDER_NAME
|
||||||
|
if cfg.OIDC_PROVIDER_URL and cfg.OIDC_CLIENT_ID else None),
|
||||||
|
"registration_open": (not sso_only
|
||||||
|
and site_settings.get_flag("registration_enabled"))}
|
||||||
|
|
||||||
|
|
||||||
@router.post("/register")
|
@router.post("/register")
|
||||||
|
|
|
||||||
|
|
@ -119,3 +119,43 @@ class OneTimeCodeTests(unittest.TestCase):
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|
||||||
|
|
||||||
|
class SignupPolicyTests(unittest.TestCase):
|
||||||
|
"""The form asks this before it draws itself.
|
||||||
|
|
||||||
|
Under SSO-only it used to draw the whole password sign-up — name, email,
|
||||||
|
password twice, invite code — and only the POST refused it. The page now
|
||||||
|
has what it needs to decline.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False},
|
||||||
|
poolclass=StaticPool)
|
||||||
|
Base.metadata.create_all(self.engine)
|
||||||
|
self.db = Session(self.engine)
|
||||||
|
self.db.add(User(id=1, name="Someone", email="someone@example.com",
|
||||||
|
hashed_password=None))
|
||||||
|
self.db.commit()
|
||||||
|
app = FastAPI()
|
||||||
|
app.include_router(auth.router, prefix="/auth")
|
||||||
|
app.dependency_overrides[get_db] = lambda: self.db
|
||||||
|
self.client = TestClient(app)
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
self.client.close()
|
||||||
|
self.db.close()
|
||||||
|
self.engine.dispose()
|
||||||
|
|
||||||
|
def test_sso_only_closes_registration_in_the_policy(self):
|
||||||
|
with patch.object(auth, "_get_sso_settings", return_value={"sso_only": True}):
|
||||||
|
body = self.client.get("/auth/signup-policy").json()
|
||||||
|
self.assertTrue(body["sso_only"])
|
||||||
|
self.assertFalse(body["registration_open"])
|
||||||
|
|
||||||
|
def test_otherwise_the_switch_decides(self):
|
||||||
|
with patch.object(auth, "_get_sso_settings", return_value={"sso_only": False}), \
|
||||||
|
patch.object(auth.site_settings, "get_flag", return_value=True):
|
||||||
|
body = self.client.get("/auth/signup-policy").json()
|
||||||
|
self.assertFalse(body["sso_only"])
|
||||||
|
self.assertTrue(body["registration_open"])
|
||||||
|
|
|
||||||
|
|
@ -161,6 +161,17 @@ export default function Navbar({ onSignIn, onRegister, onSearch }) {
|
||||||
const [peek, setPeek] = useState(false)
|
const [peek, setPeek] = useState(false)
|
||||||
// Registered by the quiz player while it is on screen without a rail.
|
// Registered by the quiz player while it is on screen without a rail.
|
||||||
const sessionDrawer = useSessionDrawer()
|
const sessionDrawer = useSessionDrawer()
|
||||||
|
//: Whether anybody may sign themselves up. Asked only when logged out —
|
||||||
|
//: there is nothing to draw otherwise, and it is one request per visit.
|
||||||
|
const [signupOpen, setSignupOpen] = useState(true)
|
||||||
|
useEffect(() => {
|
||||||
|
if (user) return undefined
|
||||||
|
let live = true
|
||||||
|
api.get('/auth/signup-policy')
|
||||||
|
.then(res => { if (live) setSignupOpen(res.data?.registration_open !== false) })
|
||||||
|
.catch(() => {})
|
||||||
|
return () => { live = false }
|
||||||
|
}, [user])
|
||||||
//: Is the thing this button opens currently open? On a page that has taken
|
//: Is the thing this button opens currently open? On a page that has taken
|
||||||
//: the button over, that is the page's drawer; otherwise it is the site
|
//: the button over, that is the page's drawer; otherwise it is the site
|
||||||
//: menu. Either way the bars fold into a cross.
|
//: menu. Either way the bars fold into a cross.
|
||||||
|
|
@ -266,10 +277,13 @@ export default function Navbar({ onSignIn, onRegister, onSearch }) {
|
||||||
? <button onClick={onSignIn} style={{ fontSize: '0.875rem', color: 'var(--navbar-fg)', opacity: 0.8, background: 'none', border: 'none', cursor: 'pointer', padding: '6px 10px' }}>Sign In</button>
|
? <button onClick={onSignIn} style={{ fontSize: '0.875rem', color: 'var(--navbar-fg)', opacity: 0.8, background: 'none', border: 'none', cursor: 'pointer', padding: '6px 10px' }}>Sign In</button>
|
||||||
: <Link to="/login" style={{ fontSize: '0.875rem', color: 'var(--navbar-fg)', opacity: 0.8, textDecoration: 'none', padding: '6px 10px' }}>Sign In</Link>
|
: <Link to="/login" style={{ fontSize: '0.875rem', color: 'var(--navbar-fg)', opacity: 0.8, textDecoration: 'none', padding: '6px 10px' }}>Sign In</Link>
|
||||||
}
|
}
|
||||||
{onRegister
|
{/* Only where there is a door. On a site that signs in through
|
||||||
|
a provider — or with registration simply switched off —
|
||||||
|
this led to a form whose only outcome was a refusal. */}
|
||||||
|
{signupOpen && (onRegister
|
||||||
? <button onClick={onRegister} className="btn btn-primary" style={{ fontSize: '0.85rem', padding: '6px 14px', borderRadius: 8 }}>Register</button>
|
? <button onClick={onRegister} className="btn btn-primary" style={{ fontSize: '0.85rem', padding: '6px 14px', borderRadius: 8 }}>Register</button>
|
||||||
: <Link to="/register" className="btn btn-primary" style={{ fontSize: '0.85rem', padding: '6px 14px', textDecoration: 'none', borderRadius: 8 }}>Register</Link>
|
: <Link to="/register" className="btn btn-primary" style={{ fontSize: '0.85rem', padding: '6px 14px', textDecoration: 'none', borderRadius: 8 }}>Register</Link>
|
||||||
}
|
)}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -253,15 +253,30 @@ function AuthModal({ mode, onClose, onSwitch }) {
|
||||||
// required" and nowhere to put one.
|
// required" and nowhere to put one.
|
||||||
const [inviteRequired, setInviteRequired] = useState(false)
|
const [inviteRequired, setInviteRequired] = useState(false)
|
||||||
const [inviteCode, setInviteCode] = useState('')
|
const [inviteCode, setInviteCode] = useState('')
|
||||||
|
const [signupOpen, setSignupOpen] = useState(true)
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
let live = true
|
let live = true
|
||||||
api.get('/auth/signup-policy')
|
api.get('/auth/signup-policy')
|
||||||
.then(res => { if (live) setInviteRequired(!!res.data?.invite_required) })
|
.then(res => {
|
||||||
|
if (!live) return
|
||||||
|
setInviteRequired(!!res.data?.invite_required)
|
||||||
|
// Registration closed — by the switch, or because the site signs in
|
||||||
|
// through a provider. Either way there is no password sign-up to
|
||||||
|
// offer, and a Register tab that leads to a refusal is worse than no
|
||||||
|
// tab at all.
|
||||||
|
setSignupOpen(res.data?.registration_open !== false)
|
||||||
|
})
|
||||||
.catch(() => {})
|
.catch(() => {})
|
||||||
return () => { live = false }
|
return () => { live = false }
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
|
// Opened straight into Register — from the header button, or a stale tab —
|
||||||
|
// on a site that has since closed it. The tab is gone; the form must go too.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!signupOpen && mode === 'register') onSwitch('login')
|
||||||
|
}, [signupOpen, mode, onSwitch])
|
||||||
|
|
||||||
const reset = () => { setError(''); setUnverified(false); setResendSent(false); setRegistered(false) }
|
const reset = () => { setError(''); setUnverified(false); setResendSent(false); setRegistered(false) }
|
||||||
const switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); setConfirm(''); onSwitch(m) }
|
const switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); setConfirm(''); onSwitch(m) }
|
||||||
|
|
||||||
|
|
@ -321,7 +336,7 @@ function AuthModal({ mode, onClose, onSwitch }) {
|
||||||
|
|
||||||
{/* Tabs */}
|
{/* Tabs */}
|
||||||
<div className="lp-tabs">
|
<div className="lp-tabs">
|
||||||
{[['login', 'Sign In'], ['register', 'Register']].map(([m, label]) => (
|
{[['login', 'Sign In'], ...(signupOpen ? [['register', 'Register']] : [])].map(([m, label]) => (
|
||||||
<button key={m} onClick={() => switchMode(m)} aria-pressed={mode === m}>{label}</button>
|
<button key={m} onClick={() => switchMode(m)} aria-pressed={mode === m}>{label}</button>
|
||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -19,13 +19,22 @@ export default function RegisterPage() {
|
||||||
// with, so the form knows whether to want a code.
|
// with, so the form knows whether to want a code.
|
||||||
const [inviteRequired, setInviteRequired] = useState(false)
|
const [inviteRequired, setInviteRequired] = useState(false)
|
||||||
const [inviteCode, setInviteCode] = useState('')
|
const [inviteCode, setInviteCode] = useState('')
|
||||||
|
//: Null until the policy is known. A site on single sign-on has no password
|
||||||
|
//: sign-up, and drawing the form and refusing the POST is asking somebody
|
||||||
|
//: for a name, an email, a password twice and an invite code before telling
|
||||||
|
//: them the door does not exist.
|
||||||
|
const [policy, setPolicy] = useState(null)
|
||||||
const { loginWithToken } = useAuth()
|
const { loginWithToken } = useAuth()
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
let live = true
|
let live = true
|
||||||
api.get('/auth/signup-policy')
|
api.get('/auth/signup-policy')
|
||||||
.then(res => { if (live) setInviteRequired(!!res.data?.invite_required) })
|
.then(res => {
|
||||||
.catch(() => {})
|
if (!live) return
|
||||||
|
setInviteRequired(!!res.data?.invite_required)
|
||||||
|
setPolicy(res.data || {})
|
||||||
|
})
|
||||||
|
.catch(() => { if (live) setPolicy({}) })
|
||||||
return () => { live = false }
|
return () => { live = false }
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
|
|
@ -59,6 +68,28 @@ export default function RegisterPage() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (policy === null) {
|
||||||
|
return <div className="auth-page"><div className="auth-card"><div className="loading"><div className="spinner" /></div></div></div>
|
||||||
|
}
|
||||||
|
|
||||||
|
// Nothing to fill in on a site where accounts come from the provider.
|
||||||
|
if (policy.sso_only) {
|
||||||
|
return (
|
||||||
|
<div className="auth-page">
|
||||||
|
<div className="auth-card" style={{ textAlign: 'center' }}>
|
||||||
|
<h1>Accounts come by invitation</h1>
|
||||||
|
<p style={{ color: 'var(--text-muted)', marginBottom: 20 }}>
|
||||||
|
This site signs in through {policy.provider_name || 'single sign-on'}.
|
||||||
|
Ask an administrator for a sign-up link.
|
||||||
|
</p>
|
||||||
|
<Link to="/login" className="btn btn-primary" style={{ display: 'inline-block' }}>
|
||||||
|
Go to sign in
|
||||||
|
</Link>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
if (done) {
|
if (done) {
|
||||||
return (
|
return (
|
||||||
<div className="auth-page">
|
<div className="auth-page">
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue