Issue/#977 Remove IRC password functionality #978
Conversation
server/lobbyconnection.py
Outdated
| await self.send({ | ||
| "command": "irc_password", | ||
| "password": new_irc_password | ||
| "password": hexlify(os.urandom(16)).decode() | ||
| }) |
There was a problem hiding this comment.
Do we even need to send the message?
I know removing it from the spec would be a backwards breaking change but it seems like just not sending it should be fine.
There was a problem hiding this comment.
Mmm, according to my notes about version numbers I think not sending it would be a breaking change. I would consider it part of the command_auth message flow where if you send a valid token then you always expect the next message that comes back to you to be irc_password, so removing that would be a breaking change to any code that was built on that message flow. This is also supported by my integration tests which fail if I remove that command.
https://github.com/FAForever/server/blob/develop/CONTRIBUTING.md#version-numbers
There was a problem hiding this comment.
Maybe there don't exist any client implementations that rely on that in practice, but IDK. I feel like it doesn't really hurt much to keep it and it ensures that the protocol definitely remains backwards compatible.
There was a problem hiding this comment.
In general yes, but the websocket change is a breaking change and we'll force all users to update to new irc+websocket for lobby
There was a problem hiding this comment.
Sure, but that's just a change to the message transport. As far as like the logic of how to handle certain messages that wouldn't be affected by the websocket change.
There was a problem hiding this comment.
Ah didn't realize it was considered part of the auth flow. I just considered it as a message that comes at some point after auth
There was a problem hiding this comment.
It's certainly a bit subjective what the 'flow' really is, but since the message is sent unconditionally after validating the token I would consider it to be part of the 'flow'.
There was a problem hiding this comment.
Wouldn't it be more expressive for future us to set it to a fixed string "deprecated" instead of using random bits?
There was a problem hiding this comment.
Sure, I can get behind that
Askaholic
force-pushed
the
issue/#977-deprecate-irc-password
branch
from
cca62a2
to
55d421d
Compare
Askaholic
force-pushed
the
issue/#977-deprecate-irc-password
branch
from
55d421d
to
061accd
Compare
This will prevent the logs from being filled with warnings about failing to update NickServ password whenever someone logs in.
Closes #977