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

Setting sasl_mechanism to '' disables the auth #719

Merged
merged 1 commit into from Jul 26, 2017

Conversation

Projects
None yet
3 participants
@LemonBoy
Member

LemonBoy commented Jun 17, 2017

There was no easy way for the user to disable the SASL authentication
once the network was created.
Closes #718

@dequis

This comment has been minimized.

Member

dequis commented Jun 17, 2017

Sounds good

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jun 17, 2017

could we use empty string instead? many other settings work like that iirc

@dequis

This comment has been minimized.

Member

dequis commented Jun 17, 2017

How do you set settings to empty strings? There's "/set -clear" for global ones but network/server settings need a parameter and a pair of empty quotes won't help.

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jun 19, 2017

try: /network add -autosendcmd 'xxx' testnet, /network add -autosendcmd '' testnet, works for me

@LemonBoy

This comment has been minimized.

Member

LemonBoy commented Jun 19, 2017

Not quite, see here.
I also don't really like using an empty string.

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jul 8, 2017

so let's allow both none and ''? afai can see simply remove the check for *opt=='\0' to allow comparison of it?

@LemonBoy

This comment has been minimized.

Member

LemonBoy commented Jul 8, 2017

I don't really like seeing sasl_mechanism = none in the output of /network, using '' is fine.

@dequis dequis changed the title from Setting sasl_mechanism to 'none' disables the auth to Setting sasl_mechanism to '' disables the auth Jul 8, 2017

@dequis

This comment has been minimized.

Member

dequis commented Jul 8, 2017

Should probably change the commit message too

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jul 21, 2017

is this good to merge? should it maybe also be possible to clear username/password?

Allow the user to clear the sasl-related fields
There was no easy way for the user to disable the SASL authentication or
to clear the username/password once the network was created.
Closes #718
@LemonBoy

This comment has been minimized.

Member

LemonBoy commented Jul 25, 2017

I've extended this behaviour to the sasl_username and sasl_password fields too.
Feel free to merge once Travis gives the green.

@ailin-nemui ailin-nemui merged commit 437fbef into irssi:master Jul 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

ailin-nemui added a commit to ailin-nemui/irssi that referenced this pull request Dec 7, 2017

Merge pull request irssi#719 from LemonBoy/sasl-disable-none
Setting sasl_mechanism to '' disables the auth

@ailin-nemui ailin-nemui added this to the 1.0.5 milestone Jan 10, 2018

lkundrak pushed a commit to lkundrak/irssi that referenced this pull request Feb 16, 2018

Merge pull request irssi#719 from LemonBoy/sasl-disable-none
Setting sasl_mechanism to '' disables the auth

(cherry picked from commit 7b97edf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment