From ab14f2cb427ec5e3ac1f0ea5d538c28f42436f71 Mon Sep 17 00:00:00 2001 From: Distronode <254259913+distronode-com@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:41:26 -0400 Subject: [PATCH] microsoft: select canEdit, so writable calendars can be booked into ListCalendars asked Graph for GET /me/calendars?$select=id,name,isDefaultCalendar&$top=100 and then read canEdit into CalendarInfo.Writable. $select limits the response to the properties it names, so Graph never sent canEdit, the bool decoded as false without an error, and every Microsoft calendar came back not writable. The calendar picker disables the Book radio and shows "(read-only)" for such a calendar, so ever since Writable arrived in 0.5.0 a Microsoft user could not choose which of their calendars bookings are written into. Nothing server-side reads Writable, so the picker is the whole of the effect. What the diagnosis relies on, from Microsoft's Graph documentation: calendar resource type (v1.0), Properties table: canEdit | Boolean | "true if the user can write to the calendar, false otherwise. This property is true for the user who created the calendar. This property is also true for a user who shared a calendar and granted write access." Customize Microsoft Graph responses with query parameters, Select: "Use the $select query parameter to return a subset of properties for a resource." OData 4.01 Part 1 (11.2.5.1) is the underlying rule: $select "requests that the service return only the properties, dynamic properties, actions and functions explicitly requested by the client." Nothing caught it because ListCalendars had no test. The two added here share a fake /me/calendars that honours $select the way Graph does, serving a calendar the user owns (canEdit true) and one shared with them read-only (canEdit false): - TestListCalendars_writableFollowsCanEdit asserts both come back with the right Writable. - TestListCalendars_selectNamesEveryDecodedProperty reads the property names msCalListResp decodes from its json tags by reflection, and fails if the request's $select omits any of them. A field added to the struct later has to be selected too, and the check does not depend on the fake filtering correctly. Both fail with canEdit taken back out of $select. With it out and the fake changed to ignore $select, the Writable test passes and the $select test still fails, which is why the fake has to filter. The other Graph calls have no such gap: calendarView selects start,end and decodes exactly those plus the @odata.nextLink paging link, event create, update and delete send no $select, and the sign-in lookup in internal/handler selects mail,userPrincipalName and decodes exactly those. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 + internal/calendar/microsoft/calendars.go | 6 +- internal/calendar/microsoft/microsoft_test.go | 133 ++++++++++++++++++ 3 files changed, 145 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f45a54ce..53bc188e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,13 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad ## [Unreleased] +### Fixed +- **Microsoft calendars can be chosen as the one bookings are written into.** Since 0.5.0 + the calendar picker marked every Microsoft calendar "(read-only)" and disabled its Book + option. The calendar list read Graph's `canEdit` but left it out of `$select`, so Graph + never returned it and every calendar decoded as not writable. It is now requested, and a + test fails if that request omits any property the response decodes. + ## [0.9.0] - 2026-09-10 ### Added diff --git a/internal/calendar/microsoft/calendars.go b/internal/calendar/microsoft/calendars.go index 9aae3151..141491ea 100644 --- a/internal/calendar/microsoft/calendars.go +++ b/internal/calendar/microsoft/calendars.go @@ -28,6 +28,10 @@ func (c *Client) clientForAccount(ctx context.Context, userID, accountEmail stri return c.buildClient(ctx, userID, accessEnc, refreshEnc, calID, expiryStr, email) } +// msCalListResp is the GET /me/calendars response. Every property decoded here must also +// be named in ListCalendars' $select: Graph returns only the selected properties, and an +// absent bool decodes as false without error. +// TestListCalendars_selectNamesEveryDecodedProperty holds the two together. type msCalListResp struct { Value []struct { ID string `json:"id"` @@ -45,7 +49,7 @@ func (c *Client) ListCalendars(ctx context.Context, userID, accountEmail string) return nil, err } req, err := http.NewRequestWithContext(ctx, http.MethodGet, - c.apiBase+"/me/calendars?$select=id,name,isDefaultCalendar&$top=100", nil) + c.apiBase+"/me/calendars?$select=id,name,isDefaultCalendar,canEdit&$top=100", nil) if err != nil { return nil, err } diff --git a/internal/calendar/microsoft/microsoft_test.go b/internal/calendar/microsoft/microsoft_test.go index 30088fa9..5e89d3bb 100644 --- a/internal/calendar/microsoft/microsoft_test.go +++ b/internal/calendar/microsoft/microsoft_test.go @@ -4,9 +4,11 @@ import ( "context" "database/sql" "encoding/base64" + "encoding/json" "io" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "time" @@ -223,6 +225,137 @@ func TestCancelEvent_alreadyGoneIsOK(t *testing.T) { } } +// fakeGraphCalendars serves GET /me/calendars the way Graph does: with $select, each +// calendar carries only the selected properties. A fake that returned every property +// regardless could never notice a decoded property missing from the request's $select. +// onSelect, when non-nil, receives the raw $select value. +func fakeGraphCalendars(t *testing.T, cals []map[string]any, onSelect func(string)) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || !strings.HasSuffix(r.URL.Path, "/me/calendars") { + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + } + sel := r.URL.Query().Get("$select") + if onSelect != nil { + onSelect(sel) + } + value := cals + if sel != "" { + value = make([]map[string]any, 0, len(cals)) + for _, cal := range cals { + picked := map[string]any{} + for _, prop := range strings.Split(sel, ",") { + if v, ok := cal[prop]; ok { + picked[prop] = v + } + } + value = append(value, picked) + } + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"value": value}) + })) +} + +// graphCalendars are two calendars as Graph describes them: the user's own default, and +// one shared with them read-only (canEdit false). +var graphCalendars = []map[string]any{ + { + "id": "cal-own", "name": "Calendar", "isDefaultCalendar": true, "canEdit": true, + "canShare": true, "color": "auto", + "owner": map[string]any{"name": "Alex", "address": "alex@example.com"}, + }, + { + "id": "cal-shared", "name": "Team rota", "isDefaultCalendar": false, "canEdit": false, + "canShare": false, "color": "lightBlue", + "owner": map[string]any{"name": "Sam", "address": "sam@example.com"}, + }, +} + +func TestListCalendars_writableFollowsCanEdit(t *testing.T) { + c := newTestClient(t) + connect(t, c, "u1") + + srv := fakeGraphCalendars(t, graphCalendars, nil) + defer srv.Close() + c.apiBase = srv.URL + + got, err := c.ListCalendars(context.Background(), "u1", "") + if err != nil { + t.Fatalf("ListCalendars: %v", err) + } + want := []calendar.CalendarInfo{ + {ID: "cal-own", Name: "Calendar", Primary: true, Writable: true}, + {ID: "cal-shared", Name: "Team rota", Primary: false, Writable: false}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("ListCalendars =\n %+v\nwant\n %+v", got, want) + } +} + +// decodedJSONNames lists the JSON property names a struct decodes. For a Graph response +// item that is the set of properties its request has to $select. +func decodedJSONNames(t *testing.T, typ reflect.Type) []string { + t.Helper() + if typ.Kind() != reflect.Struct { + t.Fatalf("decodedJSONNames(%s): want a struct", typ) + } + var names []string + for i := 0; i < typ.NumField(); i++ { + f := typ.Field(i) + if f.Anonymous { + // encoding/json promotes an embedded struct's fields; not needed yet, so refuse + // rather than under-report. + t.Fatalf("decodedJSONNames(%s): embedded field %s is not handled", typ, f.Name) + } + if !f.IsExported() { + continue + } + name, _, _ := strings.Cut(f.Tag.Get("json"), ",") + if name == "-" { + continue + } + if name == "" { + name = f.Name + } + names = append(names, name) + } + return names +} + +// Graph omits any property the request does not $select, and an absent bool decodes as +// false without error. So every property the response item decodes must be selected, or +// it silently reads as its zero value in production while a lenient fake hides it. +func TestListCalendars_selectNamesEveryDecodedProperty(t *testing.T) { + c := newTestClient(t) + connect(t, c, "u1") + + var gotSelect string + srv := fakeGraphCalendars(t, graphCalendars, func(s string) { gotSelect = s }) + defer srv.Close() + c.apiBase = srv.URL + + if _, err := c.ListCalendars(context.Background(), "u1", ""); err != nil { + t.Fatalf("ListCalendars: %v", err) + } + if gotSelect == "" { + return // no $select: Graph returns the default properties, which is not this bug + } + selected := map[string]bool{} + for _, prop := range strings.Split(gotSelect, ",") { + selected[prop] = true + } + value, ok := reflect.TypeOf(msCalListResp{}).FieldByName("Value") + if !ok { + t.Fatal("msCalListResp has no Value field") + } + for _, name := range decodedJSONNames(t, value.Type.Elem()) { + if !selected[name] { + t.Errorf("$select=%q omits %q, which msCalListResp decodes; Graph will not return it", gotSelect, name) + } + } +} + func mkIDToken(tid string) string { payload := base64.RawURLEncoding.EncodeToString([]byte(`{"tid":"` + tid + `"}`)) return "header." + payload + ".sig"