Skip to content

Commit

Permalink
Add Project field to YorkieService logs (#911)
Browse files Browse the repository at this point in the history
This commit adds project field to the YorkieService logs for easier
project identification during debugging. Additionally, removes
LoggingInterceptor to allow each service to define its own log fields,
as YorkieService and AdminService logs may differ.
  • Loading branch information
hackerwins authored Jul 2, 2024
1 parent 275cdff commit e4885cb
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 133 deletions.
24 changes: 22 additions & 2 deletions server/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import (
// Logger is a wrapper of zap.Logger.
type Logger = *zap.SugaredLogger

// Field is a wrapper of zap.Field.
type Field = zap.Field

var defaultLogger Logger
var logLevel = zapcore.InfoLevel
var loggerOnce sync.Once
Expand Down Expand Up @@ -57,8 +60,25 @@ func SetLogLevel(level string) error {
}

// New creates a new logger with the given configuration.
func New(name string) Logger {
return newLogger(name)
func New(name string, fields ...Field) Logger {
logger := newLogger(name)

if len(fields) > 0 {
var args = make([]interface{}, len(fields))
for i, field := range fields {
args[i] = field
}

logger = logger.With(args...)
}

return logger
}

// NewField creates a new field with the given key and value.
func NewField(key string, value string) Field {
return zap.String(key, value)

}

// DefaultLogger returns the default logger used by Yorkie.
Expand Down
77 changes: 0 additions & 77 deletions server/rpc/connecthelper/logging.go

This file was deleted.

1 change: 1 addition & 0 deletions server/rpc/connecthelper/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

// Package connecthelper provides helper functions for connectRPC.
package connecthelper

import (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/yorkie-team/yorkie/api/types"
"github.com/yorkie-team/yorkie/server/backend"
"github.com/yorkie-team/yorkie/server/logging"
"github.com/yorkie-team/yorkie/server/projects"
"github.com/yorkie-team/yorkie/server/rpc/auth"
"github.com/yorkie-team/yorkie/server/rpc/connecthelper"
Expand All @@ -35,22 +36,34 @@ import (
// ErrUnauthenticated is returned when authentication is failed.
var ErrUnauthenticated = errors.New("authorization is not provided")

// AdminAuthInterceptor is an interceptor for authentication.
type AdminAuthInterceptor struct {
func isAdminService(method string) bool {
return strings.HasPrefix(method, "/yorkie.v1.AdminService")
}

func isRequiredAuth(method string) bool {
return method != "/yorkie.v1.AdminService/LogIn" &&
method != "/yorkie.v1.AdminService/SignUp"
}

// AdminServiceInterceptor is an interceptor for building additional context
// and handling authentication for AdminService.
type AdminServiceInterceptor struct {
backend *backend.Backend
requestID *requestID
tokenManager *auth.TokenManager
}

// NewAdminAuthInterceptor creates a new instance of AdminAuthInterceptor.
func NewAdminAuthInterceptor(be *backend.Backend, tokenManager *auth.TokenManager) *AdminAuthInterceptor {
return &AdminAuthInterceptor{
// NewAdminServiceInterceptor creates a new instance of AdminServiceInterceptor.
func NewAdminServiceInterceptor(be *backend.Backend, tokenManager *auth.TokenManager) *AdminServiceInterceptor {
return &AdminServiceInterceptor{
backend: be,
requestID: newRequestID("a"),
tokenManager: tokenManager,
}
}

// WrapUnary creates a unary server interceptor for authentication.
func (i *AdminAuthInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
func (i *AdminServiceInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
return func(
ctx context.Context,
req connect.AnyRequest,
Expand All @@ -59,12 +72,9 @@ func (i *AdminAuthInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFu
return next(ctx, req)
}

if isRequiredAuth(req.Spec().Procedure) {
user, err := i.authenticate(ctx, req.Header())
if err != nil {
return nil, err
}
ctx = users.With(ctx, user)
ctx, err := i.buildContext(ctx, req.Spec().Procedure, req.Header())
if err != nil {
return nil, err
}

res, err := next(ctx, req)
Expand Down Expand Up @@ -92,7 +102,7 @@ func (i *AdminAuthInterceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFu
}

// WrapStreamingClient creates a stream client interceptor for authentication.
func (i *AdminAuthInterceptor) WrapStreamingClient(next connect.StreamingClientFunc) connect.StreamingClientFunc {
func (i *AdminServiceInterceptor) WrapStreamingClient(next connect.StreamingClientFunc) connect.StreamingClientFunc {
return func(
ctx context.Context,
spec connect.Spec,
Expand All @@ -102,7 +112,7 @@ func (i *AdminAuthInterceptor) WrapStreamingClient(next connect.StreamingClientF
}

// WrapStreamingHandler creates a stream server interceptor for authentication.
func (i *AdminAuthInterceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc) connect.StreamingHandlerFunc {
func (i *AdminServiceInterceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc) connect.StreamingHandlerFunc {
return func(
ctx context.Context,
conn connect.StreamingHandlerConn,
Expand All @@ -111,15 +121,12 @@ func (i *AdminAuthInterceptor) WrapStreamingHandler(next connect.StreamingHandle
return next(ctx, conn)
}

if isRequiredAuth(conn.Spec().Procedure) {
user, err := i.authenticate(ctx, conn.RequestHeader())
if err != nil {
return err
}
ctx = users.With(ctx, user)
ctx, err := i.buildContext(ctx, conn.Spec().Procedure, conn.RequestHeader())
if err != nil {
return err
}

err := next(ctx, conn)
err = next(ctx, conn)

// TODO(hackerwins, emplam27): Consider splitting between admin and sdk metrics.
sdkType, sdkVersion := connecthelper.SDKTypeAndVersion(conn.RequestHeader())
Expand All @@ -143,17 +150,27 @@ func (i *AdminAuthInterceptor) WrapStreamingHandler(next connect.StreamingHandle
}
}

func isAdminService(method string) bool {
return strings.HasPrefix(method, "/yorkie.v1.AdminService")
}
// buildContext builds a new context with the given request header.
func (i *AdminServiceInterceptor) buildContext(
ctx context.Context,
procedure string,
header http.Header,
) (context.Context, error) {
if isRequiredAuth(procedure) {
user, err := i.authenticate(ctx, header)
if err != nil {
return nil, err
}
ctx = users.With(ctx, user)
}

func isRequiredAuth(method string) bool {
return method != "/yorkie.v1.AdminService/LogIn" &&
method != "/yorkie.v1.AdminService/SignUp"
ctx = logging.With(ctx, logging.New(i.requestID.next()))

return ctx, nil
}

// authenticate does authenticate the request.
func (i *AdminAuthInterceptor) authenticate(
func (i *AdminServiceInterceptor) authenticate(
ctx context.Context,
header http.Header,
) (*types.User, error) {
Expand All @@ -163,6 +180,7 @@ func (i *AdminAuthInterceptor) authenticate(
}

// NOTE(raararaara): If the token is access token, return the user of the token.
// This is used for the case where the user uses dashboard or CLI.
claims, err := i.tokenManager.Verify(authorization)
if err == nil {
user, err := users.GetUserByName(ctx, i.backend, claims.Username)
Expand All @@ -172,6 +190,9 @@ func (i *AdminAuthInterceptor) authenticate(
}

// NOTE(raararaara): If the token is secret key, return the owner of the project.
// This is used for the case where the user uses REST API.
// TODO(hackerwins): In this case, attacker can hijack the project owner's identity.
// We need to separate project-wide API and user-wide API from AdminService.
project, err := projects.GetProjectFromSecretKey(ctx, i.backend, authorization)
if err == nil {
user, err := users.GetUserByID(ctx, i.backend, project.Owner)
Expand Down
5 changes: 2 additions & 3 deletions server/rpc/interceptors/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ import (

"connectrpc.com/connect"

"github.com/yorkie-team/yorkie/server/rpc/connecthelper"

"github.com/yorkie-team/yorkie/server/logging"
"github.com/yorkie-team/yorkie/server/rpc/connecthelper"
)

// DefaultInterceptor is a interceptor for default.
// DefaultInterceptor is an interceptor for common RPC.
type DefaultInterceptor struct{}

// NewDefaultInterceptor creates a new instance of DefaultInterceptor.
Expand Down
42 changes: 42 additions & 0 deletions server/rpc/interceptors/requestid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2022 The Yorkie Authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package interceptors

import (
"strconv"
"sync/atomic"
)

// requestID is used to generate a unique request ID.
type requestID struct {
prefix string
id int32
}

// newRequestID creates a new requestID.
func newRequestID(prefix string) *requestID {
return &requestID{
prefix: prefix,
id: 0,
}
}

// next generates a new request ID.
func (r *requestID) next() string {
next := atomic.AddInt32(&r.id, 1)
return r.prefix + strconv.Itoa(int(next))
}
Loading

0 comments on commit e4885cb

Please sign in to comment.