Merged
Conversation
-Muted file_get_contents() so that the API key is not publicly exposed -userInfo.php won't do anything if the user hasn't logged in -Added the constant STEAMAUTH_LOGOUT_LINK to make logout more flexible.
Contributor
|
Good improvements, except that constant. |
Contributor
Author
|
Hi,
|
Contributor
|
I cannot change your Pull Request. Go ahead and edit the files in your fork. It will automatically add theese changes to the PR. |
Contributor
Author
|
Ok, I've commited the new changes. |
Contributor
|
This looks ready to merge from my side now. |
Owner
|
Sorry about the delay, been really busy with exams. Thanks for your help! :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey there. First of all, I'll be using your framework for my project wich I'll upload to GitHub soon, so thank you. Now, I've made some changes that you may want to consider adding to your project:
Please, do consider making this change: with the current script, if the server doesn't reach the Steam servers (i.e: Steam is down),
file_get_contents()will display a warning to the user with the URL used, and this URL contains the API key. We do not want the user to be able to access it; this is a security issue.I know that in the example you handle this manually by including only userInfo.php when the user has logged in, but a good framework should take care of everything for you. In the current script, including this file without an user session wastes resources, increases load time and displays warnings. With this fix, it simply won't do anything.
Its value is
steamauth/logout.phpand it is an alternative tologoutbutton(), wich provides more flexibility. Withlogoutbutton()you get a simple, non-customizable button, but with the constant you can make your own button, or simply have a logout link.Many thanks for your time.