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.

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

Add PurchaseResponseCallback and RedeemGuestPassResponseCallback #1033

Merged
merged 1 commit into from
Oct 24, 2021
Merged

Add PurchaseResponseCallback and RedeemGuestPassResponseCallback #1033

merged 1 commit into from
Oct 24, 2021

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Oct 8, 2021

cc @JustArchi for review.

I didn't pull any info out of the keyvalue to leave it up to the consumer, same as the other callbacks.

Actual methods to do these actions are left as an exercise for the consumer.

@JustArchi
Copy link
Contributor

JustArchi commented Oct 8, 2021

As those callbacks are directly tied to the requests that fire them, I believe we should also add functions for that.

Actual methods to do these actions are left as an exercise for the consumer.

Does it make sense to add them then?

@xPaw
Copy link
Member Author

xPaw commented Oct 8, 2021

I can add them if maintainers are not opposed.

@JustArchi
Copy link
Contributor

Reposting my thoughts from Discord:

I understand why it was left behind
but then:
a) knowing gift gid doesn't allow you to grab the gift for yourself IIRC, as it's still tied to the account, feel free to prove me wrong if you believe otherwise
b) redeeming keys can be abused, but there is volvo limit for 1h after 10 failed activations, and brute-forcing them is rather ineffective
I don't mind either way
and I understand if SK2 devs have different view on that
but it seems a bit silly to me
based on this premise whole SK2 should be "excercise for the consumer" because it can be abused even with login procedure
we can also look at it the other way
ASF has it in plain open even with plugin system and I didn't hear from valve banning 1.3 million of ASF accounts yet
neither them contacting me saying to remove it because they ask nicely
and it's in since like ever as the core functionality, so 6 years at least

@JustArchi
Copy link
Contributor

All in all my opinion is to put it in, especially after we allowed consumers to even provide custom machine details, it doesn't have bigger abuse scope than whole login procedure, and I don't believe the potential is justifying refusal of a valid feature.

But that's me, and I'm not deciding here 😁

@yaakov-h
Copy link
Member

yaakov-h commented Oct 9, 2021

I'm conflicted on adding the methods to kick these flows off.

On one hand we don't want to make it easy for people to come in and start brute-forcing things, or malware that redeems keys that it finds. e.g. there was something I read earlier this week about malware that scrapes people's inboxes and looks for gift cards...

On the other hand, any time somebody asks, they usually land up in ASF anyway with working sample code, so the gatekeeping is minimal at best.

We could add it with [EditorBrowsable(Never)] so that it is there but not suggested by Intellisense, but that might be just as pointless.

Really this should be protected on the server side anyway, which @JustArchi indicates above that Valve already seem to be doing (yay). IIRC there are also IClient interfaces for this so people could also just do this with a legitimate first-part client...

@voided what are your thoughts?

@xPaw
Copy link
Member Author

xPaw commented Oct 9, 2021

FYI you get rate limited for an hour after 10 (or was it 5?) invalid keys.

@yaakov-h yaakov-h requested a review from voided October 18, 2021 00:49
Copy link
Member

@voided voided left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned on IRC - I'm not thrilled by adding this, but I suppose with this PR it's "officially" out of the bag anyway. The server side rate limits make me feel a little better, but that'll be moot point for malware similar to the email scanning one where it discovers primarily valid keys...

@xPaw
Copy link
Member Author

xPaw commented Oct 18, 2021

Valid key activation is also rate limit to 50 keys (if I remember correctly) an hour.

Are you approving just the callbacks, or should I add the methods?

@voided
Copy link
Member

voided commented Oct 19, 2021

Just callbacks - if anything I still believe in having some barrier to entry.

@yaakov-h yaakov-h added this to the 2.4.0 milestone Oct 24, 2021
@yaakov-h yaakov-h merged commit 8c0c2df into SteamRE:master Oct 24, 2021
@xPaw xPaw deleted the purchase-callbacks branch October 24, 2021 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants