[PR #1844] [MERGED] LDAP Public SSH Keys synchronization #16080

New Issue

GiteaMirror · 2025-11-02T12:01:51-06:00

GiteaMirror commented

2025-11-02 12:01:51 -06:00

📋 Pull Request Information

Original PR: https://github.com/go-gitea/gitea/pull/1844
Author: @dnmgns
Created: 5/31/2017
Status: ✅ Merged
Merged: 5/24/2018
Merged by: @lafriks

Base: master ← Head: master

📝 Commits (10+)

64312ab Add LDAP Key Synchronization feature
d17981b Move block: user data has changed
cc046c7 Add LDAP Key Synchronization feature
26a9205 Move block: user data has changed
d005086 Merge branch 'master' of https://github.com/dnmgns/gitea
e0da1d7 Oops
b6e2985 Fix space
a20d4f1 Merge branch 'master' of https://github.com/go-gitea/gitea
1d4dbed null def null not needed
55424bf move existsInSlice and IsEqualSlice to compare

📊 Changes

25 files changed (+620 additions, -436 deletions)

View changed files

📝 Gopkg.lock (+4 -2)
📝 integrations/auth_ldap_test.go (+58 -21)
📝 models/migrations/migrations.go (+2 -0)
➕ models/migrations/v66.go (+22 -0)
📝 models/ssh_key.go (+24 -14)
📝 models/user.go (+133 -1)
📝 modules/auth/auth_form.go (+1 -0)
📝 modules/auth/ldap/ldap.go (+33 -30)
📝 modules/util/compare.go (+29 -0)
📝 options/locale/locale_en-US.ini (+1 -0)
📝 routers/admin/auths.go (+19 -18)
📝 routers/api/v1/user/key.go (+1 -1)
📝 routers/user/setting/keys.go (+1 -1)
📝 templates/admin/auth/edit.tmpl (+4 -0)
📝 templates/admin/auth/source/ldap.tmpl (+4 -0)
📝 vendor/github.com/davecgh/go-spew/LICENSE (+1 -1)
📝 vendor/github.com/davecgh/go-spew/spew/bypass.go (+3 -3)
📝 vendor/github.com/davecgh/go-spew/spew/common.go (+1 -1)
📝 vendor/github.com/davecgh/go-spew/spew/dump.go (+5 -5)
📝 vendor/github.com/davecgh/go-spew/spew/format.go (+2 -2)

...and 5 more files

📄 Description

This PR will implement a way to synchronize Public SSH Keys from LDAP when Public SSH Key(s) are stored as LDAP attribute in LDAP server.

Not long ago, #1478 was merged, which added a LDAP user synchronization feature. This PR will sort-of extend that one and and include a (optional) Public SSH Key synchronization feature.

The PR introduces the following changes in short:

Public SSH key attribute setting for add/edit Login Source (when Authentication Type = LDAP (via BindDN))
* Enable Public SSH key synchronization checkbox for add/edit Login Source
* REPLACE_PUBLIC_SSH_KEYS setting in app.ini (Default: false), when set to true it replaces all public ssh keys for users synchronized from LDAP and replaces them with the public keys from LDAP
login_source_id column in public_key table to keep track of the public_key source (When deleting keys, we therefore can choose to only delete those synchronized with LDAP. Plus it makes life easier for sysadmin)

~~# Why d54d92f and 9e09a9c are included in this PR~~
Note that d54d92f0b81e1d71fa2a4b2aedca985e7aa841d7 and 9e09a9ccd83130cccdfa71c8000db48310d7a8ea addresses two issues found in Gitea, they are not specifically for this synchronization feature. But I choose to include them, as using this key sync feature could lead to some pretty serious issues otherwise. Depending on the update interval for LDAP User synchronization and available resources, the system could end up with lots of bak-files for authorized_keys and lots of public tmp-files causing inode issue and running out of free space. Therefore I choose to also the following changes in this PR:
* Delete the temporary public key file after it has been used to calculate the public key fingerprint
* Setting in app.ini to completely disable authorized_keys backup (default: disabled)
Edit: Created two separate PR for this.

Thoughts

While the synchronization part (where LDAP keys are added to DB) could be a lot smarter, It is a thoughtful choice to always drop all LDAP keys from DB and add all LDAP keys fetched from the LDAP server again. This reduces the complexity with handling every case (and requires fewer lines of code) while meeting the desired result.
Edit: I've added some logic for the key synchronization now, as dropping all keys and adding them again caused some auto increment leakage along with the key not being available for a split second.

This is almost my first time writing any go code at all, I've just read plenty of go code before and made some one-row-changes here and there, so feel free to comment on the code itself along with the design.

There's currently no known issues with this new functionality.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/go-gitea/gitea/pull/1844 **Author:** [@dnmgns](https://github.com/dnmgns) **Created:** 5/31/2017 **Status:** ✅ Merged **Merged:** 5/24/2018 **Merged by:** [@lafriks](https://github.com/lafriks) **Base:** `master` ← **Head:** `master` --- ### 📝 Commits (10+) - [`64312ab`](https://github.com/go-gitea/gitea/commit/64312ab09bbdbb5a440d1a251b73a032706beb88) Add LDAP Key Synchronization feature - [`d17981b`](https://github.com/go-gitea/gitea/commit/d17981b75e2190d05033aa4816648c330c6d1d3a) Move block: user data has changed - [`cc046c7`](https://github.com/go-gitea/gitea/commit/cc046c7a4452eb88ff452f8dd581dc2c56e802f1) Add LDAP Key Synchronization feature - [`26a9205`](https://github.com/go-gitea/gitea/commit/26a9205be9744ec8dd27ab52ab195ad22c3d9d9f) Move block: user data has changed - [`d005086`](https://github.com/go-gitea/gitea/commit/d005086d50f0278c1d73c61918ba5546b4425157) Merge branch 'master' of https://github.com/dnmgns/gitea - [`e0da1d7`](https://github.com/go-gitea/gitea/commit/e0da1d7601c48f9c2ae33c9e593b5f28c6c0f072) Oops - [`b6e2985`](https://github.com/go-gitea/gitea/commit/b6e298579bf96f10127743996f6fdf46b3f4c6d8) Fix space - [`a20d4f1`](https://github.com/go-gitea/gitea/commit/a20d4f1d302a484d66b7930f30ead268761de251) Merge branch 'master' of https://github.com/go-gitea/gitea - [`1d4dbed`](https://github.com/go-gitea/gitea/commit/1d4dbed6354d545e39ee18c11971ac216907015d) null def null not needed - [`55424bf`](https://github.com/go-gitea/gitea/commit/55424bf5b762667fbe4a3854997eb798e76d0b7e) move existsInSlice and IsEqualSlice to compare ### 📊 Changes **25 files changed** (+620 additions, -436 deletions) <details> <summary>View changed files</summary> 📝 `Gopkg.lock` (+4 -2) 📝 `integrations/auth_ldap_test.go` (+58 -21) 📝 `models/migrations/migrations.go` (+2 -0) ➕ `models/migrations/v66.go` (+22 -0) 📝 `models/ssh_key.go` (+24 -14) 📝 `models/user.go` (+133 -1) 📝 `modules/auth/auth_form.go` (+1 -0) 📝 `modules/auth/ldap/ldap.go` (+33 -30) 📝 `modules/util/compare.go` (+29 -0) 📝 `options/locale/locale_en-US.ini` (+1 -0) 📝 `routers/admin/auths.go` (+19 -18) 📝 `routers/api/v1/user/key.go` (+1 -1) 📝 `routers/user/setting/keys.go` (+1 -1) 📝 `templates/admin/auth/edit.tmpl` (+4 -0) 📝 `templates/admin/auth/source/ldap.tmpl` (+4 -0) 📝 `vendor/github.com/davecgh/go-spew/LICENSE` (+1 -1) 📝 `vendor/github.com/davecgh/go-spew/spew/bypass.go` (+3 -3) 📝 `vendor/github.com/davecgh/go-spew/spew/common.go` (+1 -1) 📝 `vendor/github.com/davecgh/go-spew/spew/dump.go` (+5 -5) 📝 `vendor/github.com/davecgh/go-spew/spew/format.go` (+2 -2) _...and 5 more files_ </details> ### 📄 Description This PR will implement a way to synchronize Public SSH Keys from LDAP when Public SSH Key(s) are stored as LDAP attribute in LDAP server. Not long ago, #1478 was merged, which added a LDAP user synchronization feature. This PR will sort-of extend that one and and include a (optional) Public SSH Key synchronization feature. # The PR introduces the following changes in short: * `Public SSH key attribute` setting for add/edit Login Source (when Authentication Type = LDAP (via BindDN)) ~~* `Enable Public SSH key synchronization` checkbox for add/edit Login Source~~ ~~* `REPLACE_PUBLIC_SSH_KEYS` setting in app.ini (Default: false), when set to true it replaces all public ssh keys for users synchronized from LDAP and replaces them with the public keys from LDAP~~ * `login_source_id` column in public_key table to keep track of the public_key source (When deleting keys, we therefore can choose to only delete those synchronized with LDAP. Plus it makes life easier for sysadmin) ~~# Why d54d92f and 9e09a9c are included in this PR~~ ~~Note that d54d92f0b81e1d71fa2a4b2aedca985e7aa841d7 and 9e09a9ccd83130cccdfa71c8000db48310d7a8ea addresses two issues found in Gitea, they are not specifically for this synchronization feature. But I choose to include them, as using this key sync feature could lead to some pretty serious issues otherwise. Depending on the update interval for LDAP User synchronization and available resources, the system could end up with lots of bak-files for authorized_keys and lots of public tmp-files causing inode issue and running out of free space. Therefore I choose to also the following changes in this PR:~~ ~~* Delete the temporary public key file after it has been used to calculate the public key fingerprint~~ ~~* Setting in app.ini to completely disable authorized_keys backup (default: disabled)~~ Edit: Created two separate PR for this. # Thoughts ~~While the synchronization part (where LDAP keys are added to DB) could be a lot smarter, It is a thoughtful choice to always drop all LDAP keys from DB and add all LDAP keys fetched from the LDAP server again. This reduces the complexity with handling every case (and requires fewer lines of code) while meeting the desired result.~~ Edit: I've added some logic for the key synchronization now, as dropping all keys and adding them again caused some auto increment leakage along with the key not being available for a split second. This is almost my first time writing any go code at all, I've just read plenty of go code before and made some one-row-changes here and there, so feel free to comment on the code itself along with the design. There's currently no known issues with this new functionality. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2025-11-02 12:01:51 -06:00

GiteaMirror closed this issue

2025-11-02 12:01:51 -06:00

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#16080