Skip to content

Commit

Permalink
Tightens up interview security (#3875)
Browse files Browse the repository at this point in the history
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->
<!-- You can view Contributing.MD for a detailed description of the pull
request process. -->

## About The Pull Request

Interviews have become a pretty crucial part of our administration
tools, so I figured I would audit their use a bit to make sure they're
secure. Changes the code to presume a new joiner requires an interview
until proven otherwise, and prevents interviewees from accessing donator
stuff, not that they could do anything with that, but they also almost
certainly don't need it.

## Why It's Good For The Game

Interviews good

## Changelog

:cl:
/:cl:

<!-- Both :cl:'s are required for the changelog to work! You can put
your name to the right of the first :cl: if you want to overwrite your
GitHub username as author ingame. -->
<!-- You can use multiple of the same prefix (they're only used for the
icon ingame) and delete the unneeded ones. Despite some of the tags,
changelogs should generally represent how a player might be affected by
the changes rather than a summary of the PR's contents. -->
  • Loading branch information
MarkSuckerberg authored Dec 12, 2024
1 parent dfc4824 commit d5bd3a5
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 37 deletions.
2 changes: 1 addition & 1 deletion code/modules/client/client_defines.dm
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@
var/next_move_dir_sub

/// If the client is currently under the restrictions of the interview system
var/interviewee = FALSE
var/interviewee = TRUE

/// Used by SSserver_maint to detect if a client is newly AFK.
var/last_seen_afk = 0
Expand Down
7 changes: 6 additions & 1 deletion code/modules/client/client_procs.dm
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
prefs.last_id = computer_id //these are gonna be used for banning
fps = prefs.clientfps == 0 ? 60 : prefs.clientfps //WS Edit - Client FPS Tweak

donator = GLOB.donators[ckey] || new /datum/donator(src)

if(fexists(roundend_report_file()))
add_verb(src, /client/proc/show_previous_roundend_report)

Expand Down Expand Up @@ -933,8 +935,11 @@ GLOBAL_LIST_INIT(blacklisted_builds, list(
..()

/client/proc/add_verbs_from_config()
if (interviewee)
if(interviewee)
return
if(donator.is_donator)
add_verb(src, /client/proc/do_donator_redemption)
add_verb(src, /client/proc/do_donator_wcir)
if(CONFIG_GET(flag/see_own_notes))
add_verb(src, /client/proc/self_notes)
if(CONFIG_GET(flag/use_exp_tracking))
Expand Down
38 changes: 13 additions & 25 deletions code/modules/donator/_donator.dm
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,6 @@ GLOBAL_PROTECT(donators)

/client/var/datum/donator/donator

/client/New(TopicData)
. = ..()
donator = GLOB.donators[ckey] || new /datum/donator(src)
donator.owner = src
add_verb(src, /client/proc/do_donator_redemption)
add_verb(src, /client/proc/do_donator_wcir)

/client/Destroy()
. = ..()
if(donator) // it's possible that a client was qdel'd inside the initializer
donator.owner = null
donator = null

/client/proc/do_donator_redemption()
set name = "Redeem Donator Reward"
set category = "OOC.Donator"
Expand All @@ -46,31 +33,27 @@ GLOBAL_PROTECT(donators)
/datum/donator
/// ckey of the client who this datum belongs to
var/ckey
/// reference to the client
var/client/owner

/// Whether or not this datum actually is a real donator
var/is_donator = FALSE

/// typecache of eligible rewards for this donator
var/list/flat_rewards = list(
/obj/item/reagent_containers/food/snacks/cookie = TRUE
)
var/list/flat_rewards = list()

/// list of conversion rewards for this donator
/// Expected format: base type -> list of convertible types
var/list/conversion_rewards = list(
)
var/list/conversion_rewards = list()

/// list of reskin rewards for this donator
/// Should be an assosciative list indexed by type with a value which is a list of skins
var/list/reskin_rewards = list(
)
var/list/reskin_rewards = list()

/// list of redeemed conversion types
var/list/conversions_redeemed = list()

/datum/donator/New(client/owner)
. = ..()
src.ckey = owner.ckey
src.owner = owner
load_information()
GLOB.donators[ckey] = src

Expand All @@ -79,13 +62,14 @@ GLOBAL_PROTECT(donators)
return QDEL_HINT_LETMELIVE
. = ..()
GLOB.donators -= ckey
owner.donator = null
owner = null

/datum/donator/proc/load_information() //todo: db support with config files being a backup method
var/json_file = file(REWARD_JSON_PATH + "[ckey].json")
if(!fexists(json_file))
return

is_donator = TRUE

var/list/json = safe_json_decode(file2text(json_file))

if(!json || !("ckey" in json))
Expand Down Expand Up @@ -180,6 +164,10 @@ GLOBAL_PROTECT(donators)
. += rinstance

/datum/donator/proc/what_can_i_redeem(mob/user)
if(!is_donator)
to_chat(user, span_notice("You are not a donator! If you are, please contact an admin on the discord."))
return

var/resp = list()
resp += "<span class='fakespan0'>----------</span>"
resp += "Your current redeemable rewards are as follows:"
Expand Down
15 changes: 8 additions & 7 deletions code/modules/interview/interview.dm
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,14 @@
set name = "Open Interview"
set category = "Admin.Interview"
var/mob/dead/new_player/M = usr
if (M?.client?.interviewee)
var/datum/interview/I = GLOB.interviews.interview_for_client(M.client)
if (I) // we can be returned nothing if the user is on cooldown
I.ui_interact(M)
else
to_chat(usr, "<span class='adminsay'>You are on cooldown for interviews. Please" \
+ " wait at least 3 minutes before starting a new questionnaire.</span>", confidential = TRUE)
if (!M?.client?.interviewee)
return
var/datum/interview/I = GLOB.interviews.interview_for_client(M.client)
if (I) // we can be returned nothing if the user is on cooldown
I.ui_interact(M)
else
to_chat(usr, "<span class='adminsay'>You are on cooldown for interviews. Please" \
+ " wait at least 3 minutes before starting a new questionnaire.</span>", confidential = TRUE)

/datum/interview/ui_interact(mob/user, datum/tgui/ui = null)
ui = SStgui.try_update_ui(user, src, ui)
Expand Down
2 changes: 1 addition & 1 deletion code/modules/mob/dead/new_player/login.dm
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
var/required_living_minutes = CONFIG_GET(number/panic_bunker_living)
var/living_minutes = client.get_exp_living(TRUE)
if (required_living_minutes > living_minutes)
client.interviewee = TRUE
register_for_interview()
return
client.interviewee = FALSE

new_player_panel()
if(SSticker.current_state < GAME_STATE_SETTING_UP)
Expand Down
4 changes: 2 additions & 2 deletions code/modules/mob/dead/new_player/new_player.dm
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,10 @@
return

if(src != usr)
return 0
return FALSE

if(!client)
return 0
return FALSE

if(client.interviewee)
return FALSE
Expand Down

0 comments on commit d5bd3a5

Please sign in to comment.