From 3a2e110f5790231e8020d7e0c23a3535525e66d6 Mon Sep 17 00:00:00 2001 From: Oliver Braun Date: Wed, 22 Jul 2026 17:49:20 +0200 Subject: [PATCH] feat(web): open access to any authenticated user, remove ZPA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no login allowlist anymore. The auth proxy (oauth2-proxy, restricted to the hm.edu domain) is the sole access boundary: anyone it authenticates is let in and acts strictly as their own user (per-user isolation is unchanged). This drops the users collection, the auth.seedusers seeding, and the 403 "not on the allowlist" path — a colleague no longer has to be added by hand. The ZPA (Prüfungsamt) integration is removed entirely, along with the students page it fed. Its per-email lookup let any logged-in user pull personal exam- office data (name, Matrikelnummer, gender, study programme) for arbitrary HM emails — the reason the allowlist existed. Removing ZPA removes that exposure, so opening access is safe. Existing configs keep working: leftover auth.seedusers / zpa.* keys are simply ignored. The now-unused users collection can be dropped from Mongo at leisure. Co-Authored-By: Claude Opus 4.8 (1M context) --- deploy/.glabs-web.yaml.example | 18 +- deploy/README.md | 7 +- deploy/docker-compose.yml | 5 +- web/README.md | 5 +- web/app/app.go | 13 +- web/app/courses_test.go | 2 - web/app/students.go | 90 ------- web/app/students_test.go | 82 ------ web/bootstrap/bootstrap.go | 19 +- web/bootstrap/seed.go | 48 ---- web/db/mongo.go | 1 - web/db/users.go | 57 ---- web/graph/auth.go | 29 +- web/graph/auth_test.go | 92 +++---- web/graph/generated/generated.go | 446 ------------------------------- web/graph/model/models_gen.go | 18 -- web/graph/model/user.go | 7 +- web/graph/students.graphqls | 24 -- web/graph/students.resolvers.go | 33 --- web/zpa/zpa.go | 141 ---------- web/zpa/zpa_test.go | 104 ------- 21 files changed, 61 insertions(+), 1180 deletions(-) delete mode 100644 web/app/students.go delete mode 100644 web/app/students_test.go delete mode 100644 web/bootstrap/seed.go delete mode 100644 web/db/users.go delete mode 100644 web/graph/students.graphqls delete mode 100644 web/graph/students.resolvers.go delete mode 100644 web/zpa/zpa.go delete mode 100644 web/zpa/zpa_test.go diff --git a/deploy/.glabs-web.yaml.example b/deploy/.glabs-web.yaml.example index f51d517..98b84cd 100644 --- a/deploy/.glabs-web.yaml.example +++ b/deploy/.glabs-web.yaml.example @@ -1,5 +1,5 @@ # glabs-web server config for the deployment. Copy to .glabs-web.yaml, fill in, and keep it -# SECRET (it holds secrets.key and, optionally, SMTP/ZPA tokens). It is bind-mounted read-only +# SECRET (it holds secrets.key and, optionally, SMTP credentials). It is bind-mounted read-only # into the glabs-web container at /app/.glabs-web.yaml. NEVER commit the real file. server: @@ -29,11 +29,9 @@ auth: # — matches the Caddyfile. Only used to enrich login events in the monitoring log; absent # is fine (the department is then simply empty). departmentheader: X-Remote-Department - # Seeded ONLY when the users collection is empty (a fresh deployment). Later edits never - # resurrect a removed user. Add the people allowed in; identity is the email. - seedusers: - - email: oliver.braun@hm.edu - name: Oliver Braun + # There is no allowlist: anyone the auth proxy authenticates (see OAUTH2_PROXY_EMAIL_DOMAINS + # in docker-compose.yml, which restricts to hm.edu) is let in and acts strictly as their own + # user (per-user isolation). The proxy is the access boundary. # Platform admins: emails that additionally see the admin monitoring page and receive the # nightly summary mail. This is the ONLY privilege above ordinary owner-scoped access. @@ -70,11 +68,3 @@ gitlab: # tlsInsecureSkipVerify: false # keep false (verify the cert) unless the relay needs otherwise # testRecipient: you@hm.edu # used by the dry-run smoke test # dryRun: false # true renders but never sends - -# --- Optional: ZPA (Prüfungsamt) to enrich the students page --- -# Without zpa.baseurl + zpa.token the students page shows only the roster emails. -# NOTE: per-email lookup (ask=) currently does NOT work at the HM ZPA endpoint — -# the students page stays roster-only until that is resolved. -# zpa: -# baseurl: https://zpa.example.hm.edu/api -# token: your-zpa-token diff --git a/deploy/README.md b/deploy/README.md index 58c0172..67135c3 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -38,9 +38,10 @@ Fill both in: 1. **`.env`** — `SERVER_NAME`, Mongo credentials, the `ACME_*`/EAB values, the `OAUTH2_PROXY_*` client id/secret, and a cookie secret (`openssl rand -base64 24`). The two GUI URLs default correctly; only change them if you know why. -2. **`.glabs-web.yaml`** — set `db.uri` to the SAME Mongo credentials as in `.env`, add the - allow-listed users under `auth.seedusers`, generate `secrets.key` (`openssl rand -base64 32`), - and set `gitlab.host`. SMTP and ZPA are optional (commented out). +2. **`.glabs-web.yaml`** — set `db.uri` to the SAME Mongo credentials as in `.env`, list the + platform `admins`, generate `secrets.key` (`openssl rand -base64 32`), and set `gitlab.host`. + SMTP is optional (commented out). There is no allowlist — every hm.edu account the proxy + authenticates is let in. Then: diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index bba701f..b9ca39e 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -52,7 +52,7 @@ services: mongo: condition: service_healthy volumes: - # Carries secrets (secrets.key, SMTP/ZPA tokens, Mongo credentials in db.uri) — the + # Carries secrets (secrets.key, SMTP credentials, Mongo credentials in db.uri) — the # real file is gitignored; copy .glabs-web.yaml.example and fill it in. - ./.glabs-web.yaml:/app/.glabs-web.yaml:ro # No ports: — only Caddy reaches it (http://glabs-web:8080) over the internal network. @@ -98,7 +98,8 @@ services: # fhmDepartment claim lands in X-Auth-Request-Groups, which the Caddyfile renames to # X-Remote-Department. Only used to enrich the monitoring log's login events. OAUTH2_PROXY_OIDC_GROUPS_CLAIM: "fhmDepartment" - # Extra layer on top of the DB allow-list (the backend is authoritative via auth.seedusers). + # The access boundary: only hm.edu accounts get past the proxy. The backend has no + # allowlist of its own — anyone the proxy authenticates is let in. OAUTH2_PROXY_EMAIL_DOMAINS: hm.edu OAUTH2_PROXY_HTTP_ADDRESS: 0.0.0.0:4180 OAUTH2_PROXY_REVERSE_PROXY: "true" diff --git a/web/README.md b/web/README.md index 2c67d35..f8cc5fd 100644 --- a/web/README.md +++ b/web/README.md @@ -25,7 +25,9 @@ viper beyond the config keys below. Identity comes from an auth proxy (oauth2-proxy behind Caddy) that sets `X-Remote-User` to the verified OIDC email. The server trusts that header and is -**fail-closed**: no header is 401, a user not on the allowlist is 403. The whole +**fail-closed on it**: no header is 401. There is no allowlist — anyone the proxy +authenticates is let in and acts strictly as their own user (per-user isolation), +so the proxy (restricted to the hm.edu domain) is the access boundary. The whole model assumes the server is reachable *only* through the proxy — never publish its port directly, or the header can be forged. @@ -112,7 +114,6 @@ go generate ./cmd/glabs-web | `auth.displaynameheader` | display-name header (default `X-Remote-Displayname`) | | `auth.departmentheader` | optional department header (default `X-Remote-Department`) | | `auth.devuser` | dev user email when auth is disabled | -| `auth.seedusers` | allowlist seeded only when the users collection is empty | | `admins` | emails that see the admin monitoring page and get the nightly summary | | `summary.enabled` | send the nightly admin summary mail (needs SMTP) | | `summary.hour` | local hour to send the summary (default `5`) | diff --git a/web/app/app.go b/web/app/app.go index 7018f05..62bab1b 100644 --- a/web/app/app.go +++ b/web/app/app.go @@ -13,7 +13,6 @@ import ( "github.com/obcode/glabs/v3/web/db" "github.com/obcode/glabs/v3/web/graph/model" "github.com/obcode/glabs/v3/web/secrets" - "github.com/obcode/glabs/v3/web/zpa" "github.com/spf13/viper" ) @@ -22,7 +21,6 @@ import ( // which owner each call was given, proving the owner comes from the authenticated // principal and never from a caller-supplied argument. type store interface { - GetUserByEmail(ctx context.Context, email string) (*model.User, error) CoursesOf(ctx context.Context, owner string) ([]*db.StoredCourse, error) CourseOf(ctx context.Context, owner, name string) (*db.StoredCourse, error) SaveCourse(ctx context.Context, course *db.StoredCourse) error @@ -71,9 +69,6 @@ type App struct { // configured test recipient. mailer Mailer mailDryRun bool - // zpa enriches the roster with student details; nil when ZPA is not configured - // (then the students page shows just the emails). - zpa *zpa.Client // admins holds the lowercased emails allowed to see the platform-wide monitoring // (the admin page and the nightly summary). Empty means no one is an admin. admins map[string]bool @@ -88,7 +83,7 @@ type App struct { summaryRecipients []string } -func New(database *db.DB, sealer *secrets.Sealer, gitlabHost string, mailer Mailer, mailDryRun bool, zpaClient *zpa.Client, admins []string) *App { +func New(database *db.DB, sealer *secrets.Sealer, gitlabHost string, mailer Mailer, mailDryRun bool, admins []string) *App { adminSet := make(map[string]bool, len(admins)) for _, e := range admins { if e = strings.ToLower(strings.TrimSpace(e)); e != "" { @@ -102,17 +97,11 @@ func New(database *db.DB, sealer *secrets.Sealer, gitlabHost string, mailer Mail ops: newOpGuard(), mailer: mailer, mailDryRun: mailDryRun, - zpa: zpaClient, admins: adminSet, loginSeen: map[string]time.Time{}, } } -// GetUserByEmail looks up a user for the auth middleware's allowlist check. -func (a *App) GetUserByEmail(ctx context.Context, email string) (*model.User, error) { - return a.db.GetUserByEmail(ctx, email) -} - // LocalDevUser is the identity used when auth is disabled (local development). It // is never consulted when auth is enabled. func (a *App) LocalDevUser() *model.User { diff --git a/web/app/courses_test.go b/web/app/courses_test.go index 6d841ca..dcb26dd 100644 --- a/web/app/courses_test.go +++ b/web/app/courses_test.go @@ -131,8 +131,6 @@ func (f *fakeStore) MarkNotified(_ context.Context, id string) error { return nil } -func (f *fakeStore) GetUserByEmail(context.Context, string) (*model.User, error) { return nil, nil } - func (f *fakeStore) CoursesOf(_ context.Context, owner string) ([]*db.StoredCourse, error) { f.seenOwners = append(f.seenOwners, owner) var out []*db.StoredCourse diff --git a/web/app/students.go b/web/app/students.go deleted file mode 100644 index 33168cf..0000000 --- a/web/app/students.go +++ /dev/null @@ -1,90 +0,0 @@ -package app - -import ( - "context" - "sort" - "strings" - "sync" - - "github.com/rs/zerolog/log" -) - -// zpaLookupConcurrency bounds how many ZPA lookups run at once, so enriching a -// 40-student roster is one burst of parallel calls rather than 40 sequential ones, -// without hammering ZPA. -const zpaLookupConcurrency = 8 - -// CourseStudent is one roster entry, enriched with ZPA details when they could be -// found. Found is false (and the detail fields empty) when ZPA is not configured -// or has no unambiguous match — the GUI then shows just the email. -type CourseStudent struct { - Email string - Found bool - FirstName string - LastName string - Gender string - Group string - Mtknr string -} - -// CourseStudents returns the course-level roster of one of the caller's courses, -// each email enriched with ZPA details (concurrently). A ZPA failure for one -// student is logged and leaves that entry un-enriched rather than failing the whole -// page. The result is sorted by last name, then email. -func (a *App) CourseStudents(ctx context.Context, course string) ([]*CourseStudent, error) { - stored, err := a.Course(ctx, course) - if err != nil { - return nil, err - } - - var emails []string - if stored.Source != nil { - emails = stored.Source.Students - } - students := make([]*CourseStudent, len(emails)) - for i, e := range emails { - students[i] = &CourseStudent{Email: e} - } - - if a.zpa != nil { - sem := make(chan struct{}, zpaLookupConcurrency) - var wg sync.WaitGroup - for _, cs := range students { - wg.Add(1) - go func(cs *CourseStudent) { - defer wg.Done() - sem <- struct{}{} - defer func() { <-sem }() - s, err := a.zpa.StudentByEmail(ctx, cs.Email) - if err != nil { - log.Warn().Err(err).Str("email", cs.Email).Msg("ZPA lookup failed") - return - } - if s != nil { - cs.Found = true - cs.FirstName = s.FirstName - cs.LastName = s.LastName - cs.Gender = s.Gender - cs.Group = s.Group - cs.Mtknr = s.Mtknr - } - }(cs) - } - wg.Wait() - } - - // Enriched students first (sorted by last name), then the un-enriched ones - // (sorted by email), so a missing ZPA match sinks to the bottom instead of the - // top. - sort.SliceStable(students, func(i, j int) bool { - si, sj := students[i], students[j] - if si.Found != sj.Found { - return si.Found - } - if li, lj := strings.ToLower(si.LastName), strings.ToLower(sj.LastName); li != lj { - return li < lj - } - return strings.ToLower(si.Email) < strings.ToLower(sj.Email) - }) - return students, nil -} diff --git a/web/app/students_test.go b/web/app/students_test.go deleted file mode 100644 index a421f57..0000000 --- a/web/app/students_test.go +++ /dev/null @@ -1,82 +0,0 @@ -package app - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - "github.com/obcode/glabs/v3/web/zpa" -) - -const rosterCourse = `sc: - coursepath: sc/sem - students: - - a.mueller@hm.edu - - z.unknown@hm.edu - blatt01: - per: student -` - -func TestCourseStudents_enrichesFromZPA(t *testing.T) { - const owner = "prof@hm.edu" - fs := newFakeStore() - fs.courses[owner+"/sc"] = storedCourse(t, owner, rosterCourse) - - // Mock ZPA: knows a.mueller, does not know z.unknown. - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var out []*zpa.Student - if r.URL.Query().Get("ask") == "a.mueller@hm.edu" { - out = []*zpa.Student{{Mtknr: "42", FirstName: "Anna", LastName: "Müller", Email: "a.mueller@hm.edu", Gender: "f", Group: "IF1"}} - } - _ = json.NewEncoder(w).Encode(out) - })) - t.Cleanup(srv.Close) - - a := &App{db: fs, zpa: zpa.New(zpa.Config{BaseURL: srv.URL, Token: "x"})} - students, err := a.CourseStudents(ctxAs(owner), "sc") - if err != nil { - t.Fatalf("CourseStudents: %v", err) - } - if len(students) != 2 { - t.Fatalf("got %d students, want 2", len(students)) - } - // Enriched student sorts first. - if s := students[0]; !s.Found || s.LastName != "Müller" || s.Group != "IF1" || s.Mtknr != "42" { - t.Errorf("students[0] = %+v, want the enriched Müller", s) - } - // Unknown sinks to the bottom, un-enriched. - if s := students[1]; s.Found || s.Email != "z.unknown@hm.edu" { - t.Errorf("students[1] = %+v, want the un-enriched z.unknown", s) - } -} - -// Without a ZPA client the roster still comes back, just un-enriched. -func TestCourseStudents_noZPAReturnsRosterOnly(t *testing.T) { - const owner = "prof@hm.edu" - fs := newFakeStore() - fs.courses[owner+"/sc"] = storedCourse(t, owner, rosterCourse) - - a := &App{db: fs} // no zpa - students, err := a.CourseStudents(ctxAs(owner), "sc") - if err != nil { - t.Fatalf("CourseStudents: %v", err) - } - if len(students) != 2 { - t.Fatalf("got %d students, want 2", len(students)) - } - for _, s := range students { - if s.Found { - t.Errorf("student %s should be un-enriched without ZPA", s.Email) - } - } -} - -// The roster read is owner-scoped: another user's course is invisible. -func TestCourseStudents_requiresAuthentication(t *testing.T) { - a := &App{db: newFakeStore()} - if _, err := a.CourseStudents(context.Background(), "sc"); err == nil { - t.Error("CourseStudents without a principal should fail") - } -} diff --git a/web/bootstrap/bootstrap.go b/web/bootstrap/bootstrap.go index 40f42b4..d4ebc30 100644 --- a/web/bootstrap/bootstrap.go +++ b/web/bootstrap/bootstrap.go @@ -17,7 +17,6 @@ import ( "github.com/obcode/glabs/v3/web/graph" "github.com/obcode/glabs/v3/web/mail" "github.com/obcode/glabs/v3/web/secrets" - "github.com/obcode/glabs/v3/web/zpa" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/viper" @@ -57,9 +56,6 @@ func Serve() error { if err != nil { return err } - if err := database_.EnsureUserIndexes(ctx); err != nil { - return err - } if err := database_.EnsureCourseIndexes(ctx); err != nil { return err } @@ -104,24 +100,11 @@ func Serve() error { log.Warn().Msg("no smtp.host configured; scheduled-job notifications are disabled") } - // ZPA (student directory) is optional: without zpa.baseurl + zpa.token, the - // students page just shows the roster emails. - var zpaClient *zpa.Client - if base, tok := viper.GetString("zpa.baseurl"), viper.GetString("zpa.token"); base != "" && tok != "" { - zpaClient = zpa.New(zpa.Config{BaseURL: base, Token: tok}) - log.Info().Str("baseurl", base).Msg("ZPA configured; the students page is enriched") - } else { - log.Warn().Msg("no zpa.baseurl/zpa.token configured; the students page shows only emails") - } - // admins may see the platform-wide monitoring page and receive the nightly // summary. It is the ONLY privilege above ordinary owner-scoped access. admins := viper.GetStringSlice("admins") - a := app.New(database_, sealer, viper.GetString("gitlab.host"), mailer, viper.GetBool("smtp.dryRun"), zpaClient, admins) - if err := seedUsers(ctx, database_); err != nil { - return err - } + a := app.New(database_, sealer, viper.GetString("gitlab.host"), mailer, viper.GetBool("smtp.dryRun"), admins) // The scheduled-job runner polls in the background for the life of the process // (a background context, since StartServer blocks and never returns here). diff --git a/web/bootstrap/seed.go b/web/bootstrap/seed.go deleted file mode 100644 index 7ee7600..0000000 --- a/web/bootstrap/seed.go +++ /dev/null @@ -1,48 +0,0 @@ -package bootstrap - -import ( - "context" - "strings" - - "github.com/obcode/glabs/v3/web/db" - "github.com/obcode/glabs/v3/web/graph/model" - "github.com/rs/zerolog/log" - "github.com/spf13/viper" -) - -// seedUsers inserts the allowlist from auth.seedusers, but only when the users -// collection is empty — so a fresh deployment has someone who can get in, while -// later config edits never resurrect a user removed from the database. -func seedUsers(ctx context.Context, database *db.DB) error { - count, err := database.CountUsers(ctx) - if err != nil { - return err - } - if count > 0 { - return nil - } - - var seed []struct { - Email string - Name string - } - if err := viper.UnmarshalKey("auth.seedusers", &seed); err != nil { - return err - } - if len(seed) == 0 { - log.Warn().Msg("no users in the database and no auth.seedusers configured — nobody will be able to log in") - return nil - } - - for _, s := range seed { - email := strings.ToLower(strings.TrimSpace(s.Email)) - if email == "" { - continue - } - if err := database.SaveUser(ctx, &model.User{Email: email, Name: strings.TrimSpace(s.Name)}); err != nil { - return err - } - log.Info().Str("email", email).Msg("seeded user") - } - return nil -} diff --git a/web/db/mongo.go b/web/db/mongo.go index 84ee384..000cc83 100644 --- a/web/db/mongo.go +++ b/web/db/mongo.go @@ -14,7 +14,6 @@ import ( // Collection names. One place, so a typo is a compile error rather than a silent // empty result. const ( - collectionUsers = "users" collectionCourses = "courses" collectionUserSecrets = "user_secrets" collectionActivity = "activity" diff --git a/web/db/users.go b/web/db/users.go deleted file mode 100644 index 297ee04..0000000 --- a/web/db/users.go +++ /dev/null @@ -1,57 +0,0 @@ -package db - -import ( - "context" - "errors" - "fmt" - - "github.com/obcode/glabs/v3/web/graph/model" - "go.mongodb.org/mongo-driver/v2/bson" - "go.mongodb.org/mongo-driver/v2/mongo" - "go.mongodb.org/mongo-driver/v2/mongo/options" -) - -// EnsureUserIndexes makes the email unique. Called once at startup. -func (db *DB) EnsureUserIndexes(ctx context.Context) error { - _, err := db.collection(collectionUsers).Indexes().CreateOne(ctx, mongo.IndexModel{ - Keys: bson.D{{Key: "email", Value: 1}}, - Options: options.Index().SetUnique(true), - }) - if err != nil { - return fmt.Errorf("cannot create users index: %w", err) - } - return nil -} - -// GetUserByEmail returns the user with the given email, or nil if there is none. -// The auth middleware treats nil as "not on the allowlist" — a 403 — so a missing -// user must be nil, nil rather than an error. -func (db *DB) GetUserByEmail(ctx context.Context, email string) (*model.User, error) { - var user model.User - err := db.collection(collectionUsers).FindOne(ctx, bson.M{"email": email}).Decode(&user) - if err != nil { - if errors.Is(err, mongo.ErrNoDocuments) { - return nil, nil - } - return nil, fmt.Errorf("cannot read user %s: %w", email, err) - } - return &user, nil -} - -// SaveUser inserts or updates a user, keyed by email. -func (db *DB) SaveUser(ctx context.Context, user *model.User) error { - _, err := db.collection(collectionUsers).ReplaceOne(ctx, - bson.M{"email": user.Email}, user, - options.Replace().SetUpsert(true), - ) - if err != nil { - return fmt.Errorf("cannot save user %s: %w", user.Email, err) - } - return nil -} - -// CountUsers reports how many users exist, so seeding can run only on an empty -// allowlist. -func (db *DB) CountUsers(ctx context.Context) (int64, error) { - return db.collection(collectionUsers).CountDocuments(ctx, bson.M{}) -} diff --git a/web/graph/auth.go b/web/graph/auth.go index 6aefe7e..96a3f49 100644 --- a/web/graph/auth.go +++ b/web/graph/auth.go @@ -20,7 +20,6 @@ func UserFromContext(ctx context.Context) *model.User { // tested without a database. type authProvider interface { LocalDevUser() *model.User - GetUserByEmail(ctx context.Context, email string) (*model.User, error) // NoteLogin records that a user was active (throttled); NoteRejectedLogin // records a refused request. Both feed the platform monitoring log. NoteLogin(ctx context.Context, email, name, department string) @@ -28,10 +27,11 @@ type authProvider interface { } // authMiddleware trusts the identity injected by the auth proxy (oauth2-proxy → -// Caddy sets X-Remote-User to the verified OIDC email). It is fail-closed: no -// header is 401, an unknown user is 403 — only allowlisted users get in. With -// auth.enabled=false it injects a local dev user instead, so development needs no -// proxy. +// Caddy sets X-Remote-User to the verified OIDC email). It is fail-closed on the +// header only: no header is 401. There is no allowlist — anyone the proxy +// authenticates is let in and acts strictly as their own user (per-user +// isolation). With auth.enabled=false it injects a local dev user instead, so +// development needs no proxy. // // The whole model rests on the server being reachable only through the proxy. If // it is ever exposed directly, the header is trusted unconditionally and anyone @@ -72,22 +72,9 @@ func authMiddleware(p authProvider) func(http.Handler) http.Handler { http.Error(w, "unauthenticated: no identity from the auth proxy", http.StatusUnauthorized) return } - u, err := p.GetUserByEmail(r.Context(), email) - if err != nil { - log.Error().Err(err).Str("email", email).Msg("cannot verify user") - http.Error(w, "cannot verify user", http.StatusInternalServerError) - return - } - if u == nil { - log.Warn().Str("email", email).Msg("rejected login of user not on the allowlist") - p.NoteRejectedLogin(r.Context(), email, dept, "nicht auf der Allowlist") - http.Error(w, "forbidden: user not authorized", http.StatusForbidden) - return - } - if u.Name == "" { - u.Name = strings.TrimSpace(r.Header.Get(nameHeader)) - } - user = u + // No allowlist: the proxy-verified identity is trusted directly. The + // display name comes from its header (empty is fine). + user = &model.User{Email: email, Name: strings.TrimSpace(r.Header.Get(nameHeader))} p.NoteLogin(r.Context(), user.Email, user.Name, dept) } ctx := principal.WithUser(r.Context(), user) diff --git a/web/graph/auth_test.go b/web/graph/auth_test.go index 460e6e8..7261fdd 100644 --- a/web/graph/auth_test.go +++ b/web/graph/auth_test.go @@ -13,23 +13,17 @@ import ( // fakeAuthProvider stands in for the app, so the middleware is tested without a // database. type fakeAuthProvider struct { - allow map[string]*model.User dev *model.User - err error logins int rejections int lastDept string + lastName string } func (f *fakeAuthProvider) LocalDevUser() *model.User { return f.dev } -func (f *fakeAuthProvider) GetUserByEmail(_ context.Context, email string) (*model.User, error) { - if f.err != nil { - return nil, f.err - } - return f.allow[email], nil -} -func (f *fakeAuthProvider) NoteLogin(_ context.Context, _, _, department string) { +func (f *fakeAuthProvider) NoteLogin(_ context.Context, _, name, department string) { f.logins++ + f.lastName = name f.lastDept = department } func (f *fakeAuthProvider) NoteRejectedLogin(_ context.Context, _, department, _ string) { @@ -74,85 +68,65 @@ func TestAuthMiddlewareRejectsMissingHeader(t *testing.T) { } } -func TestAuthMiddlewareRejectsUnknownUser(t *testing.T) { +// There is no allowlist: any identity the proxy asserts is let in and acts as its +// own user. The email is lowercased so a differently-cased header still yields a +// canonical identity. +func TestAuthMiddlewareAcceptsAnyAuthenticatedUser(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) viper.Set("auth.enabled", true) - code, user := serve(t, &fakeAuthProvider{allow: map[string]*model.User{}}, "X-Remote-User", "stranger@hm.edu") - if code != http.StatusForbidden { - t.Errorf("unknown user: status = %d, want 403", code) - } - if user != nil { - t.Errorf("unknown user: a user reached the handler: %+v", user) - } -} - -func TestAuthMiddlewareAcceptsAllowlistedUser(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("auth.enabled", true) - - prof := &model.User{Email: "prof@hm.edu", Name: "Prof"} - p := &fakeAuthProvider{allow: map[string]*model.User{"prof@hm.edu": prof}} - - // The email is lowercased before lookup, so a differently-cased header still - // matches. - code, user := serve(t, p, "X-Remote-User", "PROF@HM.EDU") + code, user := serve(t, &fakeAuthProvider{}, "X-Remote-User", "STRANGER@HM.EDU") if code != http.StatusOK { - t.Fatalf("allowlisted user: status = %d, want 200", code) - } - if user == nil || user.Email != "prof@hm.edu" { - t.Errorf("allowlisted user: context user = %+v, want prof@hm.edu", user) + t.Fatalf("any user: status = %d, want 200", code) } -} - -func TestAuthMiddlewareInternalErrorOnLookupFailure(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("auth.enabled", true) - - p := &fakeAuthProvider{err: context.DeadlineExceeded} - code, _ := serve(t, p, "X-Remote-User", "prof@hm.edu") - if code != http.StatusInternalServerError { - t.Errorf("lookup failure: status = %d, want 500", code) + if user == nil || user.Email != "stranger@hm.edu" { + t.Errorf("any user: context user = %+v, want stranger@hm.edu", user) } } -// An accepted login is noted once, carrying the department header; a rejected one -// is noted as a rejection. This is what feeds the monitoring log. -func TestAuthMiddlewareNotesLoginWithDepartment(t *testing.T) { +// An accepted login is noted once, carrying the display name and department +// headers. This is what feeds the monitoring log. +func TestAuthMiddlewareNotesLoginWithNameAndDepartment(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) viper.Set("auth.enabled", true) - prof := &model.User{Email: "prof@hm.edu", Name: "Prof"} - p := &fakeAuthProvider{allow: map[string]*model.User{"prof@hm.edu": prof}} - + p := &fakeAuthProvider{} h := authMiddleware(p)(capture(new(*model.User))) req := httptest.NewRequest(http.MethodPost, "/query", nil) req.Header.Set("X-Remote-User", "prof@hm.edu") + req.Header.Set("X-Remote-Displayname", "Prof Example") req.Header.Set("X-Remote-Department", "07") h.ServeHTTP(httptest.NewRecorder(), req) if p.logins != 1 || p.rejections != 0 { t.Errorf("accepted login: logins=%d rejections=%d, want 1/0", p.logins, p.rejections) } + if p.lastName != "Prof Example" { + t.Errorf("name = %q, want %q", p.lastName, "Prof Example") + } if p.lastDept != "07" { t.Errorf("department = %q, want %q", p.lastDept, "07") } +} + +// A missing identity header is the one rejection left, and it is noted as such. +func TestAuthMiddlewareNotesRejectionOnMissingHeader(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("auth.enabled", true) - // A stranger is a rejection, not a login. - p2 := &fakeAuthProvider{allow: map[string]*model.User{}} - code, _ := serve(t, p2, "X-Remote-User", "stranger@hm.edu") - if code != http.StatusForbidden || p2.rejections != 1 || p2.logins != 0 { - t.Errorf("stranger: code=%d logins=%d rejections=%d, want 403/0/1", code, p2.logins, p2.rejections) + p := &fakeAuthProvider{} + code, _ := serve(t, p, "", "") + if code != http.StatusUnauthorized || p.rejections != 1 || p.logins != 0 { + t.Errorf("missing header: code=%d logins=%d rejections=%d, want 401/0/1", code, p.logins, p.rejections) } } -// With auth disabled the middleware never consults the header or the database — -// every request runs as the local dev user. This is the local-development path; -// it must not accidentally depend on a header being present. +// With auth disabled the middleware never consults the header — every request +// runs as the local dev user. This is the local-development path; it must not +// accidentally depend on a header being present. func TestAuthMiddlewareDisabledUsesDevUser(t *testing.T) { viper.Reset() t.Cleanup(viper.Reset) diff --git a/web/graph/generated/generated.go b/web/graph/generated/generated.go index d68cb73..1260657 100644 --- a/web/graph/generated/generated.go +++ b/web/graph/generated/generated.go @@ -123,16 +123,6 @@ type ComplexityRoot struct { UseEmailDomainAsSuffix func(childComplexity int) int } - CourseStudent struct { - Email func(childComplexity int) int - FirstName func(childComplexity int) int - Found func(childComplexity int) int - Gender func(childComplexity int) int - Group func(childComplexity int) int - LastName func(childComplexity int) int - Mtknr func(childComplexity int) int - } - DockerImageReport struct { Image func(childComplexity int) int Wanted func(childComplexity int) int @@ -317,7 +307,6 @@ type ComplexityRoot struct { CourseCheck func(childComplexity int, name string) int CourseLint func(childComplexity int, name string) int CourseRepoOverview func(childComplexity int, course string) int - CourseStudents func(childComplexity int, course string) int CourseYaml func(childComplexity int, name string) int Courses func(childComplexity int) int GitlabToken func(childComplexity int) int @@ -483,7 +472,6 @@ type QueryResolver interface { CourseRepoOverview(ctx context.Context, course string) ([]*model.AssignmentRepos, error) ScheduledJobs(ctx context.Context, status []model.JobStatus) ([]*model.ScheduledJob, error) ScheduledJob(ctx context.Context, id string) (*model.ScheduledJob, error) - CourseStudents(ctx context.Context, course string) ([]*model.CourseStudent, error) } type SubscriptionResolver interface { AssignmentReportProgress(ctx context.Context, course string, name string) (<-chan *model.ReportProgress, error) @@ -864,49 +852,6 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin return e.ComplexityRoot.Course.UseEmailDomainAsSuffix(childComplexity), true - case "CourseStudent.email": - if e.ComplexityRoot.CourseStudent.Email == nil { - break - } - - return e.ComplexityRoot.CourseStudent.Email(childComplexity), true - case "CourseStudent.firstName": - if e.ComplexityRoot.CourseStudent.FirstName == nil { - break - } - - return e.ComplexityRoot.CourseStudent.FirstName(childComplexity), true - case "CourseStudent.found": - if e.ComplexityRoot.CourseStudent.Found == nil { - break - } - - return e.ComplexityRoot.CourseStudent.Found(childComplexity), true - case "CourseStudent.gender": - if e.ComplexityRoot.CourseStudent.Gender == nil { - break - } - - return e.ComplexityRoot.CourseStudent.Gender(childComplexity), true - case "CourseStudent.group": - if e.ComplexityRoot.CourseStudent.Group == nil { - break - } - - return e.ComplexityRoot.CourseStudent.Group(childComplexity), true - case "CourseStudent.lastName": - if e.ComplexityRoot.CourseStudent.LastName == nil { - break - } - - return e.ComplexityRoot.CourseStudent.LastName(childComplexity), true - case "CourseStudent.mtknr": - if e.ComplexityRoot.CourseStudent.Mtknr == nil { - break - } - - return e.ComplexityRoot.CourseStudent.Mtknr(childComplexity), true - case "DockerImageReport.image": if e.ComplexityRoot.DockerImageReport.Image == nil { break @@ -1807,17 +1752,6 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin } return e.ComplexityRoot.Query.CourseRepoOverview(childComplexity, args["course"].(string)), true - case "Query.courseStudents": - if e.ComplexityRoot.Query.CourseStudents == nil { - break - } - - args, err := ec.field_Query_courseStudents_args(ctx, rawArgs) - if err != nil { - return 0, false - } - - return e.ComplexityRoot.Query.CourseStudents(childComplexity, args["course"].(string)), true case "Query.courseYAML": if e.ComplexityRoot.Query.CourseYaml == nil { break @@ -3208,31 +3142,6 @@ type Query { "Version and build metadata." serverInfo: ServerInfo! } -`, BuiltIn: false}, - {Name: "../students.graphqls", Input: `""" -One roster entry of a course, enriched with ZPA (Prüfungsamt) student details when -they could be found. ` + "`" + `found` + "`" + ` is false and the detail fields are null when ZPA is -not configured or has no unambiguous match — the GUI then shows just the email. -""" -type CourseStudent { - "The roster email (the identity glabs keys students on)." - email: String! - "Whether ZPA details were found for this email." - found: Boolean! - firstName: String - lastName: String - "The ZPA gender field (sensitive — shown only when the course owner opts in)." - gender: String - "The ZPA group/study programme." - group: String - "The Matrikelnummer." - mtknr: String -} - -extend type Query { - "The course-level roster of one of the caller's courses, enriched with ZPA details, sorted by last name." - courseStudents(course: String!): [CourseStudent!]! -} `, BuiltIn: false}, } var parsedSchema = gqlparser.MustLoadSchema(sources...) @@ -3409,26 +3318,6 @@ func (ec *executionContext) childFields_Course(ctx context.Context, field graphq return nil, fmt.Errorf("no field named %q was found under type Course", field.Name) } -func (ec *executionContext) childFields_CourseStudent(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - switch field.Name { - case "email": - return ec.fieldContext_CourseStudent_email(ctx, field) - case "found": - return ec.fieldContext_CourseStudent_found(ctx, field) - case "firstName": - return ec.fieldContext_CourseStudent_firstName(ctx, field) - case "lastName": - return ec.fieldContext_CourseStudent_lastName(ctx, field) - case "gender": - return ec.fieldContext_CourseStudent_gender(ctx, field) - case "group": - return ec.fieldContext_CourseStudent_group(ctx, field) - case "mtknr": - return ec.fieldContext_CourseStudent_mtknr(ctx, field) - } - return nil, fmt.Errorf("no field named %q was found under type CourseStudent", field.Name) -} - func (ec *executionContext) childFields_DockerImageReport(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { switch field.Name { case "wanted": @@ -4623,20 +4512,6 @@ func (ec *executionContext) field_Query_courseRepoOverview_args(ctx context.Cont return args, nil } -func (ec *executionContext) field_Query_courseStudents_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { - var err error - args := map[string]any{} - arg0, err := graphql.ProcessArgField(ctx, rawArgs, "course", - func(ctx context.Context, v any) (string, error) { - return ec.unmarshalNString2string(ctx, v) - }) - if err != nil { - return nil, err - } - args["course"] = arg0 - return args, nil -} - func (ec *executionContext) field_Query_courseYAML_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { var err error args := map[string]any{} @@ -6283,167 +6158,6 @@ func (ec *executionContext) fieldContext_Course_updatedAt(_ context.Context, fie return graphql.NewScalarFieldContext("Course", field, false, false, errors.New("field of type Time does not have child fields")) } -func (ec *executionContext) _CourseStudent_email(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_email(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.Email, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v string) graphql.Marshaler { - return ec.marshalNString2string(ctx, selections, v) - }, - true, - true, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_email(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_found(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_found(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.Found, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v bool) graphql.Marshaler { - return ec.marshalNBoolean2bool(ctx, selections, v) - }, - true, - true, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_found(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type Boolean does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_firstName(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_firstName(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.FirstName, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v *string) graphql.Marshaler { - return ec.marshalOString2ᚖstring(ctx, selections, v) - }, - true, - false, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_firstName(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_lastName(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_lastName(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.LastName, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v *string) graphql.Marshaler { - return ec.marshalOString2ᚖstring(ctx, selections, v) - }, - true, - false, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_lastName(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_gender(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_gender(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.Gender, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v *string) graphql.Marshaler { - return ec.marshalOString2ᚖstring(ctx, selections, v) - }, - true, - false, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_gender(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_group(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_group(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.Group, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v *string) graphql.Marshaler { - return ec.marshalOString2ᚖstring(ctx, selections, v) - }, - true, - false, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_group(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - -func (ec *executionContext) _CourseStudent_mtknr(ctx context.Context, field graphql.CollectedField, obj *model.CourseStudent) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_CourseStudent_mtknr(ctx, field) - }, - func(ctx context.Context) (any, error) { - return obj.Mtknr, nil - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v *string) graphql.Marshaler { - return ec.marshalOString2ᚖstring(ctx, selections, v) - }, - true, - false, - ) -} -func (ec *executionContext) fieldContext_CourseStudent_mtknr(_ context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - return graphql.NewScalarFieldContext("CourseStudent", field, false, false, errors.New("field of type String does not have child fields")) -} - func (ec *executionContext) _DockerImageReport_wanted(ctx context.Context, field graphql.CollectedField, obj *model.DockerImageReport) (ret graphql.Marshaler) { return graphql.ResolveField( ctx, @@ -10439,50 +10153,6 @@ func (ec *executionContext) fieldContext_Query_scheduledJob(ctx context.Context, return fc, nil } -func (ec *executionContext) _Query_courseStudents(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { - return graphql.ResolveField( - ctx, - ec.OperationContext, - field, - func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.fieldContext_Query_courseStudents(ctx, field) - }, - func(ctx context.Context) (any, error) { - fc := graphql.GetFieldContext(ctx) - return ec.Resolvers.Query().CourseStudents(ctx, fc.Args["course"].(string)) - }, - nil, - func(ctx context.Context, selections ast.SelectionSet, v []*model.CourseStudent) graphql.Marshaler { - return ec.marshalNCourseStudent2ᚕᚖgithubᚗcomᚋobcodeᚋglabsᚋv3ᚋwebᚋgraphᚋmodelᚐCourseStudentᚄ(ctx, selections, v) - }, - true, - true, - ) -} -func (ec *executionContext) fieldContext_Query_courseStudents(ctx context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { - fc = &graphql.FieldContext{ - Object: "Query", - Field: field, - IsMethod: true, - IsResolver: true, - Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { - return ec.childFields_CourseStudent(ctx, field) - }, - } - defer func() { - if r := recover(); r != nil { - err = ec.Recover(ctx, r) - ec.Error(ctx, err) - } - }() - ctx = graphql.WithFieldContext(ctx, fc) - if fc.Args, err = ec.field_Query_courseStudents_args(ctx, field.ArgumentMap(ec.Variables)); err != nil { - ec.Error(ctx, err) - return fc, err - } - return fc, nil -} - func (ec *executionContext) _Query___type(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { return graphql.ResolveField( ctx, @@ -13796,74 +13466,6 @@ func (ec *executionContext) _Course(ctx context.Context, sel ast.SelectionSet, o return out } -var courseStudentImplementors = []string{"CourseStudent"} - -func (ec *executionContext) _CourseStudent(ctx context.Context, sel ast.SelectionSet, obj *model.CourseStudent) graphql.Marshaler { - fields := graphql.CollectFields(ec.OperationContext, sel, courseStudentImplementors) - - out := graphql.NewFieldSet(fields) - deferredFieldSet := graphql.NewFieldSet(nil) - deferLabelToView := make(map[string]*graphql.FieldSetView) - for i, field := range fields { - switch field.Name { - case "__typename": - out.Values[i] = graphql.MarshalString("CourseStudent") - case "email": - out.Values[i] = ec._CourseStudent_email(ctx, field, obj) - if out.Values[i] == graphql.Null { - out.Invalids++ - } - case "found": - out.Values[i] = ec._CourseStudent_found(ctx, field, obj) - if out.Values[i] == graphql.Null { - out.Invalids++ - } - case "firstName": - out.Values[i] = ec._CourseStudent_firstName(ctx, field, obj) - if out.Values[i] == graphql.RequiredNull { - out.Invalids++ - } - case "lastName": - out.Values[i] = ec._CourseStudent_lastName(ctx, field, obj) - if out.Values[i] == graphql.RequiredNull { - out.Invalids++ - } - case "gender": - out.Values[i] = ec._CourseStudent_gender(ctx, field, obj) - if out.Values[i] == graphql.RequiredNull { - out.Invalids++ - } - case "group": - out.Values[i] = ec._CourseStudent_group(ctx, field, obj) - if out.Values[i] == graphql.RequiredNull { - out.Invalids++ - } - case "mtknr": - out.Values[i] = ec._CourseStudent_mtknr(ctx, field, obj) - if out.Values[i] == graphql.RequiredNull { - out.Invalids++ - } - default: - panic("unknown field " + strconv.Quote(field.Name)) - } - } - out.Dispatch(ctx) - if out.Invalids > 0 { - return graphql.Null - } - - atomic.AddInt32(&ec.Deferred, int32(min(len(deferLabelToView), math.MaxInt32))) - - ec.ProcessDeferredGroup(graphql.DeferredGroup{ - Defers: deferLabelToView, - Path: graphql.GetPath(ctx), - FieldSet: deferredFieldSet, - Context: ctx, - }) - - return out -} - var dockerImageReportImplementors = []string{"DockerImageReport"} func (ec *executionContext) _DockerImageReport(ctx context.Context, sel ast.SelectionSet, obj *model.DockerImageReport) graphql.Marshaler { @@ -15642,28 +15244,6 @@ func (ec *executionContext) _Query(ctx context.Context, sel ast.SelectionSet) gr func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) } - out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return rrm(innerCtx) }) - case "courseStudents": - field := field - - innerFunc := func(ctx context.Context, fs *graphql.FieldSet) (res graphql.Marshaler) { - defer func() { - if r := recover(); r != nil { - ec.Error(ctx, ec.Recover(ctx, r)) - } - }() - res = ec._Query_courseStudents(ctx, field) - if res == graphql.Null { - atomic.AddUint32(&fs.Invalids, 1) - } - return res - } - - rrm := func(ctx context.Context) graphql.Marshaler { - return ec.OperationContext.RootResolverMiddleware(ctx, - func(ctx context.Context) graphql.Marshaler { return innerFunc(ctx, out) }) - } - out.Concurrently(i, func(ctx context.Context) graphql.Marshaler { return rrm(innerCtx) }) case "__type": out.Values[i] = ec.OperationContext.RootResolverMiddleware(innerCtx, func(ctx context.Context) (res graphql.Marshaler) { @@ -17009,32 +16589,6 @@ func (ec *executionContext) marshalNCourse2ᚖgithubᚗcomᚋobcodeᚋglabsᚋv3 return ec._Course(ctx, sel, v) } -func (ec *executionContext) marshalNCourseStudent2ᚕᚖgithubᚗcomᚋobcodeᚋglabsᚋv3ᚋwebᚋgraphᚋmodelᚐCourseStudentᚄ(ctx context.Context, sel ast.SelectionSet, v []*model.CourseStudent) graphql.Marshaler { - ret := graphql.MarshalSliceConcurrently(ctx, len(v), 0, false, func(ctx context.Context, i int) graphql.Marshaler { - fc := graphql.GetFieldContext(ctx) - fc.Result = &v[i] - return ec.marshalNCourseStudent2ᚖgithubᚗcomᚋobcodeᚋglabsᚋv3ᚋwebᚋgraphᚋmodelᚐCourseStudent(ctx, sel, v[i]) - }) - - for _, e := range ret { - if e == graphql.Null { - return graphql.Null - } - } - - return ret -} - -func (ec *executionContext) marshalNCourseStudent2ᚖgithubᚗcomᚋobcodeᚋglabsᚋv3ᚋwebᚋgraphᚋmodelᚐCourseStudent(ctx context.Context, sel ast.SelectionSet, v *model.CourseStudent) graphql.Marshaler { - if v == nil { - if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) { - graphql.AddErrorf(ctx, "the requested element is null which the schema does not allow") - } - return graphql.Null - } - return ec._CourseStudent(ctx, sel, v) -} - func (ec *executionContext) marshalNDockerImageReport2ᚕᚖgithubᚗcomᚋobcodeᚋglabsᚋv3ᚋwebᚋgraphᚋmodelᚐDockerImageReportᚄ(ctx context.Context, sel ast.SelectionSet, v []*model.DockerImageReport) graphql.Marshaler { ret := graphql.MarshalSliceConcurrently(ctx, len(v), 0, false, func(ctx context.Context, i int) graphql.Marshaler { fc := graphql.GetFieldContext(ctx) diff --git a/web/graph/model/models_gen.go b/web/graph/model/models_gen.go index c80051b..caca596 100644 --- a/web/graph/model/models_gen.go +++ b/web/graph/model/models_gen.go @@ -150,24 +150,6 @@ type Course struct { UpdatedAt time.Time `json:"updatedAt"` } -// One roster entry of a course, enriched with ZPA (Prüfungsamt) student details when -// they could be found. `found` is false and the detail fields are null when ZPA is -// not configured or has no unambiguous match — the GUI then shows just the email. -type CourseStudent struct { - // The roster email (the identity glabs keys students on). - Email string `json:"email"` - // Whether ZPA details were found for this email. - Found bool `json:"found"` - FirstName *string `json:"firstName,omitempty"` - LastName *string `json:"lastName,omitempty"` - // The ZPA gender field (sensitive — shown only when the course owner opts in). - Gender *string `json:"gender,omitempty"` - // The ZPA group/study programme. - Group *string `json:"group,omitempty"` - // The Matrikelnummer. - Mtknr *string `json:"mtknr,omitempty"` -} - type DockerImageReport struct { Wanted string `json:"wanted"` Image *string `json:"image,omitempty"` diff --git a/web/graph/model/user.go b/web/graph/model/user.go index d150c06..367bd6b 100644 --- a/web/graph/model/user.go +++ b/web/graph/model/user.go @@ -1,12 +1,13 @@ package model // User is an authenticated user of glabs-web. Identity comes from the auth proxy -// (the OIDC email); a user must be on the allowlist to be let in. +// (the OIDC email); there is no allowlist — anyone the proxy authenticates is let +// in. // // glabs has no role hierarchy — every user manages only their own courses (strict // per-user isolation), so there is nothing for roles to gate. The type is written -// by hand rather than generated so it carries bson tags for MongoDB; gqlgen binds -// to it via autobind. +// by hand rather than generated so it carries bson tags (it was once persisted); +// gqlgen binds to it via autobind. type User struct { Email string `json:"email" bson:"email"` Name string `json:"name" bson:"name"` diff --git a/web/graph/students.graphqls b/web/graph/students.graphqls deleted file mode 100644 index e2dcde4..0000000 --- a/web/graph/students.graphqls +++ /dev/null @@ -1,24 +0,0 @@ -""" -One roster entry of a course, enriched with ZPA (Prüfungsamt) student details when -they could be found. `found` is false and the detail fields are null when ZPA is -not configured or has no unambiguous match — the GUI then shows just the email. -""" -type CourseStudent { - "The roster email (the identity glabs keys students on)." - email: String! - "Whether ZPA details were found for this email." - found: Boolean! - firstName: String - lastName: String - "The ZPA gender field (sensitive — shown only when the course owner opts in)." - gender: String - "The ZPA group/study programme." - group: String - "The Matrikelnummer." - mtknr: String -} - -extend type Query { - "The course-level roster of one of the caller's courses, enriched with ZPA details, sorted by last name." - courseStudents(course: String!): [CourseStudent!]! -} diff --git a/web/graph/students.resolvers.go b/web/graph/students.resolvers.go deleted file mode 100644 index 25ad457..0000000 --- a/web/graph/students.resolvers.go +++ /dev/null @@ -1,33 +0,0 @@ -package graph - -// This file will be automatically regenerated based on the schema, any resolver -// implementations -// will be copied through when generating and any unknown code will be moved to the end. -// Code generated by github.com/99designs/gqlgen version v0.17.94 - -import ( - "context" - - "github.com/obcode/glabs/v3/web/graph/model" -) - -// CourseStudents is the resolver for the courseStudents field. -func (r *queryResolver) CourseStudents(ctx context.Context, course string) ([]*model.CourseStudent, error) { - students, err := r.app.CourseStudents(ctx, course) - if err != nil { - return nil, err - } - out := make([]*model.CourseStudent, 0, len(students)) - for _, s := range students { - out = append(out, &model.CourseStudent{ - Email: s.Email, - Found: s.Found, - FirstName: emptyToNil(s.FirstName), - LastName: emptyToNil(s.LastName), - Gender: emptyToNil(s.Gender), - Group: emptyToNil(s.Group), - Mtknr: emptyToNil(s.Mtknr), - }) - } - return out, nil -} diff --git a/web/zpa/zpa.go b/web/zpa/zpa.go deleted file mode 100644 index 1092c68..0000000 --- a/web/zpa/zpa.go +++ /dev/null @@ -1,141 +0,0 @@ -// Package zpa is a minimal read-only client for HM's ZPA (Prüfungsamt) REST API, -// used to enrich a course roster with student details (name, group, gender). It -// is deliberately small — glabs only needs to look a student up — and keeps the -// HTTP details confined here, mirroring plexams.go's zpa package. -// -// glabs identifies students by email, but ZPA's get_student_info search is -// primarily used with a Matrikelnummer in plexams. StudentByEmail therefore -// looks up defensively (by full email, then the email's local part) and returns -// nil when ZPA has no unambiguous match — so the students page degrades to just -// the email rather than showing wrong data. -package zpa - -import ( - "context" - "encoding/json" - "fmt" - "io" - "net/http" - "net/url" - "strings" - "time" -) - -// Config is the ZPA connection, read from the server config in bootstrap. -type Config struct { - BaseURL string - Token string -} - -// Client talks to the ZPA REST API with a fixed token. -type Client struct { - baseURL string - token string - http *http.Client -} - -// New builds a ZPA client. baseURL and token are required by the caller; an empty -// config means ZPA is disabled (bootstrap passes nil then). -func New(cfg Config) *Client { - return &Client{ - baseURL: strings.TrimRight(cfg.BaseURL, "/"), - token: cfg.Token, - http: &http.Client{Timeout: 30 * time.Second}, - } -} - -// Student is one ZPA student record. -type Student struct { - Mtknr string `json:"mtknr"` - Greeting string `json:"greeting"` - FirstName string `json:"first_name"` - LastName string `json:"last_name"` - Email string `json:"email"` - Gender string `json:"gender"` - Group string `json:"group"` -} - -// StudentByEmail looks a student up defensively: first by the full email, then by -// the email's local part. It returns (nil, nil) when ZPA has no unambiguous match, -// so callers can show just the email rather than guessing. -func (c *Client) StudentByEmail(ctx context.Context, email string) (*Student, error) { - email = strings.TrimSpace(email) - if email == "" { - return nil, nil - } - - students, err := c.search(ctx, email) - if err != nil { - return nil, err - } - if s := pickMatch(students, email); s != nil { - return s, nil - } - - // Fallback: the local part (HM emails are name-based, e.g. a.ciftci@hm.edu). - if local, _, ok := strings.Cut(email, "@"); ok && local != "" { - students, err = c.search(ctx, local) - if err != nil { - return nil, err - } - if s := pickMatch(students, email); s != nil { - return s, nil - } - } - return nil, nil -} - -// pickMatch prefers an exact (case-insensitive) email match; failing that, a sole -// result; otherwise nil (an ambiguous search must not enrich with the wrong -// person). -func pickMatch(students []*Student, email string) *Student { - for _, s := range students { - if s != nil && strings.EqualFold(strings.TrimSpace(s.Email), email) { - return s - } - } - if len(students) == 1 { - return students[0] - } - return nil -} - -// search calls get_student_info?ask=. -func (c *Client) search(ctx context.Context, ask string) ([]*Student, error) { - var students []*Student - err := c.get(ctx, "get_student_info?ask="+url.QueryEscape(ask), &students) - return students, err -} - -func (c *Client) get(ctx context.Context, path string, v any) error { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL+"/"+path, nil) - if err != nil { - return fmt.Errorf("cannot build ZPA request for %s: %w", path, err) - } - req.Header.Set("Accept", "*/*") - req.Header.Set("Authorization", "Token "+c.token) - - resp, err := c.http.Do(req) - if err != nil { - return fmt.Errorf("cannot reach ZPA for %s: %w", path, err) - } - defer resp.Body.Close() //nolint:errcheck - body, _ := io.ReadAll(resp.Body) - - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return fmt.Errorf("ZPA returned %s for %s: %s", resp.Status, path, bodySnippet(body)) - } - if err := json.Unmarshal(body, v); err != nil { - return fmt.Errorf("ZPA returned an unexpected (non-JSON) response for %s: %s", path, bodySnippet(body)) - } - return nil -} - -func bodySnippet(body []byte) string { - s := strings.TrimSpace(string(body)) - const max = 300 - if len(s) > max { - s = s[:max] + " …(truncated)" - } - return s -} diff --git a/web/zpa/zpa_test.go b/web/zpa/zpa_test.go deleted file mode 100644 index ba585bd..0000000 --- a/web/zpa/zpa_test.go +++ /dev/null @@ -1,104 +0,0 @@ -package zpa - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" -) - -// mockZPA serves get_student_info from a map of ask→students, and records the -// Authorization header of the last request. -func mockZPA(t *testing.T, byAsk map[string][]*Student) (*Client, *string) { - t.Helper() - lastAuth := new(string) - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - *lastAuth = r.Header.Get("Authorization") - if r.URL.Path != "/get_student_info" { - http.Error(w, "not found", http.StatusNotFound) - return - } - ask := r.URL.Query().Get("ask") - _ = json.NewEncoder(w).Encode(byAsk[ask]) - })) - t.Cleanup(srv.Close) - return New(Config{BaseURL: srv.URL, Token: "tok-123"}), lastAuth -} - -func TestStudentByEmail_exactMatchAndAuthHeader(t *testing.T) { - want := &Student{Mtknr: "123", FirstName: "Ada", LastName: "Ciftci", Email: "a.ciftci@hm.edu", Gender: "f", Group: "IF3A"} - c, lastAuth := mockZPA(t, map[string][]*Student{ - "a.ciftci@hm.edu": {want}, - }) - - got, err := c.StudentByEmail(context.Background(), "a.ciftci@hm.edu") - if err != nil { - t.Fatalf("StudentByEmail: %v", err) - } - if got == nil || got.Mtknr != "123" || got.FirstName != "Ada" || got.Group != "IF3A" { - t.Fatalf("got %+v, want Ada Ciftci (123)", got) - } - if *lastAuth != "Token tok-123" { - t.Errorf("Authorization = %q, want 'Token tok-123'", *lastAuth) - } -} - -func TestStudentByEmail_fallsBackToLocalPart(t *testing.T) { - want := &Student{Mtknr: "9", FirstName: "Bo", LastName: "Rueedi", Email: "benjamin.rueedi@hm.edu"} - // The full-email search returns nothing; the local-part search finds the student. - c, _ := mockZPA(t, map[string][]*Student{ - "benjamin.rueedi": {want}, - }) - - got, err := c.StudentByEmail(context.Background(), "benjamin.rueedi@hm.edu") - if err != nil { - t.Fatalf("StudentByEmail: %v", err) - } - if got == nil || got.Mtknr != "9" { - t.Fatalf("got %+v, want the local-part match (9)", got) - } -} - -func TestStudentByEmail_noMatchIsNil(t *testing.T) { - c, _ := mockZPA(t, map[string][]*Student{}) // ZPA knows nobody - - got, err := c.StudentByEmail(context.Background(), "nobody@hm.edu") - if err != nil { - t.Fatalf("StudentByEmail: %v", err) - } - if got != nil { - t.Errorf("got %+v, want nil for an unknown email", got) - } -} - -func TestStudentByEmail_ambiguousLocalPartIsNil(t *testing.T) { - // The local-part search returns two students, neither matching the email exactly - // → must not guess. - c, _ := mockZPA(t, map[string][]*Student{ - "mueller": { - {Mtknr: "1", Email: "a.mueller@hm.edu"}, - {Mtknr: "2", Email: "b.mueller@hm.edu"}, - }, - }) - - got, err := c.StudentByEmail(context.Background(), "mueller@hm.edu") - if err != nil { - t.Fatalf("StudentByEmail: %v", err) - } - if got != nil { - t.Errorf("got %+v, want nil for an ambiguous match", got) - } -} - -func TestStudentByEmail_serverErrorPropagates(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - http.Error(w, "forbidden", http.StatusForbidden) - })) - t.Cleanup(srv.Close) - c := New(Config{BaseURL: srv.URL, Token: "bad"}) - - if _, err := c.StudentByEmail(context.Background(), "a@hm.edu"); err == nil { - t.Error("a non-2xx ZPA response should be an error") - } -}