hanahmily commented on code in PR #704:
URL: 
https://github.com/apache/skywalking-banyandb/pull/704#discussion_r2238016863


##########
banyand/liaison/pkg/auth/config.go:
##########
@@ -0,0 +1,73 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 auth provides configuration management and validation logic for 
authentication.
+package auth
+
+import (
+       "os"
+       "strings"
+
+       "sigs.k8s.io/yaml"
+)
+
+// Config AuthConfig.
+type Config struct {
+       Users             []User `yaml:"users"`
+       Enabled           bool   `yaml:"-"`
+       HealthAuthEnabled bool   `yaml:"-"`
+}
+
+// User details from config file.
+type User struct {
+       Username string `yaml:"username"`
+       Password string `yaml:"password"`
+}
+
+// InitCfg returns Config with default values.
+func InitCfg() *Config {
+       return &Config{
+               Enabled:           false,
+               HealthAuthEnabled: false,
+               Users:             []User{},
+       }
+}
+
+// LoadConfig implements the reading of the authentication configuration.
+func LoadConfig(cfg *Config, filePath string) error {
+       cfg.Enabled = true
+       data, err := os.ReadFile(filePath)

Review Comment:
   Check file permissions (0600) and warn/error if too permissive



##########
banyand/liaison/grpc/auth.go:
##########
@@ -0,0 +1,95 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 grpc
+
+import (
+       "context"
+
+       "google.golang.org/grpc"
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/metadata"
+       "google.golang.org/grpc/status"
+
+       "github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+)
+
+func authInterceptor(cfg *auth.Config) grpc.UnaryServerInterceptor {
+       return func(
+               ctx context.Context,
+               req interface{},
+               info *grpc.UnaryServerInfo,
+               handler grpc.UnaryHandler,
+       ) (interface{}, error) {
+               if !cfg.Enabled {
+                       return handler(ctx, req)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(ctx, req)
+               }
+               if err := validateUser(ctx, cfg); err != nil {
+                       return nil, err
+               }
+               return handler(ctx, req)
+       }
+}
+
+func authStreamInterceptor(cfg *auth.Config) grpc.StreamServerInterceptor {
+       return func(
+               srv interface{},
+               stream grpc.ServerStream,
+               info *grpc.StreamServerInfo,
+               handler grpc.StreamHandler,
+       ) error {
+               if !cfg.Enabled {
+                       return handler(srv, stream)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(srv, stream)
+               }
+               if err := validateUser(stream.Context(), cfg); err != nil {
+                       return err
+               }
+               return handler(srv, stream)
+       }
+}
+
+func validateUser(ctx context.Context, cfg *auth.Config) error {
+       md, ok := metadata.FromIncomingContext(ctx)
+       if !ok {
+               return status.Errorf(codes.Unauthenticated, "metadata is not 
provided")
+       }
+
+       usernames := md.Get("username")
+       passwords := md.Get("password")
+
+       if len(usernames) == 0 {
+               return status.Errorf(codes.Unauthenticated, "username is 
missing")
+       }
+       if len(passwords) == 0 {
+               return status.Errorf(codes.Unauthenticated, "password is 
missing")

Review Comment:
   
   ```suggestion
                return status.Errorf(codes.Unauthenticated, "Invalid 
credentials")
   ```



##########
banyand/liaison/pkg/auth/config.go:
##########
@@ -0,0 +1,73 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 auth provides configuration management and validation logic for 
authentication.
+package auth
+
+import (
+       "os"
+       "strings"
+
+       "sigs.k8s.io/yaml"
+)
+
+// Config AuthConfig.
+type Config struct {
+       Users             []User `yaml:"users"`
+       Enabled           bool   `yaml:"-"`
+       HealthAuthEnabled bool   `yaml:"-"`
+}
+
+// User details from config file.
+type User struct {
+       Username string `yaml:"username"`
+       Password string `yaml:"password"`
+}
+
+// InitCfg returns Config with default values.
+func InitCfg() *Config {
+       return &Config{
+               Enabled:           false,
+               HealthAuthEnabled: false,
+               Users:             []User{},
+       }
+}
+
+// LoadConfig implements the reading of the authentication configuration.
+func LoadConfig(cfg *Config, filePath string) error {
+       cfg.Enabled = true
+       data, err := os.ReadFile(filePath)
+       if err != nil {
+               return err
+       }
+       err = yaml.Unmarshal(data, cfg)
+       if err != nil {
+               return err
+       }
+       return nil
+}
+
+// CheckUsernameAndPassword returns true if the provided username and password 
match any configured user.
+func CheckUsernameAndPassword(cfg *Config, username, password string) bool {
+       for _, user := range cfg.Users {
+               if strings.TrimSpace(username) == 
strings.TrimSpace(user.Username) &&
+                       strings.TrimSpace(password) == 
strings.TrimSpace(user.Password) {

Review Comment:
   Would you pls use "subtle.ConstantTimeCompare" to protect the server from 
timming attacks?



##########
banyand/liaison/grpc/auth.go:
##########
@@ -0,0 +1,95 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 grpc
+
+import (
+       "context"
+
+       "google.golang.org/grpc"
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/metadata"
+       "google.golang.org/grpc/status"
+
+       "github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+)
+
+func authInterceptor(cfg *auth.Config) grpc.UnaryServerInterceptor {
+       return func(
+               ctx context.Context,
+               req interface{},
+               info *grpc.UnaryServerInfo,
+               handler grpc.UnaryHandler,
+       ) (interface{}, error) {
+               if !cfg.Enabled {
+                       return handler(ctx, req)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(ctx, req)
+               }
+               if err := validateUser(ctx, cfg); err != nil {
+                       return nil, err
+               }
+               return handler(ctx, req)
+       }
+}
+
+func authStreamInterceptor(cfg *auth.Config) grpc.StreamServerInterceptor {
+       return func(
+               srv interface{},
+               stream grpc.ServerStream,
+               info *grpc.StreamServerInfo,
+               handler grpc.StreamHandler,
+       ) error {
+               if !cfg.Enabled {
+                       return handler(srv, stream)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(srv, stream)
+               }
+               if err := validateUser(stream.Context(), cfg); err != nil {
+                       return err
+               }
+               return handler(srv, stream)
+       }
+}
+
+func validateUser(ctx context.Context, cfg *auth.Config) error {
+       md, ok := metadata.FromIncomingContext(ctx)
+       if !ok {
+               return status.Errorf(codes.Unauthenticated, "metadata is not 
provided")
+       }
+
+       usernames := md.Get("username")
+       passwords := md.Get("password")
+
+       if len(usernames) == 0 {
+               return status.Errorf(codes.Unauthenticated, "username is 
missing")
+       }
+       if len(passwords) == 0 {
+               return status.Errorf(codes.Unauthenticated, "password is 
missing")
+       }
+
+       username := usernames[0]
+       password := passwords[0]
+
+       if !auth.CheckUsernameAndPassword(cfg, username, password) {
+               return status.Errorf(codes.Unauthenticated, "invalid username 
or password")

Review Comment:
   
   ```suggestion
                return status.Errorf(codes.Unauthenticated, "Invalid 
credentials")
   ```



##########
bydbctl/internal/cmd/root.go:
##########
@@ -152,3 +163,10 @@ func bindTLSRelatedFlag(commands ...*cobra.Command) {
                c.Flags().StringVarP(&cert, "cert", "", "", "Certificate for 
tls")
        }
 }
+
+func bindUsernameAndPasswordFlag(commands ...*cobra.Command) {

Review Comment:
   To bind them in RootCmdFlags, all sub commands will inherit them.By that, 
the cmd pkg will only change root.go.



##########
test/integration/standalone/other/testdata/.bydbctl.yaml:
##########
@@ -0,0 +1,3 @@
+group: ""

Review Comment:
   Move this file and .bydbctl1.yaml to `bydbctl/internal/cmd/testdata`



##########
bydbctl/internal/cmd/auth_test.go:
##########
@@ -0,0 +1,164 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 cmd_test
+
+import (
+       "fmt"
+       "os"
+       "path/filepath"
+       "runtime"
+       "strings"
+
+       "github.com/google/uuid"
+       g "github.com/onsi/ginkgo/v2"
+       gm "github.com/onsi/gomega"
+       "github.com/spf13/cobra"
+       "github.com/zenizh/go-capturer"
+       "sigs.k8s.io/yaml"
+
+       databasev1 
"github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1"
+       serverAuth 
"github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+       "github.com/apache/skywalking-banyandb/bydbctl/internal/cmd"
+       "github.com/apache/skywalking-banyandb/pkg/test/helpers"
+       "github.com/apache/skywalking-banyandb/pkg/test/setup"
+)
+
+var _ = g.Describe("bydbctl test with authentication", func() {
+       var deferFn func()
+       var httpAddr string
+       var authCfgFile string
+       var baseDir string
+       var bydbctlCfgFile string
+       var rootCmd *cobra.Command
+       var testUser serverAuth.User
+
+       g.BeforeEach(func() {
+               _, currentFile, _, _ := runtime.Caller(0)
+               baseDir = filepath.Dir(currentFile)
+               authCfgFile = filepath.Join(baseDir, 
"../../../test/integration/standalone/other/testdata/config.yaml")
+               // load server config.yaml
+               cfgBytes, err := os.ReadFile(authCfgFile)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               var cfg serverAuth.Config
+               err = yaml.Unmarshal(cfgBytes, &cfg)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               // take the first user as the test user
+               gm.Expect(len(cfg.Users)).Should(gm.BeNumerically(">", 0))
+               testUser = cfg.Users[0]
+               // Username and password must be provided because health checks 
require authentication when --enable-health-auth=true is enabled.
+               _, httpAddr, deferFn = setup.EmptyStandaloneWithAuth(
+                       testUser.Username, testUser.Password,
+                       fmt.Sprintf("--auth-config-file=%s", authCfgFile),
+                       "--enable-health-auth=true",
+               )
+               httpAddr = httpSchema + httpAddr
+               rootCmd = &cobra.Command{Use: "root"}
+               cmd.RootCmdFlags(rootCmd)
+       })
+
+       g.It("list groups and health check with correct username and password 
from command line", func() {
+               rootCmd.SetArgs([]string{"group", "list", "-a", httpAddr, "-u", 
testUser.Username, "-p", testUser.Password})
+               err := rootCmd.Execute()
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+
+               rootCmd.SetArgs([]string{"health", "-a", httpAddr, "-u", 
testUser.Username, "-p", testUser.Password})
+               err = rootCmd.Execute()
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+       })
+
+       g.It("list groups and health check with wrong username and password", 
func() {
+               // it will send http request with username = "admin" and 
password = ""
+               rootCmd.SetArgs([]string{"group", "list", "-a", httpAddr, "-u", 
"admin", "-p", testUser.Password + "wrong"})
+               err := rootCmd.Execute()
+               gm.Expect(err).To(gm.HaveOccurred())
+
+               rootCmd.SetArgs([]string{"health", "-a", httpAddr, "-u", 
"admin", "-p", testUser.Password + "wrong"})
+               err = rootCmd.Execute()
+               gm.Expect(err).To(gm.HaveOccurred())
+       })
+
+       g.It("create and get a group and health check with correct username and 
password in bydbctlCfgFile", func() {
+               bydbctlCfgFile = filepath.Join(baseDir, 
"../../../test/integration/standalone/other/testdata/.bydbctl.yaml")
+               // Copy .bydbctl.yaml to /tmp directory to avoid permission 
modification failure under WSL
+               tempBydbctl := filepath.Join(os.TempDir(), 
fmt.Sprintf(".bydbctl-%s.yaml", uuid.New().String()))
+               input, err := os.ReadFile(bydbctlCfgFile)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               err = os.WriteFile(tempBydbctl, input, 0o600)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               bydbctlCfgFile = tempBydbctl
+               info, _ := os.Stat(bydbctlCfgFile)
+               gm.Expect(info.Mode().Perm()).To(gm.Equal(os.FileMode(0o600)))
+
+               rootCmd.SetArgs([]string{"--config", bydbctlCfgFile, "group", 
"create", "-a", httpAddr, "-f", "-"})
+               rootCmd.SetIn(strings.NewReader(`
+metadata:
+  name: group1
+catalog: CATALOG_STREAM
+resource_opts:
+  shard_num: 2
+  segment_interval:
+    unit: UNIT_DAY
+    num: 1
+  ttl:
+    unit: UNIT_DAY
+    num: 7`))
+               out := capturer.CaptureStdout(func() {
+                       err = rootCmd.Execute()
+                       gm.Expect(err).NotTo(gm.HaveOccurred())
+               })
+               gm.Expect(out).To(gm.ContainSubstring("group group1 is 
created"))
+
+               rootCmd.SetArgs([]string{"--config", bydbctlCfgFile, "group", 
"-a", httpAddr, "get", "-g", "group1"})
+               out = capturer.CaptureStdout(func() {
+                       err = rootCmd.Execute()
+                       gm.Expect(err).NotTo(gm.HaveOccurred())
+               })
+               resp := new(databasev1.GroupRegistryServiceGetResponse)
+               helpers.UnmarshalYAML([]byte(out), resp)
+               gm.Expect(resp.Group.Metadata.Name).To(gm.Equal("group1"))
+
+               rootCmd.SetArgs([]string{"health", "-a", httpAddr})
+               err = rootCmd.Execute()
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+       })
+
+       g.It("list groups and health check with wrong username and password in 
bydbctlCfgFile", func() {
+               bydbctlCfgFile = filepath.Join(baseDir, 
"../../../test/integration/standalone/other/testdata/.bydbctl1.yaml")

Review Comment:
   Use embed.FS to access this file



##########
bydbctl/pkg/auth/util.go:
##########
@@ -0,0 +1,31 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 auth contains authentication helper functions.
+package auth
+
+import (
+       "encoding/base64"
+       "fmt"
+)
+
+// GenerateBasicAuthHeader creates a Basic Auth header from username and 
password.
+func GenerateBasicAuthHeader(username, password string) string {

Review Comment:
   Move it to 'pkg' since both "bydbctl" and "test" are using it.



##########
banyand/liaison/grpc/auth.go:
##########
@@ -0,0 +1,95 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 grpc
+
+import (
+       "context"
+
+       "google.golang.org/grpc"
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/metadata"
+       "google.golang.org/grpc/status"
+
+       "github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+)
+
+func authInterceptor(cfg *auth.Config) grpc.UnaryServerInterceptor {
+       return func(
+               ctx context.Context,
+               req interface{},
+               info *grpc.UnaryServerInfo,
+               handler grpc.UnaryHandler,
+       ) (interface{}, error) {
+               if !cfg.Enabled {
+                       return handler(ctx, req)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(ctx, req)
+               }
+               if err := validateUser(ctx, cfg); err != nil {
+                       return nil, err
+               }
+               return handler(ctx, req)
+       }
+}
+
+func authStreamInterceptor(cfg *auth.Config) grpc.StreamServerInterceptor {
+       return func(
+               srv interface{},
+               stream grpc.ServerStream,
+               info *grpc.StreamServerInfo,
+               handler grpc.StreamHandler,
+       ) error {
+               if !cfg.Enabled {
+                       return handler(srv, stream)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(srv, stream)
+               }
+               if err := validateUser(stream.Context(), cfg); err != nil {
+                       return err
+               }
+               return handler(srv, stream)
+       }
+}
+
+func validateUser(ctx context.Context, cfg *auth.Config) error {
+       md, ok := metadata.FromIncomingContext(ctx)
+       if !ok {
+               return status.Errorf(codes.Unauthenticated, "metadata is not 
provided")
+       }
+
+       usernames := md.Get("username")
+       passwords := md.Get("password")
+
+       if len(usernames) == 0 {
+               return status.Errorf(codes.Unauthenticated, "username is 
missing")

Review Comment:
   
   ```suggestion
                return status.Errorf(codes.Unauthenticated, "Invalid 
credentials")
   ```



##########
bydbctl/internal/cmd/root.go:
##########
@@ -96,14 +98,23 @@ func initConfig() {
                viper.SetConfigName(".bydbctl")
        }
 
+       viper.SetEnvPrefix("BYDBCTL")
        viper.AutomaticEnv()

Review Comment:
   You could use bindFlags from `pkg/config` to bind all flags to env vars. I'm 
free to make this function exported



##########
bydbctl/internal/cmd/root.go:
##########
@@ -96,14 +98,23 @@ func initConfig() {
                viper.SetConfigName(".bydbctl")
        }
 
+       viper.SetEnvPrefix("BYDBCTL")
        viper.AutomaticEnv()
 
        readCfg := func() error {
                if err := viper.ReadInConfig(); err != nil {
                        return err
                }
+               configFile := viper.ConfigFileUsed()
+               _, err := os.Stat(configFile)
+               if err != nil {
+                       return fmt.Errorf("unable to stat config file: %w", err)
+               }
+               // TODO if info.Mode().Perm() != 0o600 {
+               // TODO fmt.Printf("config file %s has unsafe permissions: %o 
(expected 0600)", configFile, info.Mode().Perm())
+               // TODO }

Review Comment:
   Implement the TODO - enforce secure file permissions.



##########
banyand/liaison/grpc/auth.go:
##########
@@ -0,0 +1,95 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 grpc
+
+import (
+       "context"
+
+       "google.golang.org/grpc"
+       "google.golang.org/grpc/codes"
+       "google.golang.org/grpc/metadata"
+       "google.golang.org/grpc/status"
+
+       "github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+)
+
+func authInterceptor(cfg *auth.Config) grpc.UnaryServerInterceptor {
+       return func(
+               ctx context.Context,
+               req interface{},
+               info *grpc.UnaryServerInfo,
+               handler grpc.UnaryHandler,
+       ) (interface{}, error) {
+               if !cfg.Enabled {
+                       return handler(ctx, req)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(ctx, req)
+               }
+               if err := validateUser(ctx, cfg); err != nil {
+                       return nil, err
+               }
+               return handler(ctx, req)
+       }
+}
+
+func authStreamInterceptor(cfg *auth.Config) grpc.StreamServerInterceptor {
+       return func(
+               srv interface{},
+               stream grpc.ServerStream,
+               info *grpc.StreamServerInfo,
+               handler grpc.StreamHandler,
+       ) error {
+               if !cfg.Enabled {
+                       return handler(srv, stream)
+               }
+               if info.FullMethod == "/grpc.health.v1.Health/Check" && 
!cfg.HealthAuthEnabled {
+                       return handler(srv, stream)
+               }
+               if err := validateUser(stream.Context(), cfg); err != nil {
+                       return err
+               }
+               return handler(srv, stream)
+       }
+}
+
+func validateUser(ctx context.Context, cfg *auth.Config) error {
+       md, ok := metadata.FromIncomingContext(ctx)
+       if !ok {
+               return status.Errorf(codes.Unauthenticated, "metadata is not 
provided")
+       }
+
+       usernames := md.Get("username")
+       passwords := md.Get("password")
+
+       if len(usernames) == 0 {
+               return status.Errorf(codes.Unauthenticated, "username is 
missing")
+       }
+       if len(passwords) == 0 {
+               return status.Errorf(codes.Unauthenticated, "password is 
missing")

Review Comment:
   
   ```suggestion
                return status.Errorf(codes.Unauthenticated, "Invalid 
credentials")
   ```



##########
test/integration/standalone/other/auth_test.go:
##########
@@ -0,0 +1,203 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 integration_other_test
+
+import (
+       "fmt"
+       "net/http"
+       "os"
+       "path/filepath"
+       "runtime"
+       "time"
+
+       g "github.com/onsi/ginkgo/v2"
+       gm "github.com/onsi/gomega"
+       "github.com/onsi/gomega/gleak"
+       grpclib "google.golang.org/grpc"
+       "google.golang.org/grpc/credentials/insecure"
+       "sigs.k8s.io/yaml"
+
+       serverAuth 
"github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+       "github.com/apache/skywalking-banyandb/bydbctl/pkg/auth"
+       "github.com/apache/skywalking-banyandb/pkg/grpchelper"
+       "github.com/apache/skywalking-banyandb/pkg/test/flags"
+       "github.com/apache/skywalking-banyandb/pkg/test/helpers"
+       "github.com/apache/skywalking-banyandb/pkg/test/setup"
+       "github.com/apache/skywalking-banyandb/pkg/timestamp"
+       casesMeasureData 
"github.com/apache/skywalking-banyandb/test/cases/measure/data"
+)
+
+var _ = g.Describe("Query service_cpm_minute with authentication", func() {
+       var deferFn func()
+       var baseTime time.Time
+       var interval time.Duration
+       var conn *grpclib.ClientConn
+       var goods []gleak.Goroutine
+       var grpcAddr, httpAddr string
+       var authCfgFile string
+       var testUser serverAuth.User
+
+       g.BeforeEach(func() {
+               _, currentFile, _, _ := runtime.Caller(0)
+               basePath := filepath.Dir(currentFile)
+               authCfgFile = filepath.Join(basePath, "testdata/config.yaml")

Review Comment:
   Use embed.FS to load this file. You can find some examples in this project.



##########
bydbctl/internal/cmd/auth_test.go:
##########
@@ -0,0 +1,164 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you 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 cmd_test
+
+import (
+       "fmt"
+       "os"
+       "path/filepath"
+       "runtime"
+       "strings"
+
+       "github.com/google/uuid"
+       g "github.com/onsi/ginkgo/v2"
+       gm "github.com/onsi/gomega"
+       "github.com/spf13/cobra"
+       "github.com/zenizh/go-capturer"
+       "sigs.k8s.io/yaml"
+
+       databasev1 
"github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1"
+       serverAuth 
"github.com/apache/skywalking-banyandb/banyand/liaison/pkg/auth"
+       "github.com/apache/skywalking-banyandb/bydbctl/internal/cmd"
+       "github.com/apache/skywalking-banyandb/pkg/test/helpers"
+       "github.com/apache/skywalking-banyandb/pkg/test/setup"
+)
+
+var _ = g.Describe("bydbctl test with authentication", func() {
+       var deferFn func()
+       var httpAddr string
+       var authCfgFile string
+       var baseDir string
+       var bydbctlCfgFile string
+       var rootCmd *cobra.Command
+       var testUser serverAuth.User
+
+       g.BeforeEach(func() {
+               _, currentFile, _, _ := runtime.Caller(0)
+               baseDir = filepath.Dir(currentFile)
+               authCfgFile = filepath.Join(baseDir, 
"../../../test/integration/standalone/other/testdata/config.yaml")
+               // load server config.yaml
+               cfgBytes, err := os.ReadFile(authCfgFile)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               var cfg serverAuth.Config
+               err = yaml.Unmarshal(cfgBytes, &cfg)
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+               // take the first user as the test user
+               gm.Expect(len(cfg.Users)).Should(gm.BeNumerically(">", 0))
+               testUser = cfg.Users[0]
+               // Username and password must be provided because health checks 
require authentication when --enable-health-auth=true is enabled.
+               _, httpAddr, deferFn = setup.EmptyStandaloneWithAuth(
+                       testUser.Username, testUser.Password,
+                       fmt.Sprintf("--auth-config-file=%s", authCfgFile),
+                       "--enable-health-auth=true",
+               )
+               httpAddr = httpSchema + httpAddr
+               rootCmd = &cobra.Command{Use: "root"}
+               cmd.RootCmdFlags(rootCmd)
+       })
+
+       g.It("list groups and health check with correct username and password 
from command line", func() {
+               rootCmd.SetArgs([]string{"group", "list", "-a", httpAddr, "-u", 
testUser.Username, "-p", testUser.Password})
+               err := rootCmd.Execute()
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+
+               rootCmd.SetArgs([]string{"health", "-a", httpAddr, "-u", 
testUser.Username, "-p", testUser.Password})
+               err = rootCmd.Execute()
+               gm.Expect(err).NotTo(gm.HaveOccurred())
+       })
+
+       g.It("list groups and health check with wrong username and password", 
func() {
+               // it will send http request with username = "admin" and 
password = ""
+               rootCmd.SetArgs([]string{"group", "list", "-a", httpAddr, "-u", 
"admin", "-p", testUser.Password + "wrong"})
+               err := rootCmd.Execute()
+               gm.Expect(err).To(gm.HaveOccurred())
+
+               rootCmd.SetArgs([]string{"health", "-a", httpAddr, "-u", 
"admin", "-p", testUser.Password + "wrong"})
+               err = rootCmd.Execute()
+               gm.Expect(err).To(gm.HaveOccurred())
+       })
+
+       g.It("create and get a group and health check with correct username and 
password in bydbctlCfgFile", func() {
+               bydbctlCfgFile = filepath.Join(baseDir, 
"../../../test/integration/standalone/other/testdata/.bydbctl.yaml")

Review Comment:
   Use embed.Fs to access this file.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to