Skip to content

Fix auth probe denial cache #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joecorall merged 1 commit into from
May 1, 2026
Merged

Fix auth probe denial cache #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions docs/authorization.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,13 @@ repository permissions change rarely or when the mapping is used for content
whose access is primarily stable. Shorter TTLs keep permission changes visible
sooner at the cost of more probes to the upstream Drupal URL.

Triplet caches successful probes, 401, 403, and 404 responses for the
configured TTL. Other upstream errors are not cached.

Negative auth-probe caching is conservative. For 401, 403, and 404 probe
responses, Triplet checks the upstream `Last-Modified` header before caching the
denial. If `Last-Modified` parses and is newer than
5 minutes ago, the denial is not cached. This avoids holding a stale denial
while repository access rules or file publication are still settling. If
`Last-Modified` is absent, unparseable, or older than that minimum age window,
the denial can be cached for the configured auth TTL.
Triplet caches successful probes for the configured TTL. It does not cache
401/403 authorization denials, because those commonly happen while Drupal is
still publishing or recalculating access. Other upstream errors are not cached.

It also does not cache 404 not-found responses. A 404 can happen while Drupal
or Fedora is still publishing a just-created file, so Triplet rechecks upstream
on the next request instead of holding a stale miss.

The image cache invalidation route also clears matching auth-probe entries when
the source backend supports it.
Expand Down
50 changes: 15 additions & 35 deletions internal/storage/local_url.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,12 @@ type LocalURLFallback struct {

// LocalURLMapping maps a URL identifier prefix to a local file source.
type LocalURLMapping struct {
Prefix string
File *FileOpener
OCFL bool
AuthProbe bool
AuthCacheTTL time.Duration
AuthErrorCacheMinAge time.Duration
AuthCacheMaxEntries int
Prefix string
File *FileOpener
OCFL bool
AuthProbe bool
AuthCacheTTL time.Duration
AuthCacheMaxEntries int
}

type authCacheEntry struct {
Expand All @@ -65,10 +64,9 @@ var errAuthProbeHeadUnsupported = errors.New("auth probe head unsupported")
type authCacheTier string

const (
authCacheTierAnonymous authCacheTier = "anonymous"
authCacheTierAuthenticated authCacheTier = "authenticated"
defaultAuthErrorCacheMinAge = 5 * time.Minute
defaultAuthCacheMaxEntries = 4096
authCacheTierAnonymous authCacheTier = "anonymous"
authCacheTierAuthenticated authCacheTier = "authenticated"
defaultAuthCacheMaxEntries = 4096
)

// Open implements Opener.
Expand Down Expand Up @@ -315,7 +313,7 @@ func (l *LocalURLFallback) authorize(ctx context.Context, identifier string, map
}
return l.authorizeAuthenticated(ctx, identifier, mapping, headers)
}
anonErr := l.probeCached(ctx, anonKey, identifier, nil, mapping.anonymousAuthTTL(), mapping.authErrorCacheMinAge(), mapping.authCacheMaxEntries())
anonErr := l.probeCached(ctx, anonKey, identifier, nil, mapping.anonymousAuthTTL(), mapping.authCacheMaxEntries())
if anonErr == nil || !hasAuthHeaders(headers) {
return anonErr
}
Expand All @@ -330,10 +328,10 @@ func (l *LocalURLFallback) authorizeAuthenticated(ctx context.Context, identifie
if err, ok := l.cachedAuth(key); ok {
return err
}
return l.probeCached(ctx, key, identifier, headers, mapping.authenticatedAuthTTL(), mapping.authErrorCacheMinAge(), mapping.authCacheMaxEntries())
return l.probeCached(ctx, key, identifier, headers, mapping.authenticatedAuthTTL(), mapping.authCacheMaxEntries())
}

func (l *LocalURLFallback) probeCached(ctx context.Context, key, identifier string, headers http.Header, ttl, errorMinAge time.Duration, maxEntries int) error {
func (l *LocalURLFallback) probeCached(ctx context.Context, key, identifier string, headers http.Header, ttl time.Duration, maxEntries int) error {
if wait, ok := l.beginAuthProbe(key); ok {
l.debug(ctx, "local url auth probe waiting on in-flight probe", identifier, "cache_key", redact.Hash(key))
select {
Expand All @@ -344,8 +342,8 @@ func (l *LocalURLFallback) probeCached(ctx context.Context, key, identifier stri
return ctx.Err()
}
}
respHeader, err := l.probe(ctx, identifier, headers)
cacheable := cacheableAuthProbeResult(err) && cacheableAuthProbeResponse(err, respHeader, errorMinAge, time.Now())
_, err := l.probe(ctx, identifier, headers)
cacheable := cacheableAuthProbeResult(err)
l.debug(ctx, "local url auth probe completed", identifier, "cache_key", redact.Hash(key), "cacheable", cacheable, "ttl", ttl.String(), "err", err)
if ttl > 0 && cacheable {
l.storeAuth(key, authCacheScope(key), identifier, err, ttl, maxEntries)
Expand Down Expand Up @@ -388,18 +386,7 @@ func hasAuthHeaders(headers http.Header) bool {
}

func cacheableAuthProbeResult(err error) bool {
return err == nil || errors.Is(err, ErrForbidden) || errors.Is(err, ErrNotFound)
}

func cacheableAuthProbeResponse(err error, header http.Header, errorMinAge time.Duration, now time.Time) bool {
if err == nil || (!errors.Is(err, ErrForbidden) && !errors.Is(err, ErrNotFound)) || errorMinAge <= 0 {
return true
}
modified, parseErr := http.ParseTime(header.Get("Last-Modified"))
if parseErr != nil {
return true
}
return !modified.After(now.Add(-errorMinAge))
return err == nil
}

func (m LocalURLMapping) anonymousAuthTTL() time.Duration {
Expand All @@ -410,13 +397,6 @@ func (m LocalURLMapping) authenticatedAuthTTL() time.Duration {
return m.AuthCacheTTL
}

func (m LocalURLMapping) authErrorCacheMinAge() time.Duration {
if m.AuthErrorCacheMinAge > 0 {
return m.AuthErrorCacheMinAge
}
return defaultAuthErrorCacheMinAge
}

func (m LocalURLMapping) authCacheMaxEntries() int {
if m.AuthCacheMaxEntries > 0 {
return m.AuthCacheMaxEntries
Expand Down
35 changes: 16 additions & 19 deletions internal/storage/local_url_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ func TestLocalURLFallbackAuthenticatedAuthProbeTTL(t *testing.T) {
time.Sleep(ttl - time.Nanosecond)
synctest.Wait()
openAndClose(t, op, ctx, identifier)
if got := anonProbes.Load(); got != 1 {
if got := anonProbes.Load(); got != 2 {
t.Fatalf("anonymous probes before auth ttl = %d", got)
}
if got := authProbes.Load(); got != 1 {
Expand All @@ -584,7 +584,7 @@ func TestLocalURLFallbackAuthenticatedAuthProbeTTL(t *testing.T) {
time.Sleep(2 * time.Nanosecond)
synctest.Wait()
openAndClose(t, op, ctx, identifier)
if got := anonProbes.Load(); got != 2 {
if got := anonProbes.Load(); got != 3 {
t.Fatalf("anonymous probes after auth ttl = %d", got)
}
if got := authProbes.Load(); got != 2 {
Expand Down Expand Up @@ -636,7 +636,7 @@ func TestLocalURLFallbackAuthProbeAnonymousSucceedsAndCaches(t *testing.T) {
}
}

func TestLocalURLFallbackAuthProbeDoesNotCacheRecentError(t *testing.T) {
func TestLocalURLFallbackAuthProbeDoesNotCacheForbidden(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "private.jp2"), []byte("private"), 0o600); err != nil {
t.Fatal(err)
Expand All @@ -654,11 +654,10 @@ func TestLocalURLFallbackAuthProbeDoesNotCacheRecentError(t *testing.T) {
}
op := &LocalURLFallback{
Mappings: []LocalURLMapping{{
Prefix: srv.URL + "/system/files",
File: fileOp,
AuthProbe: true,
AuthCacheTTL: time.Minute,
AuthErrorCacheMinAge: time.Hour,
Prefix: srv.URL + "/system/files",
File: fileOp,
AuthProbe: true,
AuthCacheTTL: time.Minute,
}},
Fallback: errOpener{},
AuthFallback: testAuthHTTP(t, srv),
Expand All @@ -675,16 +674,15 @@ func TestLocalURLFallbackAuthProbeDoesNotCacheRecentError(t *testing.T) {
}
}

func TestLocalURLFallbackAuthProbeCachesOldError(t *testing.T) {
func TestLocalURLFallbackAuthProbeDoesNotCacheNotFound(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "private.jp2"), []byte("private"), 0o600); err != nil {
t.Fatal(err)
}
var probes atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
probes.Add(1)
w.Header().Set("Last-Modified", time.Now().Add(-2*time.Hour).UTC().Format(http.TimeFormat))
w.WriteHeader(http.StatusForbidden)
w.WriteHeader(http.StatusNotFound)
}))
defer srv.Close()
fileOp, err := NewFileOpener(root)
Expand All @@ -693,23 +691,22 @@ func TestLocalURLFallbackAuthProbeCachesOldError(t *testing.T) {
}
op := &LocalURLFallback{
Mappings: []LocalURLMapping{{
Prefix: srv.URL + "/system/files",
File: fileOp,
AuthProbe: true,
AuthCacheTTL: time.Minute,
AuthErrorCacheMinAge: time.Hour,
Prefix: srv.URL + "/system/files",
File: fileOp,
AuthProbe: true,
AuthCacheTTL: time.Minute,
}},
Fallback: errOpener{},
AuthFallback: testAuthHTTP(t, srv),
}

for i := 0; i < 2; i++ {
_, _, err := op.Open(context.Background(), srv.URL+"/system/files/private.jp2")
if !errors.Is(err, ErrForbidden) {
if !errors.Is(err, ErrNotFound) {
t.Fatalf("err = %v", err)
}
}
if got := probes.Load(); got != 1 {
if got := probes.Load(); got != 2 {
t.Fatalf("probes = %d", got)
}
}
Expand Down Expand Up @@ -753,7 +750,7 @@ func TestLocalURLFallbackAuthProbeFallsBackToCredentialedCache(t *testing.T) {
}
_ = rc.Close()
}
if got := probes.Load(); got != 2 {
if got := probes.Load(); got != 3 {
t.Fatalf("probes = %d", got)
}
}
Expand Down
Loading