diff --git a/mcp.go b/mcp.go new file mode 100644 index 0000000..433dfa7 --- /dev/null +++ b/mcp.go @@ -0,0 +1,16 @@ +package bm + +import ( + "git.golem15.com/golem15/summercms/modules/pact" +) + +var _ pact.HasMCP = (*Plugin)(nil) + +// MCP declares the plugin's MCP tools. They are exposed with the bm_ prefix, +// bind token-guarded routes only (the token twins of the student API under +// TokenPrefix and the management API under ManagePrefix), run as the +// signed-in user under golem15.user's "user" guard, and their titles and +// descriptions are static text, never built from data. +func (p *Plugin) MCP() pact.MCP { + return pact.MCP{Tools: studentTools()} +} diff --git a/mcp_student.go b/mcp_student.go new file mode 100644 index 0000000..04a8824 --- /dev/null +++ b/mcp_student.go @@ -0,0 +1,156 @@ +package bm + +import ( + "net/http" + + "git.golem15.com/golem15/summercms/modules/pact" +) + +// The sentences every student tool description ends with (D-15). +const ( + needsScopeRead = " Needs scope read." + needsScopeWrite = " Needs scope write." +) + +// noInput is the input of a tool that takes no arguments. +type noInput struct{} + +// idInput names one record by its id. +type idInput struct { + ID uint `json:"id" path:"id" jsonschema:"the record id, as listed by the matching list tool"` +} + +// readingIDInput names one reading by its id. +type readingIDInput struct { + ID uint `json:"id" path:"id" jsonschema:"the reading id, as listed by bm_readings or bm_course"` +} + +// coursesInput is the input of bm_courses. +type coursesInput struct { + Semester int `json:"semester,omitempty" query:"semester" jsonschema:"only courses of this semester, 1 to 4"` +} + +// rangeInput is the input of bm_schedule and bm_deadlines. +type rangeInput struct { + From string `json:"from,omitempty" query:"from" jsonschema:"first day, YYYY-MM-DD (default: today)"` + To string `json:"to,omitempty" query:"to" jsonschema:"last day (inclusive), YYYY-MM-DD, not before from (default: no end)"` +} + +// readingsInput is the input of bm_readings. +type readingsInput struct { + Course uint `json:"course,omitempty" query:"course" jsonschema:"only readings of this course id"` +} + +// boolPtr returns a pointer to b, for the tool annotations. +func boolPtr(b bool) *bool { return &b } + +// studentTools are the tools over the token twins of the student API. Each +// call runs as the session's user, who reaches only their own progress and +// published data, as on the HTTP API (D-09). +func studentTools() []pact.MCPTool { + return []pact.MCPTool{ + { + Name: "dashboard", + Title: "BM dashboard", + Description: "Returns the student's dashboard: the next class sessions, the deadlines not yet due, the zjazd (meeting) under way today and the next one, and the student's reading progress counts." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/dashboard", + Input: noInput{}, + }, + { + Name: "courses", + Title: "BM courses", + Description: "Lists the programme's courses with code, form, lecturer, semester, ECTS, assessment, syllabus link and counters." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/courses", + Input: coursesInput{}, + }, + { + Name: "course", + Title: "BM course", + Description: "Returns one course with its description, syllabus link, counters, class sessions, readings (with the student's read and owned marks) and published materials." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/courses/{id}", + Input: idInput{}, + }, + { + Name: "schedule", + Title: "BM schedule", + Description: "Lists the class sessions starting in a date range (default: from today, no end) with course, zjazd, start and end time, room, kind, lecturer, notes and whether the class is online." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/schedule", + Input: rangeInput{}, + }, + { + Name: "meetings", + Title: "BM meetings", + Description: "Lists the zjazdy (weekend meetings) with number, first and last day, notes and the number of class sessions in each." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/meetings", + Input: noInput{}, + }, + { + Name: "deadlines", + Title: "BM deadlines", + Description: "Lists the deadlines due in a date range (default: from today, no end) with title, due time, kind, course and description." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/deadlines", + Input: rangeInput{}, + }, + { + Name: "readings", + Title: "BM readings", + Description: "Lists readings with author, source, link, course, zjazd, whether required, and the student's read and owned marks." + needsScopeRead, + Guard: "user", + Method: http.MethodGet, + Path: TokenPrefix + "/readings", + Input: readingsInput{}, + }, + { + Name: "mark_reading_read", + Title: "Mark a reading as read", + Description: "Marks a reading as read by the signed-in student and returns the reading id with the read time. Marking again keeps the first time." + needsScopeWrite, + Guard: "user", + Method: http.MethodPut, + Path: TokenPrefix + "/readings/{id}/progress", + Input: readingIDInput{}, + }, + { + Name: "unmark_reading_read", + Title: "Unmark a reading as read", + Description: "Removes the signed-in student's read mark from a reading (it can be marked again) and returns the reading id with an empty read time." + needsScopeWrite, + Guard: "user", + Method: http.MethodDelete, + Path: TokenPrefix + "/readings/{id}/progress", + Input: readingIDInput{}, + Destructive: boolPtr(false), + Idempotent: boolPtr(true), + }, + { + Name: "mark_reading_owned", + Title: "Mark a reading as owned", + Description: "Marks that the signed-in student has a copy of a reading and returns the reading id with the owned time." + needsScopeWrite, + Guard: "user", + Method: http.MethodPut, + Path: TokenPrefix + "/readings/{id}/owned", + Input: readingIDInput{}, + }, + { + Name: "unmark_reading_owned", + Title: "Unmark a reading as owned", + Description: "Removes the signed-in student's owned mark from a reading (it can be marked again) and returns the reading id with an empty owned time." + needsScopeWrite, + Guard: "user", + Method: http.MethodDelete, + Path: TokenPrefix + "/readings/{id}/owned", + Input: readingIDInput{}, + Destructive: boolPtr(false), + Idempotent: boolPtr(true), + }, + } +} diff --git a/mcp_test.go b/mcp_test.go new file mode 100644 index 0000000..fafdba1 --- /dev/null +++ b/mcp_test.go @@ -0,0 +1,242 @@ +package bm_test + +import ( + "net/http" + "reflect" + "slices" + "strings" + "testing" + + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/golem15/summercms/modules/pier" + "git.golem15.com/golem15/summercms/modules/surf" + bm "git.golem15.com/jakub/sm-bm-plugin" +) + +// mountHost adapts a built surf router to pier.Host for a compile-only +// mount: pier compiles every declared tool against the real route table. +type mountHost struct{ r *surf.Router } + +func (h mountHost) Routes() []pier.RouteInfo { + var out []pier.RouteInfo + for _, ri := range h.r.Routes() { + out = append(out, pier.RouteInfo{Method: ri.Method, Pattern: ri.Pattern, PluginID: ri.PluginID, Middleware: ri.Middleware}) + } + return out +} +func (h mountHost) Handler(method, pattern string) (http.Handler, bool) { + return h.r.Handler(method, pattern) +} +func (mountHost) MountStream(string, string, http.Handler, ...string) error { return nil } +func (mountHost) MountJSON(string, string, http.Handler) error { return nil } +func (h mountHost) Owner(method, path string) (string, bool) { + for _, ri := range h.r.Routes() { + if ri.Method == method && ri.Pattern == path { + return ri.PluginID, true + } + } + return "", false +} + +// pinnedTools is the sorted tool list of golem15.user plus jz.bm. +var pinnedTools = []string{ + "bm_course", + "bm_courses", + "bm_dashboard", + "bm_deadlines", + "bm_mark_reading_owned", + "bm_mark_reading_read", + "bm_meetings", + "bm_readings", + "bm_schedule", + "bm_unmark_reading_owned", + "bm_unmark_reading_read", + "user_me", +} + +// studentQueryKeys are the query keys each student handler reads +// (controllers/api: query(r, ...)), by tool name. A tool query field outside +// these would be sent and silently ignored. +var studentQueryKeys = map[string][]string{ + "courses": {"semester"}, + "schedule": {"from", "to"}, + "deadlines": {"from", "to"}, + "readings": {"course"}, +} + +// bmTools returns jz.bm's declared tools. +func bmTools() []pact.MCPTool { + return (&bm.Plugin{}).MCP().Tools +} + +// TestBMMCP (criterion 4, D-09, D-14, D-15) mounts pier against golem15.user +// and jz.bm without a database and pins the compiled tools; every tool binds +// a token-guarded jz.bm route, names its scope, and reads only keys the +// route reads. Runs under -short. +func TestBMMCP(t *testing.T) { + tools := bmTools() + + t.Run("compiles", func(t *testing.T) { + app := backpack.New(testConfig(t)) + if err := app.Config.Set("app.url", "https://bm.example.test"); err != nil { + t.Fatal(err) + } + plugins := activate(t, app) + router, err := surf.BuildRouter(app, plugins) + if err != nil { + t.Fatalf("BuildRouter: %v", err) + } + if err := app.Config.Set(pier.EnabledKey, true); err != nil { + t.Fatal(err) + } + if err := pier.Mount(mountHost{r: router}, app, plugins); err != nil { + t.Fatalf("pier.Mount: %v", err) + } + p, ok := app.Lookup[*pier.Pier]() + if !ok { + t.Fatal("pier was not mounted") + } + if got := p.ToolNames(); !slices.Equal(got, pinnedTools) { + t.Fatalf("compiled tools %v, want %v", got, pinnedTools) + } + }) + + t.Run("routes", func(t *testing.T) { + for _, tool := range tools { + if !strings.HasPrefix(tool.Path, bm.TokenPrefix+"/") && !strings.HasPrefix(tool.Path, bm.ManagePrefix+"/") { + t.Errorf("%s binds %s, outside %s and %s", tool.Name, tool.Path, bm.TokenPrefix, bm.ManagePrefix) + } + if tool.Guard != "user" { + t.Errorf("%s: Guard %q, want user", tool.Name, tool.Guard) + } + } + }) + + t.Run("descriptions", func(t *testing.T) { + for _, tool := range tools { + if !strings.Contains(tool.Description, "Needs scope read") && !strings.Contains(tool.Description, "Needs scope write") { + t.Errorf("%s: description does not name its scope: %q", tool.Name, tool.Description) + } + if tool.Title == "" { + t.Errorf("%s has no title", tool.Name) + } + } + }) + + t.Run("inputs", func(t *testing.T) { + for _, tool := range tools { + rt := reflect.TypeOf(tool.Input) + for rt.Kind() == reflect.Pointer { + rt = rt.Elem() + } + if rt.Kind() != reflect.Struct { + t.Errorf("%s: input is %s, not a struct", tool.Name, rt.Kind()) + continue + } + for i := 0; i < rt.NumField(); i++ { + f := rt.Field(i) + key, _, _ := strings.Cut(f.Tag.Get("json"), ",") + if key == "dry_run" { + t.Errorf("%s has a dry_run field (D-14)", tool.Name) + } + switch { + case f.Tag.Get("path") != "": + if !strings.Contains(tool.Path, "{"+f.Tag.Get("path")+"}") { + t.Errorf("%s.%s: path tag %q not in %s", tool.Name, f.Name, f.Tag.Get("path"), tool.Path) + } + case f.Tag.Get("query") != "": + if strings.HasPrefix(tool.Path, bm.TokenPrefix+"/") && !slices.Contains(studentQueryKeys[tool.Name], f.Tag.Get("query")) { + t.Errorf("%s.%s: query key %q is not one the handler reads", tool.Name, f.Name, f.Tag.Get("query")) + } + default: + if tool.Method == http.MethodGet || tool.Method == http.MethodDelete { + t.Errorf("%s.%s: a %s tool carries a body field %q", tool.Name, f.Name, tool.Method, key) + } + } + } + } + }) +} + +// studentRoutes are the routes mountStudent registers, in order. +var studentRoutes = []string{ + "GET /dashboard", + "GET /courses", + "GET /courses/{id}", + "GET /schedule", + "GET /meetings", + "GET /deadlines", + "GET /readings", + "PUT /readings/{id}/progress", + "DELETE /readings/{id}/progress", + "PUT /readings/{id}/owned", + "DELETE /readings/{id}/owned", + "GET /materials", + "GET /materials/{id}", +} + +// TestTokenMirrorRoutes is the invariant of the two student surfaces: every +// student route exists under APIPrefix (jwt.auth, no user.* middleware) and +// under TokenPrefix (user.api_token, the token bucket and the matching +// scope, no jwt.auth); the calendar feed and the chat have no twin. +func TestTokenMirrorRoutes(t *testing.T) { + app := backpack.New(testConfig(t)) + plugins := activate(t, app) + router, err := surf.BuildRouter(app, plugins) + if err != nil { + t.Fatalf("BuildRouter: %v", err) + } + byRoute := map[string][]string{} + for _, rt := range router.Routes() { + if rt.PluginID != "jz.bm" { + continue + } + byRoute[rt.Method+" "+rt.Pattern] = rt.Middleware + if strings.HasPrefix(rt.Pattern, bm.TokenPrefix+"/") { + rest := strings.TrimPrefix(rt.Pattern, bm.TokenPrefix) + if !slices.Contains(studentRoutes, rt.Method+" "+rest) { + t.Errorf("%s %s under the token prefix is not a student route", rt.Method, rt.Pattern) + } + } + } + for _, route := range studentRoutes { + method, pattern, _ := strings.Cut(route, " ") + jwt, ok := byRoute[method+" "+bm.APIPrefix+pattern] + if !ok { + t.Errorf("%s %s is not registered", method, bm.APIPrefix+pattern) + } else { + if !slices.Contains(jwt, "jwt.auth") { + t.Errorf("%s %s lacks jwt.auth: %v", method, bm.APIPrefix+pattern, jwt) + } + if slices.ContainsFunc(jwt, func(name string) bool { return strings.HasPrefix(name, "user.") }) { + t.Errorf("%s %s carries user.* middleware: %v", method, bm.APIPrefix+pattern, jwt) + } + } + tok, ok := byRoute[method+" "+bm.TokenPrefix+pattern] + if !ok { + t.Errorf("%s %s (the token twin) is not registered", method, bm.TokenPrefix+pattern) + continue + } + scope := "user.scope:write" + if method == http.MethodGet { + scope = "user.scope:read" + } + for _, name := range []string{"user.api_token", "throttle:jz.bm.token", scope} { + if !slices.Contains(tok, name) { + t.Errorf("%s %s lacks %s: %v", method, bm.TokenPrefix+pattern, name, tok) + } + } + if slices.Contains(tok, "jwt.auth") { + t.Errorf("%s %s carries jwt.auth: %v", method, bm.TokenPrefix+pattern, tok) + } + if slices.Index(tok, "user.api_token") > slices.Index(tok, "throttle:jz.bm.token") { + t.Errorf("%s %s: the token bucket runs before the guard: %v", method, bm.TokenPrefix+pattern, tok) + } + } + for route := range byRoute { + if strings.Contains(route, bm.TokenPrefix+"/calendar-feed") || strings.Contains(route, bm.TokenPrefix+"/chat") { + t.Errorf("%s has a token twin", route) + } + } +} diff --git a/plugin_test.go b/plugin_test.go index 563f087..10bf43c 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -119,6 +119,19 @@ func TestRoutes(t *testing.T) { "PUT /_bm/api/v1/calendar-feed", "POST /_bm/api/v1/calendar-feed/rotate", "DELETE /_bm/api/v1/calendar-feed", + "GET /_bm/api/v1/token/dashboard", + "GET /_bm/api/v1/token/courses", + "GET /_bm/api/v1/token/courses/{id}", + "GET /_bm/api/v1/token/schedule", + "GET /_bm/api/v1/token/meetings", + "GET /_bm/api/v1/token/deadlines", + "GET /_bm/api/v1/token/readings", + "PUT /_bm/api/v1/token/readings/{id}/progress", + "DELETE /_bm/api/v1/token/readings/{id}/progress", + "PUT /_bm/api/v1/token/readings/{id}/owned", + "DELETE /_bm/api/v1/token/readings/{id}/owned", + "GET /_bm/api/v1/token/materials", + "GET /_bm/api/v1/token/materials/{id}", "GET /api/v1/bm/me", "GET /api/v1/bm/materials", "GET /api/v1/bm/materials/{id}", @@ -176,7 +189,13 @@ func TestRoutes(t *testing.T) { continue } got = append(got, rt.Method+" "+rt.Pattern) - if strings.HasPrefix(rt.Pattern, "/api/v1/bm/") { + if strings.HasPrefix(rt.Pattern, bm.TokenPrefix+"/") { + // The token twins of the student API: the user API token guard + // and a scope instead of jwt.auth. + if !slices.Contains(rt.Middleware, "user.api_token") || !slices.ContainsFunc(rt.Middleware, func(name string) bool { return strings.HasPrefix(name, "user.scope:") }) || slices.Contains(rt.Middleware, "jwt.auth") { + t.Errorf("%s %s is not behind the user API token and a scope: %v", rt.Method, rt.Pattern, rt.Middleware) + } + } else if strings.HasPrefix(rt.Pattern, "/api/v1/bm/") { if !slices.Contains(rt.Middleware, "user.api_token") || !slices.ContainsFunc(rt.Middleware, func(name string) bool { return strings.HasPrefix(name, "user.scope:") }) { t.Errorf("%s %s is not behind the shared token and a scope: %v", rt.Method, rt.Pattern, rt.Middleware) } diff --git a/routes.go b/routes.go index 91f2f03..aa3632e 100644 --- a/routes.go +++ b/routes.go @@ -41,10 +41,15 @@ const ( // bucketChat throttles the BM chat routes per user (they run after // jwt.auth), or per client IP for a request without one. bucketChat = "throttle:jz.bm.chat" + // bucketToken throttles the token twins of the student API per user + // (it runs after user.api_token), or per client IP for a request + // without one. + bucketToken = "throttle:jz.bm.token" ) // Buckets declares the rate limits of the public calendar feed, the -// management API, the realtime routes and the BM chat routes. +// management API, the token twins of the student API, the realtime routes +// and the BM chat routes. func (p *Plugin) Buckets() map[string]surf.Bucket { trusted := surf.TrustedProxies(nil) if p.app != nil { @@ -81,6 +86,16 @@ func (p *Plugin) Buckets() map[string]surf.Bucket { return "jz.bm.realtime|ip:" + surf.ClientIP(r, trusted) }, }, + "jz.bm.token": { + Max: 240, + Decay: time.Minute, + Key: func(r *http.Request) string { + if u, ok := bouncer.User(r.Context()); ok && u.ID != 0 { + return "jz.bm.token|u:" + strconv.FormatUint(uint64(u.ID), 10) + } + return "jz.bm.token|ip:" + surf.ClientIP(r, trusted) + }, + }, "jz.bm.chat": { Max: 240, Decay: time.Minute, @@ -100,31 +115,18 @@ const APIPrefix = "/_bm/api/v1" // ManagePrefix is the path prefix of the management API. const ManagePrefix = "/api/v1/bm" +// TokenPrefix is the path prefix of the token twins of the student API: the +// same routes and handlers behind golem15.user's API token guard, for +// personal tokens and MCP sessions. +const TokenPrefix = APIPrefix + "/token" + // Routes declares the student API and the management API. Every student // route needs a golem15.user JWT (jwt.auth); students only read, except for // their own reading progress and owned marks. func (p *Plugin) Routes(r pact.Router) error { h := p.handlers() r.Group(APIPrefix, surf.Use("jwt.auth", "locale.from-principal"), func(g pact.Router) { - g.Get("/dashboard", h.Dashboard) - g.Get("/courses", h.Courses) - g.Get("/courses/{id}", h.Course) - g.Where("id", "[0-9]+") - g.Get("/schedule", h.Schedule) - g.Get("/meetings", h.Meetings) - g.Get("/deadlines", h.Deadlines) - g.Get("/readings", h.Readings) - g.Put("/readings/{id}/progress", h.MarkRead) - g.Where("id", "[0-9]+") - g.Delete("/readings/{id}/progress", h.UnmarkRead) - g.Where("id", "[0-9]+") - g.Put("/readings/{id}/owned", h.MarkOwned) - g.Where("id", "[0-9]+") - g.Delete("/readings/{id}/owned", h.UnmarkOwned) - g.Where("id", "[0-9]+") - g.Get("/materials", h.Materials) - g.Get("/materials/{id}", h.Material) - g.Where("id", "[0-9]+") + mountStudent(g, h, nil, nil) g.Get("/calendar-feed", h.CalendarFeed) g.Put("/calendar-feed", h.UpdateCalendarFeed) g.Post("/calendar-feed/rotate", h.RotateCalendarFeed, "throttle:10,1") @@ -136,6 +138,15 @@ func (p *Plugin) Routes(r pact.Router) error { g.Post("/chat/embeds/resolve", h.ResolveChatEmbeds, bucketChat, "body.limit:8192") } }) + // The personal-token and MCP twins of the student API: the same routes + // and handlers behind user.api_token (golem15.user's MCP guard dispatches + // only to user.api_token routes), with a scope on each. The handlers read + // bouncer.User, so a token reaches only its own user's data. The bucket + // runs after the guard, so it keys by user. The calendar feed and the + // chat have no twin. + r.Group(TokenPrefix, surf.Use("user.api_token", bucketToken, "locale.from-principal"), func(g pact.Router) { + mountStudent(g, h, []string{"user.scope:read"}, []string{"user.scope:write"}) + }) // The management API: what bm:seed does, plus materials, // behind the application-wide user API token. Scope limits delegation; // user.permission checks the owner's current group permissions. @@ -283,6 +294,31 @@ func (p *Plugin) handlers() *api.Handlers { return h } +// mountStudent mounts the student routes on g, with read appended to every +// GET route and write to the reading marks. The jwt.auth group and the token +// twins share it, so both lists cannot drift. +func mountStudent(g pact.Router, h *api.Handlers, read, write []string) { + g.Get("/dashboard", h.Dashboard, read...) + g.Get("/courses", h.Courses, read...) + g.Get("/courses/{id}", h.Course, read...) + g.Where("id", "[0-9]+") + g.Get("/schedule", h.Schedule, read...) + g.Get("/meetings", h.Meetings, read...) + g.Get("/deadlines", h.Deadlines, read...) + g.Get("/readings", h.Readings, read...) + g.Put("/readings/{id}/progress", h.MarkRead, write...) + g.Where("id", "[0-9]+") + g.Delete("/readings/{id}/progress", h.UnmarkRead, write...) + g.Where("id", "[0-9]+") + g.Put("/readings/{id}/owned", h.MarkOwned, write...) + g.Where("id", "[0-9]+") + g.Delete("/readings/{id}/owned", h.UnmarkOwned, write...) + g.Where("id", "[0-9]+") + g.Get("/materials", h.Materials, read...) + g.Get("/materials/{id}", h.Material, read...) + g.Where("id", "[0-9]+") +} + // recordRoutes is what Record provides to mountRecords. type recordRoutes interface { List(*api.Handlers) http.HandlerFunc