Merge pull request #136 from containerish/adhoc-changes

Final changes before release

.github/workflows/golangci-lint.yml

@@ -15,5 +15,5 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.42.1
+          version: v1.45.2
           args: --config golangci.yaml

auth/auth.go

@@ -30,6 +30,7 @@ type Authentication interface {
 	RenewAccessToken(ctx echo.Context) error
 	VerifyEmail(ctx echo.Context) error
 	ResetPassword(ctx echo.Context) error
+	ResetForgottenPassword(ctx echo.Context) error
 	ForgotPassword(ctx echo.Context) error
 	Invites(ctx echo.Context) error
 }

auth/github.go

@@ -3,7 +3,6 @@ package auth
 import (
 	"context"
 	"fmt"
-	"net"
 	"net/http"
 	"time"
 
@@ -38,6 +37,7 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
+
 	// no need to compare the stateToken from QueryParam \w stateToken from a.oauthStateStore
 	// the key is the actual token :p
 	delete(a.oauthStateStore, stateToken)
@@ -93,20 +93,15 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
 
 	oauthUser.Password = refreshToken
 	if err = a.pgStore.AddOAuthUser(ctx.Request().Context(), &oauthUser); err != nil {
-		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
-			"error": err.Error(),
-			"code":  "GH_OAUTH_STORE_OAUTH_USER",
-		})
+		redirectPath := fmt.Sprintf("%s/%s?error=%s", a.c.WebAppEndpoint, a.c.WebAppErrorRedirectPath, err.Error())
+		echoErr := ctx.Redirect(http.StatusTemporaryRedirect, redirectPath)
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
 
 	sessionId := uuid.NewString()
 	if err = a.pgStore.AddSession(ctx.Request().Context(), sessionId, refreshToken, oauthUser.Username); err != nil {
-		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
-			"error":   err.Error(),
-			"message": "ERR_CREATING_SESSION",
-		})
+		echoErr := ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppErrorRedirectPath)
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
@@ -134,23 +129,18 @@ func (a *auth) createCookie(name string, value string, httpOnly bool, expiresAt
 
 	secure := true
 	sameSite := http.SameSiteStrictMode
+	domain := a.c.Registry.FQDN
 	if a.c.Environment == config.Local {
 		secure = false
 		sameSite = http.SameSiteLaxMode
+		domain = "localhost"
 	}
 
-	webappEndpoint := a.c.WebAppEndpoint
-	if a.c.Environment == config.Local {
-		host, _, err := net.SplitHostPort(webappEndpoint)
-		if err != nil {
-			webappEndpoint = host
-		}
-	}
 	cookie := &http.Cookie{
 		Name:     name,
 		Value:    value,
 		Path:     "/",
-		Domain:   webappEndpoint,
+		Domain:   domain,
 		Expires:  expiresAt,
 		Secure:   secure,
 		SameSite: sameSite,

auth/jwt_middleware.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/containerish/OpenRegistry/types"
-	"github.com/fatih/color"
 	"github.com/golang-jwt/jwt"
 	"github.com/labstack/echo/v4"
 	"github.com/labstack/echo/v4/middleware"
@@ -45,7 +44,6 @@ func (a *auth) JWT() echo.MiddlewareFunc {
 		ErrorHandlerWithContext: func(err error, ctx echo.Context) error {
 			// ErrorHandlerWithContext only logs the failing requtest
 			ctx.Set(types.HandlerStartTime, time.Now())
-			color.Red(ctx.QueryParam("token"))
 			a.logger.Log(ctx, err)
 			return ctx.JSON(http.StatusUnauthorized, echo.Map{
 				"error":   err.Error(),

auth/reset_password.go

@@ -13,7 +13,7 @@ import (
 	"github.com/labstack/echo/v4"
 )
 
-func (a *auth) ResetPassword(ctx echo.Context) error {
+func (a *auth) ResetForgottenPassword(ctx echo.Context) error {
 	token, ok := ctx.Get("user").(*jwt.Token)
 	if !ok {
 		err := fmt.Errorf("ERR_EMPTY_TOKEN")
@@ -36,6 +36,18 @@ func (a *auth) ResetPassword(ctx echo.Context) error {
 		return echoErr
 	}
 
+	var pwd *types.Password
+	err := json.NewDecoder(ctx.Request().Body).Decode(&pwd)
+	if err != nil {
+		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
+			"error":   err.Error(),
+			"message": "request body could not be decoded",
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
+	_ = ctx.Request().Body.Close()
+
 	userId := c.Id
 	user, err := a.pgStore.GetUserById(ctx.Request().Context(), userId, true)
 	if err != nil {
@@ -47,79 +59,112 @@ func (a *auth) ResetPassword(ctx echo.Context) error {
 		return echoErr
 	}
 
-	var pwd *types.Password
-	kind := ctx.QueryParam("kind")
+	if err = validatePassword(pwd.NewPassword); err != nil {
+		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
+			"error": err.Error(),
+			"message": `password must be alphanumeric, at least 8 chars long, must have at least one special character 
+and an uppercase letter`,
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
 
-	err = json.NewDecoder(ctx.Request().Body).Decode(&pwd)
-	if err != nil {
+	if a.verifyPassword(user.Password, pwd.NewPassword) {
+
+		err = fmt.Errorf("new password can not be same as old password")
+		// error is already user friendly
 		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
 			"error":   err.Error(),
-			"message": "request body could not be decoded",
+			"message": err.Error(),
 		})
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
-	_ = ctx.Request().Body.Close()
 
-	if err = verifyPassword(pwd.NewPassword); err != nil {
-		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
-			"error": err.Error(),
-			"message": `password must be alphanumeric, at least 8 chars long, must have at least one special character 
-and an uppercase letter`,
+	hashPassword, err := a.hashPassword(pwd.NewPassword)
+	if err != nil {
+		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
+			"error":   err.Error(),
+			"message": "ERR_HASH_NEW_PASSWORD",
 		})
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
 
-	if kind == "forgot_password_callback" {
-		hashPassword, err := a.hashPassword(pwd.NewPassword)
-		if err != nil {
-			echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
-				"error":   err.Error(),
-				"message": "ERR_HASH_NEW_PASSWORD",
-			})
-			a.logger.Log(ctx, err)
-			return echoErr
-		}
+	if err = a.pgStore.UpdateUserPWD(ctx.Request().Context(), userId, hashPassword); err != nil {
+		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
+			"error":   err.Error(),
+			"message": "error updating new password",
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
 
-		if user.Password == hashPassword {
-			err = fmt.Errorf("new password can not be same as old password")
-			// error is already user friendly
-			echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
-				"error":   err.Error(),
-				"message": err.Error(),
-			})
-			a.logger.Log(ctx, err)
-			return echoErr
-		}
+	err = ctx.JSON(http.StatusAccepted, echo.Map{
+		"message": "password changed successfully",
+	})
+	a.logger.Log(ctx, err)
+	return err
+}
 
-		if err = a.pgStore.UpdateUserPWD(ctx.Request().Context(), userId, hashPassword); err != nil {
-			echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
-				"error":   err.Error(),
-				"message": "error updating new password",
-			})
-			a.logger.Log(ctx, err)
-			return echoErr
-		}
+func (a *auth) ResetPassword(ctx echo.Context) error {
+	token, ok := ctx.Get("user").(*jwt.Token)
+	if !ok {
+		err := fmt.Errorf("ERR_EMPTY_TOKEN")
+		echoErr := ctx.JSON(http.StatusUnauthorized, echo.Map{
+			"error":   err.Error(),
+			"message": "JWT token can not be empty",
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
 
-		err = ctx.JSON(http.StatusAccepted, echo.Map{
-			"message": "password changed successfully",
+	c, ok := token.Claims.(*Claims)
+	if !ok {
+		err := fmt.Errorf("ERR_INVALID_CLAIMS")
+		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
+			"error":   err.Error(),
+			"message": "invalid claims in JWT",
 		})
 		a.logger.Log(ctx, err)
-		return err
+		return echoErr
 	}
 
-	if pwd.OldPassword == pwd.NewPassword {
-		err = fmt.Errorf("OLD_NEW_PWD_SAME")
+	var pwd *types.Password
+	err := json.NewDecoder(ctx.Request().Body).Decode(&pwd)
+	if err != nil {
 		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
 			"error":   err.Error(),
-			"message": "new password can not be same as old password",
+			"message": "request body could not be decoded",
 		})
 		a.logger.Log(ctx, err)
 		return echoErr
 	}
+	_ = ctx.Request().Body.Close()
 
-	newHashedPwd, err := a.hashPassword(pwd.NewPassword)
+	userId := c.Id
+	user, err := a.pgStore.GetUserById(ctx.Request().Context(), userId, true)
+	if err != nil {
+		echoErr := ctx.JSON(http.StatusNotFound, echo.Map{
+			"error":   err.Error(),
+			"message": "error getting user by ID from DB",
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
+
+	// compare the current password with password hash from DB
+	if !a.verifyPassword(user.Password, pwd.OldPassword) {
+		err = fmt.Errorf("ERR_WRONG_PASSWORD")
+		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
+			"error":   err.Error(),
+			"message": "password is wrong",
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
+
+	hashPassword, err := a.hashPassword(pwd.NewPassword)
 	if err != nil {
 		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
 			"error":   err.Error(),
@@ -129,7 +174,28 @@ and an uppercase letter`,
 		return echoErr
 	}
 
-	if err = a.pgStore.UpdateUserPWD(ctx.Request().Context(), userId, newHashedPwd); err != nil {
+	if user.Password == hashPassword {
+		err = fmt.Errorf("new password can not be same as old password")
+		// error is already user friendly
+		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
+			"error":   err.Error(),
+			"message": err.Error(),
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
+
+	if err = validatePassword(pwd.NewPassword); err != nil {
+		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
+			"error": err.Error(),
+			"message": `password must be alphanumeric, at least 8 chars long, must have at least one special character 
+and an uppercase letter`,
+		})
+		a.logger.Log(ctx, err)
+		return echoErr
+	}
+
+	if err = a.pgStore.UpdateUserPWD(ctx.Request().Context(), userId, hashPassword); err != nil {
 		echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
 			"error":   err.Error(),
 			"message": "error updating new password",

auth/signup.go

@@ -40,7 +40,7 @@ func (a *auth) SignUp(ctx echo.Context) error {
 		return echoErr
 	}
 
-	if err := verifyPassword(u.Password); err != nil {
+	if err := validatePassword(u.Password); err != nil {
 		// err.Error() is already user friendly
 		echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
 			"error":   err.Error(),
@@ -155,7 +155,7 @@ func verifyEmail(email string) error {
 	return nil
 }
 
-func verifyPassword(password string) error {
+func validatePassword(password string) error {
 	var uppercasePresent bool
 	var lowercasePresent bool
 	var numberPresent bool

config.yaml.example

@@ -2,6 +2,7 @@ environment: local
 debug: true
 web_app_url: "http://localhost:3000"
 web_app_redirect_url: "/"
+web_app_error_redirect_path: "/auth/unhandled"
 registry:
   dns_address: localhost
   version: master

config/config.go

@@ -10,27 +10,35 @@ import (
 
 type (
 	OpenRegistryConfig struct {
-		Registry          *Registry `mapstructure:"registry"`
-		StoreConfig       *Store    `mapstructure:"database"`
-		AuthConfig        *Auth     `mapstructure:"auth"`
-		LogConfig         *Log      `mapstructure:"log_service"`
-		SkynetConfig      *Skynet   `mapstructure:"skynet"`
-		OAuth             *OAuth    `mapstructure:"oauth"`
-		Email             *Email    `mapstructure:"email"`
-		Environment       string    `mapstructure:"environment"`
-		WebAppEndpoint    string    `mapstructure:"web_app_url"`
-		WebAppRedirectURL string    `mapstructure:"web_app_redirect_url"`
-		Debug             bool      `mapstructure:"debug"`
+		Registry                *Registry `mapstructure:"registry"`
+		StoreConfig             *Store    `mapstructure:"database"`
+		AuthConfig              *Auth     `mapstructure:"auth"`
+		LogConfig               *Log      `mapstructure:"log_service"`
+		SkynetConfig            *Skynet   `mapstructure:"skynet"`
+		OAuth                   *OAuth    `mapstructure:"oauth"`
+		Email                   *Email    `mapstructure:"email"`
+		Environment             string    `mapstructure:"environment"`
+		WebAppEndpoint          string    `mapstructure:"web_app_url"`
+		WebAppRedirectURL       string    `mapstructure:"web_app_redirect_url"`
+		WebAppErrorRedirectPath string    `mapstructure:"web_app_error_redirect_path"`
+		Debug                   bool      `mapstructure:"debug"`
 	}
 
 	Registry struct {
+		TLS           TLS      `mapstructure:"tls"`
 		DNSAddress    string   `mapstructure:"dns_address"`
+		FQDN          string   `mapstructure:"fqdn"`
 		SigningSecret string   `mapstructure:"jwt_signing_secret"`
 		Host          string   `mapstructure:"host"`
 		Services      []string `mapstructure:"services"`
 		Port          uint     `mapstructure:"port"`
 	}
 
+	TLS struct {
+		PrivateKey string `mapstructure:"priv_key"`
+		PubKey     string `mapstructure:"pub_key"`
+	}
+
 	Auth struct {
 		SupportedServices map[string]bool `mapstructure:"supported_services"`
 	}

go.mod

@@ -20,10 +20,10 @@ require (
 	golang.org/x/crypto v0.0.0-20210817164053-32db794688a5
 )
 
-require github.com/sendgrid/rest v2.6.9+incompatible // indirect
 
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
+	github.com/sendgrid/rest v2.6.9+incompatible // indirect
 	github.com/cespare/xxhash/v2 v2.1.1 // indirect
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
 	github.com/go-playground/locales v0.14.0 // indirect