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

Csrf #6

Open
wants to merge 6 commits into
base: db2
Choose a base branch
from
Open

Csrf #6

wants to merge 6 commits into from

Conversation

source-Alexander-Rudenko
Copy link
Collaborator

No description provided.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Не очень вышло, что картинки в этом ПРе)

Comment on lines +22 to +25
func LoadEncriptionKey() []byte {
key := os.Getenv("ENCRYPTION_KEY")
return []byte(key)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Еще с Ксюшиным кодом не мерджился? Там просто как раз конфиг был

)

func AuthMiddleware(whitelist []string, authHandler *auth.AuthHandler, next http.Handler) http.Handler {
func AuthWithCSRFMiddleware(whitelist []string, authHandler *auth.AuthHandler, encryptionKey []byte, next http.Handler) http.Handler {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не сразу подумал об этом, а мы не можем передавать сервис в миддлвару вместо хэндлера? На хэндлеры зависимости плохо делать

Comment on lines +296 to +302
func (h *AuthHandler) CheckCSRFMiddleware(ctx context.Context, encryptionKey []byte, s *models.Session, inputToken string) (bool, error) {
return h.service.CheckCSRF(ctx, encryptionKey, s, inputToken)
}

func (h *AuthHandler) CreateCSRFMiddleware(ctx context.Context, encryptionKey []byte, s *models.Session) (string, error) {
return h.service.CreateCSRF(ctx, encryptionKey, s)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Странно называть методы миддлварами, если это не миддлвары. Наверное, можно отказаться от этих методов, если передавать сервис вместо хэндлера в AuthWithCSRFMiddleware?

@@ -22,6 +22,10 @@ type sessionKeyType struct{}

var sessionKey sessionKeyType

type csrfKeyType struct{}

var csrfKey sessionKeyType
Copy link
Collaborator

Choose a reason for hiding this comment

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

Кажется, должно быть csrfKeyType

type TokenData struct {
CSRFtoken string
SessionToken string
UserID int
Copy link
Collaborator

Choose a reason for hiding this comment

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

А юзер айди тут точно должен быть? Вроде он в сессии лежит

return &csrfDB{client: client}
}

func (db *csrfDB) CreateCSRF(ctx context.Context, encryptionKey []byte, s *models.Session) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Токен по-хорошему должен генериться на уровне бизнес-логики. Репозиторий должен только делать CRUD

return token, nil
}

func (db *csrfDB) CheckCSRF(ctx context.Context, encryptionKey []byte, s *models.Session, inputToken string) (bool, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Здесь тоже много логики, явно в сервисный слой надо

return s.Token == td.SessionToken && s.UserID == td.UserID, nil
}

func PrefixedKey(key string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Выглядит вспомогательным методом, если не используется в других пакетах, сделать закрытым

Comment on lines 71 to 77
func (a *authService) CreateCSRF(ctx context.Context, encryptionKey []byte, s *models.Session) (string, error) {
return a.CsrfDB.CreateCSRF(ctx, encryptionKey, s)
}

func (a *authService) CheckCSRF(ctx context.Context, encryptionKey []byte, s *models.Session, inputToken string) (bool, error) {
return a.CsrfDB.CheckCSRF(ctx, encryptionKey, s, inputToken)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Собственно да, тут должна быть логика, которая сейчас в репозиториях

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants