Repository: CTFd/CTFd (python)

Issue:
Free Hints should require unlock
Free hints should require users to unlock the hint 

* We can track when users open free hints  
* It makes it less likely that a user will unlock a hint accidentally
* This makes the flow the same for all hints

Pull request description:
Require unlocking for free hints
* Require unlocking for free hints as well as for paid hints
* Add hint title to the Challenge detail endpoint after hint is unlocked
* Add config that allows public users to see free hints without logging in
* Closes #2737

Patch:
diff --git a/CTFd/api/v1/challenges.py b/CTFd/api/v1/challenges.py
index 8cf797a2..c5788a0a 100644
--- a/CTFd/api/v1/challenges.py
+++ b/CTFd/api/v1/challenges.py
@@ -396,7 +396,12 @@ class Challenge(Resource):
         for hint in Hints.query.filter_by(challenge_id=chal.id).all():
             if hint.id in unlocked_hints or ctf_ended():
                 hints.append(
-                    {"id": hint.id, "cost": hint.cost, "content": hint.content}
+                    {
+                        "id": hint.id,
+                        "cost": hint.cost,
+                        "title": hint.title,
+                        "content": hint.content,
+                    }
                 )
             else:
                 hints.append({"id": hint.id, "cost": hint.cost, "title": hint.title})
diff --git a/CTFd/api/v1/hints.py b/CTFd/api/v1/hints.py
index 8d347fd9..afa4aba0 100644
--- a/CTFd/api/v1/hints.py
+++ b/CTFd/api/v1/hints.py
@@ -9,6 +9,7 @@ from CTFd.api.v1.schemas import APIDetailedSuccessResponse, APIListSuccessRespon
 from CTFd.constants import RawEnum
 from CTFd.models import Hints, HintUnlocks, db
 from CTFd.schemas.hints import HintSchema
+from CTFd.utils import get_config
 from CTFd.utils.decorators import admins_only, during_ctf_time_only
 from CTFd.utils.decorators.visibility import check_challenge_visibility
 from CTFd.utils.helpers.models import build_model_filters
@@ -132,6 +133,19 @@ class Hint(Resource):
                     },
                     403,
                 )
+            else:
+                # Allow admins to allow public CTFs to see free hints if they wish
+                if bool(get_config("hints_free_public_access", False)) is True:
+                    response = HintSchema(view="unlocked").dump(hint)
+                    return {"success": True, "data": response.data}
+                else:
+                    return (
+                        {
+                            "success": False,
+                            "errors": {"cost": ["You must login to unlock this hint"]},
+                        },
+                        403,
+                    )
 
         if hint.prerequisites:
             requirements = hint.prerequisites
@@ -140,13 +154,6 @@ class Hint(Resource):
             all_unlocks = HintUnlocks.query.filter_by(account_id=user.account_id).all()
             unlock_ids = {unlock.target for unlock in all_unlocks}
 
-            # Get the IDs of all free hints
-            free_hints = Hints.query.filter_by(cost=0).all()
-            free_ids = {h.id for h in free_hints}
-
-            # Add free hints to unlocked IDs
-            unlock_ids.update(free_ids)
-
             # Filter out hint IDs that don't exist
             all_hint_ids = {h.id for h in Hints.query.with_entities(Hints.id).all()}
             prereqs = set(requirements).intersection(all_hint_ids)
@@ -167,14 +174,12 @@ class Hint(Resource):
                     403,
                 )
 
-        view = "unlocked"
-        if hint.cost:
-            view = "locked"
-            unlocked = HintUnlocks.query.filter_by(
-                account_id=user.account_id, target=hint.id
-            ).first()
-            if unlocked:
-                view = "unlocked"
+        view = "locked"
+        unlocked = HintUnlocks.query.filter_by(
+            account_id=user.account_id, target=hint.id
+        ).first()
+        if unlocked:
+            view = "unlocked"
 
         if is_admin():
             if request.args.get("preview", False):
diff --git a/CTFd/forms/config.py b/CTFd/forms/config.py
index dc3857fb..7802a972 100644
--- a/CTFd/forms/config.py
+++ b/CTFd/forms/config.py
@@ -138,6 +138,15 @@ class LegalSettingsForm(BaseForm):
     submit = SubmitField("Update")
 
 
+class ChallengeSettingsForm(BaseForm):
+    hints_free_public_access = SelectField(
+        "Hints Free Public Access",
+        description="Control whether users must be logged in to see free hints (hints without cost or requirements)",
+        choices=[("true", "Enabled"), ("false", "Disabled")],
+        default="false",
+    )
+
+
 class VisibilitySettingsForm(BaseForm):
     challenge_visibility = SelectField(
         "Challenge Visibility",
diff --git a/CTFd/themes/admin/templates/config.html b/CTFd/themes/admin/templates/config.html
index e3b077b7..98957005 100644
--- a/CTFd/themes/admin/templates/config.html
+++ b/CTFd/themes/admin/templates/config.html
@@ -78,6 +78,15 @@
 					</a>
 				</li>
 
+				<!-- Challenges -->
+				<small class="section-title">Challenges</small>
+				<li class="nav-item">
+					<a class="nav-link rounded-0 py-1" href="#challenges" role="tab" data-toggle="tab">
+						<i class="fa fa-users action-icon"></i>Challenges
+					</a>
+				</li>
+
+				<!-- Backup -->
 				<small class="section-title">Backup</small>
 				<li class="nav-item">
 					<a class="nav-link rounded-0 py-1" href="#backup" role="tab" data-toggle="tab">
@@ -201,6 +210,10 @@
 
 				{% include "admin/configs/registration_code.html" %}
 
+				<!-- Challeges -->
+
+				{% include "admin/configs/challenges.html" %}
+
 				<!-- Security -->
 
 				{% include "admin/configs/backup.html" %}
diff --git a/CTFd/themes/admin/templates/configs/challenges.html b/CTFd/themes/admin/templates/configs/challenges.html
new file mode 100644
index 00000000..08eff4db
--- /dev/null
+++ b/CTFd/themes/admin/templates/configs/challenges.html
@@ -0,0 +1,20 @@
+<div role="tabpanel" class="tab-pane config-section" id="challenges">
+    {% set hints_free_public_access = "true" if hints_free_public_access == True else "false" %}
+	{% 
+		with form = Forms.config.ChallengeSettingsForm(
+			hints_free_public_access=hints_free_public_access,
+		) 
+	%}
+	<form method="POST" autocomplete="off" class="w-100">
+		<div class="form-group">
+			{{ form.hints_free_public_access.label }}
+			{{ form.hints_free_public_access(class="form-control custom-select") }}
+			<small class="form-text text-muted">
+				{{ form.hints_free_public_access.description }}
+			</small>
+		</div>
+
+		<button type="submit" class="btn btn-md btn-primary float-right">Update</button>
+	</form>
+	{% endwith %}
+</div>


Test patch:
diff --git a/tests/api/v1/user/test_hints.py b/tests/api/v1/user/test_hints.py
index e7609135..fbaef4e5 100644
--- a/tests/api/v1/user/test_hints.py
+++ b/tests/api/v1/user/test_hints.py
@@ -206,6 +206,14 @@ def test_api_hints_accessible_public():
         app.db.session.commit()
 
         with app.test_client() as non_logged_in_user:
+            # Hints cannot be seen unless free public access is on
+            r = non_logged_in_user.get("/api/v1/hints/1")
+            assert r.status_code == 403
+            errors = r.get_json()["errors"]
+            assert errors == {"cost": ["You must login to unlock this hint"]}
+
+            # Enable free public access
+            set_config("hints_free_public_access", True)
             r = non_logged_in_user.get("/api/v1/hints/1")
             hint = r.get_json()["data"]
             assert hint["content"] == "This is a free hint"
@@ -235,6 +243,13 @@ def test_api_hints_accessible_public():
 
         # Verify existing hint behavior for authed users
         with login_as_user(app) as client:
+            # Free hints require an unlock
+            r = client.get("/api/v1/hints/1")
+            hint = r.get_json()["data"]
+            assert hint.get("content") is None
+
+            # Issue the unlock and then request the content
+            r = client.post("/api/v1/unlocks", json={"target": 1, "type": "hints"})
             r = client.get("/api/v1/hints/1")
             hint = r.get_json()["data"]
             assert hint["content"] == "This is a free hint"
diff --git a/tests/users/test_hints.py b/tests/users/test_hints.py
index e1755436..f8a795b7 100644
--- a/tests/users/test_hints.py
+++ b/tests/users/test_hints.py
@@ -93,6 +93,17 @@ def test_unlocking_hints_with_no_cost():
         chal_id = chal.id
         gen_hint(app.db, chal_id)
         client = login_as_user(app)
+        # Attempt to access hint
+        r = client.get("/api/v1/hints/1")
+        resp = r.get_json()["data"]
+
+        # Hint does not provide content until an unlock is generated
+        assert resp.get("content") is None
+
+        # We generate an unlock for the free hint
+        client.post("/api/v1/unlocks", json={"target": 1, "type": "hints"})
+
+        # We should now be able to see content
         r = client.get("/api/v1/hints/1")
         resp = r.get_json()["data"]
         assert resp.get("content") == "This is a hint"
@@ -269,6 +280,8 @@ def test_unlocking_hint_for_unicode_challenge():
 
         client = login_as_user(app)
 
+        # Generate an unlock for the free hint
+        client.post("/api/v1/unlocks", json={"target": 1, "type": "hints"})
         r = client.get("/api/v1/hints/1")
         assert r.status_code == 200
         resp = r.get_json()["data"]