server: a non-admin account can manage its own uploads
Signing in and being allowed to administer the server were the same question: the OIDC callback refused a session outright to anyone outside the admin group. A legitimate user could authenticate, be told what they could not do, and be left with no way to see or delete the data their own devices had uploaded. They are separate questions now. Everyone who authenticates gets a session; the admin flag rides inside the MAC'd payload, so promoting yourself means forging a signature rather than editing a cookie, and a role that does not parse fails closed to "user". Pages scope themselves through visibleDevices/mayTouchRun rather than filtering individually — per-page scoping is what the next page added will be missing, and that failure is silent, since a listing that leaks other people's uploads looks exactly like one that does not. Someone else's run answers 404, not 403: a distinguishable refusal would confirm the run exists. Revoking devices and minting enrolment tokens affect the whole server and stay behind adminOnly at the route table, where someone looking for who-may-do-what will actually find it. Ownership is re-read per request instead of captured at sign-in, so unlinking an account takes effect immediately rather than at session expiry. Tests cover that, plus the degenerate case of an empty subject, which must own nothing rather than everything with an empty account id. Also: attribute the ICMPv6 finding per network. It compared "is IPv6 configured anywhere on this device" against "did any network answer", which on a phone reports IPv6-is-broken about a network where IPv6 was never configured. network_ref is null on every test, so the probe now records per-network outcomes structurally rather than as prose a finding would have to parse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7eaf0c4190
commit
7a5004f293
@@ -166,12 +166,21 @@ func (t *Throttle) Succeeded() {
|
||||
|
||||
// ---- sessions ---------------------------------------------------------------------------
|
||||
|
||||
// Session is an authenticated admin, however they proved it.
|
||||
// Session is an authenticated account, however it proved itself. Not necessarily an admin:
|
||||
// signing in and being allowed to administer the server are separate questions, and a plain user
|
||||
// gets a session so they can manage their own uploads.
|
||||
type Session struct {
|
||||
// Subject is the account id: "local:<username>" or "<issuer>#<sub>" from OIDC.
|
||||
Subject string
|
||||
// Display is what the UI shows.
|
||||
Display string
|
||||
// Admin is authorisation, decided at sign-in and carried inside the signed payload.
|
||||
//
|
||||
// Inside, specifically — not derived later from the subject, and not stored beside the MAC.
|
||||
// A flag outside the signature is a privilege escalation anyone can perform with a text
|
||||
// editor, and re-deriving it per request would mean re-reading group membership from the IdP
|
||||
// on a path that has no token to do it with.
|
||||
Admin bool
|
||||
Expires time.Time
|
||||
}
|
||||
|
||||
@@ -203,12 +212,16 @@ func NewSecret() ([]byte, error) {
|
||||
|
||||
var ErrSession = errors.New("session is not valid")
|
||||
|
||||
// Issue returns the cookie value for a newly authenticated admin.
|
||||
func (s *Sessions) Issue(subject, display string) string {
|
||||
// Issue returns the cookie value for a newly authenticated account.
|
||||
func (s *Sessions) Issue(subject, display string, admin bool) string {
|
||||
exp := time.Now().Add(s.ttl).Unix()
|
||||
role := "u"
|
||||
if admin {
|
||||
role = "a"
|
||||
}
|
||||
payload := base64.RawURLEncoding.EncodeToString([]byte(subject)) + "." +
|
||||
base64.RawURLEncoding.EncodeToString([]byte(display)) + "." +
|
||||
strconv.FormatInt(exp, 10)
|
||||
strconv.FormatInt(exp, 10) + "." + role
|
||||
return payload + "." + s.mac(payload)
|
||||
}
|
||||
|
||||
@@ -225,7 +238,7 @@ func (s *Sessions) Parse(value string) (*Session, error) {
|
||||
return nil, ErrSession
|
||||
}
|
||||
parts := strings.Split(payload, ".")
|
||||
if len(parts) != 3 {
|
||||
if len(parts) != 4 {
|
||||
return nil, ErrSession
|
||||
}
|
||||
subject, err := base64.RawURLEncoding.DecodeString(parts[0])
|
||||
@@ -243,7 +256,13 @@ func (s *Sessions) Parse(value string) (*Session, error) {
|
||||
if time.Now().After(time.Unix(exp, 0)) {
|
||||
return nil, fmt.Errorf("%w: expired", ErrSession)
|
||||
}
|
||||
return &Session{Subject: string(subject), Display: string(display), Expires: time.Unix(exp, 0)}, nil
|
||||
// Anything that is not exactly the admin marker is a user. A malformed role must fail closed:
|
||||
// the safe reading of an unparseable privilege claim is the smaller privilege.
|
||||
admin := parts[3] == "a"
|
||||
return &Session{
|
||||
Subject: string(subject), Display: string(display),
|
||||
Admin: admin, Expires: time.Unix(exp, 0),
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (s *Sessions) mac(payload string) string {
|
||||
|
||||
@@ -4,6 +4,8 @@
|
||||
package adminauth
|
||||
|
||||
import (
|
||||
"encoding/base64"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -96,7 +98,7 @@ func TestAnEmptyCredentialNeverVerifies(t *testing.T) {
|
||||
func TestSessionRoundTrip(t *testing.T) {
|
||||
secret, _ := NewSecret()
|
||||
s := NewSessions(secret, time.Hour)
|
||||
got, err := s.Parse(s.Issue("local:admin", "Admin"))
|
||||
got, err := s.Parse(s.Issue("local:admin", "Admin", true))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -110,7 +112,7 @@ func TestSessionRoundTrip(t *testing.T) {
|
||||
func TestTamperedSessionsAreRejected(t *testing.T) {
|
||||
secret, _ := NewSecret()
|
||||
s := NewSessions(secret, time.Hour)
|
||||
good := s.Issue("local:admin", "Admin")
|
||||
good := s.Issue("local:admin", "Admin", true)
|
||||
|
||||
parts := strings.Split(good, ".")
|
||||
tampered := []string{
|
||||
@@ -131,7 +133,7 @@ func TestTamperedSessionsAreRejected(t *testing.T) {
|
||||
func TestSessionsFromAnotherSecretAreRejected(t *testing.T) {
|
||||
a, _ := NewSecret()
|
||||
b, _ := NewSecret()
|
||||
issued := NewSessions(a, time.Hour).Issue("local:admin", "Admin")
|
||||
issued := NewSessions(a, time.Hour).Issue("local:admin", "Admin", true)
|
||||
if _, err := NewSessions(b, time.Hour).Parse(issued); err == nil {
|
||||
t.Fatal("a session signed with a different secret was accepted — rotating the secret " +
|
||||
"must invalidate every existing session")
|
||||
@@ -143,7 +145,7 @@ func TestExpiredSessionsAreRejected(t *testing.T) {
|
||||
// A negative TTL is not reachable through NewSessions, so issue with a real one and check
|
||||
// the boundary via a session that has already run out.
|
||||
s := NewSessions(secret, time.Millisecond)
|
||||
v := s.Issue("local:admin", "Admin")
|
||||
v := s.Issue("local:admin", "Admin", true)
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
if _, err := s.Parse(v); err == nil {
|
||||
t.Fatal("an expired session was accepted")
|
||||
@@ -201,3 +203,46 @@ func TestThrottleForgivesAfterAQuietPeriod(t *testing.T) {
|
||||
t.Fatalf("an operator returning later was still throttled: %v", d)
|
||||
}
|
||||
}
|
||||
|
||||
// The admin flag is an authorisation decision carried in a cookie the client holds, so the
|
||||
// interesting cases are all about what happens when the client lies about it.
|
||||
func TestSessionAdminFlag(t *testing.T) {
|
||||
s := NewSessions([]byte("secret"), time.Hour)
|
||||
|
||||
t.Run("round trips both ways", func(t *testing.T) {
|
||||
admin, err := s.Parse(s.Issue("local:admin", "Admin", true))
|
||||
if err != nil || !admin.Admin {
|
||||
t.Fatalf("admin session did not survive: %+v err=%v", admin, err)
|
||||
}
|
||||
user, err := s.Parse(s.Issue("oidc#1", "Markus", false))
|
||||
if err != nil || user.Admin {
|
||||
t.Fatalf("user session came back as admin: %+v err=%v", user, err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("promoting yourself invalidates the cookie", func(t *testing.T) {
|
||||
// The whole point of putting the flag inside the MAC: editing it must break the signature
|
||||
// rather than produce a valid admin session.
|
||||
v := s.Issue("oidc#1", "Markus", false)
|
||||
i := strings.LastIndex(v, ".")
|
||||
tampered := strings.TrimSuffix(v[:i], ".u") + ".a" + v[i:]
|
||||
if got, err := s.Parse(tampered); err == nil {
|
||||
t.Fatalf("a self-promoted cookie was accepted as %+v", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an unparseable role is not an admin", func(t *testing.T) {
|
||||
// Fail closed: whatever a malformed privilege claim means, it does not mean "more access".
|
||||
// Signed by us, so it passes the MAC — only the role parsing stands between it and admin.
|
||||
exp := strconv.FormatInt(time.Now().Add(time.Hour).Unix(), 10)
|
||||
payload := base64.RawURLEncoding.EncodeToString([]byte("oidc#1")) + "." +
|
||||
base64.RawURLEncoding.EncodeToString([]byte("Markus")) + "." + exp + ".ADMIN"
|
||||
sess, err := s.Parse(payload + "." + s.mac(payload))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected parse error: %v", err)
|
||||
}
|
||||
if sess.Admin {
|
||||
t.Fatal("a role of \"ADMIN\" was treated as the admin marker")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user