diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index b856287..74f8692 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -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 # site-wide, and the only way to find out was to fill the form in and be # 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"), "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") diff --git a/backend/tests/test_sso_hardening.py b/backend/tests/test_sso_hardening.py index c8395f5..8f9c8db 100644 --- a/backend/tests/test_sso_hardening.py +++ b/backend/tests/test_sso_hardening.py @@ -119,3 +119,43 @@ class OneTimeCodeTests(unittest.TestCase): if __name__ == "__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"]) diff --git a/frontend/src/components/Navbar.jsx b/frontend/src/components/Navbar.jsx index 75ca652..4466411 100644 --- a/frontend/src/components/Navbar.jsx +++ b/frontend/src/components/Navbar.jsx @@ -161,6 +161,17 @@ export default function Navbar({ onSignIn, onRegister, onSearch }) { const [peek, setPeek] = useState(false) // Registered by the quiz player while it is on screen without a rail. 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 //: the button over, that is the page's drawer; otherwise it is the site //: menu. Either way the bars fold into a cross. @@ -266,10 +277,13 @@ export default function Navbar({ onSignIn, onRegister, onSearch }) { ? : Sign In } - {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 ? : Register - } + )} )} diff --git a/frontend/src/pages/LandingPage.jsx b/frontend/src/pages/LandingPage.jsx index c5f1c2e..6fb86d5 100644 --- a/frontend/src/pages/LandingPage.jsx +++ b/frontend/src/pages/LandingPage.jsx @@ -253,15 +253,30 @@ function AuthModal({ mode, onClose, onSwitch }) { // required" and nowhere to put one. const [inviteRequired, setInviteRequired] = useState(false) const [inviteCode, setInviteCode] = useState('') + const [signupOpen, setSignupOpen] = useState(true) useEffect(() => { let live = true 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(() => {}) 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 switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); setConfirm(''); onSwitch(m) } @@ -321,7 +336,7 @@ function AuthModal({ mode, onClose, onSwitch }) { {/* Tabs */}
- {[['login', 'Sign In'], ['register', 'Register']].map(([m, label]) => ( + {[['login', 'Sign In'], ...(signupOpen ? [['register', 'Register']] : [])].map(([m, label]) => ( ))}
diff --git a/frontend/src/pages/RegisterPage.jsx b/frontend/src/pages/RegisterPage.jsx index 1f8361a..355d81b 100644 --- a/frontend/src/pages/RegisterPage.jsx +++ b/frontend/src/pages/RegisterPage.jsx @@ -19,13 +19,22 @@ export default function RegisterPage() { // with, so the form knows whether to want a code. const [inviteRequired, setInviteRequired] = useState(false) 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() useEffect(() => { let live = true api.get('/auth/signup-policy') - .then(res => { if (live) setInviteRequired(!!res.data?.invite_required) }) - .catch(() => {}) + .then(res => { + if (!live) return + setInviteRequired(!!res.data?.invite_required) + setPolicy(res.data || {}) + }) + .catch(() => { if (live) setPolicy({}) }) return () => { live = false } }, []) @@ -59,6 +68,28 @@ export default function RegisterPage() { } } + if (policy === null) { + return
+ } + + // Nothing to fill in on a site where accounts come from the provider. + if (policy.sso_only) { + return ( +
+
+

Accounts come by invitation

+

+ This site signs in through {policy.provider_name || 'single sign-on'}. + Ask an administrator for a sign-up link. +

+ + Go to sign in + +
+
+ ) + } + if (done) { return (