Skip to content

Commit

Permalink
changed SHA256 to bcrypt
Browse files Browse the repository at this point in the history
  • Loading branch information
yakuter committed Jun 7, 2020
1 parent f8e0ecd commit 36296f2
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 140 deletions.
71 changes: 67 additions & 4 deletions internal/api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,74 @@ var (
InvalidToken = "Token is expired or not valid!"
NoToken = "Token could not found! "
TokenCreateErr = "Token could not be created"
SignupSuccess = "User created successfully"
)

// Signup ...
func Signup(s storage.Store) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {

userDTO := new(model.UserDTO)

// 1. Decode request body to userDTO object
decoder := json.NewDecoder(r.Body)
if err := decoder.Decode(&userDTO); err != nil {
RespondWithError(w, http.StatusBadRequest, "Invalid resquest payload")
return
}
defer r.Body.Close()

// 2. Run validator according to model.UserDTO validator tags
validate := validator.New()
validateError := validate.Struct(userDTO)
if validateError != nil {
errs := GetErrors(validateError.(validator.ValidationErrors))
RespondWithErrors(w, http.StatusBadRequest, InvalidRequestPayload, errs)
return
}

// 3. Check if user exist in database
_, err := s.Users().FindByEmail(userDTO.Email)
if err == nil {

This comment has been minimized.

Copy link
@mrtrkmn

mrtrkmn Jun 8, 2020

Contributor

This is very strange if err == nil ?

This comment has been minimized.

Copy link
@yakuter

yakuter Jun 9, 2020

Author Collaborator

Well even it seems weird it is working. if err == nil means there is a record in the database. Because "record not found" is an error in gorm. So I return respond with error. However with this way it seems I also overwritten the server internal errors. If you have suggestion I would like to here for thies.

errs := []string{"This email is already used!"}
message := "User couldn't created!"
RespondWithErrors(w, http.StatusBadRequest, message, errs)
return
}

// 4. Create new user
createdUser, err := app.CreateUser(s, userDTO)
if err != nil {
RespondWithError(w, http.StatusInternalServerError, err.Error())
return
}

// 5. Update user once to generate schema
updatedUser, err := app.GenerateSchema(s, createdUser)
if err != nil {
RespondWithError(w, http.StatusInternalServerError, err.Error())
return
}

// 6. Create user schema and tables
err = s.Users().CreateSchema(updatedUser.Schema)
if err != nil {
RespondWithError(w, http.StatusInternalServerError, err.Error())
return
}

// 7. Create user tables in user schema
app.MigrateUserTables(s, updatedUser.Schema)

response := model.Response{
Code: http.StatusOK,
Status: Success,
Message: SignupSuccess,
}
RespondWithJSON(w, http.StatusOK, response)
}
}

// Signin ...
func Signin(s storage.Store) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -43,13 +109,10 @@ func Signin(s storage.Store) http.HandlerFunc {
return
}

// Create hash from master password
loginDTO.MasterPassword = app.NewSHA256([]byte(loginDTO.MasterPassword))

// Check if user exist in database and credentials are true
user, err := s.Users().FindByCredentials(loginDTO.Email, loginDTO.MasterPassword)
if err != nil {
RespondWithError(w, http.StatusUnauthorized, InvalidUser)
RespondWithError(w, http.StatusUnauthorized, err.Error())
return
}

Expand Down
14 changes: 7 additions & 7 deletions internal/api/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func FindAllLogins(s storage.Store) http.HandlerFunc {
return
}

loginList = app.DecryptLoginPasswords(loginList)
// loginList = app.DecryptLoginPasswords(loginList)
RespondWithJSON(w, http.StatusOK, loginList)
}
}
Expand All @@ -55,13 +55,13 @@ func FindLoginsByID(s storage.Store) http.HandlerFunc {
return
}

uLogin, err := app.DecryptLoginPassword(s, login)
if err != nil {
RespondWithError(w, http.StatusInternalServerError, err.Error())
return
}
// uLogin, err := app.DecryptLoginPassword(s, login)
// if err != nil {
// RespondWithError(w, http.StatusInternalServerError, err.Error())
// return
// }

RespondWithJSON(w, http.StatusOK, model.ToLoginDTO(uLogin))
RespondWithJSON(w, http.StatusOK, model.ToLoginDTO(login))
}
}

Expand Down
80 changes: 0 additions & 80 deletions internal/api/signup.go

This file was deleted.

11 changes: 5 additions & 6 deletions internal/app/encryption.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import (
"crypto/cipher"
"crypto/md5"
"crypto/rand"
"crypto/sha256"
"encoding/hex"
"io"
"io/ioutil"
"os"

"github.com/sethvargo/go-password/password"
"golang.org/x/crypto/bcrypt"

"github.com/spf13/viper"
)
Expand All @@ -36,11 +36,10 @@ func Password() (string, error) {
return res, nil
}

// NewSHA256 ...
func NewSHA256(key []byte) string {
hasher := sha256.New()
hasher.Write([]byte(key))
return hex.EncodeToString(hasher.Sum(nil))
// NewBcrypt ...
func NewBcrypt(key []byte) string {
hasher, _ := bcrypt.GenerateFromPassword(key, bcrypt.DefaultCost)
return string(hasher)
}

// CreateHash ...
Expand Down
24 changes: 0 additions & 24 deletions internal/app/encryption_test.go

This file was deleted.

4 changes: 2 additions & 2 deletions internal/app/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func CreateLogin(s storage.Store, dto *model.LoginDTO, schema string) (*model.Login, error) {

rawPass := dto.Password
dto.Password = base64.StdEncoding.EncodeToString(Encrypt(dto.Password, viper.GetString("server.passphrase")))
// dto.Password = base64.StdEncoding.EncodeToString(Encrypt(dto.Password, viper.GetString("server.passphrase")))

createdLogin, err := s.Logins().Save(model.ToLogin(dto), schema)
if err != nil {
Expand All @@ -35,7 +35,7 @@ func UpdateLogin(s storage.Store, login *model.Login, dto *model.LoginDTO, schem
dto.Password = generatedPass
}
rawPass := dto.Password
dto.Password = base64.StdEncoding.EncodeToString(Encrypt(dto.Password, viper.GetString("server.passphrase")))
// dto.Password = base64.StdEncoding.EncodeToString(Encrypt(dto.Password, viper.GetString("server.passphrase")))

login.URL = dto.URL
login.Username = dto.Username
Expand Down
6 changes: 3 additions & 3 deletions internal/app/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func CreateUser(s storage.Store, userDTO *model.UserDTO) (*model.User, error) {

// Hasing the master password with SHA256
userDTO.MasterPassword = NewSHA256([]byte(userDTO.MasterPassword))
userDTO.MasterPassword = NewBcrypt([]byte(userDTO.MasterPassword))

// New user's plan is Free and role is Member (not Admin)
userDTO.Plan = "Free"

This comment has been minimized.

Copy link
@mrtrkmn

mrtrkmn Jun 8, 2020

Contributor

Hmm, I think, casbin integration could be much nicer, of course, for the moment, the priority could be low however, as project grows, it may require to have. Good to note over here; https://github.com/casbin/casbin

Example implementation.
https://zupzup.org/casbin-http-role-auth/

Expand All @@ -33,8 +33,8 @@ func CreateUser(s storage.Store, userDTO *model.UserDTO) (*model.User, error) {
func UpdateUser(s storage.Store, user *model.User, userDTO *model.UserDTO, isAuthorized bool) (*model.User, error) {

// TODO: Refactor the contents of updated user with a logical way
if userDTO.MasterPassword != "" && NewSHA256([]byte(userDTO.MasterPassword)) != user.MasterPassword {
userDTO.MasterPassword = NewSHA256([]byte(userDTO.MasterPassword))
if userDTO.MasterPassword != "" && NewBcrypt([]byte(userDTO.MasterPassword)) != user.MasterPassword {
userDTO.MasterPassword = NewBcrypt([]byte(userDTO.MasterPassword))
} else {
userDTO.MasterPassword = user.MasterPassword
}
Expand Down
16 changes: 4 additions & 12 deletions internal/router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,10 @@ func (r *Router) initRoutes() {

// Auth endpoints
authRouter := mux.NewRouter().PathPrefix("/auth").Subrouter()
authRouter.HandleFunc("/signin", api.Signin(r.store))
authRouter.HandleFunc("/refresh", api.RefreshToken(r.store))
authRouter.HandleFunc("/check", api.CheckToken(r.store))

// Web public endpoints
webRouter := mux.NewRouter().PathPrefix("/web").Subrouter()
webRouter.HandleFunc("/signup", api.Signup(r.store)).Methods(http.MethodPost)
authRouter.HandleFunc("/signup", api.Signup(r.store)).Methods(http.MethodPost)
authRouter.HandleFunc("/signin", api.Signin(r.store)).Methods(http.MethodPost)
authRouter.HandleFunc("/refresh", api.RefreshToken(r.store)).Methods(http.MethodPost)
authRouter.HandleFunc("/check", api.CheckToken(r.store)).Methods(http.MethodPost)

n := negroni.Classic()
n.Use(negroni.HandlerFunc(CORS))
Expand All @@ -114,11 +111,6 @@ func (r *Router) initRoutes() {
negroni.Wrap(authRouter),
))

r.router.PathPrefix("/web").Handler(n.With(
LimitHandler(),
negroni.Wrap(webRouter),
))

// Insecure endpoints
r.router.HandleFunc("/health", api.HealthCheck(r.store)).Methods(http.MethodGet)

Expand Down
15 changes: 13 additions & 2 deletions internal/storage/user/user_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/jinzhu/gorm"
"github.com/pass-wall/passwall-server/model"
"golang.org/x/crypto/bcrypt"
)

// Repository ...
Expand Down Expand Up @@ -66,8 +67,18 @@ func (p *Repository) FindByEmail(email string) (*model.User, error) {
// FindByCredentials ...
func (p *Repository) FindByCredentials(email, masterPassword string) (*model.User, error) {
user := new(model.User)
err := p.db.Where(`email = ? AND master_password = ?`, email, masterPassword).First(&user).Error
return user, err
err := p.db.Where(`email = ?`, email).First(&user).Error
if err != nil {
return user, err
}

// Comparing the password with the bcrypt hash
err = bcrypt.CompareHashAndPassword([]byte(user.MasterPassword), []byte(masterPassword))
if err != nil {
return user, err
}

return user, nil
}

// Save ...
Expand Down

0 comments on commit 36296f2

Please sign in to comment.