Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update ban page SteamID check #579

Merged
merged 6 commits into from
Sep 27, 2019
Merged

Update ban page SteamID check #579

merged 6 commits into from
Sep 27, 2019

Conversation

rumblefrog
Copy link
Member

@rumblefrog rumblefrog commented Jul 20, 2019 •
edited
Loading

There are users with a much shorter Steam ID, and when attempting to ban those individuals, the check fails.

Description

Patches length

Motivation and Context

Fixes an issue with banning users with much shorter Steam ID

https://steamcommunity.com/profiles/76561197960265729 Should be the lowest with 11 characters.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

@rumblefrog
Update ban page SteamID length check
9964367 
There are users with a much shorter Steam ID, and when attempting to ban those individuals, the check fails.
@rumblefrog rumblefrog requested a review from Groruk July 20, 2019 23:00
@TheByKotik
Copy link
Contributor

TheByKotik commented Jul 21, 2019 •
edited
Loading

Why not just do it through XMLHttpRequest (JS Ajax) (or any ajax else)?

http://api.steampowered.com/ISteamUser/GetPlayerSummaries/v0002/?key=$SteamAPIKey&steamids=76561197960265729

Or (for custom url).

http://api.steampowered.com/ISteamUser/ResolveVanityURL/v0001/?key=$SteamAPIKey&vanityurl=$UserVanityURL

@CrazyHackGUT
Copy link
Contributor

Because for this checks, Steam API key is required.
XMLHttpRequest requires send for user Steam API key. This is insecure, because with key, user can generate GSLT tokens, for example: https://steamapi.xpaw.me/#IGameServersService

@TheByKotik
Copy link
Contributor

TheByKotik commented Jul 21, 2019 •
edited
Loading

Because for this checks, Steam API key is required.
XMLHttpRequest requires send for user Steam API key. This is insecure, because with key, user can generate GSLT tokens, for example: https://steamapi.xpaw.me/#IGameServersService

No-no-no.
Sorry for my "russian" english.
I'll just print a scheme.
From JS -> /ajax/is-player-valid.php -> call to SteamAPI -> return results to JS.

@Groruk
Copy link
Member

Groruk commented Jul 21, 2019

I wouldn't use steam API calls for ID validation, simply because you would risk running into rate limiting and with akamai's agressive rate limiting and IP blacklisting that would be counter productive (also one of the reasons we use the steam API so sparsely in SBPP)

@TheByKotik
Copy link
Contributor

I wouldn't use steam API calls for ID validation, simply because you would risk running into rate limiting and with akamai's agressive rate limiting and IP blacklisting that would be counter productive (also one of the reasons we use the steam API so sparsely in SBPP)

Anyway, I just suggested using more accurate methods. And maybe display more info about player in ban page.
Okay, I'll know that Steam API is not welcome here.

@geominorai
Copy link
Contributor

You could convert and copy the regex from https://github.com/sbpp/sourcebans-pp/blob/v1.x/web/includes/SteamID/SteamID.php#L64 for consistency.

@rumblefrog rumblefrog requested review from Groruk and removed request for Groruk September 25, 2019 04:09
@rumblefrog rumblefrog changed the title Update ban page SteamID length check Update ban page SteamID check Sep 25, 2019
rumblefrog and others added 4 commits September 27, 2019 14:36
@rumblefrog
remove comment
931e200
@Groruk
Change RegEx to include all SteamID formats
a60e63d 
Reference: https://developer.valvesoftware.com/wiki/SteamID
@Groruk
Use .test() instead of .match()
caccc2a 
Returns `true` if pattern is found in string, `false` otherwise
Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test
@Groruk
Use Ban Type for checks
e7d2e87 
Instead of checking if $('ip') or $('steam') are empty, just use the Ban Type selection from the form for this.
@rumblefrog rumblefrog merged commit bf074af into v1.x Sep 27, 2019
@rumblefrog rumblefrog deleted the steamid-length-check branch September 27, 2019 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Groruk Groruk Groruk approved these changes

Assignees
No one assigned
Labels
semver: patch t: ui/ux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rumblefrog @TheByKotik @CrazyHackGUT @Groruk @geominorai