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 command #231

Closed
wants to merge 3 commits into from
Closed

LOGOUT command #231

wants to merge 3 commits into from

Conversation

ValwareIRC
Copy link
Member

This creates LOGOUT command which logs a user out independent of services. It will also send out an SVSLOGIN so that services correctly implementing SVSLOGIN (for configurations of multiple SASL-servers) will be able to know that the user is logged out.

This creates LOGOUT command which logs a user out independent of services.
It will also send out an SVSLOGIN so that services correctly implementing SVSLOGIN (for configurations of multiple SASL-servers) will be able to know that the user is logged out.
@ValwareIRC
Copy link
Member Author

ValwareIRC commented Nov 13, 2022

Worth noting, a quick conversation with Sadie from Anope said that they don't process incoming SVSLOGINs which is counter-intuitive, recognising UnrealIRCd at least supports running a sasl-server separate from services-server. I was also informed that it's unlikely that Atheme supports it either.

Dalek seems to be the only supporting services package.
I feel like using SVSLOGIN is the right way to go, and I feel the fault is with the services packages unwilling to cater for other servers doing the SASL work.

That said, I will work to try and make pull requests for both Anope and Atheme for implementing this. Whether or not they will be accepted is another story, but I can't imagine it would hurt their services to include support for it.

Made it so that it will check if NIckServ is online (and part of the config-defined `services-server` and ask it to log us out. If it's not, then we still logout and assume that services will see that we are logged out when they catch up again.
@ValwareIRC
Copy link
Member Author

Made this work a bit neater for existing services, the biggest snag is we have to fail it at the end if there was no NickServ on the services server, because we don't want to desync services. This feels kinda messy as we shouldn't be messaging a bot about it really, since there is never a guarantee that the bot with the logout command is called NickServ.

@syzop
Copy link
Member

syzop commented Nov 14, 2022

I like the idea. I mean: via SASL we allow logging into an account without knowing anything about services, but there is no way to log out of it without knowing anything about services.

Surely we won't be the only one who wants/needs this, you are coordinating with others, right? Keep us updated :)
Ultimately, I don't mind throwing some fallback/compatibility code in like this NS LOGOUT, while also doing things properly with like SVSLOGIN, for example.

@ValwareIRC
Copy link
Member Author

Atheme PR requested and mentioned so both can see what I'm trying to achieve here
atheme/atheme#909

@ValwareIRC
Copy link
Member Author

Anope has now implemented support for server-side logout 😄
anope/anope@38d5b93

@syzop
Copy link
Member

syzop commented Nov 20, 2023

Just an update here, so everyone can see it:
We talked about this on IRC a few weeks ago. Anope added it to their 2.1.x branch. And atheme.. well.. you may have more news on that (or not).
I said I will wait until a new anope version is out which includes this functionality. Since, if we ship UnrealIRCd before that, it would send things like "You are now logged out" when you actually are not, which is a bit dangerous. To be fair, that may still happen when the new anope is out and users use an older anope, but right now it would affect close to 100% of the users so that would be bad.

So, we will wait a while, I don't know the schedule about anope 2.1.x but it sounds like something that will not be out very soon. Ah well, we can wait, this is nice functionality but missing it not life threatening :).

@syzop syzop added the blocked Proposed patch needs careful consideration of things, not suitable for immediate merging. label Nov 20, 2023
@ValwareIRC ValwareIRC closed this by deleting the head repository Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Proposed patch needs careful consideration of things, not suitable for immediate merging.
Projects
None yet
2 participants