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

feat: optionally add domain_hint #628

Merged
merged 2 commits into from
Oct 12, 2024
Merged

Conversation

gjasny
Copy link
Contributor

@gjasny gjasny commented Oct 7, 2024

Providing a domain hint allows to bypass the account selector on Microsoft side in case more than one account has been used in the Browser in the past. Documentation for the parameter is here.

Testing done

I locally built the plugin and installed the hpi file into a Jenkins 2.462.1. After adding our domain to the new input field I opened a separate browser. There the user account selector was bypassed and I got logged directly into Jenkins.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

I don't know how and where to add a test for the feature.

Please consider to merge and release a new plugin version.

Thanks,
Gregor

@KalleOlaviNiemitalo
Copy link

At Home Realm Discovery for an application, it says

Microsoft doesn't recommend configuring auto-acceleration any longer, as it can prevent the use of stronger authentication methods such as FIDO and hinders collaboration.

which I think applies to the domain_hint parameter.

The "hinders collaboration" part does not matter if Jenkins or Entra ID has already been configured to require all users to be in the same domain. I'm not sure about the effect on FIDO, though.

IMO, it's okay to support the domain_hint parameter in this plugin; but if there are downsides to setting it, then the help text should link to related documentation.

@gjasny
Copy link
Contributor Author

gjasny commented Oct 7, 2024

At Home Realm Discovery for an application, it says

Microsoft doesn't recommend configuring auto-acceleration any longer, as it can prevent the use of stronger authentication methods such as FIDO and hinders collaboration.

IMO, it's okay to support the domain_hint parameter in this plugin; but if there are downsides to setting it, then the help text should link to related documentation.

I tested it with FIDO2 authentication and it still works.

@@ -46,6 +46,9 @@
<f:checkbox />
</f:entry>

<f:entry title="${%Domain Hint: The realm of the user in a federated directory}" field="domainHint">
Copy link
Member

Choose a reason for hiding this comment

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

the label would normally just be the field name and then either a description or help text to explain what its for

@timja timja enabled auto-merge (squash) October 12, 2024 17:53
@timja timja merged commit 161b33e into jenkinsci:master Oct 12, 2024
15 of 16 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