-
Notifications
You must be signed in to change notification settings - Fork 413
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(auth): integrate ldap #3031
Conversation
# Conflicts: # .github/workflows/platform-docker-publish-all-features-image.yml
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Ephemeral Environment
|
@@ -120,7 +120,7 @@ jobs: | |||
context: . | |||
build-args: | | |||
SAML_INSTALLED=1 | |||
POETRY_OPTS=--with saml,auth-controller | |||
POETRY_OPTS=--with saml,auth-controller,ldap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're resorting to a private git dependency, this step will fail without adding credentials to the git client.
See one of the solutions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I was assuming that the presence of the github token secret variable would suffice here but perhaps you're right. It's working currently where we manually checkout the packages but I guess that's because we're using a pre-built action. I'll have a play with this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are providing a distinct token to the checkout action for the private repos, aren't we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, we are. Well caught, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See successful build here.
# Conflicts: # api/poetry.lock
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3031 +/- ##
==========================================
+ Coverage 95.74% 95.89% +0.15%
==========================================
Files 1045 1039 -6
Lines 30889 30688 -201
==========================================
- Hits 29574 29429 -145
+ Misses 1315 1259 -56 ☔ View full report in Codecov by Sentry. |
cb57d08
to
f3943e8
Compare
dceb49c
to
b04c793
Compare
b04c793
to
2d16bd2
Compare
@@ -43,6 +43,7 @@ jobs: | |||
|
|||
- name: Install poetry | |||
run: pipx install poetry | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, irrelevant to this PR but it was bothering me.
Changes
Adds the flagsmith-ldap module as a dependency to the main Flagsmith application. This will be installed as a dependency as part of the flagsmith-private-cloud image build workflow.
How did you test this code?
Tests are run in flagsmith/flagsmith-ldap repository.