refactor: handle url queries null values better

Change forwardauth endpoint in example compose

cmd/user/create/create.go

@@ -13,10 +13,12 @@ import (
 )
 
 var interactive bool
-var username string
-var password string
 var docker bool
 
+// i stands for input
+var iUsername string
+var iPassword string
+
 var CreateCmd = &cobra.Command{
 	Use:   "create",
 	Short: "Create a user",
@@ -30,13 +32,13 @@ var CreateCmd = &cobra.Command{
 			// Create huh form
 			form := huh.NewForm(
 				huh.NewGroup(
-					huh.NewInput().Title("Username").Value(&username).Validate((func(s string) error {
+					huh.NewInput().Title("Username").Value(&iUsername).Validate((func(s string) error {
 						if s == "" {
 							return errors.New("username cannot be empty")
 						}
 						return nil
 					})),
-					huh.NewInput().Title("Password").Value(&password).Validate((func(s string) error {
+					huh.NewInput().Title("Password").Value(&iPassword).Validate((func(s string) error {
 						if s == "" {
 							return errors.New("password cannot be empty")
 						}
@@ -57,20 +59,21 @@ var CreateCmd = &cobra.Command{
 		}
 
 		// Do we have username and password?
-		if username == "" || password == "" {
+		if iUsername == "" || iPassword == "" {
 			log.Error().Msg("Username and password cannot be empty")
 		}
 
-		log.Info().Str("username", username).Str("password", password).Bool("docker", docker).Msg("Creating user")
+		log.Info().Str("username", iUsername).Str("password", iPassword).Bool("docker", docker).Msg("Creating user")
 
 		// Hash password
-		passwordByte, passwordErr := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
+		password, passwordErr := bcrypt.GenerateFromPassword([]byte(iPassword), bcrypt.DefaultCost)
 
 		if passwordErr != nil {
 			log.Fatal().Err(passwordErr).Msg("Failed to hash password")
 		}
 
-		passwordString := string(passwordByte)
+		// Convert password to string
+		passwordString := string(password)
 
 		// Escape $ for docker
 		if docker {
@@ -78,14 +81,14 @@ var CreateCmd = &cobra.Command{
 		}
 
 		// Log user created
-		log.Info().Str("user", fmt.Sprintf("%s:%s", username, passwordString)).Msg("User created")
+		log.Info().Str("user", fmt.Sprintf("%s:%s", iUsername, passwordString)).Msg("User created")
 	},
 }
 
 func init() {
 	// Flags
-	CreateCmd.Flags().BoolVar(&interactive, "interactive", false, "Create a user interactively")
+	CreateCmd.Flags().BoolVarP(&interactive, "interactive", "i", false, "Create a user interactively")
 	CreateCmd.Flags().BoolVar(&docker, "docker", false, "Format output for docker")
-	CreateCmd.Flags().StringVar(&username, "username", "", "Username")
-	CreateCmd.Flags().StringVar(&password, "password", "", "Password")
+	CreateCmd.Flags().StringVar(&iUsername, "username", "", "Username")
+	CreateCmd.Flags().StringVar(&iPassword, "password", "", "Password")
 }

cmd/user/verify/verify.go

@@ -12,10 +12,12 @@ import (
 )
 
 var interactive bool
-var username string
-var password string
 var docker bool
-var user string
+
+// i stands for input
+var iUsername string
+var iPassword string
+var iUser string
 
 var VerifyCmd = &cobra.Command{
 	Use:   "verify",
@@ -30,19 +32,19 @@ var VerifyCmd = &cobra.Command{
 			// Create huh form
 			form := huh.NewForm(
 				huh.NewGroup(
-					huh.NewInput().Title("User (username:hash)").Value(&user).Validate((func(s string) error {
+					huh.NewInput().Title("User (username:hash)").Value(&iUser).Validate((func(s string) error {
 						if s == "" {
 							return errors.New("user cannot be empty")
 						}
 						return nil
 					})),
-					huh.NewInput().Title("Username").Value(&username).Validate((func(s string) error {
+					huh.NewInput().Title("Username").Value(&iUsername).Validate((func(s string) error {
 						if s == "" {
 							return errors.New("username cannot be empty")
 						}
 						return nil
 					})),
-					huh.NewInput().Title("Password").Value(&password).Validate((func(s string) error {
+					huh.NewInput().Title("Password").Value(&iPassword).Validate((func(s string) error {
 						if s == "" {
 							return errors.New("password cannot be empty")
 						}
@@ -63,28 +65,28 @@ var VerifyCmd = &cobra.Command{
 		}
 
 		// Do we have username, password and user?
-		if username == "" || password == "" || user == "" {
+		if iUsername == "" || iPassword == "" || iUser == "" {
 			log.Fatal().Msg("Username, password and user cannot be empty")
 		}
 
-		log.Info().Str("user", user).Str("username", username).Str("password", password).Bool("docker", docker).Msg("Verifying user")
+		log.Info().Str("user", iUser).Str("username", iUsername).Str("password", iPassword).Bool("docker", docker).Msg("Verifying user")
 
-		// Split username and password
-		userSplit := strings.Split(user, ":")
+		// Split username and password hash
+		username, hash, ok := strings.Cut(iUser, ":")
 
-		if userSplit[1] == "" {
+		if !ok {
 			log.Fatal().Msg("User is not formatted correctly")
 		}
 
 		// Replace $$ with $ if formatted for docker
 		if docker {
-			userSplit[1] = strings.ReplaceAll(userSplit[1], "$$", "$")
+			hash = strings.ReplaceAll(hash, "$$", "$")
 		}
 
 		// Compare username and password
-		verifyErr := bcrypt.CompareHashAndPassword([]byte(userSplit[1]), []byte(password))
+		verifyErr := bcrypt.CompareHashAndPassword([]byte(hash), []byte(iPassword))
 
-		if verifyErr != nil || username != userSplit[0] {
+		if verifyErr != nil || username != iUsername {
 			log.Fatal().Msg("Username or password incorrect")
 		} else {
 			log.Info().Msg("Verification successful")
@@ -96,7 +98,7 @@ func init() {
 	// Flags
 	VerifyCmd.Flags().BoolVarP(&interactive, "interactive", "i", false, "Create a user interactively")
 	VerifyCmd.Flags().BoolVar(&docker, "docker", false, "Is the user formatted for docker?")
-	VerifyCmd.Flags().StringVar(&username, "username", "", "Username")
-	VerifyCmd.Flags().StringVar(&password, "password", "", "Password")
-	VerifyCmd.Flags().StringVar(&user, "user", "", "Hash (username:hash combination)")
+	VerifyCmd.Flags().StringVar(&iUsername, "username", "", "Username")
+	VerifyCmd.Flags().StringVar(&iPassword, "password", "", "Password")
+	VerifyCmd.Flags().StringVar(&iUser, "user", "", "Hash (username:hash combination)")
 }

docker-compose.dev.yml

@@ -30,4 +30,4 @@ services:
       traefik.enable: true
       traefik.http.routers.tinyauth.rule: Host(`tinyauth.dev.local`)
       traefik.http.services.tinyauth.loadbalancer.server.port: 3000
-      traefik.http.middlewares.tinyauth.forwardauth.address: http://tinyauth:3000/api/auth/traefik
+      traefik.http.middlewares.tinyauth.forwardauth.address: http://tinyauth:3000/api/auth

docker-compose.example.yml

@@ -28,4 +28,4 @@ services:
       traefik.enable: true
       traefik.http.routers.tinyauth.rule: Host(`tinyauth.example.com`)
       traefik.http.services.tinyauth.loadbalancer.server.port: 3000
-      traefik.http.middlewares.tinyauth.forwardauth.address: http://tinyauth:3000/api/auth/traefik
+      traefik.http.middlewares.tinyauth.forwardauth.address: http://tinyauth:3000/api/auth

internal/api/api.go

@@ -121,7 +121,12 @@ func (api *API) SetupRoutes() {
 		bindErr := c.BindUri(&proxy)
 
 		// Handle error
-		if api.handleError(c, "Failed to bind URI", bindErr) {
+		if bindErr != nil {
+			log.Error().Err(bindErr).Msg("Failed to bind URI")
+			c.JSON(400, gin.H{
+				"status":  400,
+				"message": "Bad Request",
+			})
 			return
 		}
 
@@ -130,6 +135,9 @@ func (api *API) SetupRoutes() {
 		// Get user context
 		userContext := api.Hooks.UseUserContext(c)
 
+		// Check if using basic auth
+		_, _, basicAuth := c.Request.BasicAuth()
+
 		// Get headers
 		uri := c.Request.Header.Get("X-Forwarded-Uri")
 		proto := c.Request.Header.Get("X-Forwarded-Proto")
@@ -144,8 +152,8 @@ func (api *API) SetupRoutes() {
 
 			// Check if there was an error
 			if appAllowedErr != nil {
-				// Return 501 if nginx is the proxy or if the request is using an Authorization header
-				if proxy.Proxy == "nginx" || c.GetHeader("Authorization") != "" {
+				// Return 501 if nginx is the proxy or if the request is using basic auth
+				if proxy.Proxy == "nginx" || basicAuth {
 					log.Error().Err(appAllowedErr).Msg("Failed to check if app is allowed")
 					c.JSON(501, gin.H{
 						"status":  501,
@@ -166,36 +174,24 @@ func (api *API) SetupRoutes() {
 			if !appAllowed {
 				log.Warn().Str("username", userContext.Username).Str("host", host).Msg("User not allowed")
 
+				// Return 401 if nginx is the proxy or if the request is using an Authorization header
+				if proxy.Proxy == "nginx" || basicAuth {
+					c.Header("WWW-Authenticate", "Basic realm=\"tinyauth\"")
+					c.JSON(401, gin.H{
+						"status":  401,
+						"message": "Unauthorized",
+					})
+					return
+				}
+
 				// Build query
 				queries, queryErr := query.Values(types.UnauthorizedQuery{
 					Username: userContext.Username,
 					Resource: strings.Split(host, ".")[0],
 				})
 
-				// Check if there was an error
-				if queryErr != nil {
-					// Return 501 if nginx is the proxy or if the request is using an Authorization header
-					if proxy.Proxy == "nginx" || c.GetHeader("Authorization") != "" {
-						log.Error().Err(queryErr).Msg("Failed to build query")
-						c.JSON(501, gin.H{
-							"status":  501,
-							"message": "Internal Server Error",
-						})
-						return
-					}
-
-					// Return the internal server error page
-					if api.handleError(c, "Failed to build query", queryErr) {
-						return
-					}
-				}
-
-				// Return 401 if nginx is the proxy or if the request is using an Authorization header
-				if proxy.Proxy == "nginx" || c.GetHeader("Authorization") != "" {
-					c.JSON(401, gin.H{
-						"status":  401,
-						"message": "Unauthorized",
-					})
+				// Handle error (no need to check for nginx/headers since we are sure we are using caddy/traefik)
+				if api.handleError(c, "Failed to build query", queryErr) {
 					return
 				}
 
@@ -220,7 +216,8 @@ func (api *API) SetupRoutes() {
 		log.Debug().Msg("Unauthorized")
 
 		// Return 401 if nginx is the proxy or if the request is using an Authorization header
-		if proxy.Proxy == "nginx" || c.GetHeader("Authorization") != "" {
+		if proxy.Proxy == "nginx" || basicAuth {
+			c.Header("WWW-Authenticate", "Basic realm=\"tinyauth\"")
 			c.JSON(401, gin.H{
 				"status":  401,
 				"message": "Unauthorized",
@@ -233,13 +230,13 @@ func (api *API) SetupRoutes() {
 			RedirectURI: fmt.Sprintf("%s://%s%s", proto, host, uri),
 		})
 
-		log.Debug().Interface("redirect_uri", fmt.Sprintf("%s://%s%s", proto, host, uri)).Msg("Redirecting to login")
-
 		// Handle error (no need to check for nginx/headers since we are sure we are using caddy/traefik)
 		if api.handleError(c, "Failed to build query", queryErr) {
 			return
 		}
 
+		log.Debug().Interface("redirect_uri", fmt.Sprintf("%s://%s%s", proto, host, uri)).Msg("Redirecting to login")
+
 		// Redirect to login
 		c.Redirect(http.StatusTemporaryRedirect, fmt.Sprintf("%s/?%s", api.Config.AppURL, queries.Encode()))
 	})
@@ -338,6 +335,7 @@ func (api *API) SetupRoutes() {
 		// We are not logged in so return unauthorized
 		if !userContext.IsLoggedIn {
 			log.Debug().Msg("Unauthorized")
+			c.Header("WWW-Authenticate", "Basic realm=\"tinyauth\"")
 			c.JSON(200, gin.H{
 				"status":              200,
 				"message":             "Unauthorized",

internal/auth/auth.go

@@ -211,38 +211,18 @@ func (auth *Auth) ResourceAllowed(context types.UserContext, host string) (bool,
 	return true, nil
 }
 
-func (auth *Auth) GetBasicAuth(c *gin.Context) types.User {
+func (auth *Auth) GetBasicAuth(c *gin.Context) *types.User {
 	// Get the Authorization header
-	header := c.GetHeader("Authorization")
+	username, password, ok := c.Request.BasicAuth()
 
-	// If the header is empty, return an empty user
-	if header == "" {
-		return types.User{}
-	}
-
-	// Split the header
-	headerSplit := strings.Split(header, " ")
-
-	if len(headerSplit) != 2 {
-		return types.User{}
-	}
-
-	// Check if the header is Basic
-	if headerSplit[0] != "Basic" {
-		return types.User{}
-	}
-
-	// Split the credentials
-	credentials := strings.Split(headerSplit[1], ":")
-
-	// If the credentials are not in the correct format, return an empty user
-	if len(credentials) != 2 {
-		return types.User{}
+	// If not ok, return an empty user
+	if !ok {
+		return nil
 	}
 
 	// Return the user
-	return types.User{
-		Username: credentials[0],
-		Password: credentials[1],
+	return &types.User{
+		Username: username,
+		Password: password,
 	}
 }

internal/hooks/hooks.go

@@ -27,7 +27,7 @@ func (hooks *Hooks) UseUserContext(c *gin.Context) types.UserContext {
 	basic := hooks.Auth.GetBasicAuth(c)
 
 	// Check if basic auth is set
-	if basic.Username != "" {
+	if basic != nil {
 		log.Debug().Msg("Got basic auth")
 
 		// Check if user exists and password is correct

site/src/pages/continue-page.tsx

@@ -8,7 +8,7 @@ import { ReactNode } from "react";
 export const ContinuePage = () => {
   const queryString = window.location.search;
   const params = new URLSearchParams(queryString);
-  const redirectUri = params.get("redirect_uri");
+  const redirectUri = params.get("redirect_uri") ?? "";
 
   const { isLoggedIn, disableContinue } = useUserContext();
 
@@ -16,7 +16,7 @@ export const ContinuePage = () => {
     return <Navigate to={`/login?redirect_uri=${redirectUri}`} />;
   }
 
-  if (redirectUri === "null") {
+  if (redirectUri === "null" || redirectUri === "") {
     return <Navigate to="/" />;
   }
 
@@ -27,15 +27,29 @@ export const ContinuePage = () => {
       color: "blue",
     });
     setTimeout(() => {
-      window.location.href = redirectUri!;
+      window.location.href = redirectUri;
     }, 500);
   };
 
-  const urlParsed = URL.parse(redirectUri!);
+  const urlParsed = URL.parse(redirectUri);
+
+  if (urlParsed === null) {
+    return (
+      <ContinuePageLayout>
+        <Text size="xl" fw={700}>
+          Invalid Redirect
+        </Text>
+        <Text>
+          The redirect URL is invalid, please contact the app owner to fix the
+          issue.
+        </Text>
+      </ContinuePageLayout>
+    );
+  }
 
   if (
     window.location.protocol === "https:" &&
-    urlParsed!.protocol === "http:"
+    urlParsed.protocol === "http:"
   ) {
     return (
       <ContinuePageLayout>
@@ -54,7 +68,7 @@ export const ContinuePage = () => {
   }
 
   if (disableContinue) {
-    window.location.href = redirectUri!;
+    window.location.href = redirectUri;
     return (
       <ContinuePageLayout>
         <Text size="xl" fw={700}>

site/src/pages/login-page.tsx

@@ -24,9 +24,10 @@ import { TailscaleIcon } from "../icons/tailscale";
 export const LoginPage = () => {
   const queryString = window.location.search;
   const params = new URLSearchParams(queryString);
-  const redirectUri = params.get("redirect_uri");
+  const redirectUri = params.get("redirect_uri") ?? "";
 
   const { isLoggedIn, configuredProviders } = useUserContext();
+
   const oauthProviders = configuredProviders.filter(
     (value) => value !== "username",
   );
@@ -69,7 +70,7 @@ export const LoginPage = () => {
         color: "green",
       });
       setTimeout(() => {
-        if (redirectUri === "null") {
+        if (redirectUri === "null" || redirectUri === "") {
           window.location.replace("/");
         } else {
           window.location.replace(`/continue?redirect_uri=${redirectUri}`);

site/src/pages/unauthorized-page.tsx

@@ -5,10 +5,10 @@ import { Navigate } from "react-router";
 export const UnauthorizedPage = () => {
   const queryString = window.location.search;
   const params = new URLSearchParams(queryString);
-  const username = params.get("username");
-  const resource = params.get("resource");
+  const username = params.get("username") ?? "";
+  const resource = params.get("resource") ?? "";
 
-  if (username === "null") {
+  if (username === "null" || username === "") {
     return <Navigate to="/" />;
   }
 
@@ -20,7 +20,7 @@ export const UnauthorizedPage = () => {
         </Text>
         <Text>
           The user with username <Code>{username}</Code> is not authorized to{" "}
-          {resource !== "null" ? (
+          {resource !== "null" && resource !== "" ? (
             <span>
               access the <Code>{resource}</Code> resource.
             </span>