Repository: CTFd/CTFd
Pull request #918 (2019-03-22 06:34:07)

Dont calculate hidden users in place
* Users that are hidden shouldn't be factored into the calculation of place for other players. 
* Clean up a few other tests. 

codecov[bot]: # [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=h1) Report
> Merging [#918](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=desc) into [master](https://codecov.io/gh/CTFd/CTFd/commit/37dcfdc5682537e2e6b5d6a46a5da2c5b8ed32a5?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/CTFd/CTFd/pull/918/graphs/tree.svg?width=650&token=IfPqkWkt9A&height=150&src=pr)](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master    #918   +/-   ##
======================================
  Coverage    87.3%   87.3%           
======================================
  Files          91      91           
  Lines        4630    4630           
======================================
  Hits         4042    4042           
  Misses        588     588
```


| [Impacted Files](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [CTFd/models/\_\_init\_\_.py](https://codecov.io/gh/CTFd/CTFd/pull/918/diff?src=pr&el=tree#diff-Q1RGZC9tb2RlbHMvX19pbml0X18ucHk=) | `88.96% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=footer). Last update [37dcfdc...5dcf180](https://codecov.io/gh/CTFd/CTFd/pull/918?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ColdHeat: @xmsec does this PR resolve your issue? 
xmsec: Yes, it does.

Patch:
diff --git a/CTFd/models/__init__.py b/CTFd/models/__init__.py
index f93aeff9..f5484974 100644
--- a/CTFd/models/__init__.py
+++ b/CTFd/models/__init__.py
@@ -438,7 +438,7 @@ class Users(db.Model):
                 Users.id.label('user_id'),
             ) \
                 .join(sumscores, Users.id == sumscores.columns.user_id) \
-                .filter(Users.banned == False) \
+                .filter(Users.banned == False, Users.hidden == False) \
                 .order_by(sumscores.columns.score.desc(), sumscores.columns.id)
 
         standings = standings_query.all()


Test patch:
diff --git a/tests/users/test_scoreboard.py b/tests/users/test_scoreboard.py
index 39630726..ba1be4b6 100644
--- a/tests/users/test_scoreboard.py
+++ b/tests/users/test_scoreboard.py
@@ -5,34 +5,24 @@ from freezegun import freeze_time
 from tests.helpers import *
 
 
-def test_user_get_scoreboard():
-    """Can a registered user load /scoreboard"""
+def test_user_get_scoreboard_components():
     app = create_ctfd()
     with app.app_context():
         register_user(app)
         client = login_as_user(app)
+
+        # test_user_get_scoreboard
+        """Can a registered user load scoreboard components"""
         r = client.get('/scoreboard')
         assert r.status_code == 200
-    destroy_ctfd(app)
-
 
-def test_user_get_scores():
-    """Can a registered user load /api/v1/scoreboard"""
-    app = create_ctfd()
-    with app.app_context():
-        register_user(app)
-        client = login_as_user(app)
+        # test_user_get_scores
+        """Can a registered user load /api/v1/scoreboard"""
         r = client.get('/api/v1/scoreboard')
         assert r.status_code == 200
-    destroy_ctfd(app)
-
 
-def test_user_get_topteams():
-    """Can a registered user load /api/v1/scoreboard/top/10"""
-    app = create_ctfd()
-    with app.app_context():
-        register_user(app)
-        client = login_as_user(app)
+        # test_user_get_topteams
+        """Can a registered user load /api/v1/scoreboard/top/10"""
         r = client.get('/api/v1/scoreboard/top/10')
         assert r.status_code == 200
     destroy_ctfd(app)
@@ -295,3 +285,42 @@ def test_scoring_logic_with_zero_point_challenges():
         scores = get_scores(admin)
         assert scores[0]['name'] == 'user2'
     destroy_ctfd(app)
+
+
+def test_hidden_users_should_not_influence_scores():
+    app = create_ctfd()
+    with app.app_context():
+        register_user(app, name="user1", email="user1@ctfd.io", password="password")
+        register_user(app, name="user2", email="user2@ctfd.io", password="password")
+        register_user(app, name="user3", email="user3@ctfd.io", password="password")
+
+        user = Users.query.filter_by(name="user3").first()
+        user.hidden = True
+        app.db.session.commit()
+
+        client1 = login_as_user(app, name="user1", password="password")
+        client2 = login_as_user(app, name="user2", password="password")
+
+        # User 1 solves 1st challenge
+        chal1 = gen_challenge(app.db)
+        gen_solve(app.db, user_id=2, challenge_id=chal1.id)
+
+        # User 2 solves 2nd challenge
+        chal2 = gen_challenge(app.db)
+        gen_solve(app.db, user_id=3, challenge_id=chal2.id)
+
+        # User 3 solves both
+        gen_solve(app.db, user_id=4, challenge_id=chal1.id)
+        gen_solve(app.db, user_id=4, challenge_id=chal2.id)
+
+        scores = get_scores(client1)
+
+        for entry in scores:
+            assert entry['name'] != 'user3'
+
+        user1 = Users.query.filter_by(name="user1").first()
+        assert user1.place == '1st'
+
+        user2 = Users.query.filter_by(name="user2").first()
+        assert user2.place == '2nd'
+    destroy_ctfd(app)