diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index f227ffc..5acb8b8 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -54,11 +54,6 @@ jobs: go-version-file: "go.mod" cache: true - - name: "Install test runner" - run: | - go install -mod=mod github.com/onsi/ginkgo/v2/ginkgo - go get github.com/onsi/gomega/... - - name: "Configure iRODS clients" run: | mkdir -p "$HOME/.irods" diff --git a/Makefile b/Makefile index 0b0f092..960e3dd 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ export GOARCH := $(shell go env GOARCH) CGO_ENABLED := 1 -.PHONY: build build-linux build-darwin build-windows check clean coverage install lint test +.PHONY: build build-linux build-darwin build-windows check clean coverage install lint test-install test all: build @@ -37,10 +37,13 @@ lint: check: test -test: +test-install: + go install github.com/onsi/ginkgo/v2/ginkgo + +test: test-install ginkgo -r --race -coverage: +coverage: test-install ginkgo -r --cover -coverprofile=coverage.out dist: build diff --git a/server/handlers.go b/server/handlers.go index 45ecaec..d2c722b 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -338,15 +338,20 @@ func HandleIRODSGet(server *SqyrrlServer) http.Handler { return } - userZone := server.iRODSAccount.ClientZone + localZone := server.iRODSAccount.ClientZone var isReadable bool if server.isAuthenticated(r) { + // The username obtained from the email address does not include the iRODS + // zone. We use the local zone to which the Sqyrrl server is connected as + // the user's zone. userName := iRODSUsernameFromEmail(logger, server.getSessionUserEmail(r)) + userZone := localZone + logger.Debug().Str("user", userName).Msg("User is authenticated") - isReadable, err = IsReadableByUser(logger, rodsFs, userName, userZone, objPath) + isReadable, err = IsReadableByUser(logger, rodsFs, localZone, userName, userZone, objPath) if err != nil { logger.Err(err).Msg("Failed to check if the object is readable") writeErrorResponse(logger, w, http.StatusInternalServerError) @@ -364,7 +369,7 @@ func HandleIRODSGet(server *SqyrrlServer) http.Handler { } } else { logger.Debug().Msg("User is not authenticated") - isReadable, err = IsPublicReadable(logger, rodsFs, userZone, objPath) + isReadable, err = IsPublicReadable(logger, rodsFs, localZone, objPath) if err != nil { logger.Err(err).Msg("Failed to check if the object is public readable") writeErrorResponse(logger, w, http.StatusInternalServerError) diff --git a/server/irods.go b/server/irods.go index 17f899b..d24b413 100644 --- a/server/irods.go +++ b/server/irods.go @@ -18,6 +18,7 @@ package server import ( + "crypto/tls" "fmt" "net/http" "os" @@ -141,6 +142,14 @@ func NewIRODSAccount(logger zerolog.Logger, manager *config.ICommandsEnvironment return nil, err } + var tlsConfig *tls.Config + if tlsConfig, err = account.SSLConfiguration.GetTLSConfig(account.Host); err != nil { + logger.Err(err).Msg("Failed to update TLS configuration") + return nil, err + } + // Add a cipher suite that is compatible with older iRODS servers (4.2.7) + tlsConfig.CipherSuites = append(tlsConfig.CipherSuites, tls.TLS_RSA_WITH_AES_256_CBC_SHA) + if password != "" { if err = InitIRODS(logger, manager, password); err != nil { logger.Err(err). @@ -190,18 +199,41 @@ func NewIRODSAccount(logger zerolog.Logger, manager *config.ICommandsEnvironment } // IsReadableByUser checks if the data object at the given path is readable by the -// given user in the zone hosting the file. +// given user in the zone the Sqyrrl server is logged into. // // If iRODS is federated, there may be multiple zones, each with their own users. -// The zone argument is the zone of user whose read permission is to be checked, -// which is normally the current zone. This is consulted only if the ACL user zone is -// empty. +// +// The localZone argument is the zone that Sqyrrl is logged into. This is consulted +// only if an access permission in the data object's ACL has an empty user zone, in which +// case the local zone is assumed. +// +// The userZone argument is the zone of the user whose read permission is to be checked. // // A file is considered readable if the user has read access or is in a group that has // read access. func IsReadableByUser(logger zerolog.Logger, filesystem *ifs.FileSystem, - userName string, userZone string, rodsPath string) (_ bool, err error) { + localZone string, userName string, userZone string, rodsPath string) (_ bool, err error) { var acl []*types.IRODSAccess + var users []*types.IRODSUser + + localUserExists := false + if users, err = filesystem.ListUsers(); err != nil { + return false, err + } + for _, user := range users { + if user.Name == userName && user.Zone == userZone { + localUserExists = true + break + } + } + if !localUserExists { + logger.Warn(). + Str("path", rodsPath). + Str("user", userName). + Str("zone", userZone). + Msg("iRODS user not found") + return false, nil + } if acl, err = filesystem.ListACLs(rodsPath); err != nil { return false, err @@ -231,11 +263,11 @@ func IsReadableByUser(logger zerolog.Logger, filesystem *ifs.FileSystem, for _, ac := range acl { // ACL user zone may be empty if it refers to the local zone - var effectiveUserZone string + var acUserZone string if ac.UserZone != "" { - effectiveUserZone = ac.UserZone + acUserZone = ac.UserZone } else { - effectiveUserZone = userZone + acUserZone = localZone } hasRead := ac.AccessLevel == types.IRODSAccessLevelReadObject @@ -243,13 +275,13 @@ func IsReadableByUser(logger zerolog.Logger, filesystem *ifs.FileSystem, // There is permission directly for the user if ac.UserType == types.IRODSUserRodsUser || ac.UserType == types.IRODSUserRodsAdmin { - if effectiveUserZone == userZone && ac.UserName == userName && (hasRead || hasOwn) { + if acUserZone == userZone && ac.UserName == userName && (hasRead || hasOwn) { logger.Trace(). Str("path", rodsPath). Str("user", userName). Str("zone", userZone). - Str("effective_zone", effectiveUserZone). Str("ac_user", ac.UserName). + Str("ac_zone", acUserZone). Str("ac_level", string(ac.AccessLevel)). Bool("read", hasRead). Bool("own", hasOwn). @@ -263,7 +295,7 @@ func IsReadableByUser(logger zerolog.Logger, filesystem *ifs.FileSystem, Str("user", userName). Str("zone", userZone). Str("ac_user", ac.UserName). - Str("effective_zone", effectiveUserZone). + Str("ac_zone", acUserZone). Str("ac_level", string(ac.AccessLevel)). Bool("read", hasRead). Bool("own", hasOwn). @@ -275,13 +307,13 @@ func IsReadableByUser(logger zerolog.Logger, filesystem *ifs.FileSystem, // Check if user in the group of this AC (ac.UserName is the name of the AC's group, unfortunately) _, userInGroup := userGroupLookup[ac.UserName] - if effectiveUserZone == userZone && userInGroup && (hasRead || hasOwn) { + if acUserZone == userZone && userInGroup && (hasRead || hasOwn) { logger.Trace(). Str("path", rodsPath). Str("user", userName). Str("zone", userZone). - Str("effective_zone", effectiveUserZone). Str("ac_user", ac.UserName). + Str("ac_zone", acUserZone). Str("ac_level", string(ac.AccessLevel)). Bool("read", hasRead). Bool("own", hasOwn). diff --git a/server/irods_test.go b/server/irods_test.go index 797677a..e0c28e6 100644 --- a/server/irods_test.go +++ b/server/irods_test.go @@ -34,13 +34,15 @@ import ( var _ = Describe("iRODS functions", func() { var conn *connection.IRODSConnection - var zone string + var localZone, userZone string var workColl string var testFile, localPath, remotePath string var err error BeforeEach(func(ctx SpecContext) { - zone = "testZone" + localZone = "testZone" + userZone = localZone + workColl = TmpRodsPath(rootColl, "iRODSGetHandler") err = irodsFS.MakeDir(workColl, true) Expect(err).NotTo(HaveOccurred()) @@ -67,8 +69,8 @@ var _ = Describe("iRODS functions", func() { When("a non-existent path is given", func() { It("should return a FileNotFoundError", func() { - _, err := server.IsReadableByUser(suiteLogger, irodsFS, userInPublic, - zone, "/no/such/path") + _, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, userZone, "/no/such/path") Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(&types.FileNotFoundError{})) }) @@ -76,8 +78,8 @@ var _ = Describe("iRODS functions", func() { When("a non-existent user is given", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, "no_such_user", - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + "no_such_user", userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) }) @@ -108,21 +110,31 @@ var _ = Describe("iRODS functions", func() { When("the user is in the public group", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) }) + + When("the user is in a different zone", func() { + It("should return false", func() { + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, "anotherZone", remotePath) + Expect(err).NotTo(HaveOccurred()) + Expect(readable).To(BeFalse()) + }) + }) }) When("the user is not in the public group", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userNotInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userNotInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) }) }) + }) When("the data object has read permissions for the public group", func() { @@ -134,21 +146,31 @@ var _ = Describe("iRODS functions", func() { When("the user is in the public group", func() { It("should return true", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeTrue()) }) + + When("the user is in a different zone", func() { + It("should return false", func() { + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, "anotherZone", remotePath) + Expect(err).NotTo(HaveOccurred()) + Expect(readable).To(BeFalse()) + }) + }) }) When("the user is not in the public group", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userNotInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userNotInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) }) }) + }) When("the data object has own permissions for the public group", func() { @@ -160,21 +182,31 @@ var _ = Describe("iRODS functions", func() { When("the user is in the public group", func() { It("should return true", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeTrue()) }) + + When("the user is in a different zone", func() { + It("should return false", func() { + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInPublic, "anotherZone", remotePath) + Expect(err).NotTo(HaveOccurred()) + Expect(readable).To(BeFalse()) + }) + }) }) When("the user is not in the public group", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userNotInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userNotInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) }) }) + }) When("the data object has read permissions for several groups", func() { @@ -188,17 +220,26 @@ var _ = Describe("iRODS functions", func() { When("the user is in one of the groups", func() { It("should return true", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userInOthers, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInOthers, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeTrue()) }) + + When("the user is in a different zone", func() { + It("should return false", func() { + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userInOthers, "anotherZone", remotePath) + Expect(err).NotTo(HaveOccurred()) + Expect(readable).To(BeFalse()) + }) + }) }) When("the user is not in one of the groups", func() { It("should return false", func() { - readable, err := server.IsReadableByUser(suiteLogger, irodsFS, userNotInPublic, - zone, remotePath) + readable, err := server.IsReadableByUser(suiteLogger, irodsFS, localZone, + userNotInPublic, userZone, remotePath) Expect(err).NotTo(HaveOccurred()) Expect(readable).To(BeFalse()) })