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

Add support for prompting user account at each login #538

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

bhartshorn
Copy link
Contributor

@bhartshorn bhartshorn commented Feb 9, 2024

This is a PR to solve my issue #533. This change was made fairly naively by looking at the other settings available, copy pasting, and adding an if statement in (hopefully) the right place. As mentioned in my issue, I have little experience with Jenkins plug-ins, so forgive me and guide me if I did something dumb here.

Testing done

I have not yet tested this, I'm looking in to how to manually build and install the plugin for testing. I'll also go back and look if there's automated testing that could be extended for this.

I took the .hpi file built by the Jenkins CI process for this PR, spun up a new Jenkins instance in docker with the PR .hpi installed, and checked the behavior:

  1. when "Prompt for user account on each login" is unchecked, Entra ID automatically uses the MS account attached to windows, without prompting.
  2. when "Prompt for user account on each login" is checked, Entra ID prompts me for which account to sign in to every time I click "Sign in"

This is successful testing as far as the functionality I need.

I do not see a straightforward way to add a test case (the tests are quite minimal unless I'm missing something).

Submitter checklist

@bhartshorn
Copy link
Contributor Author

Pinging @timja as hopefully someone who can review and merge. Thanks!

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, untested but everything I would expect to be needed is there

@timja timja merged commit f85d61f into jenkinsci:master Feb 9, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants