From 800fb8260a07f5405bace81d1946c0a091bc02fe Mon Sep 17 00:00:00 2001
From: Kevin Chung <kchung@ctfd.io>
Date: Sat, 3 Dec 2022 12:16:11 -0500
Subject: [PATCH] Clarify Score Visibility and Account Visibility (#2227)

* Don't show /scoreboard if we do not have account_visibility
* Clarify the behavior of Score Visibility with respect to Account Visibility
---
 CTFd/scoreboard.py                            |  6 ++-
 .../admin/templates/configs/settings.html     | 47 +++++++++----------
 .../core/templates/components/navbar.html     |  2 +-
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/CTFd/scoreboard.py b/CTFd/scoreboard.py
index 7e9b3409..261813b5 100644
--- a/CTFd/scoreboard.py
+++ b/CTFd/scoreboard.py
@@ -2,7 +2,10 @@ from flask import Blueprint, render_template
 
 from CTFd.utils import config
 from CTFd.utils.config.visibility import scores_visible
-from CTFd.utils.decorators.visibility import check_score_visibility
+from CTFd.utils.decorators.visibility import (
+    check_account_visibility,
+    check_score_visibility,
+)
 from CTFd.utils.helpers import get_infos
 from CTFd.utils.scores import get_standings
 from CTFd.utils.user import is_admin
@@ -11,6 +14,7 @@ scoreboard = Blueprint("scoreboard", __name__)
 
 
 @scoreboard.route("/scoreboard")
+@check_account_visibility
 @check_score_visibility
 def listing():
     infos = get_infos()
diff --git a/CTFd/themes/admin/templates/configs/settings.html b/CTFd/themes/admin/templates/configs/settings.html
index cc074b2c..876a4d2a 100644
--- a/CTFd/themes/admin/templates/configs/settings.html
+++ b/CTFd/themes/admin/templates/configs/settings.html
@@ -20,6 +20,26 @@
 			</select>
 		</div>
 
+		<div class="form-group">
+			<label>
+				Account Visibility<br>
+				<small class="form-text text-muted">
+					Control whether accounts (users &amp; teams) are shown to everyone, only to authenticated users, or only to admins
+				</small>
+			</label>
+			<select class="form-control custom-select" name="account_visibility">
+				<option value="public" {% if account_visibility == 'public' %}selected{% endif %}>
+					Public
+				</option>
+				<option value="private" {% if account_visibility == 'private' %}selected{% endif %}>
+					Private
+				</option>
+				<option value="admins" {% if account_visibility == 'admins' %}selected{% endif %}>
+					Admins Only
+				</option>
+			</select>
+		</div>
+
 		<div class="form-group">
 			<label>
 				Score Visibility<br>
@@ -42,30 +62,9 @@
 				</option>
 			</select>
 			<small class="form-text text-muted">
-				This setting should generally be the same as Account Visibility to avoid conflicts.
-			</small>
-		</div>
-
-		<div class="form-group">
-			<label>
-				Account Visibility<br>
-				<small class="form-text text-muted">
-					Control whether accounts (users &amp; teams) are shown to everyone, only to authenticated users, or only to admins
-				</small>
-			</label>
-			<select class="form-control custom-select" name="account_visibility">
-				<option value="public" {% if account_visibility == 'public' %}selected{% endif %}>
-					Public
-				</option>
-				<option value="private" {% if account_visibility == 'private' %}selected{% endif %}>
-					Private
-				</option>
-				<option value="admins" {% if account_visibility == 'admins' %}selected{% endif %}>
-					Admins Only
-				</option>
-			</select>
-			<small class="form-text text-muted">
-				This setting should generally be the same as Score Visibility to avoid conflicts.
+				Score Visibility is a subset of Account Visibility. 
+				This means that if accounts are visible to a user then score visibility will control whether they can see the score of that user. 
+				If accounts are not visibile then score visibility has no effect.
 			</small>
 		</div>
 
diff --git a/CTFd/themes/core/templates/components/navbar.html b/CTFd/themes/core/templates/components/navbar.html
index c2a1f580..1a5f0453 100644
--- a/CTFd/themes/core/templates/components/navbar.html
+++ b/CTFd/themes/core/templates/components/navbar.html
@@ -30,7 +30,7 @@
                     {% endif %}
                 {% endif %}
 
-                {% if Configs.score_visibility != 'admins' %}
+                {% if Configs.account_visibility != 'admins' and Configs.score_visibility != 'admins' %}
                     <li class="nav-item">
                         <a class="nav-link" href="{{ url_for('scoreboard.listing') }}">Scoreboard</a>
                     </li>