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

Login username/password for IRC server is not stored correctly #334

Closed
ukcb opened this Issue Apr 16, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@ukcb

ukcb commented Apr 16, 2017

Where to set a password for a irc-server exactly? I can set a password in the Advanced settings, but when I restart convos I get no more connection to the irc-server (Access denied: Bad password?).
I need to re-enter the password, then it works again. Why is the password not saved?

@ukcb

This comment has been minimized.

Show comment
Hide comment
@ukcb

ukcb Apr 17, 2017

I must specify both in the advanced settings - username AND password! I had only entered the
password, which was not saved. I should have known. I checked the url-syntax in my connection.json.

username and password
"url":"irc://convos:password@irc-server:6697?nick=convos&tls=1"
it's working

username only
"url":"irc://convos:@irc-server:6697?nick=convos&tls=1"
does not work

password only
"url":"irc://irc-server:6697?nick=convos&tls=1"
does not work

password is a space
"url":"irc://convos:%20@irc-server:6697?nick=convos&tls=1"
PASS Syntax error
does not work

username and password is a space
"url":"irc://%20:%20@irc-server:6697?nick=convos&tls=1"
USER Syntax error
does not work

The last two examples are a suggestion for the future, if an error should occur.
An empty entry should be prevented.

ukcb commented Apr 17, 2017

I must specify both in the advanced settings - username AND password! I had only entered the
password, which was not saved. I should have known. I checked the url-syntax in my connection.json.

username and password
"url":"irc://convos:password@irc-server:6697?nick=convos&tls=1"
it's working

username only
"url":"irc://convos:@irc-server:6697?nick=convos&tls=1"
does not work

password only
"url":"irc://irc-server:6697?nick=convos&tls=1"
does not work

password is a space
"url":"irc://convos:%20@irc-server:6697?nick=convos&tls=1"
PASS Syntax error
does not work

username and password is a space
"url":"irc://%20:%20@irc-server:6697?nick=convos&tls=1"
USER Syntax error
does not work

The last two examples are a suggestion for the future, if an error should occur.
An empty entry should be prevented.

@ukcb ukcb closed this Apr 17, 2017

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 17, 2017

Collaborator

Why did you close this issue? Isn't this a bug..? Sounds like Convos should support just setting a password..?

Collaborator

jhthorsen commented Apr 17, 2017

Why did you close this issue? Isn't this a bug..? Sounds like Convos should support just setting a password..?

@jhthorsen jhthorsen added the question label Apr 17, 2017

@jhthorsen jhthorsen self-assigned this Apr 17, 2017

@ukcb

This comment has been minimized.

Show comment
Hide comment
@ukcb

ukcb Apr 18, 2017

It's not really a mistake, but it should be better documented.
For my taste, a irc-server password should be specified for a new connection.
A unique password per irc-server should be sufficient.
Look into the RFC for command PASS https://tools.ietf.org/html/rfc1459#section-4.1.1

ukcb commented Apr 18, 2017

It's not really a mistake, but it should be better documented.
For my taste, a irc-server password should be specified for a new connection.
A unique password per irc-server should be sufficient.
Look into the RFC for command PASS https://tools.ietf.org/html/rfc1459#section-4.1.1

@jhthorsen jhthorsen added bug and removed question labels Apr 18, 2017

@jhthorsen jhthorsen reopened this Apr 18, 2017

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 18, 2017

Collaborator

I think this is a mistake which should be fixed.

Collaborator

jhthorsen commented Apr 18, 2017

I think this is a mistake which should be fixed.

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 18, 2017

Collaborator

I've written a failing test, and the implemented logic seems to be pretty broken. It's not easy to fix, since I need to figure out if a new password is entered or not. The way it is now, I don't want to send the password to the web browser, but rather just store it on the server side once. But maybe it's ok to send it to the browser..? The reason behind not wanting to send it to the browser is in case the Convos installation is not served over TLS/SSL.

I have some ideas:

  • Serve the connection URL back with a flag set. Example: irc://username@irc.example.com/?password=1 (Convos can then use "password=1" to render the form differently)
  • Serve the URL back with username and password. Example: irc://username:secret@irc.example.com/
  • Not quite sure, but maybe blank password is good enough indicator Example: irc://username:@irc.example.com
  • Have some special password string "******" where the password goes. This might be in conflict with the actual password set, even though I find that very unlikely.

Will look at it later this week - Hopefully tomorrow.

Collaborator

jhthorsen commented Apr 18, 2017

I've written a failing test, and the implemented logic seems to be pretty broken. It's not easy to fix, since I need to figure out if a new password is entered or not. The way it is now, I don't want to send the password to the web browser, but rather just store it on the server side once. But maybe it's ok to send it to the browser..? The reason behind not wanting to send it to the browser is in case the Convos installation is not served over TLS/SSL.

I have some ideas:

  • Serve the connection URL back with a flag set. Example: irc://username@irc.example.com/?password=1 (Convos can then use "password=1" to render the form differently)
  • Serve the URL back with username and password. Example: irc://username:secret@irc.example.com/
  • Not quite sure, but maybe blank password is good enough indicator Example: irc://username:@irc.example.com
  • Have some special password string "******" where the password goes. This might be in conflict with the actual password set, even though I find that very unlikely.

Will look at it later this week - Hopefully tomorrow.

@jhthorsen jhthorsen changed the title from login-password of irc-server to Login username/password for IRC server is not stored correctly Apr 18, 2017

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 18, 2017

Collaborator

And also... How do I know if someone has submitted a blank password or not changed the password at all? I would like to avoid a "delete password" button. The special password string "******" could be used for this and it would also avoid password managers from autofilling the form. Though the password manager might pick it up as an actual password.

Collaborator

jhthorsen commented Apr 18, 2017

And also... How do I know if someone has submitted a blank password or not changed the password at all? I would like to avoid a "delete password" button. The special password string "******" could be used for this and it would also avoid password managers from autofilling the form. Though the password manager might pick it up as an actual password.

@ukcb

This comment has been minimized.

Show comment
Hide comment
@ukcb

ukcb Apr 19, 2017

Can not you simply send a PASS command before a SERVER command?

Pseudo-code: irc->write("PASS $pass") if $pass

ukcb commented Apr 19, 2017

Can not you simply send a PASS command before a SERVER command?

Pseudo-code: irc->write("PASS $pass") if $pass

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 19, 2017

Collaborator

@ukcb: That works as expected, but the problem is that the sending the password from frontend to backend does not (always) work.

I've discussed it with @marcusramberg, and we came to the conclusion that it's ok to pass the username/password to the frontend from the backend. This is OK as long as Convos is protected with TLS/SSL, and there's other overlapping issues if you don't protect Convos with a secure connection.

Collaborator

jhthorsen commented Apr 19, 2017

@ukcb: That works as expected, but the problem is that the sending the password from frontend to backend does not (always) work.

I've discussed it with @marcusramberg, and we came to the conclusion that it's ok to pass the username/password to the frontend from the backend. This is OK as long as Convos is protected with TLS/SSL, and there's other overlapping issues if you don't protect Convos with a secure connection.

jhthorsen added a commit that referenced this issue Apr 21, 2017

@jhthorsen jhthorsen closed this in 738d8d3 Apr 21, 2017

@jhthorsen

This comment has been minimized.

Show comment
Hide comment
@jhthorsen

jhthorsen Apr 21, 2017

Collaborator

This is now fixed and will be part of the next release, version 0.99_33.

Collaborator

jhthorsen commented Apr 21, 2017

This is now fixed and will be part of the next release, version 0.99_33.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment