From 77d87aeecdafaa6aa4fde0b62cf7159fcd00061c Mon Sep 17 00:00:00 2001 From: Thomas Miceli <27960254+thomiceli@users.noreply.github.com> Date: Tue, 28 May 2024 00:00:05 +0200 Subject: [PATCH 1/2] Fix CI check for additional translations only (#289) --- scripts/check-translations.sh | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/scripts/check-translations.sh b/scripts/check-translations.sh index bfe88915..ccda7dc7 100755 --- a/scripts/check-translations.sh +++ b/scripts/check-translations.sh @@ -8,8 +8,6 @@ sed -i '/^\s*$/d' sorted_reference_keys.txt for new_file in internal/i18n/locales/*.yml; do filename=$(basename $new_file) - echo "" - echo "Checking $filename..." # Extract keys from the current file and sort them sort <(awk -F':' '{print $1}' $new_file) > sorted_new_keys.txt @@ -18,17 +16,12 @@ for new_file in internal/i18n/locales/*.yml; do comm -3 sorted_reference_keys.txt sorted_new_keys.txt > differences.txt if [ -s differences.txt ]; then - echo "Error in $filename: The YAML file has differences in keys." while IFS= read -r line; do if [[ $line == $'\t'* ]]; then echo "+ Additional key in $filename: $(echo $line | awk '{$1=$1; print}')" - else - echo "- Missing key in $filename: $(echo $line | awk '{$1=$1; print}')" + differences_found=1 fi done < differences.txt - differences_found=1 - else - echo "All keys in $filename match perfectly." fi rm sorted_new_keys.txt From 38892d8a4a4115b6fcb399dc6e3c0fa7ecb3dab5 Mon Sep 17 00:00:00 2001 From: Thomas Miceli <27960254+thomiceli@users.noreply.github.com> Date: Tue, 28 May 2024 01:30:08 +0200 Subject: [PATCH 2/2] Fix perms for http/ssh clone (#288) --- internal/actions/actions.go | 2 +- internal/cli/main.go | 2 +- internal/db/sshkey.go | 13 ++- internal/db/user.go | 9 ++ internal/i18n/locales/en-US.yml | 1 + internal/ssh/git_ssh.go | 11 ++- internal/ssh/run.go | 4 +- internal/web/git_http.go | 11 ++- internal/web/server.go | 10 +- internal/web/settings.go | 8 ++ internal/web/test/auth_test.go | 169 ++++++++++++++++++++++++++++++++ internal/web/test/server.go | 7 +- 12 files changed, 225 insertions(+), 22 deletions(-) diff --git a/internal/actions/actions.go b/internal/actions/actions.go index 404ae66f..5c8b460d 100644 --- a/internal/actions/actions.go +++ b/internal/actions/actions.go @@ -74,7 +74,7 @@ func Run(actionType int) { case IndexGists: functionToRun = indexGists default: - panic("unhandled default case") + log.Error().Msg("Unknown action type") } functionToRun() diff --git a/internal/cli/main.go b/internal/cli/main.go index 533695f5..e313c233 100644 --- a/internal/cli/main.go +++ b/internal/cli/main.go @@ -30,7 +30,7 @@ var CmdStart = cli.Command{ Usage: "Start Opengist server", Action: func(ctx *cli.Context) error { Initialize(ctx) - go web.NewServer(os.Getenv("OG_DEV") == "1").Start() + go web.NewServer(os.Getenv("OG_DEV") == "1", path.Join(config.GetHomeDir(), "sessions")).Start() go ssh.Start() select {} }, diff --git a/internal/db/sshkey.go b/internal/db/sshkey.go index e79d9739..0ff1a2ae 100644 --- a/internal/db/sshkey.go +++ b/internal/db/sshkey.go @@ -48,13 +48,12 @@ func GetSSHKeyByID(sshKeyId uint) (*SSHKey, error) { return sshKey, err } -func SSHKeyDoesExists(sshKeyContent string) (*SSHKey, error) { - sshKey := new(SSHKey) - err := db. - Where("content like ?", sshKeyContent+"%"). - First(&sshKey).Error - - return sshKey, err +func SSHKeyDoesExists(sshKeyContent string) (bool, error) { + var count int64 + err := db.Model(&SSHKey{}). + Where("content = ?", sshKeyContent). + Count(&count).Error + return count > 0, err } func (sshKey *SSHKey) Create() error { diff --git a/internal/db/user.go b/internal/db/user.go index 88208870..37a01912 100644 --- a/internal/db/user.go +++ b/internal/db/user.go @@ -118,6 +118,15 @@ func GetUsersFromEmails(emailsSet map[string]struct{}) (map[string]*User, error) return userMap, nil } +func GetUserFromSSHKey(sshKey string) (*User, error) { + user := new(User) + err := db. + Joins("JOIN ssh_keys ON users.id = ssh_keys.user_id"). + Where("ssh_keys.content = ?", sshKey). + First(&user).Error + return user, err +} + func SSHKeyExistsForUser(sshKey string, userId uint) (*SSHKey, error) { key := new(SSHKey) err := db. diff --git a/internal/i18n/locales/en-US.yml b/internal/i18n/locales/en-US.yml index 9a34da6f..55d7cf06 100644 --- a/internal/i18n/locales/en-US.yml +++ b/internal/i18n/locales/en-US.yml @@ -126,6 +126,7 @@ settings.delete-ssh-key-confirm: Confirm deletion of SSH key settings.ssh-key-added-at: Added settings.ssh-key-never-used: Never used settings.ssh-key-last-used: Last used +settings.ssh-key-exists: SSH key already exists settings.change-username: Change username settings.create-password: Create password settings.create-password-help: Create your password to login to Opengist via HTTP diff --git a/internal/ssh/git_ssh.go b/internal/ssh/git_ssh.go index caf9c9f6..fe6f2b16 100644 --- a/internal/ssh/git_ssh.go +++ b/internal/ssh/git_ssh.go @@ -50,11 +50,18 @@ func runGitCommand(ch ssh.Channel, gitCmd string, key string, ip string) error { // - gist is not found (obfuscation) // - admin setting to require login is set to true if verb == "receive-pack" || - gist.Private == 2 || + gist.Private == db.PrivateVisibility || gist.ID == 0 || !allowUnauthenticated { - pubKey, err := db.SSHKeyExistsForUser(key, gist.UserID) + var userToCheckPermissions *db.User + if gist.Private != db.PrivateVisibility && verb == "upload-pack" { + userToCheckPermissions, _ = db.GetUserFromSSHKey(key) + } else { + userToCheckPermissions = &gist.User + } + + pubKey, err := db.SSHKeyExistsForUser(key, userToCheckPermissions.ID) if err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { log.Warn().Msg("Invalid SSH authentication attempt from " + ip) diff --git a/internal/ssh/run.go b/internal/ssh/run.go index e4eaccb0..6d624384 100644 --- a/internal/ssh/run.go +++ b/internal/ssh/run.go @@ -24,8 +24,8 @@ func Start() { sshConfig := &ssh.ServerConfig{ PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { strKey := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(key))) - _, err := db.SSHKeyDoesExists(strKey) - if err != nil { + exists, err := db.SSHKeyDoesExists(strKey) + if !exists { if !errors.Is(err, gorm.ErrRecordNotFound) { return nil, err } diff --git a/internal/web/git_http.go b/internal/web/git_http.go index bf8bbf88..a0341fff 100644 --- a/internal/web/git_http.go +++ b/internal/web/git_http.go @@ -73,7 +73,7 @@ func gitHttp(ctx echo.Context) error { allow, err := auth.ShouldAllowUnauthenticatedGistAccess(ContextAuthInfo{ctx}, true) if err != nil { - panic("impossible") + log.Fatal().Err(err).Msg("Cannot check if unauthenticated access is allowed") } // Shows basic auth if : @@ -105,7 +105,14 @@ func gitHttp(ctx echo.Context) error { return plainText(ctx, 404, "Check your credentials or make sure you have access to the Gist") } - if ok, err := utils.Argon2id.Verify(authPassword, gist.User.Password); !ok || gist.User.Username != authUsername { + var userToCheckPermissions *db.User + if gist.Private != db.PrivateVisibility && isPull { + userToCheckPermissions, _ = db.GetUserByUsername(authUsername) + } else { + userToCheckPermissions = &gist.User + } + + if ok, err := utils.Argon2id.Verify(authPassword, userToCheckPermissions.Password); !ok { if err != nil { return errorRes(500, "Cannot verify password", err) } diff --git a/internal/web/server.go b/internal/web/server.go index 9da58485..60b4e8eb 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -161,12 +161,12 @@ type Server struct { dev bool } -func NewServer(isDev bool) *Server { +func NewServer(isDev bool, sessionsPath string) *Server { dev = isDev flashStore = sessions.NewCookieStore([]byte("opengist")) - userStore = sessions.NewFilesystemStore(path.Join(config.GetHomeDir(), "sessions"), - utils.ReadKey(path.Join(config.GetHomeDir(), "sessions", "session-auth.key")), - utils.ReadKey(path.Join(config.GetHomeDir(), "sessions", "session-encrypt.key")), + userStore = sessions.NewFilesystemStore(sessionsPath, + utils.ReadKey(path.Join(sessionsPath, "session-auth.key")), + utils.ReadKey(path.Join(sessionsPath, "session-encrypt.key")), ) userStore.MaxLength(10 * 1024) gothic.Store = userStore @@ -526,7 +526,7 @@ func makeCheckRequireLogin(isSingleGistAccess bool) echo.MiddlewareFunc { allow, err := auth.ShouldAllowUnauthenticatedGistAccess(ContextAuthInfo{ctx}, isSingleGistAccess) if err != nil { - panic("impossible") + log.Fatal().Err(err).Msg("Failed to check if unauthenticated access is allowed") } if !allow { diff --git a/internal/web/settings.go b/internal/web/settings.go index 14d64802..d3d814bb 100644 --- a/internal/web/settings.go +++ b/internal/web/settings.go @@ -89,6 +89,14 @@ func sshKeysProcess(ctx echo.Context) error { } key.Content = strings.TrimSpace(string(ssh.MarshalAuthorizedKey(pubKey))) + if exists, err := db.SSHKeyDoesExists(key.Content); exists { + if err != nil { + return errorRes(500, "Cannot check if SSH key exists", err) + } + addFlash(ctx, tr(ctx, "settings.ssh-key-exists"), "error") + return redirect(ctx, "/settings") + } + if err := key.Create(); err != nil { return errorRes(500, "Cannot add SSH key", err) } diff --git a/internal/web/test/auth_test.go b/internal/web/test/auth_test.go index f648815e..cc13dc69 100644 --- a/internal/web/test/auth_test.go +++ b/internal/web/test/auth_test.go @@ -1,8 +1,13 @@ package test import ( + "fmt" "github.com/stretchr/testify/require" + "github.com/thomiceli/opengist/internal/config" "github.com/thomiceli/opengist/internal/db" + "os" + "os/exec" + "path" "testing" ) @@ -147,3 +152,167 @@ func TestAnonymous(t *testing.T) { require.NoError(t, err) } + +func TestGitOperations(t *testing.T) { + setup(t) + s, err := newTestServer() + require.NoError(t, err, "Failed to create test server") + defer teardown(t, s) + + admin := db.UserDTO{Username: "thomas", Password: "thomas"} + register(t, s, admin) + s.sessionCookie = "" + register(t, s, db.UserDTO{Username: "fujiwara", Password: "fujiwara"}) + s.sessionCookie = "" + register(t, s, db.UserDTO{Username: "kaguya", Password: "kaguya"}) + + gist1 := db.GistDTO{ + Title: "kaguya-pub-gist", + URL: "kaguya-pub-gist", + Description: "kaguya's first gist", + VisibilityDTO: db.VisibilityDTO{ + Private: db.PublicVisibility, + }, + Name: []string{"kaguya-file.txt"}, + Content: []string{ + "yeah", + }, + } + err = s.request("POST", "/", gist1, 302) + require.NoError(t, err) + + gist2 := db.GistDTO{ + Title: "kaguya-unl-gist", + URL: "kaguya-unl-gist", + Description: "kaguya's second gist", + VisibilityDTO: db.VisibilityDTO{ + Private: db.UnlistedVisibility, + }, + Name: []string{"kaguya-file.txt"}, + Content: []string{ + "cool", + }, + } + err = s.request("POST", "/", gist2, 302) + require.NoError(t, err) + + gist3 := db.GistDTO{ + Title: "kaguya-priv-gist", + URL: "kaguya-priv-gist", + Description: "kaguya's second gist", + VisibilityDTO: db.VisibilityDTO{ + Private: db.PrivateVisibility, + }, + Name: []string{"kaguya-file.txt"}, + Content: []string{ + "super", + }, + } + err = s.request("POST", "/", gist3, 302) + require.NoError(t, err) + + gitOperations := func(credentials, owner, url, filename string, expectErrorClone, expectErrorCheck, expectErrorPush bool) { + fmt.Println("Testing", credentials, url, expectErrorClone, expectErrorCheck, expectErrorPush) + err := clientGitClone(credentials, owner, url) + if expectErrorClone { + require.Error(t, err) + } else { + require.NoError(t, err) + } + err = clientCheckRepo(url, filename) + if expectErrorCheck { + require.Error(t, err) + } else { + require.NoError(t, err) + } + err = clientGitPush(url) + if expectErrorPush { + require.Error(t, err) + } else { + require.NoError(t, err) + } + } + + tests := []struct { + credentials string + user string + url string + expectErrorClone bool + expectErrorCheck bool + expectErrorPush bool + }{ + {":", "kaguya", "kaguya-pub-gist", false, false, true}, + {":", "kaguya", "kaguya-unl-gist", false, false, true}, + {":", "kaguya", "kaguya-priv-gist", true, true, true}, + {"kaguya:kaguya", "kaguya", "kaguya-pub-gist", false, false, false}, + {"kaguya:kaguya", "kaguya", "kaguya-unl-gist", false, false, false}, + {"kaguya:kaguya", "kaguya", "kaguya-priv-gist", false, false, false}, + {"fujiwara:fujiwara", "kaguya", "kaguya-pub-gist", false, false, true}, + {"fujiwara:fujiwara", "kaguya", "kaguya-unl-gist", false, false, true}, + {"fujiwara:fujiwara", "kaguya", "kaguya-priv-gist", true, true, true}, + } + + for _, test := range tests { + gitOperations(test.credentials, test.user, test.url, "kaguya-file.txt", test.expectErrorClone, test.expectErrorCheck, test.expectErrorPush) + } + + login(t, s, admin) + err = s.request("PUT", "/admin-panel/set-config", settingSet{"require-login", "1"}, 200) + require.NoError(t, err) + + testsRequireLogin := []struct { + credentials string + user string + url string + expectErrorClone bool + expectErrorCheck bool + expectErrorPush bool + }{ + {":", "kaguya", "kaguya-pub-gist", true, true, true}, + {":", "kaguya", "kaguya-unl-gist", true, true, true}, + {":", "kaguya", "kaguya-priv-gist", true, true, true}, + {"kaguya:kaguya", "kaguya", "kaguya-pub-gist", false, false, false}, + {"kaguya:kaguya", "kaguya", "kaguya-unl-gist", false, false, false}, + {"kaguya:kaguya", "kaguya", "kaguya-priv-gist", false, false, false}, + {"fujiwara:fujiwara", "kaguya", "kaguya-pub-gist", false, false, true}, + {"fujiwara:fujiwara", "kaguya", "kaguya-unl-gist", false, false, true}, + {"fujiwara:fujiwara", "kaguya", "kaguya-priv-gist", true, true, true}, + } + + for _, test := range testsRequireLogin { + gitOperations(test.credentials, test.user, test.url, "kaguya-file.txt", test.expectErrorClone, test.expectErrorCheck, test.expectErrorPush) + } + + login(t, s, admin) + err = s.request("PUT", "/admin-panel/set-config", settingSet{"allow-gists-without-login", "1"}, 200) + require.NoError(t, err) + + for _, test := range tests { + gitOperations(test.credentials, test.user, test.url, "kaguya-file.txt", test.expectErrorClone, test.expectErrorCheck, test.expectErrorPush) + } +} + +func clientGitClone(creds string, user string, url string) error { + return exec.Command("git", "clone", "http://"+creds+"@localhost:6157/"+user+"/"+url, path.Join(config.GetHomeDir(), "tmp", url)).Run() +} + +func clientGitPush(url string) error { + f, err := os.Create(path.Join(config.GetHomeDir(), "tmp", url, "newfile.txt")) + if err != nil { + return err + } + f.Close() + + _ = exec.Command("git", "-C", path.Join(config.GetHomeDir(), "tmp", url), "add", "newfile.txt").Run() + _ = exec.Command("git", "-C", path.Join(config.GetHomeDir(), "tmp", url), "commit", "-m", "new file").Run() + err = exec.Command("git", "-C", path.Join(config.GetHomeDir(), "tmp", url), "push", "origin", "master").Run() + + _ = os.RemoveAll(path.Join(config.GetHomeDir(), "tmp", url)) + + return err +} + +func clientCheckRepo(url string, file string) error { + _, err := os.ReadFile(path.Join(config.GetHomeDir(), "tmp", url, file)) + return err +} diff --git a/internal/web/test/server.go b/internal/web/test/server.go index 5ef8d252..b10e40e9 100644 --- a/internal/web/test/server.go +++ b/internal/web/test/server.go @@ -31,7 +31,7 @@ type testServer struct { func newTestServer() (*testServer, error) { s := &testServer{ - server: web.NewServer(true), + server: web.NewServer(true, path.Join(config.GetHomeDir(), "tmp", "sessions")), } go s.start() @@ -149,7 +149,7 @@ func setup(t *testing.T) { homePath := config.GetHomeDir() log.Info().Msg("Data directory: " + homePath) - err = os.MkdirAll(filepath.Join(homePath, "sessions"), 0755) + err = os.MkdirAll(filepath.Join(homePath, "tmp", "sessions"), 0755) require.NoError(t, err, "Could not create sessions directory") err = os.MkdirAll(filepath.Join(homePath, "tmp", "repos"), 0755) @@ -177,6 +177,9 @@ func teardown(t *testing.T, s *testServer) { err = os.RemoveAll(path.Join(config.GetHomeDir(), "tmp", "repos")) require.NoError(t, err, "Could not remove repos directory") + err = os.RemoveAll(path.Join(config.GetHomeDir(), "tmp", "sessions")) + require.NoError(t, err, "Could not remove repos directory") + // err = os.RemoveAll(path.Join(config.C.OpengistHome, "testsindex")) // require.NoError(t, err, "Could not remove repos directory")