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

/logout should invalidate a player's session #816

Closed
Twonox opened this issue Jul 2, 2016 · 14 comments
Closed

/logout should invalidate a player's session #816

Twonox opened this issue Jul 2, 2016 · 14 comments
Assignees
Labels
Priority: critical Marks issues that should be fixed/closed as soon as possible. Type: enhancement
Milestone

Comments

@Twonox
Copy link
Contributor

Twonox commented Jul 2, 2016

When we do /logout during the session, the session don't expire. We can do /logout, disconnect then connect to the server without entering our pass.

@sgdc3
Copy link
Member

sgdc3 commented Jul 2, 2016

@ljacqu This is related to the Bungeecord messaging refactor (we need to create an iasue for it) and the session refactor

@EbonJaeger
Copy link
Contributor

EbonJaeger commented Jul 11, 2016

Id say this is the same issue as #824, and thus related to #582.

@Twonox
Copy link
Contributor Author

Twonox commented Jul 12, 2016

I don't use bungeecord

@ljacqu ljacqu changed the title /logout & session /logout should invalidate a player's session Jul 12, 2016
@sgdc3 sgdc3 self-assigned this Jul 12, 2016
@sgdc3 sgdc3 added Status: in progress Priority: critical Marks issues that should be fixed/closed as soon as possible. labels Jul 12, 2016
@sgdc3 sgdc3 added this to the 5.2 Release milestone Jul 12, 2016
@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

@Twonox Done ;)

@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

Whoops found that we already do this

@ljacqu
Copy link
Member

ljacqu commented Jul 12, 2016

Reverted -> code was uglier (let SessionManager worry about session settings, why check if player is authenticated inside the logout process?) and did not fix the issue. You didn't test this.

@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

:( you're evil

@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

        // Session logic
        if (service.getProperty(PluginSettings.SESSIONS_ENABLED) && (sessionManager.hasSession(name) || database.isLogged(name))) {
            sessionManager.cancelSession(name);

            PlayerAuth auth = database.getAuth(name);
            database.setUnlogged(name);
            playerCache.removePlayer(name);
            if (auth != null && auth.getIp().equals(ip)) {
                service.send(player, MessageKey.SESSION_RECONNECTION);
                plugin.getManagement().performLogin(player, "dontneed", true);
                return;
            } else if (service.getProperty(PluginSettings.SESSIONS_EXPIRE_ON_IP_CHANGE)) {
                service.send(player, MessageKey.SESSION_EXPIRED);
            }
        }
    }

@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

L 137 AsyncJoin

@sgdc3 sgdc3 removed their assignment Jul 12, 2016
@ljacqu
Copy link
Member

ljacqu commented Jul 12, 2016

Ooh bad, good catch on that, and sorry. I expected more logic to be inside SessionManager; I guess the encapsulation story is still open.
Abd next time I implement some feature I will ask you to verify @sgdc3 so you can exercise revenge ;) 😄

@sgdc3
Copy link
Member

sgdc3 commented Jul 12, 2016

😈

@ljacqu ljacqu self-assigned this Jul 15, 2016
@ljacqu
Copy link
Member

ljacqu commented Jul 15, 2016

Thank you for reporting this @Twonox. Please verify with build nr. 1219 or above. http://ci.xephi.fr/job/AuthMeReloaded/1219/

@Twonox
Copy link
Contributor Author

Twonox commented Jul 16, 2016

That works thanks :)

@Twonox Twonox closed this as completed Jul 16, 2016
@ljacqu
Copy link
Member

ljacqu commented Jul 16, 2016

Thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: critical Marks issues that should be fixed/closed as soon as possible. Type: enhancement
Development

No branches or pull requests

5 participants