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

IMConnectionProvider.java: getAuthenticationHolder(): fix check for Bot user name… #211

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

jimklimov
Copy link
Contributor

…to exempt empty strings (lack of specific configuration) [JENKINS-72590]

Per https://issues.jenkins.io/browse/JENKINS-72590 the check was insufficient - it only excluded null values, but an empty saved config conveys an empty non-null String instead.

Testing done

Stumbled on this while dev-testing other PRs, and the temporary Jenkins controller had just a trivial configuration to log into IRC but only had an admin user defined locally. Replies from Jenkins for commands which required authorization, such as "help", failed with inability to "impersonate" and did not make it to IRC.

With this change, replies are present in a private chat and commands are honoured.

Submitter checklist

UPDATE: Probably this is counter-productive after all: can't assign permission constraints to a non-existing user. Would change to a more reasonable exception that helps troubleshooting instead.

…ot user name, to exempt empty strings (lack of specific configuration) [JENKINS-72590]

Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
@jimklimov jimklimov requested a review from a team as a code owner January 22, 2024 22:50
…ot user name, to exempt empty strings (lack of specific configuration) if Jenkins security setting allows that [JENKINS-72590]

Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
@jimklimov
Copy link
Contributor Author

With the mood change to throw an exception for the empty user-name config in the later commit, the IRC client in fact is precluded from starting (which may be counter-productive for sites that only want to post notifications):

2024-01-22 23:13:37.779+0000 [id=82]    INFO    h.p.i.IMConnectionProvider$ConnectorRunnable#run: Trying to reconnect
2024-01-22 23:13:37.793+0000 [id=82]    SEVERE  h.i.i.InstallUncaughtExceptionHandler$DefaultUncaughtExceptionHandler#uncaughtException: A thread (IM-Reconnector-Thread/82) died unexpectedly due to an uncaught exception. This may leave your
server corrupted and usually indicates a software bug.
org.acegisecurity.userdetails.UsernameNotFoundException: No local Jenkins user name is configured for instant messaging to act as
        at hudson.plugins.im.IMConnectionProvider.getAuthenticationHolder(IMConnectionProvider.java:127)
        at hudson.plugins.ircbot.v2.IRCConnectionProvider.createConnection(IRCConnectionProvider.java:40)
        at hudson.plugins.im.IMConnectionProvider.create(IMConnectionProvider.java:64)
        at hudson.plugins.im.IMConnectionProvider$ConnectorRunnable.run(IMConnectionProvider.java:193)
        at java.base/java.lang.Thread.run(Thread.java:833)

…row UsernameNotFoundException when we fail to impersonate() or to User.get() AND the name is blank [JENKINS-72590]

This allows e.g. IRC Bot to start and send messages ("are you talking to
me?" and build notifications) but not send commands when no Jenkins user
is attached to the IRC configuration.

Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
@jimklimov
Copy link
Contributor Author

With the latest change, this gets well-balanced: IRC bot can start and send notifications, but can not run commands, when not properly configured with a Jenkins user.

…g command execution, outline this to private message sender [JENKINS-72590]

Signed-off-by: Jim Klimov <jimklimov+jenkinsci@gmail.com>
@jimklimov jimklimov added java Pull requests that update Java code enhancement labels Jan 23, 2024
@jimklimov jimklimov merged commit fc1db63 into jenkinsci:master Jan 23, 2024
14 checks passed
@jimklimov jimklimov deleted the JENKINS-72590 branch January 23, 2024 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant