Require OIDC bearer tokens on remote/virtual config writes when an issuer is set
This commit is contained in:
@@ -0,0 +1,105 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/coreos/go-oidc/v3/oidc"
|
||||
)
|
||||
|
||||
// BearerOIDC verifies `Authorization: Bearer <jwt>` against an OIDC issuer's
|
||||
// JWKS. Discovery is lazy and retried per request so startup never depends on
|
||||
// the issuer being reachable.
|
||||
type BearerOIDC struct {
|
||||
issuer string
|
||||
audiences []string
|
||||
|
||||
mu sync.Mutex
|
||||
verifier *oidc.IDTokenVerifier
|
||||
}
|
||||
|
||||
// NewBearerOIDC returns nil when issuer is empty, meaning auth is disabled.
|
||||
func NewBearerOIDC(issuer string, audiences []string) *BearerOIDC {
|
||||
if issuer == "" {
|
||||
return nil
|
||||
}
|
||||
return &BearerOIDC{issuer: issuer, audiences: audiences}
|
||||
}
|
||||
|
||||
func (b *BearerOIDC) getVerifier(ctx context.Context) (*oidc.IDTokenVerifier, error) {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
if b.verifier != nil {
|
||||
return b.verifier, nil
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
|
||||
defer cancel()
|
||||
// The provider keeps this context for JWKS refreshes; it must outlive the request.
|
||||
p, err := oidc.NewProvider(oidc.ClientContext(context.WithoutCancel(ctx), http.DefaultClient), b.issuer)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
b.verifier = p.Verifier(&oidc.Config{SkipClientIDCheck: true})
|
||||
return b.verifier, nil
|
||||
}
|
||||
|
||||
// Require gates every request through next on a valid token. A nil receiver
|
||||
// passes everything through.
|
||||
func (b *BearerOIDC) Require(next http.Handler) http.Handler {
|
||||
if b == nil {
|
||||
return next
|
||||
}
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
raw, ok := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ")
|
||||
if !ok || raw == "" {
|
||||
unauthorized(w, "missing bearer token")
|
||||
return
|
||||
}
|
||||
v, err := b.getVerifier(r.Context())
|
||||
if err != nil {
|
||||
slog.Error("oidc discovery failed", "issuer", b.issuer, "error", err)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusServiceUnavailable)
|
||||
_, _ = w.Write([]byte(`{"error":"token issuer unavailable"}`))
|
||||
return
|
||||
}
|
||||
tok, err := v.Verify(r.Context(), raw)
|
||||
if err != nil {
|
||||
slog.Info("bearer token rejected", "error", err)
|
||||
unauthorized(w, "invalid token")
|
||||
return
|
||||
}
|
||||
if len(b.audiences) > 0 && !slices.ContainsFunc(tok.Audience, func(a string) bool { return slices.Contains(b.audiences, a) }) {
|
||||
slog.Info("bearer token rejected", "error", "audience not allowed", "aud", tok.Audience)
|
||||
unauthorized(w, "invalid token")
|
||||
return
|
||||
}
|
||||
slog.Info("bearer token accepted", "sub", tok.Subject, "method", r.Method, "path", r.URL.Path)
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// RequireWrites gates only mutating methods.
|
||||
func (b *BearerOIDC) RequireWrites(next http.Handler) http.Handler {
|
||||
gated := b.Require(next)
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.Method {
|
||||
case http.MethodGet, http.MethodHead, http.MethodOptions:
|
||||
next.ServeHTTP(w, r)
|
||||
default:
|
||||
gated.ServeHTTP(w, r)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func unauthorized(w http.ResponseWriter, msg string) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Header().Set("WWW-Authenticate", "Bearer")
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
_, _ = w.Write([]byte(`{"error":"` + msg + `"}`))
|
||||
}
|
||||
@@ -0,0 +1,81 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/testsupport"
|
||||
)
|
||||
|
||||
func serve(h http.Handler, method, token string) int {
|
||||
r := httptest.NewRequest(method, "/x", nil)
|
||||
if token != "" {
|
||||
r.Header.Set("Authorization", "Bearer "+token)
|
||||
}
|
||||
w := httptest.NewRecorder()
|
||||
h.ServeHTTP(w, r)
|
||||
return w.Code
|
||||
}
|
||||
|
||||
var ok = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) })
|
||||
|
||||
func TestBearerOIDC(t *testing.T) {
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
defer iss.Close()
|
||||
h := NewBearerOIDC(iss.URL, []string{"other", "artifactapi"}).Require(ok)
|
||||
|
||||
otherKey, _ := rsa.GenerateKey(rand.Reader, 2048)
|
||||
cases := []struct {
|
||||
name string
|
||||
token string
|
||||
want int
|
||||
}{
|
||||
{"valid", iss.Token(nil, "artifactapi", "ci", time.Hour), http.StatusNoContent},
|
||||
{"missing", "", http.StatusUnauthorized},
|
||||
{"garbage", "not.a.jwt", http.StatusUnauthorized},
|
||||
{"bad signature", iss.Token(otherKey, "artifactapi", "ci", time.Hour), http.StatusUnauthorized},
|
||||
{"wrong aud", iss.Token(nil, "nope", "ci", time.Hour), http.StatusUnauthorized},
|
||||
{"expired", iss.Token(nil, "artifactapi", "ci", -time.Hour), http.StatusUnauthorized},
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := serve(h, http.MethodPost, c.token); got != c.want {
|
||||
t.Errorf("%s: status %d, want %d", c.name, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestBearerOIDCIssuerDown(t *testing.T) {
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
b := NewBearerOIDC(iss.URL, []string{"artifactapi"})
|
||||
tok := iss.Token(nil, "artifactapi", "ci", time.Hour)
|
||||
iss.Close()
|
||||
if got := serve(b.Require(ok), http.MethodPost, tok); got != http.StatusServiceUnavailable {
|
||||
t.Errorf("issuer down: status %d, want 503", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBearerOIDCDisabled(t *testing.T) {
|
||||
if got := serve(NewBearerOIDC("", nil).RequireWrites(ok), http.MethodDelete, ""); got != http.StatusNoContent {
|
||||
t.Errorf("disabled: status %d, want 204", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireWritesLeavesReadsOpen(t *testing.T) {
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
defer iss.Close()
|
||||
h := NewBearerOIDC(iss.URL, []string{"artifactapi"}).RequireWrites(ok)
|
||||
for _, m := range []string{http.MethodGet, http.MethodHead, http.MethodOptions} {
|
||||
if got := serve(h, m, ""); got != http.StatusNoContent {
|
||||
t.Errorf("%s: status %d, want 204", m, got)
|
||||
}
|
||||
}
|
||||
for _, m := range []string{http.MethodPost, http.MethodPut, http.MethodDelete} {
|
||||
if got := serve(h, m, ""); got != http.StatusUnauthorized {
|
||||
t.Errorf("%s: status %d, want 401", m, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
type Config struct {
|
||||
@@ -56,6 +57,10 @@ type Config struct {
|
||||
GitHubAppInstallationID string
|
||||
GitHubAppPrivateKey string
|
||||
GitHubAppPrivateKeyPath string
|
||||
|
||||
// Bearer-JWT auth on configuration writes; empty issuer disables it.
|
||||
OIDCIssuer string
|
||||
OIDCAudiences []string
|
||||
}
|
||||
|
||||
func (c *Config) DatabaseDSN() string {
|
||||
@@ -90,6 +95,17 @@ func Load() (*Config, error) {
|
||||
return nil, fmt.Errorf("invalid GITHUB_SYNC_POLL_INTERVAL: %w", err)
|
||||
}
|
||||
|
||||
oidcIssuer := getenv("ARTIFACTAPI_OIDC_ISSUER", "")
|
||||
var oidcAudiences []string
|
||||
for _, a := range strings.Split(getenv("ARTIFACTAPI_OIDC_AUDIENCE", ""), ",") {
|
||||
if a = strings.TrimSpace(a); a != "" {
|
||||
oidcAudiences = append(oidcAudiences, a)
|
||||
}
|
||||
}
|
||||
if oidcIssuer != "" && len(oidcAudiences) == 0 {
|
||||
return nil, fmt.Errorf("ARTIFACTAPI_OIDC_AUDIENCE is required when ARTIFACTAPI_OIDC_ISSUER is set")
|
||||
}
|
||||
|
||||
cfg := &Config{
|
||||
ListenAddr: getenv("LISTEN_ADDR", ":8000"),
|
||||
|
||||
@@ -123,6 +139,9 @@ func Load() (*Config, error) {
|
||||
GitHubAppInstallationID: getenv("GITHUB_APP_INSTALLATION_ID", ""),
|
||||
GitHubAppPrivateKey: getenv("GITHUB_APP_PRIVATE_KEY", ""),
|
||||
GitHubAppPrivateKeyPath: getenv("GITHUB_APP_PRIVATE_KEY_PATH", ""),
|
||||
|
||||
OIDCIssuer: oidcIssuer,
|
||||
OIDCAudiences: oidcAudiences,
|
||||
}
|
||||
|
||||
return cfg, nil
|
||||
|
||||
@@ -64,3 +64,20 @@ func TestLoadInvalidPort(t *testing.T) {
|
||||
t.Error("expected error for invalid DBPORT")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadOIDC(t *testing.T) {
|
||||
t.Setenv("ARTIFACTAPI_OIDC_ISSUER", "https://vault.example/v1/identity/oidc")
|
||||
t.Setenv("ARTIFACTAPI_OIDC_AUDIENCE", " a, ,b ")
|
||||
cfg, err := Load()
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
}
|
||||
if len(cfg.OIDCAudiences) != 2 || cfg.OIDCAudiences[0] != "a" || cfg.OIDCAudiences[1] != "b" {
|
||||
t.Errorf("audiences = %q", cfg.OIDCAudiences)
|
||||
}
|
||||
|
||||
t.Setenv("ARTIFACTAPI_OIDC_AUDIENCE", "")
|
||||
if _, err := Load(); err == nil {
|
||||
t.Error("expected error for issuer without audience")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@ func cors(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Access-Control-Allow-Origin", "*")
|
||||
w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Content-Type")
|
||||
w.Header().Set("Access-Control-Allow-Headers", "Content-Type, Authorization")
|
||||
if r.Method == http.MethodOptions {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.unkin.net/unkin/artifactapi/internal/testsupport"
|
||||
)
|
||||
|
||||
func TestServerOIDCWriteAuth(t *testing.T) {
|
||||
requireStack(t)
|
||||
iss := testsupport.NewOIDCIssuer()
|
||||
defer iss.Close()
|
||||
|
||||
cfg := *testSrv.cfg
|
||||
cfg.OIDCIssuer = iss.URL
|
||||
cfg.OIDCAudiences = []string{"artifactapi"}
|
||||
s := *testSrv
|
||||
s.cfg = &cfg
|
||||
ts := httptest.NewServer(s.routes())
|
||||
defer ts.Close()
|
||||
|
||||
good := iss.Token(nil, "artifactapi", "ci", time.Hour)
|
||||
do := func(method, path, body, token string) int {
|
||||
t.Helper()
|
||||
rq, _ := http.NewRequest(method, ts.URL+path, strings.NewReader(body))
|
||||
rq.Header.Set("Content-Type", "application/json")
|
||||
if token != "" {
|
||||
rq.Header.Set("Authorization", "Bearer "+token)
|
||||
}
|
||||
resp, err := http.DefaultClient.Do(rq)
|
||||
if err != nil {
|
||||
t.Fatalf("%s %s: %v", method, path, err)
|
||||
}
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
_ = resp.Body.Close()
|
||||
return resp.StatusCode
|
||||
}
|
||||
expect := func(want int, method, path, body, token string) {
|
||||
t.Helper()
|
||||
if got := do(method, path, body, token); got != want {
|
||||
t.Errorf("%s %s: status %d, want %d", method, path, got, want)
|
||||
}
|
||||
}
|
||||
|
||||
remote := `{"name":"srv-oidc","package_type":"generic","repo_type":"local"}`
|
||||
virtual := `{"name":"srv-oidc-v","package_type":"generic","members":["srv-oidc"]}`
|
||||
|
||||
expect(401, "POST", "/api/v2/remotes", remote, "")
|
||||
expect(401, "POST", "/api/v2/remotes", remote, iss.Token(nil, "other", "ci", time.Hour))
|
||||
expect(201, "POST", "/api/v2/remotes", remote, good)
|
||||
expect(401, "PUT", "/api/v2/remotes/srv-oidc", `{"package_type":"generic"}`, "")
|
||||
expect(401, "DELETE", "/api/v2/remotes/srv-oidc", "", "")
|
||||
expect(401, "POST", "/api/v2/virtuals", virtual, "")
|
||||
expect(201, "POST", "/api/v2/virtuals", virtual, good)
|
||||
expect(401, "PUT", "/api/v2/virtuals/srv-oidc-v", virtual, "")
|
||||
expect(401, "DELETE", "/api/v2/virtuals/srv-oidc-v", "", "")
|
||||
|
||||
expect(200, "GET", "/api/v2/remotes", "", "")
|
||||
expect(200, "GET", "/api/v2/remotes/srv-oidc", "", "")
|
||||
expect(200, "GET", "/api/v2/virtuals/srv-oidc-v", "", "")
|
||||
expect(201, "PUT", "/api/v2/remotes/srv-oidc/files/a.bin", "payload", "")
|
||||
expect(200, "GET", "/api/v2/remotes/srv-oidc/files/a.bin", "", "")
|
||||
if got := do("DELETE", "/api/v2/remotes/srv-oidc/files/a.bin", "", ""); got >= 400 {
|
||||
t.Errorf("delete file: status %d", got)
|
||||
}
|
||||
if got := do("POST", "/api/v2/probe", `{}`, ""); got == 401 {
|
||||
t.Error("probe gated")
|
||||
}
|
||||
if got := do("GET", "/v2/", "", ""); got == 401 {
|
||||
t.Error("docker /v2 gated")
|
||||
}
|
||||
|
||||
expect(204, "DELETE", "/api/v2/virtuals/srv-oidc-v", "", good)
|
||||
expect(204, "DELETE", "/api/v2/remotes/srv-oidc", "", good)
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
tfregistry "git.unkin.net/unkin/artifactapi/internal/api/terraform"
|
||||
v1 "git.unkin.net/unkin/artifactapi/internal/api/v1"
|
||||
v2 "git.unkin.net/unkin/artifactapi/internal/api/v2"
|
||||
"git.unkin.net/unkin/artifactapi/internal/auth"
|
||||
"git.unkin.net/unkin/artifactapi/internal/cache"
|
||||
"git.unkin.net/unkin/artifactapi/internal/config"
|
||||
"git.unkin.net/unkin/artifactapi/internal/database"
|
||||
@@ -186,10 +187,11 @@ func (s *Server) routes() chi.Router {
|
||||
statsHandler := v2.NewStatsHandler(s.db)
|
||||
eventsHandler := v2.NewEventsHandler()
|
||||
probeHandler := v2.NewProbeHandler(s.engine, s.db)
|
||||
writeAuth := auth.NewBearerOIDC(s.cfg.OIDCIssuer, s.cfg.OIDCAudiences)
|
||||
|
||||
r.Route("/api/v2", func(r chi.Router) {
|
||||
r.Mount("/remotes", remotesHandler.Routes())
|
||||
r.Mount("/virtuals", virtualsHandler.Routes())
|
||||
r.Mount("/remotes", writeAuth.RequireWrites(remotesHandler.Routes()))
|
||||
r.Mount("/virtuals", writeAuth.RequireWrites(virtualsHandler.Routes()))
|
||||
r.Mount("/health", healthHandler.Routes())
|
||||
r.Mount("/stats", statsHandler.Routes())
|
||||
r.Mount("/events", eventsHandler.Routes())
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
package testsupport
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"time"
|
||||
|
||||
"github.com/go-jose/go-jose/v4"
|
||||
"github.com/go-jose/go-jose/v4/jwt"
|
||||
)
|
||||
|
||||
// OIDCIssuer is a minimal OIDC issuer serving discovery and JWKS, signing
|
||||
// RS256 tokens with a generated key.
|
||||
type OIDCIssuer struct {
|
||||
*httptest.Server
|
||||
key *rsa.PrivateKey
|
||||
}
|
||||
|
||||
func NewOIDCIssuer() *OIDCIssuer {
|
||||
key, err := rsa.GenerateKey(rand.Reader, 2048)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
iss := &OIDCIssuer{key: key}
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{
|
||||
"issuer": iss.URL,
|
||||
"jwks_uri": iss.URL + "/keys",
|
||||
"id_token_signing_alg_values_supported": []string{"RS256"},
|
||||
})
|
||||
})
|
||||
mux.HandleFunc("/keys", func(w http.ResponseWriter, r *http.Request) {
|
||||
_ = json.NewEncoder(w).Encode(jose.JSONWebKeySet{Keys: []jose.JSONWebKey{
|
||||
{Key: &key.PublicKey, KeyID: "k1", Algorithm: "RS256", Use: "sig"},
|
||||
}})
|
||||
})
|
||||
iss.Server = httptest.NewServer(mux)
|
||||
return iss
|
||||
}
|
||||
|
||||
// Token signs a token for aud and sub. Pass a nil key to use the issuer's own;
|
||||
// expiry is relative to now.
|
||||
func (o *OIDCIssuer) Token(key *rsa.PrivateKey, aud, sub string, expiry time.Duration) string {
|
||||
if key == nil {
|
||||
key = o.key
|
||||
}
|
||||
sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: key},
|
||||
(&jose.SignerOptions{}).WithType("JWT").WithHeader("kid", "k1"))
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
now := time.Now()
|
||||
tok, err := jwt.Signed(sig).Claims(jwt.Claims{
|
||||
Issuer: o.URL,
|
||||
Subject: sub,
|
||||
Audience: jwt.Audience{aud},
|
||||
IssuedAt: jwt.NewNumericDate(now.Add(-time.Minute)),
|
||||
Expiry: jwt.NewNumericDate(now.Add(expiry)),
|
||||
}).Serialize()
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return tok
|
||||
}
|
||||
Reference in New Issue
Block a user