fix: allow for auth with activedirectory again#1061
Merged
JamieSlome merged 4 commits intofinos:mainfrom Jul 1, 2025
Merged
Conversation
Current /login is forced to use local auth. This change resolves this regression in a backwards compatible manner while still supporting alternative auth like OIDC. Auth methods compatible with /login are filtered out from the git-proxy config, if there isn't one /login is essentially disabled to avoid throwing. If there is multiple methods it'll prioritize the first. In the future we can support multiple user & password login methods simultaneously with relevant TODOs added.
loadFromGit should be sufficiently authenticated and if it isn't it's not expected to get interactive input. Inform git not to wait for interactive action. This also resolves an issue in the tests where "should throw error if repository is valid URL but not a git repository" test times out waiting for a username.
If we want to ensure this git repository isn't created it should be pointed at the FINOS org, the parent of this repo currently. In theory test-org (an org that currently exists) could add a repo called test-project.
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
Contributor
Author
|
cc: @jescalada to double check this doesn't impact OIDC & also may be relevant to #1024 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1061 +/- ##
==========================================
+ Coverage 76.73% 76.78% +0.04%
==========================================
Files 55 55
Lines 2244 2261 +17
Branches 251 251
==========================================
+ Hits 1722 1736 +14
- Misses 492 495 +3
Partials 30 30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
06kellyjac
commented
Jun 25, 2025
06kellyjac
commented
Jun 25, 2025
06kellyjac
commented
Jun 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft because my commit signing seems to be playing up but should otherwise be ready for review.After actually posting the PR commit signatures look fine.
Main change - allow for auth with activedirectory again
Current /login is forced to use local auth.
This change resolves this regression in a backwards compatible manner while still supporting alternative auth like OIDC.
Auth methods compatible with /login are filtered out from the git-proxy config, if there isn't one /login is essentially disabled to avoid throwing.
If there is multiple methods it'll prioritize the first.
In the future we can support multiple user & password login methods simultaneously with relevant TODOs added.
other fixes: