diff --git a/ocis/tests/acceptance/expected-failures-on-OCIS-storage.txt b/ocis/tests/acceptance/expected-failures-on-OCIS-storage.txt index bdb8bc467..b653fdbd1 100644 --- a/ocis/tests/acceptance/expected-failures-on-OCIS-storage.txt +++ b/ocis/tests/acceptance/expected-failures-on-OCIS-storage.txt @@ -684,11 +684,6 @@ apiSharePublicLink2/uploadToPublicLinkShare.feature:103 apiSharePublicLink2/uploadToPublicLinkShare.feature:121 apiSharePublicLink2/uploadToPublicLinkShare.feature:139 # -# https://github.com/owncloud/ocis/issues/801 deleting a folder should delete share links to it as well -# -apiSharePublicLink2/uploadToPublicLinkShare.feature:48 -apiSharePublicLink2/uploadToPublicLinkShare.feature:49 -# # https://github.com/owncloud/ocis-reva/issues/286 Upload-only shares must not overwrite but create a separate file # apiSharePublicLink2/uploadToPublicLinkShare.feature:23 diff --git a/ocis/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt b/ocis/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt index b9fff579b..edc1d58b4 100644 --- a/ocis/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt +++ b/ocis/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt @@ -651,11 +651,6 @@ apiSharePublicLink2/uploadToPublicLinkShare.feature:103 apiSharePublicLink2/uploadToPublicLinkShare.feature:121 apiSharePublicLink2/uploadToPublicLinkShare.feature:139 # -# https://github.com/owncloud/ocis/issues/801 deleting a folder should delete share links to it as well -# -apiSharePublicLink2/uploadToPublicLinkShare.feature:48 -apiSharePublicLink2/uploadToPublicLinkShare.feature:49 -# # https://github.com/owncloud/ocis-reva/issues/286 Upload-only shares must not overwrite but create a separate file # apiSharePublicLink2/uploadToPublicLinkShare.feature:23 diff --git a/ocis/tests/acceptance/features/apiOcisSpecific/apiSharePublicLink2-uploadToPublicLinkShare.feature b/ocis/tests/acceptance/features/apiOcisSpecific/apiSharePublicLink2-uploadToPublicLinkShare.feature index 51646d2b4..320ce525b 100644 --- a/ocis/tests/acceptance/features/apiOcisSpecific/apiSharePublicLink2-uploadToPublicLinkShare.feature +++ b/ocis/tests/acceptance/features/apiOcisSpecific/apiSharePublicLink2-uploadToPublicLinkShare.feature @@ -15,19 +15,3 @@ Feature: upload to a public link share When user "Alice" deletes file "/FOLDER" using the WebDAV API And the public uploads file "does-not-matter.txt" with content "does not matter" using the new public WebDAV API Then the HTTP status code should be "500" - - @issue-ocis-801 - # after fixing all issues delete this Scenario and use the one from oC10 core - Scenario Outline: Uploading file to a public upload-only share using old public API that was deleted does not work - Given using DAV path - And user "Alice" has created a public link share with settings - | path | FOLDER | - | permissions | create | - When user "Alice" deletes file "/FOLDER" using the WebDAV API - Then uploading a file should not work using the old public WebDAV API - And the HTTP status code should be "401" - - Examples: - | dav-path | - | old | - | new | \ No newline at end of file diff --git a/proxy/go.mod b/proxy/go.mod index 3ed47e589..e504e956a 100644 --- a/proxy/go.mod +++ b/proxy/go.mod @@ -9,6 +9,7 @@ require ( github.com/coreos/go-oidc v2.2.1+incompatible github.com/cs3org/go-cs3apis v0.0.0-20201007120910-416ed6cf8b00 github.com/cs3org/reva v1.3.1-0.20201023144216-cdb3d6688da5 + github.com/google/uuid v1.1.2 github.com/justinas/alice v1.2.0 github.com/micro/cli/v2 v2.1.2 github.com/micro/go-micro/v2 v2.9.1 diff --git a/proxy/pkg/command/server.go b/proxy/pkg/command/server.go index ddd24972a..0765bc101 100644 --- a/proxy/pkg/command/server.go +++ b/proxy/pkg/command/server.go @@ -15,13 +15,12 @@ import ( "github.com/coreos/go-oidc" "github.com/justinas/alice" "github.com/micro/cli/v2" - "github.com/micro/go-micro/v2/client/grpc" + "github.com/owncloud/ocis/ocis-pkg/service/grpc" "github.com/oklog/run" openzipkin "github.com/openzipkin/zipkin-go" zipkinhttp "github.com/openzipkin/zipkin-go/reporter/http" acc "github.com/owncloud/ocis/accounts/pkg/proto/v0" "github.com/owncloud/ocis/ocis-pkg/log" - ogrpc "github.com/owncloud/ocis/ocis-pkg/service/grpc" "github.com/owncloud/ocis/proxy/pkg/config" "github.com/owncloud/ocis/proxy/pkg/cs3" "github.com/owncloud/ocis/proxy/pkg/flagset" @@ -247,73 +246,67 @@ func Server(cfg *config.Config) *cli.Command { } func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alice.Chain { - - psMW := middleware.PresignedURL( - middleware.Logger(l), - middleware.Store(storepb.NewStoreService("com.owncloud.api.store", grpc.NewClient())), - middleware.PreSignedURLConfig(cfg.PreSignedURL), - ) - - accounts := acc.NewAccountsService("com.owncloud.api.accounts", ogrpc.DefaultClient) - roles := settings.NewRoleService("com.owncloud.api.settings", ogrpc.DefaultClient) - - uuidMW := middleware.AccountUUID( - middleware.Logger(l), - middleware.TokenManagerConfig(cfg.TokenManager), - middleware.AccountsClient(accounts), - middleware.SettingsRoleService(roles), - middleware.AutoprovisionAccounts(cfg.AutoprovisionAccounts), - middleware.EnableBasicAuth(cfg.EnableBasicAuth), - middleware.OIDCIss(cfg.OIDC.Issuer), - ) - - // the connection will be established in a non blocking fashion - sc, err := cs3.GetGatewayServiceClient(cfg.Reva.Address) + accountsClient := acc.NewAccountsService("com.owncloud.api.accounts", grpc.DefaultClient) + rolesClient := settings.NewRoleService("com.owncloud.api.settings", grpc.DefaultClient) + storeClient := storepb.NewStoreService("com.owncloud.api.store", grpc.DefaultClient) + revaClient, err := cs3.GetGatewayServiceClient(cfg.Reva.Address) if err != nil { l.Error().Err(err). Str("gateway", cfg.Reva.Address). Msg("Failed to create reva gateway service client") } - chMW := middleware.CreateHome( - middleware.Logger(l), - middleware.RevaGatewayClient(sc), - middleware.AccountsClient(accounts), - middleware.TokenManagerConfig(cfg.TokenManager), - ) - - if cfg.OIDC.Issuer != "" { - l.Info().Msg("loading OIDC middleware") - l.Debug().Interface("oidc_config", cfg.OIDC).Msg("OIDC-Config") - - var oidcHTTPClient = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: cfg.OIDC.Insecure, - }, - DisableKeepAlives: true, + var oidcHTTPClient = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: cfg.OIDC.Insecure, }, - Timeout: time.Second * 10, - } - - customCtx := context.WithValue(ctx, oauth2.HTTPClient, oidcHTTPClient) - - // Initialize a provider by specifying the issuer URL. - // it will fetch the keys from the issuer using the .well-known - // endpoint - provider := func() (middleware.OIDCProvider, error) { - return oidc.NewProvider(customCtx, cfg.OIDC.Issuer) - } - - oidcMW := middleware.OpenIDConnect( - middleware.Logger(l), - middleware.HTTPClient(oidcHTTPClient), - middleware.OIDCProviderFunc(provider), - middleware.OIDCIss(cfg.OIDC.Issuer), - ) - - return alice.New(middleware.RedirectToHTTPS, oidcMW, psMW, uuidMW, chMW) + DisableKeepAlives: true, + }, + Timeout: time.Second * 10, } - return alice.New(middleware.RedirectToHTTPS, psMW, uuidMW, chMW) + return alice.New( + middleware.HTTPSRedirect, + middleware.OIDCAuth( + middleware.Logger(l), + middleware.OIDCProviderFunc(func() (middleware.OIDCProvider, error) { + // Initialize a provider by specifying the issuer URL. + // it will fetch the keys from the issuer using the .well-known + // endpoint + return oidc.NewProvider( + context.WithValue(ctx, oauth2.HTTPClient, oidcHTTPClient), + cfg.OIDC.Issuer, + ) + }), + middleware.HTTPClient(oidcHTTPClient), + middleware.OIDCIss(cfg.OIDC.Issuer), + ), + middleware.BasicAuth( + middleware.Logger(l), + middleware.EnableBasicAuth(cfg.EnableBasicAuth), + middleware.AccountsClient(accountsClient), + middleware.OIDCIss(cfg.OIDC.Issuer), + ), + middleware.SignedURLAuth( + middleware.Logger(l), + middleware.PreSignedURLConfig(cfg.PreSignedURL), + middleware.AccountsClient(accountsClient), + middleware.Store(storeClient), + ), + middleware.AccountResolver( + middleware.Logger(l), + middleware.AccountsClient(accountsClient), + middleware.OIDCIss(cfg.OIDC.Issuer), + middleware.TokenManagerConfig(cfg.TokenManager), + middleware.AutoprovisionAccounts(cfg.AutoprovisionAccounts), + middleware.SettingsRoleService(rolesClient), + ), + middleware.CreateHome( + middleware.Logger(l), + middleware.AccountsClient(accountsClient), + middleware.TokenManagerConfig(cfg.TokenManager), + middleware.RevaGatewayClient(revaClient), + ), + ) } diff --git a/proxy/pkg/middleware/account_resolver.go b/proxy/pkg/middleware/account_resolver.go new file mode 100644 index 000000000..8099af32b --- /dev/null +++ b/proxy/pkg/middleware/account_resolver.go @@ -0,0 +1,210 @@ +package middleware + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strconv" + "strings" + + revaUser "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + tokenPkg "github.com/cs3org/reva/pkg/token" + "github.com/cs3org/reva/pkg/token/manager/jwt" + accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/ocis-pkg/oidc" + settings "github.com/owncloud/ocis/settings/pkg/proto/v0" +) + +// AccountResolver provides a middleware which mints a jwt and adds it to the proxied request based +// on the oidc-claims +func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + logger := options.Logger + + return func(next http.Handler) http.Handler { + tokenManager, err := jwt.New(map[string]interface{}{ + "secret": options.TokenManagerConfig.JWTSecret, + "expires": int64(60), + }) + if err != nil { + logger.Fatal().Err(err).Msgf("Could not initialize token-manager") + } + + return &accountResolver{ + next: next, + logger: logger, + tokenManager: tokenManager, + accountsClient: options.AccountsClient, + oidcIss: options.OIDCIss, + autoprovisionAccounts: options.AutoprovisionAccounts, + settingsRoleService: options.SettingsRoleService, + } + } +} + +type accountResolver struct { + oidcIss string + autoprovisionAccounts bool + next http.Handler + logger log.Logger + tokenManager tokenPkg.Manager + accountsClient accounts.AccountsService + settingsRoleService settings.RoleService +} + +func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) { + var account *accounts.Account + var status int + + claims := oidc.FromContext(req.Context()) + + if claims == nil { + m.next.ServeHTTP(w, req) + return + } + + switch { + case claims.Email != "": + account, status = getAccount(m.logger, m.accountsClient, fmt.Sprintf("mail eq '%s'", strings.ReplaceAll(claims.Email, "'", "''"))) + case claims.PreferredUsername != "": + account, status = getAccount(m.logger, m.accountsClient, fmt.Sprintf("preferred_name eq '%s'", strings.ReplaceAll(claims.PreferredUsername, "'", "''"))) + case claims.OcisID != "": + account, status = getAccount(m.logger, m.accountsClient, fmt.Sprintf("id eq '%s'", strings.ReplaceAll(claims.OcisID, "'", "''"))) + default: + // TODO allow lookup by custom claim, eg an id ... or sub + m.logger.Error().Msg("Could not lookup account, no mail or preferred_username claim set") + w.WriteHeader(http.StatusInternalServerError) + } + + if m.autoprovisionAccounts && status == http.StatusNotFound { + account, status = createAccount(m.logger, claims, m.accountsClient) + } + + if status != 0 || account == nil { + w.WriteHeader(status) + return + } + + if !account.AccountEnabled { + m.logger.Debug().Interface("account", account).Msg("account is disabled") + w.WriteHeader(http.StatusUnauthorized) + return + } + + groups := make([]string, len(account.MemberOf)) + for i := range account.MemberOf { + // reva needs the unix group name + groups[i] = account.MemberOf[i].OnPremisesSamAccountName + } + + // fetch active roles from ocis-settings + assignmentResponse, err := m.settingsRoleService.ListRoleAssignments(req.Context(), &settings.ListRoleAssignmentsRequest{AccountUuid: account.Id}) + roleIDs := make([]string, 0) + if err != nil { + m.logger.Err(err).Str("accountID", account.Id).Msg("failed to fetch role assignments") + } else { + for _, assignment := range assignmentResponse.Assignments { + roleIDs = append(roleIDs, assignment.RoleId) + } + } + + m.logger.Debug().Interface("claims", claims).Interface("account", account).Msgf("associated claims with uuid") + + user := &revaUser.User{ + Id: &revaUser.UserId{ + OpaqueId: account.Id, + Idp: claims.Iss, + }, + Username: account.OnPremisesSamAccountName, + DisplayName: account.DisplayName, + Mail: account.Mail, + MailVerified: account.ExternalUserState == "" || account.ExternalUserState == "Accepted", + Groups: groups, + Opaque: &types.Opaque{ + Map: map[string]*types.OpaqueEntry{}, + }, + } + user.Opaque.Map["uid"] = &types.OpaqueEntry{ + Decoder: "plain", + Value: []byte(strconv.FormatInt(account.UidNumber, 10)), + } + user.Opaque.Map["gid"] = &types.OpaqueEntry{ + Decoder: "plain", + Value: []byte(strconv.FormatInt(account.GidNumber, 10)), + } + + // encode roleIDs as json string + roleIDsJSON, jsonErr := json.Marshal(roleIDs) + if jsonErr != nil { + m.logger.Err(jsonErr).Str("accountID", account.Id).Msg("failed to marshal roleIDs into json") + } else { + user.Opaque.Map["roles"] = &types.OpaqueEntry{ + Decoder: "json", + Value: roleIDsJSON, + } + } + + token, err := m.tokenManager.MintToken(req.Context(), user) + + if err != nil { + m.logger.Error().Err(err).Msgf("could not mint token") + w.WriteHeader(http.StatusInternalServerError) + return + } + + req.Header.Set("x-access-token", token) + + m.next.ServeHTTP(w, req) +} + +func getAccount(logger log.Logger, ac accounts.AccountsService, query string) (account *accounts.Account, status int) { + resp, err := ac.ListAccounts(context.Background(), &accounts.ListAccountsRequest{ + Query: query, + PageSize: 2, + }) + + if err != nil { + logger.Error().Err(err).Str("query", query).Msgf("error fetching from accounts-service") + status = http.StatusInternalServerError + return + } + + if len(resp.Accounts) <= 0 { + logger.Error().Str("query", query).Msgf("account not found") + status = http.StatusNotFound + return + } + + if len(resp.Accounts) > 1 { + logger.Error().Str("query", query).Msgf("more than one account found, aborting") + status = http.StatusForbidden + return + } + + account = resp.Accounts[0] + return +} + +func createAccount(l log.Logger, claims *oidc.StandardClaims, ac accounts.AccountsService) (*accounts.Account, int) { + // TODO check if fields are missing. + req := &accounts.CreateAccountRequest{ + Account: &accounts.Account{ + DisplayName: claims.DisplayName, + PreferredName: claims.PreferredUsername, + OnPremisesSamAccountName: claims.PreferredUsername, + Mail: claims.Email, + CreationType: "LocalAccount", + AccountEnabled: true, + }, + } + created, err := ac.CreateAccount(context.Background(), req) + if err != nil { + l.Error().Err(err).Interface("account", req.Account).Msg("could not create account") + return nil, http.StatusInternalServerError + } + + return created, 0 +} diff --git a/proxy/pkg/middleware/account_uuid_test.go b/proxy/pkg/middleware/account_resolver_test.go similarity index 50% rename from proxy/pkg/middleware/account_uuid_test.go rename to proxy/pkg/middleware/account_resolver_test.go index f7e9186a9..ff111829e 100644 --- a/proxy/pkg/middleware/account_uuid_test.go +++ b/proxy/pkg/middleware/account_resolver_test.go @@ -1,5 +1,110 @@ package middleware +import ( + "context" + "fmt" + "github.com/micro/go-micro/v2/client" + "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/ocis-pkg/oidc" + "github.com/owncloud/ocis/proxy/pkg/config" + settings "github.com/owncloud/ocis/settings/pkg/proto/v0" + "net/http" + "net/http/httptest" + "testing" +) + +func TestGetAccountSuccess(t *testing.T) { + svcCache.Invalidate(AccountsKey, "success") + if _, status := getAccount(log.NewLogger(), mockAccountResolverMiddlewareAccSvc(false, true), "mail eq 'success'"); status != 0 { + t.Errorf("expected an account") + } +} + +func TestGetAccountInternalError(t *testing.T) { + svcCache.Invalidate(AccountsKey, "failure") + if _, status := getAccount(log.NewLogger(), mockAccountResolverMiddlewareAccSvc(true, false), "mail eq 'failure'"); status != http.StatusInternalServerError { + t.Errorf("expected an internal server error") + } +} + +func TestAccountResolverMiddleware(t *testing.T) { + svcCache.Invalidate(AccountsKey, "success") + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) + m := AccountResolver( + Logger(log.NewLogger()), + TokenManagerConfig(config.TokenManager{JWTSecret: "secret"}), + AccountsClient(mockAccountResolverMiddlewareAccSvc(false, true)), + SettingsRoleService(mockAccountResolverMiddlewareRolesSvc(false)), + )(next) + + r := httptest.NewRequest(http.MethodGet, "http://www.example.com", nil) + w := httptest.NewRecorder() + ctx := oidc.NewContext(r.Context(), &oidc.StandardClaims{Email: "success"}) + r = r.WithContext(ctx) + m.ServeHTTP(w, r) + + if r.Header.Get("x-access-token") == "" { + t.Errorf("expected a token") + } +} + +func TestAccountResolverMiddlewareWithDisabledAccount(t *testing.T) { + svcCache.Invalidate(AccountsKey, "failure") + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) + m := AccountResolver( + Logger(log.NewLogger()), + TokenManagerConfig(config.TokenManager{JWTSecret: "secret"}), + AccountsClient(mockAccountResolverMiddlewareAccSvc(false, false)), + SettingsRoleService(mockAccountResolverMiddlewareRolesSvc(false)), + )(next) + + r := httptest.NewRequest(http.MethodGet, "http://www.example.com", nil) + w := httptest.NewRecorder() + ctx := oidc.NewContext(r.Context(), &oidc.StandardClaims{Email: "failure"}) + r = r.WithContext(ctx) + m.ServeHTTP(w, r) + + rsp := w.Result() + defer rsp.Body.Close() + + if rsp.StatusCode != http.StatusUnauthorized { + t.Errorf("expected a disabled account to be unauthorized, got: %d", rsp.StatusCode) + } +} + +func mockAccountResolverMiddlewareAccSvc(retErr, accEnabled bool) proto.AccountsService { + return &proto.MockAccountsService{ + ListFunc: func(ctx context.Context, in *proto.ListAccountsRequest, opts ...client.CallOption) (out *proto.ListAccountsResponse, err error) { + if retErr { + return nil, fmt.Errorf("error returned by mockAccountsService LIST") + } + return &proto.ListAccountsResponse{ + Accounts: []*proto.Account{ + { + Id: "yay", + AccountEnabled: accEnabled, + }, + }, + }, nil + }, + } +} + +func mockAccountResolverMiddlewareRolesSvc(returnError bool) settings.RoleService { + return &settings.MockRoleService{ + ListRoleAssignmentsFunc: func(ctx context.Context, req *settings.ListRoleAssignmentsRequest, opts ...client.CallOption) (res *settings.ListRoleAssignmentsResponse, err error) { + if returnError { + return nil, fmt.Errorf("error returned by mockRoleService.ListRoleAssignments") + } + return &settings.ListRoleAssignmentsResponse{ + Assignments: []*settings.UserRoleAssignment{}, + }, nil + }, + } +} + +/* import ( "context" "fmt" @@ -103,4 +208,4 @@ func mockAccountUUIDMiddlewareRolesSvc(returnError bool) settings.RoleService { }, nil }, } -} +}*/ diff --git a/proxy/pkg/middleware/account_uuid.go b/proxy/pkg/middleware/account_uuid.go deleted file mode 100644 index 5f955e097..000000000 --- a/proxy/pkg/middleware/account_uuid.go +++ /dev/null @@ -1,212 +0,0 @@ -package middleware - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "strconv" - "strings" - - revauser "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" - "github.com/cs3org/reva/pkg/token/manager/jwt" - acc "github.com/owncloud/ocis/accounts/pkg/proto/v0" - "github.com/owncloud/ocis/ocis-pkg/log" - "github.com/owncloud/ocis/ocis-pkg/oidc" - settings "github.com/owncloud/ocis/settings/pkg/proto/v0" -) - -func getAccount(l log.Logger, ac acc.AccountsService, query string) (account *acc.Account, status int) { - resp, err := ac.ListAccounts(context.Background(), &acc.ListAccountsRequest{ - Query: query, - PageSize: 2, - }) - - if err != nil { - l.Error().Err(err).Str("query", query).Msgf("Error fetching from accounts-service") - status = http.StatusInternalServerError - return - } - - if len(resp.Accounts) <= 0 { - l.Error().Str("query", query).Msgf("Account not found") - status = http.StatusNotFound - return - } - - if len(resp.Accounts) > 1 { - l.Error().Str("query", query).Msgf("More than one account found. Not logging user in.") - status = http.StatusForbidden - return - } - - account = resp.Accounts[0] - return -} - -func createAccount(l log.Logger, claims *oidc.StandardClaims, ac acc.AccountsService) (*acc.Account, int) { - // TODO check if fields are missing. - req := &acc.CreateAccountRequest{ - Account: &acc.Account{ - DisplayName: claims.DisplayName, - PreferredName: claims.PreferredUsername, - OnPremisesSamAccountName: claims.PreferredUsername, - Mail: claims.Email, - CreationType: "LocalAccount", - AccountEnabled: true, - // TODO assign uidnumber and gidnumber? better do that in ocis-accounts as it can keep track of the next numbers - }, - } - created, err := ac.CreateAccount(context.Background(), req) - if err != nil { - l.Error().Err(err).Interface("account", req.Account).Msg("could not create account") - return nil, http.StatusInternalServerError - } - - return created, 0 -} - -// AccountUUID provides a middleware which mints a jwt and adds it to the proxied request based -// on the oidc-claims -func AccountUUID(opts ...Option) func(next http.Handler) http.Handler { - opt := newOptions(opts...) - - publicFilesEndpoint := "/remote.php/dav/public-files/" - - return func(next http.Handler) http.Handler { - // TODO: handle error - tokenManager, err := jwt.New(map[string]interface{}{ - "secret": opt.TokenManagerConfig.JWTSecret, - "expires": int64(60), - }) - if err != nil { - opt.Logger.Fatal().Err(err).Msgf("Could not initialize token-manager") - } - - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - l := opt.Logger - claims := oidc.FromContext(r.Context()) - var account *acc.Account - var status int - switch { - case claims == nil: - login, password, ok := r.BasicAuth() - // check if we are dealing with a public link - if ok && login == "public" && strings.HasPrefix(r.URL.Path, publicFilesEndpoint) { - // forward to reva frontend - next.ServeHTTP(w, r) - return - } - if opt.EnableBasicAuth && ok { - l.Warn().Msg("basic auth enabled, use only for testing or development") - account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("login eq '%s' and password eq '%s'", strings.ReplaceAll(login, "'", "''"), strings.ReplaceAll(password, "'", "''"))) - if status == 0 { - // fake claims for the subsequent code flow - claims = &oidc.StandardClaims{ - Iss: opt.OIDCIss, - } - } else { - // tell client to reauthenticate - w.WriteHeader(http.StatusUnauthorized) - return - } - } else { - next.ServeHTTP(w, r) - return - } - case claims.Email != "": - account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("mail eq '%s'", strings.ReplaceAll(claims.Email, "'", "''"))) - case claims.PreferredUsername != "": - account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("preferred_name eq '%s'", strings.ReplaceAll(claims.PreferredUsername, "'", "''"))) - case claims.OcisID != "": - account, status = getAccount(l, opt.AccountsClient, fmt.Sprintf("id eq '%s'", strings.ReplaceAll(claims.OcisID, "'", "''"))) - default: - // TODO allow lookup by custom claim, eg an id ... or sub - l.Error().Err(err).Msg("Could not lookup account, no mail or preferred_username claim set") - w.WriteHeader(http.StatusInternalServerError) - } - if status != 0 || account == nil { - if opt.AutoprovisionAccounts && status == http.StatusNotFound { - account, status = createAccount(l, claims, opt.AccountsClient) - if status != 0 { - w.WriteHeader(status) - return - } - } else { - w.WriteHeader(status) - return - } - } - if !account.AccountEnabled { - l.Debug().Interface("account", account).Msg("account is disabled") - w.WriteHeader(http.StatusUnauthorized) - return - } - - groups := make([]string, len(account.MemberOf)) - for i := range account.MemberOf { - // reva needs the unix group name - groups[i] = account.MemberOf[i].OnPremisesSamAccountName - } - - // fetch active roles from ocis-settings - assignmentResponse, err := opt.SettingsRoleService.ListRoleAssignments(r.Context(), &settings.ListRoleAssignmentsRequest{AccountUuid: account.Id}) - roleIDs := make([]string, 0) - if err != nil { - l.Err(err).Str("accountID", account.Id).Msg("failed to fetch role assignments") - } else { - for _, assignment := range assignmentResponse.Assignments { - roleIDs = append(roleIDs, assignment.RoleId) - } - } - - l.Debug().Interface("claims", claims).Interface("account", account).Msgf("Associated claims with uuid") - user := &revauser.User{ - Id: &revauser.UserId{ - OpaqueId: account.Id, - Idp: claims.Iss, - }, - Username: account.OnPremisesSamAccountName, - DisplayName: account.DisplayName, - Mail: account.Mail, - MailVerified: account.ExternalUserState == "" || account.ExternalUserState == "Accepted", - Groups: groups, - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{}, - }, - } - - user.Opaque.Map["uid"] = &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte(strconv.FormatInt(account.UidNumber, 10)), - } - user.Opaque.Map["gid"] = &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte(strconv.FormatInt(account.GidNumber, 10)), - } - - // encode roleIDs as json string - roleIDsJSON, jsonErr := json.Marshal(roleIDs) - if jsonErr != nil { - l.Err(jsonErr).Str("accountID", account.Id).Msg("failed to marshal roleIDs into json") - } else { - user.Opaque.Map["roles"] = &types.OpaqueEntry{ - Decoder: "json", - Value: roleIDsJSON, - } - } - - token, err := tokenManager.MintToken(r.Context(), user) - - if err != nil { - l.Error().Err(err).Msgf("Could not mint token") - w.WriteHeader(http.StatusInternalServerError) - return - } - - r.Header.Set("x-access-token", token) - next.ServeHTTP(w, r) - }) - } -} diff --git a/proxy/pkg/middleware/basic_auth.go b/proxy/pkg/middleware/basic_auth.go new file mode 100644 index 000000000..607efa758 --- /dev/null +++ b/proxy/pkg/middleware/basic_auth.go @@ -0,0 +1,73 @@ +package middleware + +import ( + "fmt" + "net/http" + "strings" + + accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/ocis-pkg/oidc" +) + +const publicFilesEndpoint = "/remote.php/dav/public-files/" + +// BasicAuth provides a middleware to check if BasicAuth is provided +func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + + return func(next http.Handler) http.Handler { + return &basicAuth{ + next: next, + logger: options.Logger, + enabled: options.EnableBasicAuth, + accountsClient: options.AccountsClient, + oidcIss: options.OIDCIss, + } + } +} + +type basicAuth struct { + next http.Handler + logger log.Logger + enabled bool + accountsClient accounts.AccountsService + oidcIss string +} + +func (m basicAuth) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if m.isPublicLink(req) || !m.isBasicAuth(req) { + m.next.ServeHTTP(w, req) + return + } + + m.logger.Warn().Msg("basic auth enabled, use only for testing or development") + + login, password, _ := req.BasicAuth() + + account, status := getAccount(m.logger, m.accountsClient, fmt.Sprintf("login eq '%s' and password eq '%s'", strings.ReplaceAll(login, "'", "''"), strings.ReplaceAll(password, "'", "''"))) + + if status != 0 { + w.WriteHeader(http.StatusUnauthorized) + return + } + + claims := &oidc.StandardClaims{ + OcisID: account.Id, + Iss: m.oidcIss, + } + + m.next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), claims))) +} + +func (m basicAuth) isPublicLink(req *http.Request) bool { + login, _, ok := req.BasicAuth() + + return ok && login == "public" && strings.HasPrefix(req.URL.Path, publicFilesEndpoint) +} + +func (m basicAuth) isBasicAuth(req *http.Request) bool { + login, password, ok := req.BasicAuth() + + return m.enabled && ok && login != "" && password != "" +} diff --git a/proxy/pkg/middleware/basic_auth_test.go b/proxy/pkg/middleware/basic_auth_test.go new file mode 100644 index 000000000..fbdfab057 --- /dev/null +++ b/proxy/pkg/middleware/basic_auth_test.go @@ -0,0 +1,3 @@ +package middleware + +/**/ diff --git a/proxy/pkg/middleware/create_home.go b/proxy/pkg/middleware/create_home.go index 386aac919..2d86e8f1b 100644 --- a/proxy/pkg/middleware/create_home.go +++ b/proxy/pkg/middleware/create_home.go @@ -1,77 +1,100 @@ package middleware import ( - "net/http" - + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/rgrpc/status" - tokenpkg "github.com/cs3org/reva/pkg/token" + tokenPkg "github.com/cs3org/reva/pkg/token" "github.com/cs3org/reva/pkg/token/manager/jwt" - "github.com/micro/go-micro/v2/errors" - "github.com/owncloud/ocis/accounts/pkg/proto/v0" + microErrors "github.com/micro/go-micro/v2/errors" + accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/log" "google.golang.org/grpc/metadata" + "net/http" ) // CreateHome provides a middleware which sends a CreateHome request to the reva gateway -func CreateHome(opts ...Option) func(next http.Handler) http.Handler { - opt := newOptions(opts...) +func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + logger := options.Logger return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - accounts := opt.AccountsClient - - tokenManager, err := jwt.New(map[string]interface{}{ - "secret": opt.TokenManagerConfig.JWTSecret, - }) - if err != nil { - opt.Logger.Error().Err(err).Msg("error creating a token manager") - w.WriteHeader(http.StatusInternalServerError) - return - } - - token := r.Header.Get("x-access-token") - if token == "" { - next.ServeHTTP(w, r) - return - } - - user, err := tokenManager.DismantleToken(r.Context(), token) - if err != nil { - opt.Logger.Err(err).Msg("error getting user from access token") - w.WriteHeader(http.StatusInternalServerError) - return - } - _, err = accounts.GetAccount(r.Context(), &proto.GetAccountRequest{ - Id: user.Id.OpaqueId, - }) - if err != nil { - e := errors.Parse(err.Error()) - if e.Code == http.StatusNotFound { - opt.Logger.Debug().Msgf("account with id %s not found", user.Id.OpaqueId) - next.ServeHTTP(w, r) - return - } - opt.Logger.Err(err).Msgf("error getting user with id %s from accounts service", user.Id.OpaqueId) - w.WriteHeader(http.StatusInternalServerError) - return - } - - // we need to pass the token to authenticate the CreateHome request. - //ctx := tokenpkg.ContextSetToken(r.Context(), token) - ctx := metadata.AppendToOutgoingContext(r.Context(), tokenpkg.TokenHeader, token) - - createHomeReq := &provider.CreateHomeRequest{} - createHomeRes, err := opt.RevaGatewayClient.CreateHome(ctx, createHomeReq) - - if err != nil { - opt.Logger.Err(err).Msg("error calling CreateHome") - } else if createHomeRes.Status.Code != rpc.Code_CODE_OK { - err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway") - opt.Logger.Err(err).Msg("error when calling Createhome") - } - - next.ServeHTTP(w, r) + tokenManager, err := jwt.New(map[string]interface{}{ + "secret": options.TokenManagerConfig.JWTSecret, }) + if err != nil { + logger.Fatal().Err(err).Msgf("Could not initialize token-manager") + } + + return &createHome{ + next: next, + logger: logger, + accountsClient: options.AccountsClient, + tokenManager: tokenManager, + revaGatewayClient: options.RevaGatewayClient, + } } } + +type createHome struct { + next http.Handler + logger log.Logger + accountsClient accounts.AccountsService + tokenManager tokenPkg.Manager + revaGatewayClient gateway.GatewayAPIClient +} + +func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if !m.shouldServe(req) { + m.next.ServeHTTP(w, req) + return + } + + token := req.Header.Get("x-access-token") + + user, err := m.tokenManager.DismantleToken(req.Context(), token) + if err != nil { + m.logger.Logger.Err(err).Msg("error getting user from access token") + w.WriteHeader(http.StatusInternalServerError) + return + } + + _, err = m.accountsClient.GetAccount(req.Context(), &accounts.GetAccountRequest{ + Id: user.Id.OpaqueId, + }) + + if err != nil { + e := microErrors.Parse(err.Error()) + + if e.Code == http.StatusNotFound { + m.logger.Debug().Msgf("account with id %s not found", user.Id.OpaqueId) + m.next.ServeHTTP(w, req) + return + } + + m.logger.Err(err).Msgf("error getting user with id %s from accounts service", user.Id.OpaqueId) + w.WriteHeader(http.StatusInternalServerError) + return + } + + // we need to pass the token to authenticate the CreateHome request. + //ctx := tokenpkg.ContextSetToken(r.Context(), token) + ctx := metadata.AppendToOutgoingContext(req.Context(), tokenPkg.TokenHeader, token) + + createHomeReq := &provider.CreateHomeRequest{} + createHomeRes, err := m.revaGatewayClient.CreateHome(ctx, createHomeReq) + + if err != nil { + m.logger.Err(err).Msg("error calling CreateHome") + } else if createHomeRes.Status.Code != rpc.Code_CODE_OK { + err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway") + m.logger.Err(err).Msg("error when calling Createhome") + } + + m.next.ServeHTTP(w, req) +} + +func (m createHome) shouldServe(req *http.Request) bool { + return req.Header.Get("x-access-token") != "" +} diff --git a/proxy/pkg/middleware/https_redirect.go b/proxy/pkg/middleware/https_redirect.go index 60b468446..f5d94144c 100644 --- a/proxy/pkg/middleware/https_redirect.go +++ b/proxy/pkg/middleware/https_redirect.go @@ -5,8 +5,8 @@ import ( "net/http" ) -// RedirectToHTTPS redirects insecure requests to https -func RedirectToHTTPS(next http.Handler) http.Handler { +// HTTPSRedirect redirects insecure requests to https +func HTTPSRedirect(next http.Handler) http.Handler { return http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { proto := req.Header.Get("x-forwarded-proto") if proto == "http" || proto == "HTTP" { diff --git a/proxy/pkg/middleware/middleware.go b/proxy/pkg/middleware/middleware.go index a8c591783..c97da6e2b 100644 --- a/proxy/pkg/middleware/middleware.go +++ b/proxy/pkg/middleware/middleware.go @@ -1,6 +1,16 @@ package middleware -import "net/http" +import ( + "errors" +) -// M undocummented -type M func(next http.Handler) http.Handler +var ( + // ErrInvalidToken is returned when the request token is invalid. + ErrInvalidToken = errors.New("invalid or missing token") + + // ErrUnauthorized is returned if the request is not authorized + ErrUnauthorized = errors.New("unauthorized") + + // ErrInternal is returned if something went wrong + ErrInternal = errors.New("internal error") +) diff --git a/proxy/pkg/middleware/middleware_test.go b/proxy/pkg/middleware/middleware_test.go new file mode 100644 index 000000000..6bb707d17 --- /dev/null +++ b/proxy/pkg/middleware/middleware_test.go @@ -0,0 +1,17 @@ +package middleware + +import ( + "github.com/owncloud/ocis/proxy/pkg/cache" +) + +const ( + // AccountsKey declares the svcKey for the Accounts service. + AccountsKey = "accounts" +) + +var ( + // svcCache caches requests for given services to prevent round trips to the service + svcCache = cache.NewCache( + cache.Size(256), + ) +) diff --git a/proxy/pkg/middleware/oidc_auth.go b/proxy/pkg/middleware/oidc_auth.go new file mode 100644 index 000000000..7c805aab2 --- /dev/null +++ b/proxy/pkg/middleware/oidc_auth.go @@ -0,0 +1,117 @@ +package middleware + +import ( + "context" + gOidc "github.com/coreos/go-oidc" + "github.com/owncloud/ocis/ocis-pkg/log" + "github.com/owncloud/ocis/ocis-pkg/oidc" + "golang.org/x/oauth2" + "net/http" + "strings" +) + +// OIDCProvider used to mock the oidc provider during tests +type OIDCProvider interface { + UserInfo(ctx context.Context, ts oauth2.TokenSource) (*gOidc.UserInfo, error) +} + +// OIDCAuth provides a middleware to check access secured by a static token. +func OIDCAuth(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + + return func(next http.Handler) http.Handler { + return &oidcAuth{ + next: next, + logger: options.Logger, + providerFunc: options.OIDCProviderFunc, + httpClient: options.HTTPClient, + oidcIss: options.OIDCIss, + } + } +} + +type oidcAuth struct { + next http.Handler + logger log.Logger + provider OIDCProvider + providerFunc func() (OIDCProvider, error) + httpClient *http.Client + oidcIss string +} + +func (m oidcAuth) ServeHTTP(w http.ResponseWriter, req *http.Request) { + + if !m.shouldServe(req) { + m.next.ServeHTTP(w, req) + return + } + + if m.provider == nil { + // Lazily initialize a provider + + // provider needs to be cached as when it is created + // it will fetch the keys from the issuer using the .well-known + // endpoint + provider, err := m.providerFunc() + if err != nil { + m.logger.Error().Err(err).Msg("could not initialize oidcAuth provider") + w.WriteHeader(http.StatusInternalServerError) + return + } + + m.provider = provider + } + + token := strings.TrimPrefix(req.Header.Get("Authorization"), "Bearer ") + + // TODO cache userinfo for access token if we can determine the expiry (which works in case it is a jwt based access token) + oauth2Token := &oauth2.Token{ + AccessToken: token, + } + + userInfo, err := m.provider.UserInfo( + context.WithValue(req.Context(), oauth2.HTTPClient, m.httpClient), + oauth2.StaticTokenSource(oauth2Token), + ) + if err != nil { + m.logger.Error().Err(err).Str("token", token).Msg("Failed to get userinfo") + http.Error(w, ErrInvalidToken.Error(), http.StatusUnauthorized) + return + } + + var claims oidc.StandardClaims + if err := userInfo.Claims(&claims); err != nil { + m.logger.Error().Err(err).Interface("userinfo", userInfo).Msg("failed to unmarshal userinfo claims") + w.WriteHeader(http.StatusInternalServerError) + return + } + + //TODO: This should be read from the token instead of config + claims.Iss = m.oidcIss + + // inject claims to the request context for the account_uuid middleware. + req = req.WithContext(oidc.NewContext(req.Context(), &claims)) + + m.logger.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Msg("unmarshalled userinfo") + + // store claims in context + // uses the original context, not the one with probably reduced security + m.next.ServeHTTP(w, req.WithContext(oidc.NewContext(req.Context(), &claims))) +} + +func (m oidcAuth) shouldServe(req *http.Request) bool { + header := req.Header.Get("Authorization") + + if m.oidcIss == "" { + return false + } + + // todo: looks dirty, check later + for _, ignoringPath := range []string{"/konnect/v1/userinfo"} { + if req.URL.Path == ignoringPath { + return false + } + } + + return strings.HasPrefix(header, "Bearer ") +} diff --git a/proxy/pkg/middleware/openidconnect_test.go b/proxy/pkg/middleware/oidc_auth_test.go similarity index 94% rename from proxy/pkg/middleware/openidconnect_test.go rename to proxy/pkg/middleware/oidc_auth_test.go index 3a02b9ac9..b4b899c1c 100644 --- a/proxy/pkg/middleware/openidconnect_test.go +++ b/proxy/pkg/middleware/oidc_auth_test.go @@ -3,24 +3,25 @@ package middleware import ( "context" "fmt" - "net/http" - "net/http/httptest" - "testing" - "github.com/coreos/go-oidc" "github.com/owncloud/ocis/ocis-pkg/log" "golang.org/x/oauth2" + "net/http" + "net/http/httptest" + "testing" ) -func TestOpenIDConnectMiddleware(t *testing.T) { +func TestOIDCAuthMiddleware(t *testing.T) { svcCache.Invalidate(AccountsKey, "success") + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) - m := OpenIDConnect( + m := OIDCAuth( Logger(log.NewLogger()), OIDCProviderFunc(func() (OIDCProvider, error) { return mockOP(false), nil }), + OIDCIss("https://localhost:9200"), )(next) r := httptest.NewRequest(http.MethodGet, "https://idp.example.com", nil) diff --git a/proxy/pkg/middleware/openidconnect.go b/proxy/pkg/middleware/openidconnect.go deleted file mode 100644 index 651026cb0..000000000 --- a/proxy/pkg/middleware/openidconnect.go +++ /dev/null @@ -1,119 +0,0 @@ -package middleware - -import ( - "context" - "errors" - "net/http" - "strings" - - "github.com/coreos/go-oidc" - ocisoidc "github.com/owncloud/ocis/ocis-pkg/oidc" - "github.com/owncloud/ocis/proxy/pkg/cache" - "golang.org/x/oauth2" -) - -var ( - // ErrInvalidToken is returned when the request token is invalid. - ErrInvalidToken = errors.New("invalid or missing token") - - // svcCache caches requests for given services to prevent round trips to the service - svcCache = cache.NewCache( - cache.Size(256), - ) -) - -// OIDCProvider used to mock the oidc provider during tests -type OIDCProvider interface { - UserInfo(ctx context.Context, ts oauth2.TokenSource) (*oidc.UserInfo, error) -} - -// OpenIDConnect provides a middleware to check access secured by a static token. -func OpenIDConnect(opts ...Option) func(next http.Handler) http.Handler { - opt := newOptions(opts...) - - return func(next http.Handler) http.Handler { - - var oidcProvider OIDCProvider - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - header := r.Header.Get("Authorization") - path := r.URL.Path - - // Ignore request to "/konnect/v1/userinfo" as this will cause endless loop when getting userinfo - // needs a better idea on how to not hardcode this - if header == "" || !strings.HasPrefix(header, "Bearer ") || path == "/konnect/v1/userinfo" { - next.ServeHTTP(w, r) - return - } - - customCtx := context.WithValue(r.Context(), oauth2.HTTPClient, opt.HTTPClient) - - // check if oidc provider is initialized - if oidcProvider == nil { - // Lazily initialize a provider - - // provider needs to be cached as when it is created - // it will fetch the keys from the issuer using the .well-known - // endpoint - var err error - oidcProvider, err = opt.OIDCProviderFunc() - if err != nil { - opt.Logger.Error().Err(err).Msg("could not initialize oidc provider") - w.WriteHeader(http.StatusInternalServerError) - return - } - } - - token := strings.TrimPrefix(header, "Bearer ") - - // TODO cache userinfo for access token if we can determine the expiry (which works in case it is a jwt based access token) - oauth2Token := &oauth2.Token{ - AccessToken: token, - } - - // The claims we want to have - var claims ocisoidc.StandardClaims - userInfo, err := oidcProvider.UserInfo(customCtx, oauth2.StaticTokenSource(oauth2Token)) - if err != nil { - opt.Logger.Error().Err(err).Str("token", token).Msg("Failed to get userinfo") - http.Error(w, ErrInvalidToken.Error(), http.StatusUnauthorized) - return - } - - if err := userInfo.Claims(&claims); err != nil { - opt.Logger.Error().Err(err).Interface("userinfo", userInfo).Msg("failed to unmarshal userinfo claims") - w.WriteHeader(http.StatusInternalServerError) - return - } - - //TODO: This should be read from the token instead of config - claims.Iss = opt.OIDCIss - - // inject claims to the request context for the account_uuid middleware. - ctxWithClaims := ocisoidc.NewContext(r.Context(), &claims) - r = r.WithContext(ctxWithClaims) - - opt.Logger.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Msg("unmarshalled userinfo") - // store claims in context - // uses the original context, not the one with probably reduced security - nr := r.WithContext(ocisoidc.NewContext(r.Context(), &claims)) - - next.ServeHTTP(w, nr) - }) - } -} - -// AccountsCacheEntry stores a request to the accounts service on the cache. -// this type declaration should be on each respective service. -type AccountsCacheEntry struct { - Email string - UUID string -} - -const ( - // AccountsKey declares the svcKey for the Accounts service. - AccountsKey = "accounts" - - // NodeKey declares the key that will be used to store the node address. - // It is shared between services. - NodeKey = "node" -) diff --git a/proxy/pkg/middleware/options.go b/proxy/pkg/middleware/options.go index b012c8cf5..32ecaa622 100644 --- a/proxy/pkg/middleware/options.go +++ b/proxy/pkg/middleware/options.go @@ -21,7 +21,7 @@ type Options struct { Logger log.Logger // TokenManagerConfig for communicating with the reva token manager TokenManagerConfig config.TokenManager - // HTTPClient to use for communication with the oidc provider + // HTTPClient to use for communication with the oidcAuth provider HTTPClient *http.Client // AccountsClient for resolving accounts AccountsClient acc.AccountsService @@ -29,7 +29,7 @@ type Options struct { SettingsRoleService settings.RoleService // OIDCProviderFunc to lazily initialize a provider, must be set for the oidcProvider middleware OIDCProviderFunc func() (OIDCProvider, error) - // OIDCIss is the oidc-issuer + // OIDCIss is the oidcAuth-issuer OIDCIss string // RevaGatewayClient to send requests to the reva gateway RevaGatewayClient gateway.GatewayAPIClient @@ -37,7 +37,7 @@ type Options struct { Store storepb.StoreService // PreSignedURLConfig to configure the middleware PreSignedURLConfig config.PreSignedURL - // AutoprovisionAccounts when an account does not exist. + // AutoprovisionAccounts when an accountResolver does not exist. AutoprovisionAccounts bool // EnableBasicAuth to allow basic auth EnableBasicAuth bool @@ -89,14 +89,14 @@ func SettingsRoleService(rc settings.RoleService) Option { } } -// OIDCProviderFunc provides a function to set the the oidc provider function option. +// OIDCProviderFunc provides a function to set the the oidcAuth provider function option. func OIDCProviderFunc(f func() (OIDCProvider, error)) Option { return func(o *Options) { o.OIDCProviderFunc = f } } -// OIDCIss sets the oidc issuer url +// OIDCIss sets the oidcAuth issuer url func OIDCIss(iss string) Option { return func(o *Options) { o.OIDCIss = iss diff --git a/proxy/pkg/middleware/presigned_url.go b/proxy/pkg/middleware/presigned_url.go deleted file mode 100644 index 40d9f5d50..000000000 --- a/proxy/pkg/middleware/presigned_url.go +++ /dev/null @@ -1,151 +0,0 @@ -package middleware - -import ( - "context" - "crypto/sha512" - "encoding/hex" - "net/http" - "strings" - "time" - - "github.com/owncloud/ocis/ocis-pkg/log" - ocisoidc "github.com/owncloud/ocis/ocis-pkg/oidc" - "github.com/owncloud/ocis/proxy/pkg/config" - storepb "github.com/owncloud/ocis/store/pkg/proto/v0" - "golang.org/x/crypto/pbkdf2" -) - -const ( - iterations = 10000 - keyLen = 32 -) - -// PresignedURL provides a middleware to check access secured by a presigned URL. -func PresignedURL(opts ...Option) func(next http.Handler) http.Handler { - opt := newOptions(opts...) - l := opt.Logger - cfg := opt.PreSignedURLConfig - - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if isSignedRequest(r) { - if signedRequestIsValid(l, r, opt.Store, cfg) { - // use openid claims to let the account_uuid middleware do a lookup by username - claims := ocisoidc.StandardClaims{ - OcisID: r.URL.Query().Get("OC-Credential"), - } - - // inject claims to the request context for the account_uuid middleware - ctxWithClaims := ocisoidc.NewContext(r.Context(), &claims) - r = r.WithContext(ctxWithClaims) - - next.ServeHTTP(w, r) - } else { - http.Error(w, "Invalid url signature", http.StatusUnauthorized) - return - } - } - next.ServeHTTP(w, r) - }) - } -} - -func isSignedRequest(r *http.Request) bool { - return r.URL.Query().Get("OC-Signature") != "" -} - -func signedRequestIsValid(l log.Logger, r *http.Request, s storepb.StoreService, cfg config.PreSignedURL) bool { - // TODO OC-Algorithm - defined the used algo (e.g. sha256 or sha512 - we should agree on one default algo and make this parameter optional) - // TODO OC-Verb - defines for which http verb the request is valid - defaults to GET OPTIONAL - - return allRequiredParametersArePresent(r) && - requestMethodMatches(r) && - requestMethodIsAllowed(r.Method, cfg.AllowedHTTPMethods) && - !urlIsExpired(r, time.Now) && - signatureIsValid(l, r, s) -} - -func allRequiredParametersArePresent(r *http.Request) bool { - // OC-Credential - defines the user scope (shall we use the owncloud user id here - this might leak internal data ....) REQUIRED - // OC-Date - defined the date the url was signed (ISO 8601 UTC) REQUIRED - // OC-Expires - defines the expiry interval in seconds (between 1 and 604800 = 7 days) REQUIRED - // OC-Signature - the computed signature - server will verify the request upon this REQUIRED - return r.URL.Query().Get("OC-Signature") != "" && - r.URL.Query().Get("OC-Credential") != "" && - r.URL.Query().Get("OC-Date") != "" && - r.URL.Query().Get("OC-Expires") != "" && - r.URL.Query().Get("OC-Verb") != "" -} - -func requestMethodMatches(r *http.Request) bool { - return strings.EqualFold(r.Method, r.URL.Query().Get("OC-Verb")) -} - -func requestMethodIsAllowed(m string, allowedMethods []string) bool { - for _, allowed := range allowedMethods { - if strings.EqualFold(m, allowed) { - return true - } - } - return false -} - -func urlIsExpired(r *http.Request, now func() time.Time) bool { - t, err := time.Parse(time.RFC3339, r.URL.Query().Get("OC-Date")) - if err != nil { - return true - } - expires, err := time.ParseDuration(r.URL.Query().Get("OC-Expires") + "s") - if err != nil { - return true - } - t.Add(expires) - return t.After(now()) -} - -func signatureIsValid(l log.Logger, r *http.Request, s storepb.StoreService) bool { - signingKey, err := getSigningKey(r.Context(), s, r.URL.Query().Get("OC-Credential")) - if err != nil { - l.Error().Err(err).Msg("could not retrieve signing key") - return false - } - if len(signingKey) == 0 { - l.Error().Err(err).Msg("signing key empty") - return false - } - - q := r.URL.Query() - signature := q.Get("OC-Signature") - q.Del("OC-Signature") - r.URL.RawQuery = q.Encode() - url := r.URL.String() - if !r.URL.IsAbs() { - url = "https://" + r.Host + url // TODO where do we get the scheme from - } - return createSignature(url, signingKey) == signature -} - -func createSignature(url string, signingKey []byte) string { - // the oc10 signature check: $hash = \hash_pbkdf2("sha512", $url, $signingKey, 10000, 64, false); - // - sets the length of the output string to 64 - // - sets raw output to false -> if raw_output is FALSE length corresponds to twice the byte-length of the derived key (as every byte of the key is returned as two hexits). - // TODO change to length 128 in oc10? - // fo golangs pbkdf2.Key we need to use 32 because it will be encoded into 64 hexits later - hash := pbkdf2.Key([]byte(url), signingKey, iterations, keyLen, sha512.New) - return hex.EncodeToString(hash) -} - -func getSigningKey(ctx context.Context, s storepb.StoreService, credential string) ([]byte, error) { - res, err := s.Read(ctx, &storepb.ReadRequest{ - Options: &storepb.ReadOptions{ - Database: "proxy", - Table: "signing-keys", - }, - Key: credential, - }) - if err != nil || len(res.Records) < 1 { - return []byte{}, err - } - - return res.Records[0].Value, nil -} diff --git a/proxy/pkg/middleware/signed_url_auth.go b/proxy/pkg/middleware/signed_url_auth.go new file mode 100644 index 000000000..247e8da3e --- /dev/null +++ b/proxy/pkg/middleware/signed_url_auth.go @@ -0,0 +1,242 @@ +package middleware + +import ( + "context" + "crypto/sha512" + "encoding/hex" + "errors" + "fmt" + "github.com/google/uuid" + accounts "github.com/owncloud/ocis/accounts/pkg/proto/v0" + "github.com/owncloud/ocis/ocis-pkg/log" + ocisoidc "github.com/owncloud/ocis/ocis-pkg/oidc" + "github.com/owncloud/ocis/proxy/pkg/config" + store "github.com/owncloud/ocis/store/pkg/proto/v0" + "golang.org/x/crypto/pbkdf2" + "net/http" + "net/url" + "strings" + "time" +) + +// SignedURLAuth provides a middleware to check access secured by a signed URL. +func SignedURLAuth(optionSetters ...Option) func(next http.Handler) http.Handler { + options := newOptions(optionSetters...) + + return func(next http.Handler) http.Handler { + return &signedURLAuth{ + next: next, + logger: options.Logger, + preSignedURLConfig: options.PreSignedURLConfig, + accountsClient: options.AccountsClient, + store: options.Store, + } + } +} + +type signedURLAuth struct { + next http.Handler + logger log.Logger + preSignedURLConfig config.PreSignedURL + accountsClient accounts.AccountsService + store store.StoreService +} + +func (m signedURLAuth) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if !m.shouldServe(req) { + m.next.ServeHTTP(w, req) + return + } + + if err := m.validate(req); err != nil { + http.Error(w, "Invalid url signature", http.StatusUnauthorized) + return + } + + claims, err := m.claims(req.URL.Query().Get("OC-Credential")) + if err != nil { + http.Error(w, "Invalid url signature", http.StatusUnauthorized) + return + } + + m.next.ServeHTTP(w, req.WithContext(ocisoidc.NewContext(req.Context(), claims))) +} + +func (m signedURLAuth) claims(credential string) (*ocisoidc.StandardClaims, error) { + // use openid claims to let the account_uuid middleware do a lookup by username + claims := ocisoidc.StandardClaims{ + OcisID: credential, + } + + // OC10 username is handled as id, if we get a credantial that is not of type uuid we expect + // that it is a PreferredUsername und we need to get the corresponding uuid + if _, err := uuid.Parse(claims.OcisID); err != nil { + // todo caching + account, status := getAccount( + m.logger, + m.accountsClient, + fmt.Sprintf( + "preferred_name eq '%s'", + strings.ReplaceAll( + claims.OcisID, + "'", + "''", + ), + ), + ) + + if status != 0 || account == nil { + return nil, fmt.Errorf("no oc-credential found for %v", claims.OcisID) + } + + claims.OcisID = account.Id + } + + return &claims, nil +} + +func (m signedURLAuth) shouldServe(req *http.Request) bool { + return req.URL.Query().Get("OC-Signature") != "" +} + +func (m signedURLAuth) validate(req *http.Request) (err error) { + query := req.URL.Query() + + if ok, err := m.allRequiredParametersArePresent(query); !ok { + return err + } + + if ok, err := m.requestMethodMatches(req.Method, query); !ok { + return err + } + + if ok, err := m.requestMethodIsAllowed(req.Method); !ok { + return err + } + + if expired, err := m.urlIsExpired(query, time.Now); expired { + return err + } + + if ok, err := m.signatureIsValid(req); !ok { + return err + } + + return nil +} + +func (m signedURLAuth) allRequiredParametersArePresent(query url.Values) (ok bool, err error) { + // check if required query parameters exist in given request query parameters + // OC-Signature - the computed signature - server will verify the request upon this REQUIRED + // OC-Credential - defines the user scope (shall we use the owncloud user id here - this might leak internal data ....) REQUIRED + // OC-Date - defined the date the url was signed (ISO 8601 UTC) REQUIRED + // OC-Expires - defines the expiry interval in seconds (between 1 and 604800 = 7 days) REQUIRED + // TODO OC-Verb - defines for which http verb the request is valid - defaults to GET OPTIONAL + for _, p := range []string{ + "OC-Signature", + "OC-Credential", + "OC-Date", + "OC-Expires", + "OC-Verb", + } { + if query.Get(p) == "" { + return false, fmt.Errorf("required %s parameter not found", p) + } + } + + return true, nil +} + +func (m signedURLAuth) requestMethodMatches(meth string, query url.Values) (ok bool, err error) { + // check if given url query parameter OC-Verb matches given request method + if !strings.EqualFold(meth, query.Get("OC-Verb")) { + return false, errors.New("required OC-Verb parameter did not match request method") + } + + return true, nil +} + +func (m signedURLAuth) requestMethodIsAllowed(meth string) (ok bool, err error) { + // check if given request method is allowed + methodIsAllowed := false + for _, am := range m.preSignedURLConfig.AllowedHTTPMethods { + if strings.EqualFold(meth, am) { + methodIsAllowed = true + break + } + } + + if !methodIsAllowed { + return false, errors.New("request method is not listed in PreSignedURLConfig AllowedHTTPMethods") + } + + return true, nil +} +func (m signedURLAuth) urlIsExpired(query url.Values, now func() time.Time) (expired bool, err error) { + // check if url is expired by checking if given date (OC-Date) + expires in seconds (OC-Expires) is after now + validFrom, err := time.Parse(time.RFC3339, query.Get("OC-Date")) + if err != nil { + return true, err + } + + requestExpiry, err := time.ParseDuration(query.Get("OC-Expires") + "s") + if err != nil { + return true, err + } + + validTo := validFrom.Add(requestExpiry) + + return !(now().After(validFrom) && now().Before(validTo)), nil +} + +func (m signedURLAuth) signatureIsValid(req *http.Request) (ok bool, err error) { + signingKey, err := m.getSigningKey(req.Context(), req.URL.Query().Get("OC-Credential")) + if err != nil { + m.logger.Error().Err(err).Msg("could not retrieve signing key") + return false, err + } + if len(signingKey) == 0 { + m.logger.Error().Err(err).Msg("signing key empty") + return false, err + } + q := req.URL.Query() + signature := q.Get("OC-Signature") + q.Del("OC-Signature") + req.URL.RawQuery = q.Encode() + url := req.URL.String() + if !req.URL.IsAbs() { + url = "https://" + req.Host + url // TODO where do we get the scheme from + } + + return m.createSignature(url, signingKey) == signature, nil +} + +func (m signedURLAuth) createSignature(url string, signingKey []byte) string { + // the oc10 signature check: $hash = \hash_pbkdf2("sha512", $url, $signingKey, 10000, 64, false); + // - sets the length of the output string to 64 + // - sets raw output to false -> if raw_output is FALSE length corresponds to twice the byte-length of the derived key (as every byte of the key is returned as two hexits). + // TODO change to length 128 in oc10? + // fo golangs pbkdf2.Key we need to use 32 because it will be encoded into 64 hexits later + hash := pbkdf2.Key([]byte(url), signingKey, 10000, 32, sha512.New) + return hex.EncodeToString(hash) +} + +func (m signedURLAuth) getSigningKey(ctx context.Context, credential string) ([]byte, error) { + claims, err := m.claims(credential) + if err != nil { + return []byte{}, err + } + + res, err := m.store.Read(ctx, &store.ReadRequest{ + Options: &store.ReadOptions{ + Database: "proxy", + Table: "signing-keys", + }, + Key: claims.OcisID, + }) + if err != nil || len(res.Records) < 1 { + return []byte{}, err + } + + return res.Records[0].Value, nil +} diff --git a/proxy/pkg/middleware/presigned_url_test.go b/proxy/pkg/middleware/signed_url_auth_test.go similarity index 58% rename from proxy/pkg/middleware/presigned_url_test.go rename to proxy/pkg/middleware/signed_url_auth_test.go index 0e41d78dd..e421ee0a1 100644 --- a/proxy/pkg/middleware/presigned_url_test.go +++ b/proxy/pkg/middleware/signed_url_auth_test.go @@ -6,7 +6,8 @@ import ( "time" ) -func TestIsSignedRequest(t *testing.T) { +func TestSignedURLAuth_shouldServe(t *testing.T) { + pua := signedURLAuth{} tests := []struct { url string expected bool @@ -17,14 +18,16 @@ func TestIsSignedRequest(t *testing.T) { for _, tt := range tests { r := httptest.NewRequest("", tt.url, nil) - result := isSignedRequest(r) + result := pua.shouldServe(r) + if result != tt.expected { t.Errorf("with %s expected %t got %t", tt.url, tt.expected, result) } } } -func TestAllRequiredParametersPresent(t *testing.T) { +func TestSignedURLAuth_allRequiredParametersPresent(t *testing.T) { + pua := signedURLAuth{} baseURL := "https://example.com/example.jpg?" tests := []struct { params string @@ -39,14 +42,15 @@ func TestAllRequiredParametersPresent(t *testing.T) { } for _, tt := range tests { r := httptest.NewRequest("", baseURL+tt.params, nil) - result := allRequiredParametersArePresent(r) - if result != tt.expected { - t.Errorf("with %s expected %t got %t", tt.params, tt.expected, result) + ok, _ := pua.allRequiredParametersArePresent(r.URL.Query()) + if ok != tt.expected { + t.Errorf("with %s expected %t got %t", tt.params, tt.expected, ok) } } } -func TestRequestMethodMatches(t *testing.T) { +func TestSignedURLAuth_requestMethodMatches(t *testing.T) { + pua := signedURLAuth{} tests := []struct { method string url string @@ -59,14 +63,15 @@ func TestRequestMethodMatches(t *testing.T) { for _, tt := range tests { r := httptest.NewRequest(tt.method, tt.url, nil) - result := requestMethodMatches(r) - if result != tt.expected { - t.Errorf("with method %s and url %s expected %t got %t", tt.method, tt.url, tt.expected, result) + ok, _ := pua.requestMethodMatches(r.Method, r.URL.Query()) + if ok != tt.expected { + t.Errorf("with method %s and url %s expected %t got %t", tt.method, tt.url, tt.expected, ok) } } } -func TestRequestMethodIsAllowed(t *testing.T) { +func TestSignedURLAuth_requestMethodIsAllowed(t *testing.T) { + pua := signedURLAuth{} tests := []struct { method string allowed []string @@ -80,40 +85,46 @@ func TestRequestMethodIsAllowed(t *testing.T) { } for _, tt := range tests { - result := requestMethodIsAllowed(tt.method, tt.allowed) - if result != tt.expected { - t.Errorf("with method %s and allowed methods %s expected %t got %t", tt.method, tt.allowed, tt.expected, result) + pua.preSignedURLConfig.AllowedHTTPMethods = tt.allowed + ok, _ := pua.requestMethodIsAllowed(tt.method) + + if ok != tt.expected { + t.Errorf("with method %s and allowed methods %s expected %t got %t", tt.method, tt.allowed, tt.expected, ok) } } } -func TestUrlIsExpired(t *testing.T) { +func TestSignedURLAuth_urlIsExpired(t *testing.T) { + pua := signedURLAuth{} nowFunc := func() time.Time { - t, _ := time.Parse(time.RFC3339, "2020-08-19T15:12:43.478Z") + t, _ := time.Parse(time.RFC3339, "2020-02-02T12:30:00.000Z") return t } tests := []struct { url string - expected bool + isExpired bool }{ - {"http://example.com/example.jpg?OC-Date=2020-08-19T15:02:43.478Z&OC-Expires=1200", false}, - {"http://example.com/example.jpg?OC-Date=invalid&OC-Expires=1200", true}, - {"http://example.com/example.jpg?OC-Date=2020-08-19T15:02:43.478Z&OC-Expires=invalid", true}, + {"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=61", false}, + {"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", true}, + {"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=59", true}, + {"http://example.com/example.jpg?OC-Date=2020-02-03T12:29:00.000Z&OC-Expires=59", true}, + {"http://example.com/example.jpg?OC-Date=2020-02-01T12:29:00.000Z&OC-Expires=59", true}, } for _, tt := range tests { r := httptest.NewRequest("", tt.url, nil) - result := urlIsExpired(r, nowFunc) - if result != tt.expected { - t.Errorf("with %s expected %t got %t", tt.url, tt.expected, result) + expired, _ := pua.urlIsExpired(r.URL.Query(), nowFunc) + if expired != tt.isExpired { + t.Errorf("with %s expected %t got %t", tt.url, tt.isExpired, expired) } } } -func TestCreateSignature(t *testing.T) { +func TestSignedURLAuth_createSignature(t *testing.T) { + pua := signedURLAuth{} expected := "27d2ebea381384af3179235114801dcd00f91e46f99fca72575301cf3948101d" - s := createSignature("something", []byte("somerandomkey")) + s := pua.createSignature("something", []byte("somerandomkey")) if s != expected { t.Fail()