feat(auth): pluggable auth with static-token and OIDC/JWT modes#32
feat(auth): pluggable auth with static-token and OIDC/JWT modes#32mickamy wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 87.80% 87.87% +0.07%
==========================================
Files 38 39 +1
Lines 2582 2688 +106
==========================================
+ Hits 2267 2362 +95
- Misses 209 218 +9
- Partials 106 108 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the authentication system of adms to support multiple authentication modes: none, static (shared bearer token), and oidc (OIDC/JWT validation). It replaces the previous auth_token_env configuration with a structured auth block, introduces an Authenticator interface with context-based Principal storage, and adds OIDC token validation using the go-oidc library. Feedback on these changes suggests using the standard library's slices.Equal helper in tests instead of a manual slice comparison function. Additionally, a potential issue was raised regarding synchronous network I/O (OIDC provider discovery) during server instantiation without a cancelable context, which could cause the startup process to hang if the issuer is unreachable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func equalStringSlice(a, b []string) bool { | ||
| if len(a) != len(b) { | ||
| return false | ||
| } | ||
|
|
||
| for i := range a { | ||
| if a[i] != b[i] { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| return true | ||
| } |
There was a problem hiding this comment.
Instead of implementing a manual slice comparison helper, prefer using the standard library's slices.Equal helper to write more idiomatic and clean Go code. Note: You will need to import the slices package.
func equalStringSlice(a, b []string) bool {
return slices.Equal(a, b)
}References
- Prefer using standard library
sliceshelpers instead of manual slice operations to write more idiomatic and clean Go code.
| func newAuthenticator(auth config.Auth) (Authenticator, error) { | ||
| switch auth.Mode { | ||
| case config.AuthModeNone, "": | ||
| // The empty zero value means "unset", which is open — same as none. | ||
| return noneAuth{}, nil | ||
| case config.AuthModeStatic: | ||
| return newStaticTokenAuth(auth.Token), nil | ||
| case config.AuthModeOIDC: | ||
| return newOIDCAuth(auth.OIDC) | ||
| default: | ||
| // config.buildAuth rejects unknown modes, so this is unreachable via | ||
| // the config path. Fail closed rather than silently serving an open | ||
| // API if a caller ever constructs an unexpected mode directly. | ||
| return nil, fmt.Errorf("unknown auth mode %q", auth.Mode) | ||
| } | ||
| } |
There was a problem hiding this comment.
Performing synchronous network I/O (OIDC provider discovery) during server instantiation (newServer / New) without a cancelable context can cause the startup process to hang (up to 10 seconds) if the OIDC issuer is unreachable, ignoring any interrupt signals (like Ctrl+C). Consider initializing the OIDC authenticator lazily or during the Prepare(ctx) phase where a cancelable context is available.
1 participant
Summary
Introduces a pluggable authentication layer so
admscan identify who is calling, not just whether a shared secret is present. Authentication is now selected byauth.mode, with three modes:none(open, the default),static(a shared bearer token), andoidc(validate JWTs against an OIDC issuer). Every request carries aPrincipal(subject, roles, claims) on its context, which lays the groundwork for role-based authorization.Config
The previous top-level
auth_token_envkey is removed in favor ofauth.static.token_env;auth.mode: nonekeeps the API fully open as before.What changed
Authenticatoris a small interface (Authenticate(r) (Principal, error)) behind theauthenticatemiddleware. The middleware stores the resolvedPrincipalon the request context and renders a401(with the appropriateWWW-Authenticatechallenge) when an authenticator rejects a request./healthzstays open for every mode.noneAuth— anonymous principal, fully open.staticTokenAuth— the existing shared-token check, now behind the interface (SHA-256 constant-time compare preserved).oidcAuth— verifies the bearer JWT's signature (against the issuer's JWKS), issuer, audience, and expiry viagithub.com/coreos/go-oidc/v3.subbecomes the principal subject,roles_claim(a JSON array or a space-separated string) becomes its roles, and the full validated claim set is retained.The OIDC issuer is discovered at startup, so a misconfigured or unreachable issuer fails startup rather than every request. The JWKS is refreshed automatically on key rotation; the discovery round-trip and later key fetches are bounded by an HTTP client timeout rather than a cancelable context, so key rotation keeps working for the process lifetime.
Security
auth.mode: staticrequiresstatic.token_env, andoidcrequiresissuerandaudience; missing values are a startup error.401.Testing
none/staticpaths (status codes,WWW-Authenticateheaders,/healthzbypass) plus a check that thePrincipalreaches the handler context.httptest, RS256 tokens signed in-test): valid tokens, expired tokens, audience mismatch, tampered signatures, and a missing header, plus role extraction from both array and space-separated claims.go build ./...,go test ./...,gofmt,go vet, andgolangci-lintall pass.Dependency
Adds
github.com/coreos/go-oidc/v3(withgo-jose/v4andgolang.org/x/oauth2as indirect dependencies) for OIDC discovery and JWKS verification. The static-token path stays on the standard library.