Skip to content

Commit

Permalink
rm manual password salting, minor fixes and refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
adwski committed Jan 26, 2024
1 parent e1f2e88 commit 2c8ee68
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 52 deletions.
3 changes: 2 additions & 1 deletion internal/api/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
29 changes: 9 additions & 20 deletions internal/api/store/migrate.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package store

import (
"embed"
"errors"
"fmt"

Expand All @@ -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
}
16 changes: 10 additions & 6 deletions internal/api/user/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const (

jwtCookieName = "vidiSessID"

SessionContextKey = "vUser"
sessionContextKey = "vUser"

RoleNameService = "service"
roleNameService = "service"
)

// Auth us an authenticator that can
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()),
},
Expand All @@ -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,
Expand All @@ -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!
Expand All @@ -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")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/api/user/store/migrations/00001_init.up.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
20 changes: 2 additions & 18 deletions internal/api/user/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
)

const (
saltLen = 10

bcryptCost = 10

constrainUID = "users_id"
Expand All @@ -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,
Expand All @@ -56,7 +49,6 @@ func New(ctx context.Context, cfg *Config) (*Store, error) {
return &Store{
Database: s,
logger: logger,
salt: []byte(cfg.Salt),
}, nil
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions internal/api/video/model/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/api/video/serviceapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 0 additions & 1 deletion internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion internal/app/user/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 2c8ee68

Please sign in to comment.