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 Microsoft AD FS #368

Merged
merged 4 commits into from
Aug 27, 2024
Merged

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Aug 14, 2024

Enable the plugin to work with AD FS which sends the user back to securityRealm/finishLogin with a HTTP Post.

Adds a Crumb exclusion extension for this endpoint.
Extract any POST data provided into a fake URL so it can be fed to the google code in a way that it can work with
Adds some rudimentary docs

fixes #352

Testing done

  • setup a local LAB ADFS (OMG!) and configured the plugin to use it.
  • validated I can login and obtain groups.
  • unit test for the crumb exclusion

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

@jtnord jtnord requested a review from a team as a code owner August 14, 2024 16:28
docs/configuration/ADFS.md Outdated Show resolved Hide resolved
@@ -129,6 +129,36 @@
</pluginRepositories>

<build>
<pluginManagement>
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated to the change, but due to eclipse-m2e/m2e-core/issues/1291 would cause an error in the project in eclipse.

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.88%. Comparing base (b7205f1) to head (a08b47e).
Report is 22 commits behind head on master.

Files Patch % Lines
...ain/java/org/jenkinsci/plugins/oic/OicSession.java 91.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #368      +/-   ##
============================================
- Coverage     72.02%   71.88%   -0.14%     
- Complexity      234      239       +5     
============================================
  Files            11       12       +1     
  Lines           990     1010      +20     
  Branches        142      145       +3     
============================================
+ Hits            713      726      +13     
- Misses          199      206       +7     
  Partials         78       78              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AD FS returns data via a POST not a get, so this adapts the code to
handle a call to finishLogin with via an HTTP POST.
@jtnord jtnord changed the title Adfs support Add support for Microsoft AD FS Aug 15, 2024
@jtnord
Copy link
Member Author

jtnord commented Aug 15, 2024

@michael-doubez the metrics for code coverage appear broken.

https://app.codecov.io/gh/jenkinsci/oic-auth-plugin/pull/368?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=jenkinsci#7dda26682abc4c20fa99de9ee4c7c0aa-L1423 is showing -0.75% coverage (src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java) yet this class in unchanged nor do any changes in test or production code affect it..

The base used for the diff is 11 commits behind master, yet the base for this PR is jtnord@3df5338 which is the same as jenkinsci/master at the time of writing this.

image

image

https://app.codecov.io/gh/jenkinsci/oic-auth-plugin is having the last report as b7205f1 so this PR is being penalised for all other changes on master since that was merged.

@jtnord
Copy link
Member Author

jtnord commented Aug 15, 2024

code coverage upload is failing on the master branch

Error: Codecov token not found. Please provide Codecov token with -t flag.
Warning: Codecov: Failed to properly create commit: The process '/home/runner/work/_actions/codecov/codecov-action/v4/dist/codecov' failed with exit code 1

#369

Copy link
Contributor

@michael-doubez michael-doubez left a comment

Choose a reason for hiding this comment

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

LGTM

@michael-doubez michael-doubez merged commit 23537cb into jenkinsci:master Aug 27, 2024
18 of 19 checks passed
@jtnord jtnord deleted the adfs branch September 19, 2024 10:31
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.

unable to use with Microsoft AD FS server
3 participants