Skip to content

Commit

Permalink
- Smarter error checking on database
Browse files Browse the repository at this point in the history
- Add unique constraint to email field on user model
- Validate token expiry on auth check
  • Loading branch information
jmorganca committed May 27, 2021
1 parent b185674 commit 0ff55b0
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 24 deletions.
18 changes: 9 additions & 9 deletions internal/server/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

type User struct {
ID uint `gorm:"primaryKey"`
Email string `json:"email"`
Email string `json:"email" gorm:"unique"`
Password []byte `json:"-"`
Provider string `json:"provider"`
Created int64 `json:"created" gorm:"autoCreateTime"`
Expand Down Expand Up @@ -49,13 +49,13 @@ func SyncUsers(db *gorm.DB, emails []string, provider string) error {
user.Email = email
user.Provider = "okta"

if result := tx.Save(&user); result.Error != nil {
return result.Error
if err := tx.Save(&user).Error; err != nil {
return err
}

var users []User
if result := tx.Find(&users); result.Error != nil {
return result.Error
if err := tx.Find(&users).Error; err != nil {
return err
}

emailsMap := make(map[string]bool)
Expand All @@ -69,13 +69,13 @@ func SyncUsers(db *gorm.DB, emails []string, provider string) error {
// Only delete user if they don't have built in auth
// TODO (jmorganca): properly refactor this into a provider check
if user.Provider == "okta" && len(user.Password) == 0 {
if result := tx.Delete(&user); result.Error != nil {
return result.Error
if err := tx.Delete(&user).Error; err != nil {
return err
}
} else {
user.Provider = "infra"
if result := tx.Save(&user); result.Error != nil {
return result.Error
if err := tx.Save(&user).Error; err != nil {
return err
}
}
}
Expand Down
44 changes: 29 additions & 15 deletions internal/server/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -45,8 +46,22 @@ func TokenAuthMiddleware(secret []byte) gin.HandlerFunc {
return
}

cl := jwt.Claims{}
out := make(map[string]interface{})
if err := tok.Claims(secret, &out); err != nil {
if err := tok.Claims(secret, &cl, &out); err != nil {
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"})
return
}

err = cl.Validate(jwt.Expected{
Issuer: "infra",
Time: time.Now(),
})
switch {
case errors.Is(err, jwt.ErrExpired):
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "expired"})
return
case err != nil:
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"})
return
}
Expand Down Expand Up @@ -223,13 +238,13 @@ func addRoutes(router *gin.Engine, db *gorm.DB, kube *Kubernetes, cfg *Config, s
}

var user User
if result := db.Where("email = ?", emailParams.Email).First(&user); result.Error != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "incorrect credentials"})
if err := db.Where("email = ?", emailParams.Email).First(&user).Error; err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "unauthorized"})
return
}

if err = bcrypt.CompareHashAndPassword(user.Password, []byte(params.Password)); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "incorrect credentials"})
if err = bcrypt.CompareHashAndPassword(user.Password, []byte(emailParams.Password)); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "unauthorized"})
return
}

Expand All @@ -246,9 +261,8 @@ func addRoutes(router *gin.Engine, db *gorm.DB, kube *Kubernetes, cfg *Config, s

router.GET("/v1/users", TokenAuthMiddleware(settings.TokenSecret), PermissionMiddleware("view", cfg), func(c *gin.Context) {
var users []User
result := db.Find(&users)

if result.Error != nil {
err := db.Find(&users).Error
if err != nil {
c.JSON(http.StatusInternalServerError, ErrorResponse{"could not list users"})
return
}
Expand Down Expand Up @@ -284,8 +298,8 @@ func addRoutes(router *gin.Engine, db *gorm.DB, kube *Kubernetes, cfg *Config, s
user.Password = hashedPassword
user.Provider = "infra"

result := db.Create(&user)
if result.Error != nil {
err = db.Create(&user).Error
if err != nil {
c.JSON(http.StatusInternalServerError, ErrorResponse{"could not create user"})
return
}
Expand Down Expand Up @@ -313,14 +327,14 @@ func addRoutes(router *gin.Engine, db *gorm.DB, kube *Kubernetes, cfg *Config, s
return
}

result := db.Where("email = ?", params.Email).Delete(User{})
if result.Error != nil {
c.JSON(http.StatusInternalServerError, ErrorResponse{"could not delete user"})
err := db.Where("email = ?", params.Email).Delete(User{}).Error
if errors.Is(err, gorm.ErrRecordNotFound) {
c.JSON(http.StatusBadRequest, ErrorResponse{"user does not exist"})
return
}

if result.RowsAffected == 0 {
c.JSON(http.StatusBadRequest, ErrorResponse{"user does not exist"})
if err != nil {
c.JSON(http.StatusInternalServerError, ErrorResponse{"could not delete user"})
return
}

Expand Down

0 comments on commit 0ff55b0

Please sign in to comment.