try and provide more helpful reponses when oauth expires and when pushing manifest lists

This commit is contained in:
Evan Jarrett
2025-11-25 09:25:38 -06:00
parent 66037c332e
commit 5f1eb05a96
7 changed files with 434 additions and 42 deletions
+6 -32
View File
@@ -2,40 +2,16 @@
# Triggers on version tags and builds cross-platform binaries using buildah
when:
- event: ["manual"]
- event: ["push"]
tag: ["v*"]
engine: "nixery"
dependencies:
nixpkgs:
- buildah
- gnugrep # Required for tag detection
engine: "kubernetes"
environment:
IMAGE_REGISTRY: atcr.io
IMAGE_USER: evan.jarrett.net
steps:
- name: Get tag for current commit
command: |
# Fetch tags (shallow clone doesn't include them by default)
git fetch --tags
# Find the tag that points to the current commit
TAG=$(git tag --points-at HEAD | grep -E '^v[0-9]' | head -n1)
if [ -z "$TAG" ]; then
echo "Error: No version tag found for current commit"
echo "Available tags:"
git tag
echo "Current commit:"
git rev-parse HEAD
exit 1
fi
echo "Building version: $TAG"
echo "$TAG" > .version
- name: Setup build environment
command: |
@@ -53,11 +29,10 @@ steps:
- name: Build and push AppView image
command: |
TAG=$(cat .version)
echo ${TANGLED_REF_NAME}
buildah bud \
--storage-driver vfs \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-appview:${TAG} \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-appview:${TANGLED_REF_NAME} \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-appview:latest \
--file ./Dockerfile.appview \
.
@@ -68,11 +43,10 @@ steps:
- name: Build and push Hold image
command: |
TAG=$(cat .version)
echo ${TANGLED_REF_NAME}
buildah bud \
--storage-driver vfs \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-hold:${TAG} \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-hold:${TANGLED_REF_NAME} \
--tag ${IMAGE_REGISTRY}/${IMAGE_USER}/atcr-hold:latest \
--file ./Dockerfile.hold \
.
+4
View File
@@ -409,6 +409,10 @@ func serveRegistry(cmd *cobra.Command, args []string) error {
// Basic Auth token endpoint (supports device secrets and app passwords)
tokenHandler := token.NewHandler(issuer, deviceStore)
// Register OAuth session checker for device auth validation
// This ensures device secrets only work when the linked OAuth session exists
tokenHandler.SetOAuthSessionChecker(oauthStore)
// Register token post-auth callback for profile management
// This decouples the token package from AppView-specific dependencies
tokenHandler.SetPostAuthCallback(func(ctx context.Context, did, handle, pdsEndpoint, accessToken string) error {
+79 -6
View File
@@ -67,6 +67,20 @@ type DeviceTokenResponse struct {
Error string `json:"error,omitempty"`
}
// AuthErrorResponse is the JSON error response from /auth/token
type AuthErrorResponse struct {
Error string `json:"error"`
Message string `json:"message"`
LoginURL string `json:"login_url,omitempty"`
}
// ValidationResult represents the result of credential validation
type ValidationResult struct {
Valid bool
OAuthSessionExpired bool
LoginURL string
}
var (
version = "dev"
commit = "none"
@@ -123,7 +137,44 @@ func handleGet() {
// If credentials exist, validate them
if found && deviceConfig.DeviceSecret != "" {
if !validateCredentials(appViewURL, deviceConfig.Handle, deviceConfig.DeviceSecret) {
result := validateCredentials(appViewURL, deviceConfig.Handle, deviceConfig.DeviceSecret)
if !result.Valid {
if result.OAuthSessionExpired {
// OAuth session expired - need to re-authenticate via browser
// Device secret is still valid, just need to restore OAuth session
fmt.Fprintf(os.Stderr, "OAuth session expired. Opening browser to re-authenticate...\n")
loginURL := result.LoginURL
if loginURL == "" {
loginURL = appViewURL + "/auth/oauth/login"
}
// Try to open browser
if err := openBrowser(loginURL); err != nil {
fmt.Fprintf(os.Stderr, "Could not open browser automatically.\n")
fmt.Fprintf(os.Stderr, "Please visit: %s\n", loginURL)
} else {
fmt.Fprintf(os.Stderr, "Please complete authentication in your browser.\n")
}
// Wait for user to complete OAuth flow, then retry
fmt.Fprintf(os.Stderr, "Waiting for authentication")
for i := 0; i < 60; i++ { // Wait up to 2 minutes
time.Sleep(2 * time.Second)
fmt.Fprintf(os.Stderr, ".")
// Retry validation
retryResult := validateCredentials(appViewURL, deviceConfig.Handle, deviceConfig.DeviceSecret)
if retryResult.Valid {
fmt.Fprintf(os.Stderr, "\n✓ Re-authenticated successfully!\n")
goto credentialsValid
}
}
fmt.Fprintf(os.Stderr, "\nAuthentication timed out. Please try again.\n")
os.Exit(1)
}
// Generic auth failure - delete credentials and re-authorize
fmt.Fprintf(os.Stderr, "Stored credentials for %s are invalid or expired\n", appViewURL)
// Delete the invalid credentials
delete(allCreds.Credentials, appViewURL)
@@ -134,6 +185,7 @@ func handleGet() {
found = false
}
}
credentialsValid:
if !found || deviceConfig.DeviceSecret == "" {
// No credentials for this AppView
@@ -550,7 +602,7 @@ func isTerminal(f *os.File) bool {
}
// validateCredentials checks if the credentials are still valid by making a test request
func validateCredentials(appViewURL, handle, deviceSecret string) bool {
func validateCredentials(appViewURL, handle, deviceSecret string) ValidationResult {
// Call /auth/token to validate device secret and get JWT
// This is the proper way to validate credentials - /v2/ requires JWT, not Basic Auth
client := &http.Client{
@@ -562,7 +614,7 @@ func validateCredentials(appViewURL, handle, deviceSecret string) bool {
req, err := http.NewRequest("GET", tokenURL, nil)
if err != nil {
return false
return ValidationResult{Valid: false}
}
// Set basic auth with device credentials
@@ -572,12 +624,33 @@ func validateCredentials(appViewURL, handle, deviceSecret string) bool {
if err != nil {
// Network error - assume credentials are valid but server unreachable
// Don't trigger re-auth on network issues
return true
return ValidationResult{Valid: true}
}
defer resp.Body.Close()
// 200 = valid credentials
// 401 = invalid/expired credentials
if resp.StatusCode == http.StatusOK {
return ValidationResult{Valid: true}
}
// 401 = check if it's OAuth session expired
if resp.StatusCode == http.StatusUnauthorized {
// Try to parse JSON error response
body, err := io.ReadAll(resp.Body)
if err == nil {
var authErr AuthErrorResponse
if json.Unmarshal(body, &authErr) == nil && authErr.Error == "oauth_session_expired" {
return ValidationResult{
Valid: false,
OAuthSessionExpired: true,
LoginURL: authErr.LoginURL,
}
}
}
// Generic auth failure
return ValidationResult{Valid: false}
}
// Any other error = assume valid (don't re-auth on server issues)
return resp.StatusCode == http.StatusOK
return ValidationResult{Valid: true}
}
+14
View File
@@ -212,6 +212,20 @@ func (s *OAuthStore) GetLatestSessionForDID(ctx context.Context, did string) (*o
return &sessionData, sessionID, nil
}
// HasSessionForDID checks if an OAuth session exists for the given DID
// This is a lightweight check used by the token handler to verify device auth
func (s *OAuthStore) HasSessionForDID(ctx context.Context, did string) bool {
var count int
err := s.db.QueryRowContext(ctx, `
SELECT COUNT(*) FROM oauth_sessions WHERE account_did = ?
`, did).Scan(&count)
if err != nil {
slog.Debug("Failed to check session existence", "did", did, "error", err)
return false
}
return count > 0
}
// CleanupOldSessions removes sessions older than the specified duration
func (s *OAuthStore) CleanupOldSessions(ctx context.Context, olderThan time.Duration) error {
cutoff := time.Now().Add(-olderThan)
+31
View File
@@ -143,6 +143,37 @@ func (s *ManifestStore) Put(ctx context.Context, manifest distribution.Manifest,
isManifestList := strings.Contains(manifestRecord.MediaType, "manifest.list") ||
strings.Contains(manifestRecord.MediaType, "image.index")
// Validate manifest list child references
// Reject manifest lists that reference non-existent child manifests
// This matches Docker Hub/ECR behavior and prevents users from accidentally pushing
// manifest lists where the underlying images don't exist
if isManifestList {
for _, ref := range manifestRecord.Manifests {
// Check if referenced manifest exists in user's PDS
refDigest, err := digest.Parse(ref.Digest)
if err != nil {
return "", fmt.Errorf("invalid digest in manifest list: %s", ref.Digest)
}
exists, err := s.Exists(ctx, refDigest)
if err != nil {
return "", fmt.Errorf("failed to check manifest reference: %w", err)
}
if !exists {
platform := "unknown"
if ref.Platform != nil {
platform = fmt.Sprintf("%s/%s", ref.Platform.OS, ref.Platform.Architecture)
}
slog.Warn("Manifest list references non-existent child manifest",
"repository", s.ctx.Repository,
"missingDigest", ref.Digest,
"platform", platform)
return "", distribution.ErrManifestBlobUnknown{Digest: refDigest}
}
}
}
if !isManifestList && s.blobStore != nil && manifestRecord.Config != nil && manifestRecord.Config.Digest != "" {
labels, err := s.extractConfigLabels(ctx, manifestRecord.Config.Digest)
if err != nil {
+247
View File
@@ -3,6 +3,7 @@ package storage
import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"net/http/httptest"
@@ -912,3 +913,249 @@ func TestManifestStore_Delete(t *testing.T) {
})
}
}
// TestManifestStore_Put_ManifestListValidation tests validation of manifest list child references
func TestManifestStore_Put_ManifestListValidation(t *testing.T) {
// Create a valid child manifest that exists
childManifest := []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.manifest.v1+json",
"config":{"digest":"sha256:config123","size":100},
"layers":[{"digest":"sha256:layer1","size":200}]
}`)
childDigest := digest.FromBytes(childManifest)
tests := []struct {
name string
manifestList []byte
childExists bool // Whether the child manifest exists
wantErr bool
wantErrType string // "ErrManifestBlobUnknown" or empty
checkErrDigest string // Expected digest in error
}{
{
name: "valid manifest list - child exists",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"` + childDigest.String() + `","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"amd64"}}
]
}`),
childExists: true,
wantErr: false,
},
{
name: "invalid manifest list - child does not exist",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"amd64"}}
]
}`),
childExists: false,
wantErr: true,
wantErrType: "ErrManifestBlobUnknown",
checkErrDigest: "sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
},
{
name: "attestation-only manifest list - attestation must also exist",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"sha256:4444444444444444444444444444444444444444444444444444444444444444","size":100,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"unknown","architecture":"unknown"}}
]
}`),
childExists: false,
wantErr: true,
wantErrType: "ErrManifestBlobUnknown",
checkErrDigest: "sha256:4444444444444444444444444444444444444444444444444444444444444444",
},
{
name: "mixed manifest list - real platform missing, attestation present",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"sha256:1111111111111111111111111111111111111111111111111111111111111111","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"arm64"}},
{"digest":"sha256:5555555555555555555555555555555555555555555555555555555555555555","size":100,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"unknown","architecture":"unknown"}}
]
}`),
childExists: false,
wantErr: true,
wantErrType: "ErrManifestBlobUnknown",
checkErrDigest: "sha256:1111111111111111111111111111111111111111111111111111111111111111",
},
{
name: "docker manifest list media type - child missing",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.docker.distribution.manifest.list.v2+json",
"manifests":[
{"digest":"sha256:2222222222222222222222222222222222222222222222222222222222222222","size":300,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","platform":{"os":"linux","architecture":"amd64"}}
]
}`),
childExists: false,
wantErr: true,
wantErrType: "ErrManifestBlobUnknown",
checkErrDigest: "sha256:2222222222222222222222222222222222222222222222222222222222222222",
},
{
name: "manifest list with nil platform - should still validate",
manifestList: []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"sha256:3333333333333333333333333333333333333333333333333333333333333333","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json"}
]
}`),
childExists: false,
wantErr: true,
wantErrType: "ErrManifestBlobUnknown",
checkErrDigest: "sha256:3333333333333333333333333333333333333333333333333333333333333333",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Track GetRecord calls for manifest existence checks
getRecordCalls := make(map[string]bool)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Handle uploadBlob
if r.URL.Path == atproto.RepoUploadBlob {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"blob":{"$type":"blob","ref":{"$link":"bafytest"},"mimeType":"application/json","size":100}}`))
return
}
// Handle getRecord (for Exists check)
if r.URL.Path == atproto.RepoGetRecord {
rkey := r.URL.Query().Get("rkey")
getRecordCalls[rkey] = true
// If child should exist, return it; otherwise return RecordNotFound
if tt.childExists || rkey == childDigest.Encoded() {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.manifest/` + rkey + `","cid":"bafytest","value":{}}`))
} else {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(`{"error":"RecordNotFound","message":"Record not found"}`))
}
return
}
// Handle putRecord
if r.URL.Path == atproto.RepoPutRecord {
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.manifest/test123","cid":"bafytest"}`))
return
}
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
client := atproto.NewClient(server.URL, "did:plc:test123", "token")
db := &mockDatabaseMetrics{}
ctx := mockRegistryContext(client, "myapp", "did:web:hold.example.com", "did:plc:test123", "test.handle", db)
store := NewManifestStore(ctx, nil)
manifest := &rawManifest{
mediaType: "application/vnd.oci.image.index.v1+json",
payload: tt.manifestList,
}
_, err := store.Put(context.Background(), manifest)
if (err != nil) != tt.wantErr {
t.Errorf("Put() error = %v, wantErr %v", err, tt.wantErr)
return
}
if tt.wantErr && tt.wantErrType == "ErrManifestBlobUnknown" {
// Check that the error is of the correct type
var blobErr distribution.ErrManifestBlobUnknown
if !errors.As(err, &blobErr) {
t.Errorf("Put() error type = %T, want distribution.ErrManifestBlobUnknown", err)
return
}
// Check that the error contains the expected digest
if tt.checkErrDigest != "" {
expectedDigest, _ := digest.Parse(tt.checkErrDigest)
if blobErr.Digest != expectedDigest {
t.Errorf("ErrManifestBlobUnknown.Digest = %v, want %v", blobErr.Digest, expectedDigest)
}
}
}
})
}
}
// TestManifestStore_Put_ManifestListValidation_MultipleChildren tests validation with multiple child manifests
func TestManifestStore_Put_ManifestListValidation_MultipleChildren(t *testing.T) {
// Create two valid child manifests
childManifest1 := []byte(`{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"digest":"sha256:config1","size":100},"layers":[]}`)
childManifest2 := []byte(`{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"digest":"sha256:config2","size":100},"layers":[]}`)
childDigest1 := digest.FromBytes(childManifest1)
childDigest2 := digest.FromBytes(childManifest2)
// Track which manifests exist
existingManifests := map[string]bool{
childDigest1.Encoded(): true,
childDigest2.Encoded(): true,
}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == atproto.RepoUploadBlob {
w.Write([]byte(`{"blob":{"$type":"blob","ref":{"$link":"bafytest"},"size":100}}`))
return
}
if r.URL.Path == atproto.RepoGetRecord {
rkey := r.URL.Query().Get("rkey")
if existingManifests[rkey] {
w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.manifest/` + rkey + `","cid":"bafytest","value":{}}`))
} else {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(`{"error":"RecordNotFound"}`))
}
return
}
if r.URL.Path == atproto.RepoPutRecord {
w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.manifest/test123","cid":"bafytest"}`))
return
}
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
client := atproto.NewClient(server.URL, "did:plc:test123", "token")
ctx := mockRegistryContext(client, "myapp", "did:web:hold.example.com", "did:plc:test123", "test.handle", nil)
store := NewManifestStore(ctx, nil)
// Create manifest list with both children
manifestList := []byte(`{
"schemaVersion":2,
"mediaType":"application/vnd.oci.image.index.v1+json",
"manifests":[
{"digest":"` + childDigest1.String() + `","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"amd64"}},
{"digest":"` + childDigest2.String() + `","size":300,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"arm64"}}
]
}`)
manifest := &rawManifest{
mediaType: "application/vnd.oci.image.index.v1+json",
payload: manifestList,
}
_, err := store.Put(context.Background(), manifest)
if err != nil {
t.Errorf("Put() should succeed when all child manifests exist, got error: %v", err)
}
}
+53 -4
View File
@@ -20,12 +20,20 @@ import (
// without coupling the token package to AppView-specific dependencies.
type PostAuthCallback func(ctx context.Context, did, handle, pdsEndpoint, accessToken string) error
// OAuthSessionChecker checks if an OAuth session exists for a DID
// This interface allows the token handler to verify OAuth sessions without
// depending directly on the OAuth store implementation.
type OAuthSessionChecker interface {
HasSessionForDID(ctx context.Context, did string) bool
}
// Handler handles /auth/token requests
type Handler struct {
issuer *Issuer
validator *auth.SessionValidator
deviceStore *db.DeviceStore // For validating device secrets
postAuthCallback PostAuthCallback
issuer *Issuer
validator *auth.SessionValidator
deviceStore *db.DeviceStore // For validating device secrets
postAuthCallback PostAuthCallback
oauthSessionChecker OAuthSessionChecker
}
// NewHandler creates a new token handler
@@ -43,6 +51,12 @@ func (h *Handler) SetPostAuthCallback(callback PostAuthCallback) {
h.postAuthCallback = callback
}
// SetOAuthSessionChecker sets the OAuth session checker for validating device auth
// When set, the handler will verify OAuth sessions exist before issuing tokens for device auth
func (h *Handler) SetOAuthSessionChecker(checker OAuthSessionChecker) {
h.oauthSessionChecker = checker
}
// TokenResponse represents the response from /auth/token
type TokenResponse struct {
Token string `json:"token,omitempty"` // Legacy field
@@ -80,6 +94,31 @@ To authenticate:
(use your ATProto handle + app-password)`, message, baseURL, r.Host), http.StatusUnauthorized)
}
// AuthErrorResponse is returned when authentication fails in a way the credential helper can handle
type AuthErrorResponse struct {
Error string `json:"error"`
Message string `json:"message"`
LoginURL string `json:"login_url,omitempty"`
}
// sendOAuthSessionExpiredError sends a JSON error response when OAuth session is missing
// This allows the credential helper to detect this specific error and open the browser
func sendOAuthSessionExpiredError(w http.ResponseWriter, r *http.Request) {
baseURL := getBaseURL(r)
loginURL := baseURL + "/auth/oauth/login"
w.Header().Set("WWW-Authenticate", `Basic realm="ATCR Registry"`)
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusUnauthorized)
resp := AuthErrorResponse{
Error: "oauth_session_expired",
Message: "OAuth session expired or invalidated. Please re-authenticate in your browser.",
LoginURL: loginURL,
}
json.NewEncoder(w).Encode(resp)
}
// ServeHTTP handles the token request
func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
slog.Debug("Received token request", "method", r.Method, "path", r.URL.Path)
@@ -130,6 +169,16 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}
// Check if OAuth session exists for this device's DID
// Device secrets are permanent, but they require an active OAuth session to work
if h.oauthSessionChecker != nil {
if !h.oauthSessionChecker.HasSessionForDID(r.Context(), device.DID) {
slog.Debug("No OAuth session for device", "did", device.DID)
sendOAuthSessionExpiredError(w, r)
return
}
}
did = device.DID
handle = device.Handle
authMethod = AuthMethodOAuth