Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use reva's Authenticate method instead of spawning token managers #2528

Merged
merged 4 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelog/unreleased/reva-auth-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Enhancement: Use reva's Authenticate method instead of spawning token managers

When using the CS3 proxy backend, we previously obtained the user from reva's
userprovider service and minted the token ourselves. This required maintaining
a shared JWT secret between ocis and reva, as well duplication of logic. This
PR delegates this logic by using the `Authenticate` method provided by the reva
gateway service to obtain this token, making it an arbitrary, indestructible
entry. Currently, the changes have been made to the proxy service but will be
extended to others as well.

https://github.com/owncloud/ocis/pull/2528
1 change: 1 addition & 0 deletions ocs/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type Config struct {
AccountBackend string
RevaAddress string
StorageUsersDriver string
MachineAuthAPIKey string
IdentityManagement IdentityManagement

Context context.Context
Expand Down
7 changes: 7 additions & 0 deletions ocs/pkg/flagset/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
EnvVars: []string{"OCS_REVA_GATEWAY_ADDR"},
Destination: &cfg.RevaAddress,
},
&cli.StringFlag{
Name: "machine-auth-api-key",
Value: flags.OverrideDefaultString(cfg.MachineAuthAPIKey, "change-me-please"),
Usage: "the API key to be used for the machine auth driver in reva",
EnvVars: []string{"OCS_MACHINE_AUTH_API_KEY", "OCIS_MACHINE_AUTH_API_KEY"},
Destination: &cfg.MachineAuthAPIKey,
},
&cli.StringFlag{
Name: "idm-address",
Value: flags.OverrideDefaultString(cfg.IdentityManagement.Address, "https://localhost:9200"),
Expand Down
2 changes: 1 addition & 1 deletion ocs/pkg/service/v0/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (o Ocs) getCS3Backend() backend.UserBackend {
if err != nil {
o.logger.Fatal().Msgf("could not get reva client at address %s", o.config.RevaAddress)
}
return backend.NewCS3UserBackend(revaClient, nil, revaClient, o.logger)
return backend.NewCS3UserBackend(nil, revaClient, o.config.MachineAuthAPIKey, o.logger)
}

func (o Ocs) getGroupsService() accounts.GroupsService {
Expand Down
2 changes: 1 addition & 1 deletion ocs/pkg/service/v0/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ func (o Ocs) fetchAccountByUsername(ctx context.Context, name string) (*accounts

func (o Ocs) fetchAccountFromCS3Backend(ctx context.Context, name string) (*accounts.Account, error) {
backend := o.getCS3Backend()
u, err := backend.GetUserByClaims(ctx, "username", name, false)
u, _, err := backend.GetUserByClaims(ctx, "username", name, false)
if err != nil {
return nil, err
}
Expand Down
12 changes: 11 additions & 1 deletion proxy/pkg/command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/cs3org/reva/pkg/token/manager/jwt"
chimiddleware "github.com/go-chi/chi/v5/middleware"
"github.com/justinas/alice"
"github.com/micro/cli/v2"
Expand Down Expand Up @@ -148,14 +149,23 @@ func loadMiddlewares(ctx context.Context, l log.Logger, cfg *config.Config) alic
var userProvider backend.UserBackend
switch cfg.AccountBackend {
case "accounts":
tokenManager, err := jwt.New(map[string]interface{}{
"secret": cfg.TokenManager.JWTSecret,
"expires": int64(24 * 60 * 60),
})
if err != nil {
l.Error().Err(err).
Msg("Failed to create token manager")
}
userProvider = backend.NewAccountsServiceUserBackend(
acc.NewAccountsService("com.owncloud.api.accounts", grpc.DefaultClient),
rolesClient,
cfg.OIDC.Issuer,
tokenManager,
l,
)
case "cs3":
userProvider = backend.NewCS3UserBackend(revaClient, rolesClient, revaClient, l)
userProvider = backend.NewCS3UserBackend(rolesClient, revaClient, cfg.MachineAuthAPIKey, l)
default:
l.Fatal().Msgf("Invalid accounts backend type '%s'", cfg.AccountBackend)
}
Expand Down
5 changes: 3 additions & 2 deletions proxy/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ type Config struct {
Reva Reva
PreSignedURL PreSignedURL
AccountBackend string
UserOIDCClaim string
UserCS3Claim string
UserOIDCClaim string
UserCS3Claim string
MachineAuthAPIKey string
AutoprovisionAccounts bool
EnableBasicAuth bool
InsecureBackends bool
Expand Down
8 changes: 8 additions & 0 deletions proxy/pkg/flagset/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,14 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
Destination: &cfg.AccountBackend,
},

&cli.StringFlag{
Name: "machine-auth-api-key",
Value: flags.OverrideDefaultString(cfg.MachineAuthAPIKey, "change-me-please"),
Usage: "the API key to be used for the machine auth driver in reva",
EnvVars: []string{"PROXY_MACHINE_AUTH_API_KEY", "OCIS_MACHINE_AUTH_API_KEY"},
Destination: &cfg.MachineAuthAPIKey,
},

// Reva Middlewares Config
&cli.StringSliceFlag{
Name: "proxy-user-agent-lock-in",
Expand Down
28 changes: 2 additions & 26 deletions proxy/pkg/middleware/account_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ import (
"errors"
"net/http"

"github.com/cs3org/reva/pkg/auth/scope"
"github.com/owncloud/ocis/proxy/pkg/user/backend"

revactx "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/token"
"github.com/cs3org/reva/pkg/token/manager/jwt"
"github.com/owncloud/ocis/ocis-pkg/log"
"github.com/owncloud/ocis/ocis-pkg/oidc"
)
Expand All @@ -21,18 +18,9 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl
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).Msg("Could not initialize token-manager")
}

return &accountResolver{
next: next,
logger: logger,
tokenManager: tokenManager,
userProvider: options.UserProvider,
userOIDCClaim: options.UserOIDCClaim,
userCS3Claim: options.UserCS3Claim,
Expand All @@ -44,7 +32,6 @@ func AccountResolver(optionSetters ...Option) func(next http.Handler) http.Handl
type accountResolver struct {
next http.Handler
logger log.Logger
tokenManager token.Manager
userProvider backend.UserBackend
autoProvisionAccounts bool
userOIDCClaim string
Expand All @@ -56,6 +43,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
ctx := req.Context()
claims := oidc.FromContext(ctx)
u, ok := revactx.ContextGetUser(ctx)
token := ""
// TODO what if an X-Access-Token is set? happens eg for download requests to the /data endpoint in the reva frontend

if claims == nil && !ok {
Expand All @@ -74,7 +62,7 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

u, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value, true)
u, token, err = m.userProvider.GetUserByClaims(req.Context(), m.userCS3Claim, value, true)

if errors.Is(err, backend.ErrAccountNotFound) {
m.logger.Debug().Str("claim", m.userOIDCClaim).Str("value", value).Msg("User by claim not found")
Expand Down Expand Up @@ -108,18 +96,6 @@ func (m accountResolver) ServeHTTP(w http.ResponseWriter, req *http.Request) {
m.logger.Debug().Interface("claims", claims).Interface("user", u).Msg("associated claims with user")
}

s, err := scope.AddOwnerScope(nil)
if err != nil {
m.logger.Error().Err(err).Msg("could not get owner scope")
return
}
token, err := m.tokenManager.MintToken(ctx, u, s)
if err != nil {
m.logger.Error().Err(err).Msg("could not mint token")
w.WriteHeader(http.StatusInternalServerError)
return
}

req.Header.Set(revactx.TokenHeader, token)

m.next.ServeHTTP(w, req)
Expand Down
17 changes: 15 additions & 2 deletions proxy/pkg/middleware/account_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"testing"

userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/pkg/auth/scope"
revactx "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/token/manager/jwt"
"github.com/owncloud/ocis/ocis-pkg/log"
"github.com/owncloud/ocis/ocis-pkg/oidc"
"github.com/owncloud/ocis/proxy/pkg/config"
Expand Down Expand Up @@ -106,9 +108,20 @@ func TestInternalServerErrorOnMissingMailAndUsername(t *testing.T) {
}

func newMockAccountResolver(userBackendResult *userv1beta1.User, userBackendErr error, oidcclaim, cs3claim string) http.Handler {
tokenManager, _ := jwt.New(map[string]interface{}{
"secret": "change-me",
"expires": int64(60),
})

token := ""
if userBackendResult != nil {
s, _ := scope.AddOwnerScope(nil)
token, _ = tokenManager.MintToken(context.Background(), userBackendResult, s)
}

mock := &test.UserBackendMock{
GetUserByClaimsFunc: func(ctx context.Context, claim string, value string, withRoles bool) (*userv1beta1.User, error) {
return userBackendResult, userBackendErr
GetUserByClaimsFunc: func(ctx context.Context, claim string, value string, withRoles bool) (*userv1beta1.User, string, error) {
return userBackendResult, token, userBackendErr
},
}

Expand Down
2 changes: 1 addition & 1 deletion proxy/pkg/middleware/basic_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func BasicAuth(optionSetters ...Option) func(next http.Handler) http.Handler {

removeSuperfluousAuthenticate(w)
login, password, _ := req.BasicAuth()
user, err := h.userProvider.Authenticate(req.Context(), login, password)
user, _, err := h.userProvider.Authenticate(req.Context(), login, password)

// touch is a user agent locking guard, when touched changes to true it indicates the User-Agent on the
// request is configured to support only one challenge, it it remains untouched, there are no considera-
Expand Down
11 changes: 0 additions & 11 deletions proxy/pkg/middleware/create_home.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
revactx "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/cs3org/reva/pkg/token"
"github.com/cs3org/reva/pkg/token/manager/jwt"
"github.com/owncloud/ocis/ocis-pkg/log"
"google.golang.org/grpc/metadata"
)
Expand All @@ -20,17 +18,9 @@ func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler {
logger := options.Logger

return func(next http.Handler) http.Handler {
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,
tokenManager: tokenManager,
revaGatewayClient: options.RevaGatewayClient,
}
}
Expand All @@ -39,7 +29,6 @@ func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler {
type createHome struct {
next http.Handler
logger log.Logger
tokenManager token.Manager
revaGatewayClient gateway.GatewayAPIClient
}

Expand Down
4 changes: 2 additions & 2 deletions proxy/pkg/middleware/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ type Options struct {
// PreSignedURLConfig to configure the middleware
PreSignedURLConfig config.PreSignedURL
// UserOIDCClaim to read from the oidc claims
UserOIDCClaim string
UserOIDCClaim string
// UserCS3Claim to use when looking up a user in the CS3 API
UserCS3Claim string
UserCS3Claim string
// AutoprovisionAccounts when an accountResolver does not exist.
AutoprovisionAccounts bool
// EnableBasicAuth to allow basic auth
Expand Down
2 changes: 1 addition & 1 deletion proxy/pkg/middleware/signed_url_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (m signedURLAuth) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

user, err := m.userProvider.GetUserByClaims(req.Context(), "username", req.URL.Query().Get("OC-Credential"), true)
user, _, err := m.userProvider.GetUserByClaims(req.Context(), "username", req.URL.Query().Get("OC-Credential"), true)
if err != nil {
m.logger.Error().Err(err).Msg("Could not get user by claim")
w.WriteHeader(http.StatusInternalServerError)
Expand Down
Loading