Feat/env auth migration#22
Conversation
omarelkashef
left a comment
There was a problem hiding this comment.
Thank you for the PR. Code looks good. Only a couple of nitpicks. For QA, the port is not exposed unless I run docker with -p 8090:8090 and I needed to inject the .env file.
| const apiUser = "bauer" | ||
|
|
||
| func APIBasicAuth(next http.Handler, secret string) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Would it be easier to add middlewares to handlers that need it than using an if statement?
| username, password, ok := r.BasicAuth() | ||
| if !ok || !secureEquals(username, apiUser) || !secureEquals(password, secret) { | ||
| w.Header().Set("WWW-Authenticate", `Basic realm="bauer-api"`) | ||
| http.Error(w, "unauthorized", http.StatusUnauthorized) |
There was a problem hiding this comment.
Just for the same of consistency, we can use types.UnAuthorized function.
| @@ -30,10 +30,16 @@ N.B. You need to install [Copilot CLI](https://docs.github.com/en/copilot/how-to | |||
| ## Configuration | |||
There was a problem hiding this comment.
Can you add docker instruction to the README?
|
|
||
|
|
||
| func GetHealth(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") |
There was a problem hiding this comment.
Unrelated to your PR, but can you move these two lines? Success().Render() already sets the header
There was a problem hiding this comment.
Pull request overview
This PR migrates Google service-account authentication/config from a checked-in credentials.json approach to GOOGLE_* environment variables (optionally loaded from .env files), and adds basic auth + containerization to support hosting the API (e.g., on Railway).
Changes:
- Replace credentials-file auth with env-based credential loading and provide a migration script +
.env.example. - Add
.envloading behavior to CLI/API config and update validation/tests/docs accordingly. - Add API HTTP basic auth middleware and a multi-stage Dockerfile for building/running
bauer-api.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/migrate_credentials_to_env.py | Adds helper to convert service-account JSON keys into .env variables. |
| internal/orchestrator/orchestrator.go | Updates GDocs client initialization to no longer require a credentials path. |
| internal/gdocs/service.go | Switches client auth to use credentials loaded from environment variables. |
| internal/gdocs/credentials.go | Implements env-based credential loading, validation, and JSON marshaling for Google SDK auth. |
| internal/config/env.go | Adds .env.local / .env loading via godotenv. |
| internal/config/config_test.go | Updates config validation tests to use GOOGLE_* env vars instead of temp credentials files. |
| internal/config/config.go | Removes CredentialsPath and validates required GOOGLE_* env vars; centralizes defaults. |
| internal/config/cli.go | Removes --credentials flag; loads env files before validation. |
| go.mod | Adds godotenv dependency. |
| go.sum | Adds checksum entries for godotenv. |
| docs/ARCHITECTURE.md | Updates architecture docs to reflect env-based config requirements. |
| credentials.json | Removes the template credentials JSON file from the repo. |
| cmd/bauer/main.go | Updates credential-related error guidance to reference GOOGLE_* variables instead of a file path. |
| cmd/app/v1/api.go | Removes credentials path usage in job config and tweaks request/response error handling. |
| cmd/app/types/config.go | Removes credentials-path config, loads env files, and introduces API_SECRET validation. |
| cmd/app/main.go | Adds basic auth middleware and minor cleanup around orchestrator naming and Getwd error handling. |
| cmd/app/core/middleware/basic_auth.go | Introduces HTTP basic auth middleware (excluding /api/v1/health). |
| Taskfile.yml | Updates local run task to drop the --credentials flag. |
| README.md | Updates setup instructions to use .env + migration helper; documents API basic auth usage. |
| Dockerfile | Adds a multi-stage build for producing a minimal bauer-api runtime image. |
| .gitignore | Ignores .env* (except .env.example) and credentials.json. |
| .env.example | Provides a template for required GOOGLE_* vars and API_SECRET. |
| .env | Removes previously committed .env contents. |
| .dockerignore | Prevents secrets and local artifacts from being copied into Docker build context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| github.com/google/uuid v1.6.0 // indirect | ||
| github.com/googleapis/enterprise-certificate-proxy v0.3.7 // indirect | ||
| github.com/googleapis/gax-go/v2 v2.15.0 // indirect | ||
| github.com/joho/godotenv v1.5.1 // indirect |
There was a problem hiding this comment.
github.com/joho/godotenv is imported directly (e.g. internal/config/env.go), so it shouldn't be marked // indirect. Running go mod tidy (or removing the indirect marker) will keep go.mod accurate and avoid churn in future dependency updates.
| github.com/joho/godotenv v1.5.1 // indirect | |
| github.com/joho/godotenv v1.5.1 |
| slog.Error("error writing response", "error", renderErr.Error(), "requestID", requestID) | ||
| } | ||
| return nil, err | ||
| return nil, renderErr |
There was a problem hiding this comment.
In getJobFromRequest, on JSON decode failure you return renderErr (the error from writing the HTTP response) instead of the decode error. If rendering succeeds renderErr will be nil, causing the handler to continue with a nil payload and likely panic or misbehave. Return the original decode error (or a wrapped version) regardless of whether rendering succeeded, and log/ignore the render error separately.
| return nil, renderErr | |
| return nil, fmt.Errorf("invalid request body: %w", err) |
|
|
||
| credentialsJSON, err := credentials.JSON() | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
credentials.JSON() errors are returned without context (return nil, err). Wrapping this with a descriptive message (similar to the other error returns in this function) will make troubleshooting misconfigured env credentials easier.
| return nil, err | |
| return nil, fmt.Errorf("failed to get credentials JSON: %w", err) |
Done
credentials.json-based auth/config and use.envvariablesQA
credentials.jsontoenvvariables (I made a small helper script to do this):i. make sure you don't have a
.envfile alreadyii. Run
python3 scripts/migrate_credentials_to_env.py --input credentials.json --output .envTo verify migration:
envvariables:set -asource .envset +ago run cmd/bauer/main.go --doc-id "1w6nU2yt9Sec3XC_qfsGaKBIsWzkzUuHODHzRBReEctk" --dry-run(sample doc, can use a different one if you like)--credentialsflagTo verify API:
API_SECRET=qa-secret-123 go run cmd/app/main.goi. server should start on port 8090
curl http://localhost:8090/api/v1/health- should return 200curl -X POST http://localhost:8090/api/v1/job -H 'Content-Type: application/json' -d '{"doc_id":"x","chunk_size":1,"page_refresh":false}'unauthorized(no auth passed in)curl -X POST http://localhost:8090/api/v1/job -u "bauer:qa-secret-123" -H 'Content-Type: application/json' -d '{"doc_id":"x","chunk_size":1,"page_refresh":false}'202curl -X POST http://localhost:8090/api/v1/job -u "bauer:wrong-secret" -H 'Content-Type: application/json' -d '{"doc_id":"x","chunk_size":1,"page_refresh":false}'unauthorizedTo verify dockerfile
docker build -t bauer-api-test .