-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix issue where ecdsa and other key types are not synced from LDAP (#5092) #5094
Conversation
…thentication provider fixes #5092
Codecov Report
@@ Coverage Diff @@
## master #5094 +/- ##
=========================================
+ Coverage 37.48% 37.5% +0.01%
=========================================
Files 310 310
Lines 45922 45923 +1
=========================================
+ Hits 17215 17223 +8
+ Misses 26233 26229 -4
+ Partials 2474 2471 -3
Continue to review full report at Codecov.
|
Could somebody help me out how to test this case? |
Yes I understand, I had a look at the CI some time ago. We should add at least an ecdsa key to the LDAP server test image then. At here https://github.com/go-gitea/test-openldap/blob/master/bootstrap/data/10_people_hermes.ldif#L24 |
I have added an ecdsa key to the test-openldap image as PR. |
@xor-gate feel free to submit PR to add key to that test ldap server |
I updated the PR with the latest changes from master e2292c7 seems master has broken CI build: https://drone.gitea.io/go-gitea/gitea/3652/10 |
@xor-gate you need to update tests as more ssh keys are now returned from ldap in test |
@lafriks finaly the ldap integration test passes now horay 👍 gives me more trust with the added test key |
Implements #5092
Currently LDAP sync support only
ssh
prefixed keytypes, so other keytypes are simply ignored. This PR improves the check by using the native parsing of the public authorized key line. I tried to find out to add a test ecdsa key but not sure where to place it.I think this are only the calculated fingerprints:
https://github.com/go-gitea/gitea/blob/master/integrations/auth_ldap_test.go#L43-L46
Based on this
https://github.com/go-gitea/gitea/blob/master/integrations/api_admin_test.go
or this
https://github.com/go-gitea/gitea/blob/master/integrations/api_keys_test.go
I would like some advice on the test, as it is highly valuable.