From 5484f0e23897ca252a4b891f8409221dd1d8f9e1 Mon Sep 17 00:00:00 2001 From: sanjay7178 Date: Sat, 6 Sep 2025 20:17:29 +0530 Subject: [PATCH 1/2] feat: Enhance OAuth user handling with fallback email and certificate generation Signed-off-by: sanjay7178 --- Dockerfile | 8 +- Tiltfile | 8 + deployments/isito.yml | 16 +- deployments/manifest.yml | 38 +++- frontend/Caddyfile.dev | 48 ++++- frontend/src/components/auth/LoginForm.jsx | 17 +- frontend/src/context/AuthContext.jsx | 53 +++++- frontend/src/routes/AuthSuccessRoute.jsx | 206 +++++++++++++++------ internal/auth/handlers.go | 18 +- internal/auth/service.go | 47 ++++- 10 files changed, 365 insertions(+), 94 deletions(-) diff --git a/Dockerfile b/Dockerfile index c49c5b7..866d32b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,14 +44,8 @@ COPY --from=builder /app/guac /app/guac # Copy templates (if your application uses them from filesystem at runtime) COPY --from=builder /app/templates /app/templates -# Create and copy certificates as before +# Create certificates directory (certificates will be generated by init container) RUN mkdir -p /app/certs -# COPY --from=builder /app/certs/ /app/certs/ # This line might not be needed if certs are always generated -RUN echo "Generating self-signed certificates..." && \ - openssl req -x509 -nodes -days 365 -newkey rsa:2048 \ - -keyout /app/certs/private.key \ - -out /app/certs/certificate.crt \ - -subj "/C=US/ST=California/L=San Francisco/O=My Company/CN=mydomain.com" ENV CERT_PATH=/app/certs/certificate.crt ENV CERT_KEY_PATH=/app/certs/private.key diff --git a/Tiltfile b/Tiltfile index 5411ed0..73aca97 100644 --- a/Tiltfile +++ b/Tiltfile @@ -25,10 +25,18 @@ docker_build_with_restart( live_update=[ sync('./.tilt/guac', '/app/guac'), sync('./templates', '/app/templates'), + sync('./certs', '/app/certs'), run('chmod +x /app/guac') # Ensure binary is executable ] ) +k8s_resource( + 'browser-sandbox-frontend', + port_forwards=['8080:80'], + labels=["frontend"], + auto_init=False +) + # Frontend (optional dev build) - uses Caddyfile.dev via build arg # Build only when needed; does not deploy any k8s resources here docker_build( diff --git a/deployments/isito.yml b/deployments/isito.yml index 811fa9a..393de2e 100644 --- a/deployments/isito.yml +++ b/deployments/isito.yml @@ -37,22 +37,24 @@ spec: - browser-sandbox-gateway http: - match: - - uri: - prefix: "/api" + - headers: + ":authority": + exact: "api.kubebrowse1.ghcat.tech" route: - destination: host: browser-sandbox-api port: number: 4567 - match: - - uri: - prefix: "/websocket" + - headers: + ":authority": + exact: "app.kubebrowse1.ghcat.tech" route: - destination: - host: browser-sandbox-api + host: browser-sandbox-frontend port: - number: 4567 - - route: # Default route for frontend + number: 80 + - route: # Default route (fallback) - destination: host: browser-sandbox-frontend port: diff --git a/deployments/manifest.yml b/deployments/manifest.yml index 77d8559..c5da043 100644 --- a/deployments/manifest.yml +++ b/deployments/manifest.yml @@ -253,6 +253,24 @@ spec: app: browser-sandbox-api spec: serviceAccountName: browser-sandbox-sa + initContainers: + - name: cert-generator + image: alpine:3.20 + command: ['sh', '-c'] + args: + - | + apk add --no-cache openssl + mkdir -p /app/certs + echo "Generating self-signed certificates..." + openssl req -x509 -nodes -days 365 -newkey rsa:2048 \ + -keyout /app/certs/private.key \ + -out /app/certs/certificate.crt \ + -subj "/C=US/ST=California/L=San Francisco/O=My Company/CN=mydomain.com" + echo "Certificates generated successfully" + ls -la /app/certs/ + volumeMounts: + - name: certs-volume + mountPath: /app/certs containers: - name: api image: ghcr.io/browsersec/kubebrowse:sha-09dfa1f @@ -320,15 +338,15 @@ spec: - name: DISTRIBUTED_TRACING value: "true" - name: GITHUB_CLIENT_ID - value: "Ov23li974J8UoG1CH0HJ" + value: "Ov23liQQQ5lMyIrOULYx" - name: GITHUB_CLIENT_SECRET - value: "f16b7dc2b85d4a7f817e273b4a2d64124153ce8f" + value: "47afea55e0e19231e1cc82c3a57d56aaeedb480e" - name: GITHUB_CALLBACK_URL value: "https://localhost:4567/auth/oauth/github/callback" - name: SESSION_SECRET value: "McX0xoYlJYpO0EGdUZC6aJxVh2rKYrXZM7SPMpVREeXMXoMBI20v90PICD-LfEn7jr07c5vH2CWcKzjl8hoNvQ" - name: FRONTEND_URL - value: "http://localhost:5173" + value: "http://localhost:8080" - name: ENVIRONMENT value: "development" - name: SMTP_HOST @@ -341,11 +359,17 @@ spec: value: "ulagdeliigobhdgr" - name: FROM_EMAIL value: "sanjay.obsidian@gmail.com" - # - name: CERT_PATH - # value: "" - # - name: CERT_KEY_PATH - # value: "" + - name: CERT_PATH + value: "/app/certs/certificate.crt" + - name: CERT_KEY_PATH + value: "/app/certs/private.key" + volumeMounts: + - name: certs-volume + mountPath: /app/certs securityContext: {} + volumes: + - name: certs-volume + emptyDir: {} --- # API Service apiVersion: v1 diff --git a/frontend/Caddyfile.dev b/frontend/Caddyfile.dev index e139535..6b39060 100644 --- a/frontend/Caddyfile.dev +++ b/frontend/Caddyfile.dev @@ -8,14 +8,50 @@ root * /srv file_server - # Handle OAuth callback redirects - handle /auth/success { - file_server - header Cache-Control "no-cache, no-store, must-revalidate" + # Authentication endpoints - proxy to backend API (excluding frontend routes) + handle /auth/oauth/* { + reverse_proxy {$CADDY_GUAC_CLIENT_URL} { + transport http { + tls + tls_insecure_skip_verify + } + header_up Host {http.reverse_proxy.upstream.hostport} + header_up Connection {header.Connection} + header_up Upgrade {header.Upgrade} + + # Cookie handling for session management + } } - # Authentication endpoints - proxy to backend API - handle /auth/* { + handle /auth/login { + reverse_proxy {$CADDY_GUAC_CLIENT_URL} { + transport http { + tls + tls_insecure_skip_verify + } + header_up Host {http.reverse_proxy.upstream.hostport} + header_up Connection {header.Connection} + header_up Upgrade {header.Upgrade} + + # Cookie handling for session management + } + } + + handle /auth/me { + reverse_proxy {$CADDY_GUAC_CLIENT_URL} { + transport http { + tls + tls_insecure_skip_verify + } + header_up Host {http.reverse_proxy.upstream.hostport} + header_up Connection {header.Connection} + header_up Upgrade {header.Upgrade} + + # Cookie handling for session management + } + } + + handle /auth/logout { reverse_proxy {$CADDY_GUAC_CLIENT_URL} { transport http { tls diff --git a/frontend/src/components/auth/LoginForm.jsx b/frontend/src/components/auth/LoginForm.jsx index 987af3b..4b131ba 100644 --- a/frontend/src/components/auth/LoginForm.jsx +++ b/frontend/src/components/auth/LoginForm.jsx @@ -6,6 +6,7 @@ import { Label } from '../ui/label'; import { Card } from '../ui/card'; import { Alert } from '../ui/alert'; import EmailVerification from './EmailVerification'; +import toast from 'react-hot-toast'; export function LoginForm({ onSwitchToRegister }) { const [formData, setFormData] = useState({ @@ -32,13 +33,25 @@ export function LoginForm({ onSwitchToRegister }) { const result = await login(formData.email, formData.password); - if (!result.success) { + if (result.success) { + // Show success toast similar to OAuth flow + toast.success(`Welcome back, ${result.user?.name || result.user?.email || 'User'}!`, { + duration: 3000, + position: "top-right", + style: { + background: "#10B981", + color: "#fff", + fontWeight: "600", + }, + iconTheme: { primary: "#fff", secondary: "#10B981" }, + }); + setIsSubmitting(false); + } else { if (result.needsVerification) { setShowEmailVerification(true); } setIsSubmitting(false); } - // If successful, the auth context will handle the state update }; const handleGitHubLogin = () => { diff --git a/frontend/src/context/AuthContext.jsx b/frontend/src/context/AuthContext.jsx index 94deec9..dbb0c49 100644 --- a/frontend/src/context/AuthContext.jsx +++ b/frontend/src/context/AuthContext.jsx @@ -28,14 +28,23 @@ function authReducer(state, action) { ...state, isLoading: action.payload }; - case AUTH_ACTIONS.SET_USER: + case AUTH_ACTIONS.SET_USER: { + // Check if user object has meaningful data + const hasUserData = action.payload && ( + action.payload.id || + action.payload.email || + action.payload.username || + (action.payload.name && action.payload.name.trim() !== '') + ); + return { ...state, user: action.payload, - isAuthenticated: !!action.payload, + isAuthenticated: hasUserData, isLoading: false, error: null }; + } case AUTH_ACTIONS.SET_ERROR: return { ...state, @@ -73,23 +82,35 @@ const AuthContext = createContext(); // Auth provider component export function AuthProvider({ children }) { const [state, dispatch] = useReducer(authReducer, initialState); + const lastAuthCheck = React.useRef(0); + const RATE_LIMIT_MS = 1000; // Minimum 1 second between auth checks // Check if user is authenticated on app load useEffect(() => { - checkAuthStatus(); + // Don't auto-check auth if we're on the success page - let that page handle it + if (window.location.pathname !== '/auth/success') { + checkAuthStatus(); + } // Check if user is coming from OAuth callback - const urlParams = new URLSearchParams(window.location.search); if (window.location.pathname === '/auth/success') { - // User just completed OAuth, check auth status immediately + // User just completed OAuth, check auth status after a short delay console.log('OAuth callback detected, checking auth status...'); setTimeout(() => { checkAuthStatus(); - }, 1000); // Give backend time to set cookies + }, 1500); // Give backend time to set cookies } }, []); const checkAuthStatus = async () => { + // Rate limiting to prevent excessive API calls + const now = Date.now(); + if (now - lastAuthCheck.current < RATE_LIMIT_MS) { + console.log('Auth check rate limited, skipping'); + return; + } + lastAuthCheck.current = now; + try { console.log('Checking auth status...'); const response = await fetch('/auth/me', { @@ -106,6 +127,18 @@ export function AuthProvider({ children }) { if (response.ok) { const data = await response.json(); console.log('Auth successful, user data:', data); + console.log('User object details:', { + id: data.user?.id, + email: data.user?.email, + username: data.user?.username, + name: data.user?.name, + provider: data.user?.provider, + hasId: !!data.user?.id, + hasEmail: !!data.user?.email, + hasUsername: !!data.user?.username, + hasName: !!(data.user?.name && data.user.name.trim() !== ''), + isEmpty: Object.keys(data.user || {}).length === 0 + }); dispatch({ type: AUTH_ACTIONS.SET_USER, payload: data.user }); } else { console.log('Auth failed, status:', response.status); @@ -145,7 +178,7 @@ export function AuthProvider({ children }) { dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: data.error }); return { success: false, error: data.error }; } - } catch (error) { + } catch { const errorMessage = 'Login failed. Please try again.'; dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: errorMessage }); return { success: false, error: errorMessage }; @@ -176,7 +209,7 @@ export function AuthProvider({ children }) { dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: data.error }); return { success: false, error: data.error }; } - } catch (error) { + } catch { const errorMessage = 'Registration failed. Please try again.'; dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: errorMessage }); return { success: false, error: errorMessage }; @@ -228,7 +261,7 @@ export function AuthProvider({ children }) { dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: data.error }); return { success: false, error: data.error }; } - } catch (error) { + } catch { const errorMessage = 'Email verification failed. Please try again.'; dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: errorMessage }); return { success: false, error: errorMessage }; @@ -258,7 +291,7 @@ export function AuthProvider({ children }) { dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: data.error }); return { success: false, error: data.error }; } - } catch (error) { + } catch { const errorMessage = 'Failed to resend verification email. Please try again.'; dispatch({ type: AUTH_ACTIONS.SET_ERROR, payload: errorMessage }); return { success: false, error: errorMessage }; diff --git a/frontend/src/routes/AuthSuccessRoute.jsx b/frontend/src/routes/AuthSuccessRoute.jsx index 28618aa..af04146 100644 --- a/frontend/src/routes/AuthSuccessRoute.jsx +++ b/frontend/src/routes/AuthSuccessRoute.jsx @@ -9,75 +9,177 @@ export function AuthSuccessRoute() { const [status, setStatus] = useState("checking"); const [kick, setKick] = useState(0); const authRef = React.useRef({ user, isAuthenticated }); + useEffect(() => { authRef.current = { user, isAuthenticated }; }, [user, isAuthenticated]); + // Listen for changes in authentication state from AuthContext + useEffect(() => { + const { user: u, isAuthenticated: ok } = authRef.current; + + // More robust check for valid user data + const hasValidUserData = u && ( + u.id || + u.email || + u.username || + (u.name && u.name.trim() !== '') + ); + + console.log('AuthSuccessRoute: Auth state changed:', { + isAuthenticated: ok, + user: u, + hasValidUserData, + userKeys: u ? Object.keys(u) : 'no user' + }); + + if (ok && hasValidUserData) { + console.log('AuthSuccessRoute: Authentication detected, showing success'); + setStatus("success"); + toast.success(`Welcome back, ${u.name || u.email || u.username || 'User'}!`, { + duration: 3000, + position: "top-right", + style: { + background: "#10B981", + color: "#fff", + fontWeight: "600", + }, + iconTheme: { primary: "#fff", secondary: "#10B981" }, + }); + + // Redirect after a short delay + const redirectTimer = setTimeout( + () => navigate("/", { replace: true }), + 2000 + ); + + return () => clearTimeout(redirectTimer); + } + }, [user, isAuthenticated, navigate]); + useEffect(() => { let cancelled = false; let redirectTimer; - const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); - const run = async () => { + let pollInterval; + + // If user is already authenticated, skip the whole polling process + const currentAuth = authRef.current; + const hasValidUserData = currentAuth.user && ( + currentAuth.user.id || + currentAuth.user.email || + currentAuth.user.username || + (currentAuth.user.name && currentAuth.user.name.trim() !== '') + ); + + if (currentAuth.isAuthenticated && hasValidUserData) { + console.log('AuthSuccessRoute: User already authenticated, redirecting immediately'); + setStatus("success"); + redirectTimer = setTimeout( + () => navigate("/", { replace: true }), + 1000 + ); + return () => clearTimeout(redirectTimer); + } + + const checkAuth = async () => { try { - for (let attempt = 1; attempt <= 3; attempt++) { - setStatus("checking"); - await checkAuthStatus(); - await sleep(1000); - const { user: u, isAuthenticated: ok } = authRef.current; - if (cancelled) return; - if (ok && u) { - setStatus("success"); - toast.success(`Welcome back, ${u.name || u.email}!`, { - duration: 3000, - position: "top-right", - style: { - background: "#10B981", - color: "#fff", - fontWeight: "600", - }, - iconTheme: { primary: "#fff", secondary: "#10B981" }, - }); - redirectTimer = setTimeout( - () => navigate("/", { replace: true }), - 2000 - ); - return; - } - setStatus("retrying"); - toast(`Authentication attempt ${attempt}/3...`, { - duration: 2000, - position: "top-right", - style: { background: "#F59E0B", color: "#fff", fontWeight: "600" }, - iconTheme: { primary: "#fff", secondary: "#F59E0B" }, - }); - await sleep(2000); - } - if (!cancelled) { - setStatus("failed"); - toast.error("Authentication failed. Please try again.", { - duration: 4000, - position: "top-right", - style: { background: "#EF4444", color: "#fff", fontWeight: "600" }, - iconTheme: { primary: "#fff", secondary: "#EF4444" }, - }); + // Check if already authenticated from context first + const currentAuth = authRef.current; + const hasValidUserData = currentAuth.user && ( + currentAuth.user.id || + currentAuth.user.email || + currentAuth.user.username || + (currentAuth.user.name && currentAuth.user.name.trim() !== '') + ); + + if (currentAuth.isAuthenticated && hasValidUserData) { + console.log('AuthSuccessRoute: Already authenticated, skipping API call'); + return true; } + + console.log('AuthSuccessRoute: Checking auth status with API...'); + setStatus("checking"); + await checkAuthStatus(); + + if (cancelled) return false; + + // Check auth status again after the API call + const { user: u, isAuthenticated: ok } = authRef.current; + const hasValidUserDataAfter = u && ( + u.id || + u.email || + u.username || + (u.name && u.name.trim() !== '') + ); + + console.log('AuthSuccessRoute: Auth check result:', { + isAuthenticated: ok, + user: u, + hasValidUserData: hasValidUserDataAfter, + userKeys: u ? Object.keys(u) : 'no user' + }); + + return ok && hasValidUserDataAfter; } catch (error) { if (!cancelled) { - console.error("Auth check failed:", error); - setStatus("failed"); - toast.error("Authentication check failed. Please try again.", { - duration: 4000, - position: "top-right", - style: { background: "#EF4444", color: "#fff", fontWeight: "600" }, - iconTheme: { primary: "#fff", secondary: "#EF4444" }, - }); + console.error("AuthSuccessRoute: Auth check failed:", error); } + return false; } }; + + const run = async () => { + // Initial check + const success = await checkAuth(); + if (success || cancelled) return; + + // If not successful, start polling with exponential backoff + let attempt = 1; + const maxAttempts = 5; // Reduced from 8 + let delay = 2000; // Start with 2 seconds + + const poll = async () => { + if (cancelled || attempt > maxAttempts) { + if (!cancelled && attempt > maxAttempts) { + console.log('AuthSuccessRoute: Max attempts reached, showing failed state'); + setStatus("failed"); + toast.error("Authentication failed. Please try again.", { + duration: 4000, + position: "top-right", + style: { background: "#EF4444", color: "#fff", fontWeight: "600" }, + iconTheme: { primary: "#fff", secondary: "#EF4444" }, + }); + } + return; + } + + setStatus("retrying"); + console.log(`AuthSuccessRoute: Authentication attempt ${attempt}/${maxAttempts}...`); + toast(`Authentication attempt ${attempt}/${maxAttempts}...`, { + duration: 2000, + position: "top-right", + style: { background: "#F59E0B", color: "#fff", fontWeight: "600" }, + iconTheme: { primary: "#fff", secondary: "#F59E0B" }, + }); + + const success = await checkAuth(); + if (success || cancelled) return; + + attempt++; + delay = Math.min(delay * 1.2, 4000); // Slower exponential backoff, max 4 seconds + + pollInterval = setTimeout(poll, delay); + }; + + pollInterval = setTimeout(poll, delay); + }; + run(); + return () => { cancelled = true; if (redirectTimer) clearTimeout(redirectTimer); + if (pollInterval) clearTimeout(pollInterval); }; }, [checkAuthStatus, navigate, kick]); @@ -210,7 +312,7 @@ export function AuthSuccessRoute() { Welcome back!

- {user?.name || user?.email} - Showing success notification and + {user?.name || user?.email || user?.username || 'User'} - Showing success notification and redirecting to home page...

diff --git a/internal/auth/handlers.go b/internal/auth/handlers.go index e6c7818..b38264c 100644 --- a/internal/auth/handlers.go +++ b/internal/auth/handlers.go @@ -80,6 +80,7 @@ func InitializeGoth() { os.Getenv("GITHUB_CLIENT_ID"), os.Getenv("GITHUB_CLIENT_SECRET"), os.Getenv("GITHUB_CALLBACK_URL"), + "user:email", // Request email scope ), ) @@ -356,6 +357,20 @@ func (h *Handler) callbackOAuthWithRedis(c *gin.Context, provider, state, code s return } + // If email is empty, use a fallback email based on the GitHub username + if user.Email == "" { + logrus.Warnf("User email is empty, using fallback email for user: %s", user.UserID) + // Create a fallback email using the GitHub username + if user.NickName != "" { + user.Email = user.NickName + "@github.local" + } else if user.UserID != "" { + user.Email = user.UserID + "@github.local" + } else { + user.Email = "user@github.local" + } + logrus.Infof("Using fallback email: %s", user.Email) + } + logrus.Infof("Successfully authenticated user: %s (%s)", user.Email, user.Provider) h.processOAuthUser(c, user) } @@ -555,7 +570,8 @@ func (h *Handler) setSessionCookie(c *gin.Context, sessionToken string) { // For local development with proxy, set cookie for localhost if strings.Contains(host, "localhost") || strings.Contains(host, "127.0.0.1") { domain = "localhost" - secure = false // HTTP for local development + // Check if the request is coming through HTTPS (even in development) + secure = c.Request.TLS != nil || c.GetHeader("X-Forwarded-Proto") == "https" } else { // For production, extract domain from host if strings.Contains(host, ":") { diff --git a/internal/auth/service.go b/internal/auth/service.go index 48d6af7..c0ff1d5 100644 --- a/internal/auth/service.go +++ b/internal/auth/service.go @@ -150,6 +150,18 @@ func (s *Service) LoginWithEmail(email, password string) (*User, *Session, error // CreateOrUpdateOAuthUser creates or updates a user from OAuth provider func (s *Service) CreateOrUpdateOAuthUser(email, provider, providerID, avatarURL, name, username string) (*User, error) { + // If email is empty, create a fallback email + if email == "" { + if username != "" { + email = username + "@github.local" + } else if providerID != "" { + email = providerID + "@github.local" + } else { + email = "user@github.local" + } + logrus.Infof("Using fallback email for OAuth user: %s", email) + } + // Try to find existing user by provider existingUser, err := s.db.GetUserByProvider(s.ctx, sqlc.GetUserByProviderParams{ Provider: sql.NullString{String: provider, Valid: true}, @@ -157,7 +169,25 @@ func (s *Service) CreateOrUpdateOAuthUser(email, provider, providerID, avatarURL }) if err == nil { - // User exists, return it + // User exists, update email if it's empty + if existingUser.Email == "" { + logrus.Infof("Updating empty email for existing user %s to %s", existingUser.ID, email) + // Update the user's email + _, updateErr := s.db.UpdateUser(s.ctx, sqlc.UpdateUserParams{ + ID: existingUser.ID, + Username: existingUser.Username, + Email: email, + }) + if updateErr != nil { + logrus.Errorf("Failed to update user email: %v", updateErr) + } else { + // Refresh the user data + existingUser, err = s.db.GetUser(s.ctx, existingUser.ID) + if err != nil { + logrus.Errorf("Failed to refresh user data: %v", err) + } + } + } return s.convertDBUser(existingUser), nil } @@ -304,9 +334,22 @@ func (s *Service) ValidateSession(token string) (*User, *Session, error) { logrus.Debugf("ValidateSession: Retrieved session data - UserID: %s, Email: %s, Provider: %s", dbSession.UserID, dbSession.Email, dbSession.Provider.String) + // Ensure email is not empty - if it is, try to get fresh user data + email := dbSession.Email + if email == "" { + logrus.Warnf("ValidateSession: Email is empty for user %s, fetching fresh user data", dbSession.UserID) + freshUser, err := s.db.GetUser(s.ctx, dbSession.UserID) + if err != nil { + logrus.Errorf("ValidateSession: Failed to get fresh user data: %v", err) + } else { + email = freshUser.Email + logrus.Infof("ValidateSession: Fresh user data - Email: %s", email) + } + } + user := &User{ ID: dbSession.UserID, - Email: dbSession.Email, + Email: email, Provider: dbSession.Provider.String, } From 028b7a82d5540208afd4d944f6d54c6d4d795c42 Mon Sep 17 00:00:00 2001 From: sanjay7178 Date: Sat, 6 Sep 2025 20:26:16 +0530 Subject: [PATCH 2/2] feat: Add DCO check to commit message validation in Lefthook Signed-off-by: sanjay7178 --- docs/CONTRIBUTING.md | 56 +++++++++++++++++++++++++++++++++++++++----- lefthook.yml | 14 +++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index ece83eb..4fda59d 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -6,7 +6,7 @@ order: 800 # Contributing to KubeBrowse -Thank you for your interest in contributing to GUAC! This document provides guidelines and instructions for contributing to this project. +Thank you for your interest in contributing to KubeBrowse! This document provides guidelines and instructions for contributing to this project. ## Table of Contents @@ -16,10 +16,14 @@ Thank you for your interest in contributing to GUAC! This document provides guid - [Getting Started](#getting-started) - [Development Environment Setup](#development-environment-setup) - [Troubleshooting Common Issues](#troubleshooting-common-issues) + - [Go Version Mismatch](#go-version-mismatch) - [Project Structure](#project-structure) - [Development Workflow](#development-workflow) - [Branching Strategy](#branching-strategy) - [Commit Guidelines](#commit-guidelines) + - [Conventional Commits Format](#conventional-commits-format) + - [DCO Sign-off Requirement](#dco-sign-off-requirement) + - [Example Commit](#example-commit) - [Pre-commit Hooks](#pre-commit-hooks) - [Pull Requests](#pull-requests) - [PR Process](#pr-process) @@ -146,9 +150,11 @@ If you encounter errors like `compile: version "go1.24.0" does not match go tool ### Commit Guidelines -We follow [Conventional Commits](https://www.conventionalcommits.org/) for commit messages: +We follow [Conventional Commits](https://www.conventionalcommits.org/) for commit messages and require [Developer Certificate of Origin (DCO)](https://developercertificate.org/) sign-off for all contributions. -``` +#### Conventional Commits Format + +```text (): @@ -157,6 +163,7 @@ We follow [Conventional Commits](https://www.conventionalcommits.org/) for commi ``` Types include: + - `feat`: A new feature - `fix`: A bug fix - `docs`: Documentation changes @@ -165,14 +172,50 @@ Types include: - `test`: Adding or modifying tests - `chore`: Changes to the build process or auxiliary tools -Example: +#### DCO Sign-off Requirement + +All commits must include a `Signed-off-by` line to certify that you have the right to submit the code under the project's license. This is required by the [Developer Certificate of Origin](https://developercertificate.org/). + +**To sign off your commits automatically:** + +```bash +# Configure git to always sign off commits (recommended) +git config --global format.signoff true + +# Or use the -s flag for individual commits +git commit -s -m "feat(api): add new authentication endpoint" +``` + +**To sign off existing commits:** + +```bash +# Sign off the last commit +git commit --amend --signoff + +# Sign off multiple commits using interactive rebase +git rebase --signoff HEAD~ + +# Force push to update the PR +git push --force-with-lease +``` + +The sign-off line should look like: + +```text +Signed-off-by: Your Name ``` + +#### Example Commit + +```text feat(api): add endpoint for user authentication - Implement JWT token generation - Add middleware for token validation Fixes #123 + +Signed-off-by: John Doe ``` ### Pre-commit Hooks @@ -186,11 +229,13 @@ This project uses [Lefthook](https://github.com/evilmartians/lefthook) for manag - Potential secrets Install the hooks with: + ```bash make hooks ``` You can manually run the pre-commit checks: + ```bash # Check only staged files make lint @@ -251,5 +296,4 @@ If Lefthook is not found, you'll be prompted to install it. Lefthook hooks are a 5. Tag the release with the version number 6. Update documentation with release notes -Thank you for contributing to GUAC! -* \ No newline at end of file +Thank you for contributing to KubeBrowse! diff --git a/lefthook.yml b/lefthook.yml index 5f32963..ccecdba 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -53,3 +53,17 @@ commit-msg: exit 1 fi + dco-check: + run: | + # if no arg was passed, fallback to the usual commit-msg file + file="${1:-$(git rev-parse --git-dir)/COMMIT_EDITMSG}" + message="$(cat "$file")" + + if ! echo "$message" | grep -q "Signed-off-by:"; then + echo "DCO check failed!" + echo "Commit message must include a 'Signed-off-by' line." + echo "To fix this, commit with: git commit -s" + echo "Or amend your commit: git commit --amend -s" + exit 1 + fi +