8000 Updated Functions | Easier Logout | Easier Login by RedSparr0w · Pull Request #116 · SmItH197/SteamAuthentication · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Updated Functions | Easier Logout | Easier Login #116

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

Merged
merged 5 commits into from
Mar 25, 2016

Conversation

RedSparr0w
Copy link
Contributor

made demo.php file a bit tidier and easier to read

Also added check for !isset($_GET['login']) as to avoid having to refresh on login


echo "welcome guest! please login \n \n";
steamlogin(); //login button

} else {
steamlogin(); //login
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesnt make sense at all.
Youre displaying a login button even if the user is logged in.

@RedSparr0w
Copy link
Contributor Author

Sorry, Had not set a "loginpage" hence the refresh problem. XD

Have updated "settings.php" page so when you login, it will redirect you back to the page where you originally clicked the login button as i think this would make more sense?

Also Tidied up "demo.php" page for easier readability

@RedSparr0w RedSparr0w changed the title Readability & Fix Example Needing Refresh Readability & Example Redirect back to refering page Feb 27, 2016
@BlackCetha
Copy link
Contributor

Did you test it?
AFAIK PHP_SELF will be either /steamauth/settings.php or / because of the redirection from steam.
Also, what exactly is your "refresh problem"?
While the changes to the demo make sense the one in the settings doesnt.

Please split them up at least. 2 Features/Changes = 2 PRs.

@RedSparr0w
Copy link
Contributor Author
  • steam redirects back to the origin page so PHP_SELF is the origin page ie /demo.php (doesnt refer to /steamauth/settings.php page) - have tested with demo page and example page
  • the "refresh problem" i was getting was because no $steamauth['loginpage'] was specified so was not redirecting/reloading after signin
  • will split it into 2 different PRs now

@RedSparr0w RedSparr0w force-pushed the master branch 2 times, most recently from 93ea1f9 to d8c53c0 Compare February 27, 2016 22:04
@RedSparr0w RedSparr0w changed the title Readability & Example Redirect back to refering page Redirect back to refering page Feb 27, 2016
@RedSparr0w
Copy link
Contributor Author

Have updated now.

Changed they way i did it so "settings.php" file remains unchanged but updated the steamlogin() function so if $steamauth['loginpage'] is not set to a specific page in the settings then it will reload the page you are currently on.

Update steamlogin() Function to reload refering page if no "loginpage" specified
@BlackCetha
Copy link
Contributor

Seems fine now.
I actually made the same change a few month ago, but we made a rollback because there were too many bugs in a bunch of commits.

@RedSparr0w
Copy link
Contributor Author

just had a look

if($steamauth['loginpage'] !== "") {
    $returnTo = $steamauth['loginpage'];
} else {
    //Determine the return to page. We substract "login&"" to remove the login var from the URL.
    //"file.php?login&foo=bar" would become "file.php?foo=bar"
    $returnTo = str_replace('login&', '', $_GET['openid_return_to']);
    //If it didn't change anything, it means that there's no additionals vars, so remove the login var so that we don't get redirected to Steam over and over.
    if($returnTo === $_GET['openid_return_to']) $returnTo = str_replace('?login', '', $_GET['openid_return_to']);
}

not sure why that wouldn't have worked.
only thing i can see is $returnTo = str_replace('login&', '', $_GET['openid_return_to']); wouldnt do anything due to if there was a "&" in the return url it would be interpreted as a new $_GET param in the current url and therefore disregarded/not included in the $_GET['openid_return_to'] param?

this could also work instead:

if (empty($steamauth['loginpage'])) {
    $steamauth['loginpage'] = str_replace('?login', '', $_GET['openid_return_to']);
}
header('Location: '.$steamauth['loginpage']);

@BlackCetha
Copy link
Contributor

Steam strips any GET-parameters from the return-url automatically, so it didnt work.
After that we tried to get it to work in some way, but it only made things worse.

8000

@RedSparr0w
Copy link
Contributor Author

i feel like my changes should work, have tested it out on a localserver and c9 server and my own site, all seemed to work fine.

Removed logout.php file - changed to function in steamauth.php

Added
- loginbutton() | Will display login button
- if (isset($_GET['login'])) | will log user in (regardless of page)
- if (isset($_GET['logout'])) | will log user out (regardless of page)
- if (isset($_GET['update'])) | will update user info (regardless of
page)

if no login/logout page set will refresh current page by default
@RedSparr0w RedSparr0w changed the title Redirect back to refering page Updated Functions | Easier Logout | Easier Login Feb 28, 2016
@RedSparr0w
Copy link
Contributor Author

RedSparr0w@228e868 fixes #107

@RedSparr0w
Copy link
Contributor Author

also feel free to test out my version of this code here (demo.php) and here (example.php)

@bman46
Copy link
Contributor
bman46 commented Feb 28, 2016

Btw the version # is 3.0

@bman46
Copy link
Contributor
bman46 commented Feb 28, 2016

Make sure u change that on the website view b/c it says 2. Something

@RedSparr0w RedSparr0w force-pushed the master branch 2 times, most recently from f01b0a5 to 2c80793 Compare February 28, 2016 03:59
@RedSparr0w
Copy link
Contributor Author

Thanks, Updated [=

Destroy / Unset Session vars on logout
@SmItH197 SmItH197 merged commit 1869d2c into SmItH197:master Mar 25, 2016
@SmItH197
Copy link
Owner

1869d2c created a conflict in the README file, I've fixed this and merged.

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
0