From 2c8ee682375bf042457d890ddfb47795322b28a5 Mon Sep 17 00:00:00 2001 From: Artem Dvoretskii Date: Fri, 26 Jan 2024 19:23:45 +0300 Subject: [PATCH] rm manual password salting, minor fixes and refactor --- internal/api/middleware/middleware.go | 3 +- internal/api/store/migrate.go | 29 ++++++------------- internal/api/user/auth/auth.go | 16 ++++++---- .../user/store/migrations/00001_init.up.sql | 4 +-- internal/api/user/store/store.go | 20 ++----------- internal/api/video/model/status.go | 3 +- internal/api/video/serviceapi.go | 2 +- internal/app/app.go | 1 - internal/app/user/app.go | 1 - 9 files changed, 27 insertions(+), 52 deletions(-) diff --git a/internal/api/middleware/middleware.go b/internal/api/middleware/middleware.go index 088c165..b21d666 100644 --- a/internal/api/middleware/middleware.go +++ b/internal/api/middleware/middleware.go @@ -11,6 +11,8 @@ func GetEchoWithDefaultMiddleware() *echo.Echo { gen := generators.NewID() + e.Use(middleware.Recover()) + e.Use(middleware.RequestIDWithConfig(middleware.RequestIDConfig{ Skipper: middleware.DefaultSkipper, Generator: gen.GetStringOrPanic, @@ -26,6 +28,5 @@ func GetEchoWithDefaultMiddleware() *echo.Echo { `"status":${status},"error":"${error}","latency":"${latency_human}"` + `,"bytes_in":${bytes_in},"bytes_out":${bytes_out}}` + "\n" e.Use(middleware.LoggerWithConfig(loggerCfg)) - e.Use(middleware.Recover()) return e } diff --git a/internal/api/store/migrate.go b/internal/api/store/migrate.go index 627d4f8..d23f8aa 100644 --- a/internal/api/store/migrate.go +++ b/internal/api/store/migrate.go @@ -1,7 +1,6 @@ package store import ( - "embed" "errors" "fmt" @@ -18,34 +17,24 @@ func (db *Database) migrate(cfg *Config) error { return errors.New("migrations dir is not defined") } db.log.Debug("starting migration") - change, err := runMigrations(cfg.DSN, cfg.MigrationsDir) - if err != nil { - return err - } - if change { - db.log.Info("migration is complete") - } else { - db.log.Debug("db is up to date") - } - return nil -} -func runMigrations(dsn string, migrationsDir *embed.FS) (bool, error) { - d, err := iofs.New(migrationsDir, "migrations") + d, err := iofs.New(cfg.MigrationsDir, "migrations") if err != nil { - return false, fmt.Errorf("failed to return an iofs driver: %w", err) + return fmt.Errorf("failed to return an iofs driver: %w", err) } - m, err := migrate.NewWithSourceInstance("iofs", d, dsn) + m, err := migrate.NewWithSourceInstance("iofs", d, cfg.DSN) if err != nil { - return false, fmt.Errorf("failed to get a new migrate instance: %w", err) + return fmt.Errorf("failed to get a new migrate instance: %w", err) } if err = m.Up(); err != nil { if !errors.Is(err, migrate.ErrNoChange) { - return false, fmt.Errorf("failed to apply migrations to the DB: %w", err) + return fmt.Errorf("failed to apply migrations to the DB: %w", err) } - return false, nil + db.log.Debug("db is up to date") + } else { + db.log.Info("migration is complete") } - return true, nil + return nil } diff --git a/internal/api/user/auth/auth.go b/internal/api/user/auth/auth.go index 6e329bf..c2f9443 100644 --- a/internal/api/user/auth/auth.go +++ b/internal/api/user/auth/auth.go @@ -19,9 +19,9 @@ const ( jwtCookieName = "vidiSessID" - SessionContextKey = "vUser" + sessionContextKey = "vUser" - RoleNameService = "service" + roleNameService = "service" ) // Auth us an authenticator that can @@ -62,6 +62,10 @@ type Claims struct { jwt.RegisteredClaims } +func (c *Claims) IsService() bool { + return c.Role == roleNameService +} + func (a *Auth) expirationTime() time.Time { return time.Now().Add(a.expiration) } @@ -85,7 +89,7 @@ func (a *Auth) NewTokenForService(name string) (string, error) { claims := &Claims{ UserID: fmt.Sprintf("svc-%s", id), Name: name, - Role: RoleNameService, + Role: roleNameService, RegisteredClaims: jwt.RegisteredClaims{ ExpiresAt: jwt.NewNumericDate(a.expirationTime()), }, @@ -105,7 +109,7 @@ func (a *Auth) makeSignedJwt(claims *Claims) (string, error) { func (a *Auth) MiddlewareUser() echo.MiddlewareFunc { return echojwt.WithConfig(echojwt.Config{ ContinueOnIgnoredError: false, - ContextKey: SessionContextKey, + ContextKey: sessionContextKey, SigningKey: a.secret, SigningMethod: echojwt.AlgorithmHS256, TokenLookup: "cookie:" + jwtCookieName, @@ -118,7 +122,7 @@ func (a *Auth) MiddlewareUser() echo.MiddlewareFunc { func (a *Auth) MiddlewareService() echo.MiddlewareFunc { return echojwt.WithConfig(echojwt.Config{ ContinueOnIgnoredError: false, - ContextKey: SessionContextKey, + ContextKey: sessionContextKey, SigningKey: a.secret, SigningMethod: echojwt.AlgorithmHS256, TokenLookup: "header:Authorization:Bearer ", // space is important! @@ -129,7 +133,7 @@ func (a *Auth) MiddlewareService() echo.MiddlewareFunc { } func GetClaimFromContext(c echo.Context) (*Claims, error) { - token, ok := c.Get(SessionContextKey).(*jwt.Token) + token, ok := c.Get(sessionContextKey).(*jwt.Token) if !ok || token == nil { return nil, errors.New("cannot get jwt token from session context") } diff --git a/internal/api/user/store/migrations/00001_init.up.sql b/internal/api/user/store/migrations/00001_init.up.sql index 70bf631..b6e3eae 100644 --- a/internal/api/user/store/migrations/00001_init.up.sql +++ b/internal/api/user/store/migrations/00001_init.up.sql @@ -10,8 +10,8 @@ CREATE TABLE users ( CONSTRAINT hash_not_empty CHECK (hash != '') ); -CREATE UNIQUE INDEX users_id On users (id); -CREATE UNIQUE INDEX users_name On users (name); +CREATE UNIQUE INDEX users_id ON users (id); +CREATE UNIQUE INDEX users_name ON users (name); CREATE INDEX users_created_at ON users (created_at); COMMIT; diff --git a/internal/api/user/store/store.go b/internal/api/user/store/store.go index 855f1ad..47b00b4 100644 --- a/internal/api/user/store/store.go +++ b/internal/api/user/store/store.go @@ -16,8 +16,6 @@ import ( ) const ( - saltLen = 10 - bcryptCost = 10 constrainUID = "users_id" @@ -30,19 +28,14 @@ var migrations embed.FS type Store struct { *store.Database logger *zap.Logger - salt []byte } type Config struct { Logger *zap.Logger DSN string - Salt string } func New(ctx context.Context, cfg *Config) (*Store, error) { - if len(cfg.Salt) != saltLen { - return nil, fmt.Errorf("salt length must be %d", saltLen) - } logger := cfg.Logger.With(zap.String("component", "database")) s, err := store.New(ctx, &store.Config{ Logger: logger, @@ -56,7 +49,6 @@ func New(ctx context.Context, cfg *Config) (*Store, error) { return &Store{ Database: s, logger: logger, - salt: []byte(cfg.Salt), }, nil } @@ -111,16 +103,8 @@ func handleDBErr(err error) error { return fmt.Errorf("postgress error: %w", pgErr) } -// salted prepends statically configured string to password. -func (s *Store) salted(pwd string) []byte { - // TODO I dunno if manual salting is helping anything here - // because bcrypt also uses random salt by itself - // and we can't reproduce same hash anyway. - return append(s.salt, []byte(pwd)...) -} - func (s *Store) hashPwd(pwd string) (string, error) { - b, err := bcrypt.GenerateFromPassword(s.salted(pwd), bcryptCost) + b, err := bcrypt.GenerateFromPassword([]byte(pwd), bcryptCost) if err != nil { return "", fmt.Errorf("cannot hash password: %w", err) } @@ -129,7 +113,7 @@ func (s *Store) hashPwd(pwd string) (string, error) { // compare does 'special' bcrypt-comparison of hashes since we cannot compare them directly. func (s *Store) compare(hash, pwd string) error { - if err := bcrypt.CompareHashAndPassword([]byte(hash), s.salted(pwd)); err != nil { + if err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(pwd)); err != nil { if errors.Is(err, bcrypt.ErrMismatchedHashAndPassword) { return model.ErrIncorrectCredentials } diff --git a/internal/api/video/model/status.go b/internal/api/video/model/status.go index ca485c1..f480c38 100644 --- a/internal/api/video/model/status.go +++ b/internal/api/video/model/status.go @@ -8,9 +8,8 @@ import ( // Video statuses. // TODO: May be transitive statuses are redundant? -// TODO: How these statuses will map to DB enums? (or don't use enums may be?) const ( - VideoStatusError = iota - 1 + VideoStatusError Status = iota - 1 VideoStatusCreated VideoStatusUploading VideoStatusUploaded diff --git a/internal/api/video/serviceapi.go b/internal/api/video/serviceapi.go index f00ab30..9328226 100644 --- a/internal/api/video/serviceapi.go +++ b/internal/api/video/serviceapi.go @@ -25,7 +25,7 @@ func (svc *Service) getServiceSession(c echo.Context) error { zap.String("name", claims.Name), zap.String("role", claims.Role)) - if claims.Role == auth.RoleNameService { + if claims.IsService() { svc.logger.Debug("service auth ok") return nil } diff --git a/internal/app/app.go b/internal/app/app.go index c9f21bd..5e6b02b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -180,7 +180,6 @@ func (app *App) setConfigDefaults() { v.SetDefault("api.prefix", "/api") // DB v.SetDefault("database.dsn", "postgres://postgres:postgres@localhost:5432/postgres") - v.SetDefault("database.salt", "changeMe") // Processor v.SetDefault("processor.segment_duration", defaultSegmentDuration) v.SetDefault("processor.video_check_period", defaultVideoCheckInterval) diff --git a/internal/app/user/app.go b/internal/app/user/app.go index 6a3d673..db3d0cb 100644 --- a/internal/app/user/app.go +++ b/internal/app/user/app.go @@ -30,7 +30,6 @@ func (a *App) configure(ctx context.Context) ([]app.Runner, []app.Closer, bool) storeCfg := &store.Config{ Logger: logger, DSN: v.GetURL("database.dsn"), - Salt: v.GetString("database.salt"), } svcCfg := &user.ServiceConfig{ Logger: logger,