Log client SPIFFE ID in ping-pong-cofide-server

[markgoddard]

Also move logging to slog

[gemini-code-assist]

Summary of Changes

Hello @markgoddard, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the ping-pong-cofide-server by introducing the logging of client SPIFFE IDs for secure connections and modernizing the logging infrastructure by adopting the slog package. These changes improve observability and align with current Go logging best practices.

Highlights

• Logging Client SPIFFE ID: The server now extracts and logs the client's SPIFFE ID from the TLS peer certificate for incoming requests, enhancing traceability and security auditing.
• Migrate to slog: The logging implementation has been migrated from the standard log and fmt packages to log/slog, providing structured logging capabilities.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request successfully migrates the server's logging from the standard log package to the structured slog package, and adds functionality to log the client's SPIFFE ID from its certificate. The changes are well-implemented and improve the observability of the service. I have one suggestion to refactor the new authentication logic into a separate HTTP middleware to improve code structure and maintainability.

[gemini-code-assist]

The new logic for authenticating the client and extracting the SPIFFE ID is correct. However, adding this logic directly into the main request handler mixes authentication/authorization concerns with the business logic of the handler.

As the application grows, this can make the handler difficult to read, maintain, and test.

Consider extracting this authentication logic into a separate HTTP middleware. A middleware is a function that wraps an http.Handler to perform pre-processing of the request.

This would provide several benefits:

• Separation of Concerns: The middleware would be responsible only for authentication, and the handler only for its core logic.
• Reusability: The authentication middleware could be reused for other endpoints if needed.
• Testability: You could test the middleware and the handler in isolation.

Here is a conceptual example of what an authentication middleware might look like:

func spiffeAuthMiddleware(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
			slog.Error("No client certificate provided")
			http.Error(w, "No client certificate provided", http.StatusUnauthorized)
			return
		}

		peerCert := r.TLS.PeerCertificates[0]
		clientID, err := x509svid.IDFromCert(peerCert)
		if err != nil {
			slog.Error("Failed to get SPIFFE ID from peer cert", "error", err)
			http.Error(w, "Invalid client SVID", http.StatusUnauthorized)
			return
		}

		// Store client ID in request context for the handler to use
		ctx := context.WithValue(r.Context(), "clientID", clientID)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

You would then apply it when setting up your routes:
secureMux.Handle("/", spiffeAuthMiddleware(handler(secureServer)))