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

Security add #18

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 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
5 changes: 3 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ toolchain go1.22.0

require (
github.com/alicebob/miniredis/v2 v2.32.1
github.com/dgrijalva/jwt-go v3.2.0+incompatible
github.com/go-redis/redis v6.15.9+incompatible
github.com/golang/mock v1.6.0
github.com/google/uuid v1.6.0
Expand All @@ -15,6 +16,8 @@ require (
github.com/minio/minio-go/v7 v7.0.69
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.9.0
github.com/thejerf/slogassert v0.3.2
golang.org/x/crypto v0.19.0
)

require (
Expand All @@ -33,9 +36,7 @@ require (
github.com/onsi/gomega v1.18.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rs/xid v1.5.0 // indirect
github.com/thejerf/slogassert v0.3.2 // indirect
github.com/yuin/gopher-lua v1.1.1 // indirect
golang.org/x/crypto v0.19.0 // indirect
golang.org/x/net v0.21.0 // indirect
golang.org/x/sys v0.17.0 // indirect
golang.org/x/text v0.14.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
Expand Down
2 changes: 2 additions & 0 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
delivery "github.com/go-park-mail-ru/2024_1_FullFocus/internal/delivery/http"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/logger"
corsmw "github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/middleware/cors"
csrfmw "github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/middleware/csrf"
logmw "github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/middleware/logging"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/repository"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/server"
Expand Down Expand Up @@ -57,6 +58,7 @@ func Init() *App {

r.Use(logmw.NewLoggingMiddleware(log))
r.Use(corsmw.NewCORSMiddleware([]string{}))
r.Use(csrfmw.CSRFMiddleware())

// Redis

Expand Down
55 changes: 55 additions & 0 deletions internal/models/csrf.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package models
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

нужны тесты


import (
"fmt"

"github.com/dgrijalva/jwt-go"
"github.com/pkg/errors"

"time"
)

type JwtToken struct {
Secret []byte
}

func NewJwtToken(secret string) (*JwtToken, error) {
return &JwtToken{Secret: []byte(secret)}, nil
}

type JwtCsrfClaims struct {
SessionID string `json:"sid"`
jwt.StandardClaims
}

func (tk *JwtToken) Create(sID string, tokenExpTime int64) (string, error) {
data := JwtCsrfClaims{
SessionID: sID,
StandardClaims: jwt.StandardClaims{
ExpiresAt: tokenExpTime,
IssuedAt: time.Now().Unix(),
},
}
token := jwt.NewWithClaims(jwt.SigningMethodHS256, data)
return token.SignedString(tk.Secret)
}

func (tk *JwtToken) parseSecretGetter(token *jwt.Token) (interface{}, error) {
method, ok := token.Method.(*jwt.SigningMethodHMAC)
if !ok || method.Alg() != "HS256" {
return nil, errors.New("bad sign method")
}
return tk.Secret, nil
}

func (tk *JwtToken) Check(sID string, inputToken string) (bool, error) {
payload := &JwtCsrfClaims{}
_, err := jwt.ParseWithClaims(inputToken, payload, tk.parseSecretGetter)
if err != nil {
return false, fmt.Errorf("cant parse jwt token: %w", err)
}
if payload.Valid() != nil {
return false, fmt.Errorf("invalid jwt token: %w", err)
}
return payload.SessionID == sID, nil
}
77 changes: 77 additions & 0 deletions internal/pkg/middleware/csrf/csrfmw.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package middleware

import (
"fmt"
"slices"
"time"

csrf "github.com/go-park-mail-ru/2024_1_FullFocus/internal/models"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/logger"
"github.com/gorilla/mux"

"net/http"
)

func CSRFMiddleware() mux.MiddlewareFunc {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
httpMethods := []string{http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

проще проверить, что не GET и не HEAD, нет?

found := slices.Contains(httpMethods, r.Method)
if found {
err := CheckSCRFToken(r)
if err != nil {
logger.Debug(ctx, fmt.Sprintf("csrf token creation error: %v", err))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут тоже, нужно на запрос ответить

return
}
}
if r.Method == http.MethodGet {
err := SetSCRFToken(w, r)
if err != nil {
logger.Debug(ctx, fmt.Sprintf("csrf token creation error: %v", err))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

надо на запрос ответить, не просто завершать функцию

return
}
}
next.ServeHTTP(w, r)
})
}
}

func SetSCRFToken(w http.ResponseWriter, r *http.Request) error {
tokens, _ := csrf.NewJwtToken("qsRY2e4hcM5T7X984E9WQ5uZ8Nty7fxB")
session, err := r.Cookie("session_id")

if err != nil {
return err
}

sID := session.Value
token, err := tokens.Create(sID, time.Now().Add(1*time.Hour).Unix())

if err != nil {
return err
}

w.Header().Set("X-Csrf-Token", token)

return nil
}

func CheckSCRFToken(r *http.Request) error {
tokens, _ := csrf.NewJwtToken("qsRY2e4hcM5T7X984E9WQ5uZ8Nty7fxB")
session, err := r.Cookie("session_id")

if err != nil {
return err
}

sID := session.Value
csrfToken := r.FormValue("X-Csrf-Token")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо из хедера брать, у вас формы не используются в запросах

_, err = tokens.Check(sID, csrfToken)

if err != nil {
return err
}

return nil
}
37 changes: 34 additions & 3 deletions internal/usecase/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,34 @@ package usecase
import (
"context"
"crypto/md5"
"crypto/rand"
"encoding/hex"
"strconv"
"errors"

"github.com/go-park-mail-ru/2024_1_FullFocus/internal/models"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/pkg/helper"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/repository"

"golang.org/x/crypto/argon2"

"strconv"
)

const (
_minLoginLength = 4
_maxLoginLength = 32
_minPasswordLength = 8
_maxPasswordLength = 32
_countBytes = 8
_countMemory = 65536
_countTreads = 4
_countKeyLen = 32
)

func PasswordArgon2(plainPassword []byte, salt []byte) []byte {
return argon2.IDKey(plainPassword, salt, 1, _countMemory, _countTreads, _countKeyLen)
}

type AuthUsecase struct {
userRepo repository.Users
sessionRepo repository.Sessions
Expand All @@ -41,13 +54,20 @@ func (u *AuthUsecase) Login(ctx context.Context, login string, password string)
return "", models.NewValidationError("invalid password input",
"Пароль должен содержать от 8 до 32 букв английского алфавита или цифр")
}

user, err := u.userRepo.GetUser(ctx, login)
if err != nil {
return "", models.ErrNoUser
}
if password != user.Password {

salt := ([]byte(user.Password))[0:8]
passwordHash := PasswordArgon2([]byte(password), salt)
saltWithPasswordHash := string(salt) + string(passwordHash)

if saltWithPasswordHash != user.Password {
return "", models.ErrWrongPassword
}

return u.sessionRepo.CreateSession(ctx, user.ID), nil
}

Expand All @@ -62,10 +82,20 @@ func (u *AuthUsecase) Signup(ctx context.Context, login string, password string)
return "", "", models.NewValidationError("invalid password input",
"Пароль должен содержать от 8 до 32 букв английского алфавита или цифр")
}

salt := make([]byte, _countBytes)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

мне кажется или соль при логине и при регистрации отличаются, если так, то это не будет работать

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

сделай 1 функцию, которая хеширует пароль. при регистрации просто пришедший пароль хешируешь и дальше по цепочке передаешь. а при логине используешь эту же функцию для пришедшего пароля

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему не будет работать? Сначала в signup рандомно генерим соль, после чего хешируем и клеим соль+хеш
А в login вытаскиваем из пароля соль, хешируем пришедший пароль, клеим соль + хеш от пришедшего пароля и сравниваем полученное значение со значением из базы
Вроде все корректно работать должно

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERROR: invalid byte sequence for encoding "UTF8": 0x83 (SQLSTATE 22021)
На кодер из лекции ругается, я перепробовал кучу способов исправить, но там рандомно ставится соль => формируется эта ошибка при SQL запросе, хотя руками вносится нормально

_, errSalt := rand.Read(salt)
if errSalt != nil {
return "", "", errors.New("err with making salt")
}
passwordHash := PasswordArgon2([]byte(password), salt)
saltWithPasswordHash := string(salt) + string(passwordHash)

user := models.User{
Username: login,
Password: password,
Password: saltWithPasswordHash,
}

uID, err := u.userRepo.CreateUser(ctx, user)
if err != nil {
return "", "", models.ErrUserAlreadyExists
Expand All @@ -74,6 +104,7 @@ func (u *AuthUsecase) Signup(ctx context.Context, login string, password string)

uIDHash := md5.Sum([]byte(strconv.Itoa(int(uID))))
stringUID := hex.EncodeToString(uIDHash[:])

return sID, stringUID, nil
}

Expand Down
31 changes: 17 additions & 14 deletions internal/usecase/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ import (
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/models"
mock_repository "github.com/go-park-mail-ru/2024_1_FullFocus/internal/repository/mocks"
"github.com/go-park-mail-ru/2024_1_FullFocus/internal/usecase"

"golang.org/x/crypto/argon2"
)

func PasswordArgon2(plainPassword []byte, salt []byte) []byte {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему в тестах эта функция дублируется

return argon2.IDKey(plainPassword, salt, 1, 64*1024, 4, 32)
}

func TestNewAuthUsecase(t *testing.T) {
t.Run("Check Auth Usecase creation", func(t *testing.T) {
ctrl := gomock.NewController(t)
Expand Down Expand Up @@ -41,8 +47,8 @@ func TestSignUp(t *testing.T) {
name: "Check valid user signup",
login: "test123",
password: "Qa5yAbrLhkwT4Y9u",
userMockBehavior: func(r *mock_repository.MockUsers, user models.User) {
r.EXPECT().CreateUser(context.Background(), user).Return(uint(0), nil)
userMockBehavior: func(r *mock_repository.MockUsers, _ models.User) {
r.EXPECT().CreateUser(context.Background(), gomock.Any()).Return(uint(0), nil)
},
sessionMockBehavior: func(r *mock_repository.MockSessions, userID uint) {
r.EXPECT().CreateSession(context.Background(), userID).Return("123")
Expand All @@ -56,8 +62,8 @@ func TestSignUp(t *testing.T) {
name: "Check valid user signup",
login: "test123",
password: "testtest1",
userMockBehavior: func(r *mock_repository.MockUsers, user models.User) {
r.EXPECT().CreateUser(context.Background(), user).Return(uint(0), nil)
userMockBehavior: func(r *mock_repository.MockUsers, _ models.User) {
r.EXPECT().CreateUser(context.Background(), gomock.Any()).Return(uint(0), nil)
},
sessionMockBehavior: func(r *mock_repository.MockSessions, userID uint) {
r.EXPECT().CreateSession(context.Background(), userID).Return("123")
Expand All @@ -71,8 +77,8 @@ func TestSignUp(t *testing.T) {
name: "Check duplicate user signup",
login: "test123",
password: "Qa5yAbrLhkwT4Y9u",
userMockBehavior: func(r *mock_repository.MockUsers, user models.User) {
r.EXPECT().CreateUser(context.Background(), user).Return(uint(0), models.ErrUserAlreadyExists)
userMockBehavior: func(r *mock_repository.MockUsers, _ models.User) {
r.EXPECT().CreateUser(context.Background(), gomock.Any()).Return(uint(0), models.ErrUserAlreadyExists)
},
expectedSID: "",
expectedErr: models.ErrUserAlreadyExists,
Expand Down Expand Up @@ -114,15 +120,11 @@ func TestSignUp(t *testing.T) {
defer ctrl.Finish()
mockUserRepo := mock_repository.NewMockUsers(ctrl)
mockSessionRepo := mock_repository.NewMockSessions(ctrl)
testUser := models.User{
ID: 0,
Username: testCase.login,
Password: testCase.password,
}
someUser := models.User{}
if testCase.callUserMock {
testCase.userMockBehavior(mockUserRepo, testUser)
testCase.userMockBehavior(mockUserRepo, someUser)
if testCase.callSessionMock {
testCase.sessionMockBehavior(mockSessionRepo, testUser.ID)
testCase.sessionMockBehavior(mockSessionRepo, someUser.ID)
}
}
au := usecase.NewAuthUsecase(mockUserRepo, mockSessionRepo)
Expand Down Expand Up @@ -150,7 +152,8 @@ func TestLogin(t *testing.T) {
login: "test123",
password: "test12345",
userMockBehavior: func(r *mock_repository.MockUsers, username string) {
r.EXPECT().GetUser(context.Background(), username).Return(models.User{ID: 0, Username: "test123", Password: "test12345"}, nil)
r.EXPECT().GetUser(context.Background(), username).Return(models.User{ID: 0, Username: "test123",
Password: string([]byte{0xd7, 0xc2, 0xf2, 0x51, 0xaa, 0x6a, 0x4e, 0x7b}) + string(PasswordArgon2([]byte("test12345"), []byte{0xd7, 0xc2, 0xf2, 0x51, 0xaa, 0x6a, 0x4e, 0x7b}))}, nil)
},
sessionMockBehavior: func(r *mock_repository.MockSessions, userID uint) {
r.EXPECT().CreateSession(context.Background(), userID).Return("123")
Expand Down