From c3b4d71ca4ccb6f4135ced353ce43921cd1cd886 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Tue, 6 Oct 2026 12:00:40 +0000 Subject: [PATCH] feat: a Room can ask one question at the door, and the answer becomes a group spec.question is optional: a prompt and two to eight answers, each a label and a demo: group. Everyone who joins picks one; the browser will not submit without it, and the server accepts only an answer the Room offers. The answer's group is kept on the Participant (spec.groups, new and optional) and sent to Dex beside the audienceGroup at every sign-in, comma-separated, which Dex's authproxy connector splits into the token's groups. The apiserver's demo:-only containment rule covers it unchanged. A Room without a question sends exactly the group it sent before, and a forged answer adds nothing. Groups that escaped the schema are dropped before they reach the header. The demo client now shows the token's groups, and its write form works in a real browser: under Referrer-Policy no-referrer a browser sends Origin "null", which its own check refused. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 + api/v1alpha1/types.go | 59 ++++ api/v1alpha1/zz_generated.deepcopy.go | 47 +++- .../room-pass.koudijs.dev_participants.yaml | 16 ++ .../bases/room-pass.koudijs.dev_rooms.yaml | 58 ++++ deploy/example/dex-config.yaml | 4 +- deploy/example/room.yaml | 10 + docs/install.md | 41 +++ docs/metrics.md | 5 +- internal/server/metrics_test.go | 4 +- internal/server/server.go | 108 +++++++- internal/server/server_test.go | 252 +++++++++++++++++- requirements.md | 10 +- test/browser/README.md | 3 +- test/browser/room-auth.spec.js | 72 +++++ test/e2e/demo-client/main.go | 21 +- test/integration/api_test.go | 45 +++- 17 files changed, 723 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 10eb259..7690505 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,9 @@ variable added to the base later still arrives. A Room needs a future end time, `spec.appearance`: a tagline, a picture, an accent colour and a background, served from the application's own host ([install guide](docs/install.md#dress-the-join-page-for-the-talk-optional)). +With `spec.question` it asks one question at the door, and the `demo:` group of +the answer joins `audienceGroup` in the participant's token +([install guide](docs/install.md#ask-one-question-at-the-door-optional)). Required cookie Secret: ```sh diff --git a/api/v1alpha1/types.go b/api/v1alpha1/types.go index 80e057a..c3d5ad9 100644 --- a/api/v1alpha1/types.go +++ b/api/v1alpha1/types.go @@ -63,6 +63,20 @@ type RoomSpec struct { // invalidates the code on screen. Set it before the event. // +optional Appearance *RoomAppearance `json:"appearance,omitempty"` + // Question asks everyone who joins to pick one answer, and the group of + // that answer joins the audienceGroup in their token. Empty, nobody is + // asked and the audienceGroup is their only group, as before. + // + // The participant chooses, so an answer is as unverified as a display + // name: bind its group only to what anyone in the room may choose to have. + // + // The answer is kept on the Participant (spec.groups) at enrollment. + // Changing the question later changes nothing for those already enrolled, + // and nobody who enrolled before it existed is asked. Like any spec + // change, editing it starts a fresh join-code epoch. Set it before the + // event. + // +optional + Question *RoomQuestion `json:"question,omitempty"` // EndsAt can be extended without changing participant identities. EndsAt metav1.Time `json:"endsAt"` // +kubebuilder:default=Closed @@ -139,6 +153,38 @@ type RoomAppearance struct { BackgroundImage string `json:"backgroundImage,omitempty"` } +// RoomQuestion is the one question a Room asks at the door. Every answer names +// a group, so a group from this list is all a participant can choose: never a +// username, never a group outside demo:, never one the operator did not write. +type RoomQuestion struct { + // Prompt is the question, shown above the answers. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=120 + // +kubebuilder:validation:Pattern="^[^<>\\x00-\\x1f\\x7f]*$" + Prompt string `json:"prompt"` + // Answers are shown in this order, and every participant picks one. + // +kubebuilder:validation:MinItems=2 + // +kubebuilder:validation:MaxItems=8 + // +listType=map + // +listMapKey=group + // +kubebuilder:validation:XValidation:rule="self.all(a, self.exists_one(b, b.label == a.label))",message="answer labels must be unique" + Answers []RoomAnswer `json:"answers"` +} + +type RoomAnswer struct { + // Label is what the participant picks, such as Svelte. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=60 + // +kubebuilder:validation:Pattern="^[^<>\\x00-\\x1f\\x7f]*$" + Label string `json:"label"` + // Group is what picking it adds to the participant's groups, such as + // demo:framework-svelte. It follows audienceGroup's rules, so the + // apiserver's demo:-only containment rule covers it unchanged. + // +kubebuilder:validation:Pattern="^demo:[a-zA-Z0-9][a-zA-Z0-9:_-]*$" + // +kubebuilder:validation:MaxLength=128 + Group string `json:"group"` +} + type Code struct { // +kubebuilder:validation:MaxLength=12 Code string `json:"code"` @@ -230,6 +276,19 @@ type ParticipantSpec struct { // +kubebuilder:validation:Pattern="^[A-Za-z0-9]([-A-Za-z0-9]*[A-Za-z0-9])?$" // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="displayName is immutable" DisplayName string `json:"displayName"` + // Groups are this participant's groups beside the Room's audienceGroup: + // the group of the answer they picked, when the Room asked a question. + // Room Pass sends them to Dex at every sign-in. Like the display name, they + // are the participant's choice, never verified. + // + // An operator may change them. The next sign-in carries the change; a + // token already issued keeps the groups it was issued with. + // +kubebuilder:validation:MaxItems=8 + // +kubebuilder:validation:items:Pattern="^demo:[a-zA-Z0-9][a-zA-Z0-9:_-]*$" + // +kubebuilder:validation:items:MaxLength=128 + // +listType=set + // +optional + Groups []string `json:"groups,omitempty"` // +kubebuilder:default=false // +kubebuilder:validation:XValidation:rule="!oldSelf || self",message="revocation is irreversible" Revoked bool `json:"revoked"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 3c69431..5bbc364 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -46,7 +46,7 @@ func (in *Participant) DeepCopyInto(out *Participant) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Participant. @@ -103,6 +103,11 @@ func (in *ParticipantList) DeepCopyObject() runtime.Object { func (in *ParticipantSpec) DeepCopyInto(out *ParticipantSpec) { *out = *in out.RoomRef = in.RoomRef + if in.Groups != nil { + in, out := &in.Groups, &out.Groups + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParticipantSpec. @@ -142,6 +147,21 @@ func (in *Room) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RoomAnswer) DeepCopyInto(out *RoomAnswer) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoomAnswer. +func (in *RoomAnswer) DeepCopy() *RoomAnswer { + if in == nil { + return nil + } + out := new(RoomAnswer) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RoomAppearance) DeepCopyInto(out *RoomAppearance) { *out = *in @@ -189,6 +209,26 @@ func (in *RoomList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RoomQuestion) DeepCopyInto(out *RoomQuestion) { + *out = *in + if in.Answers != nil { + in, out := &in.Answers, &out.Answers + *out = make([]RoomAnswer, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoomQuestion. +func (in *RoomQuestion) DeepCopy() *RoomQuestion { + if in == nil { + return nil + } + out := new(RoomQuestion) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RoomRef) DeepCopyInto(out *RoomRef) { *out = *in @@ -212,6 +252,11 @@ func (in *RoomSpec) DeepCopyInto(out *RoomSpec) { *out = new(RoomAppearance) **out = **in } + if in.Question != nil { + in, out := &in.Question, &out.Question + *out = new(RoomQuestion) + (*in).DeepCopyInto(*out) + } in.EndsAt.DeepCopyInto(&out.EndsAt) if in.AllowedReturnURLs != nil { in, out := &in.AllowedReturnURLs, &out.AllowedReturnURLs diff --git a/config/crd/bases/room-pass.koudijs.dev_participants.yaml b/config/crd/bases/room-pass.koudijs.dev_participants.yaml index 3f1a356..5c3a3cf 100644 --- a/config/crd/bases/room-pass.koudijs.dev_participants.yaml +++ b/config/crd/bases/room-pass.koudijs.dev_participants.yaml @@ -66,6 +66,22 @@ spec: x-kubernetes-validations: - message: displayName is immutable rule: self == oldSelf + groups: + description: |- + Groups are this participant's groups beside the Room's audienceGroup: + the group of the answer they picked, when the Room asked a question. + Room Pass sends them to Dex at every sign-in. Like the display name, they + are the participant's choice, never verified. + + An operator may change them. The next sign-in carries the change; a + token already issued keeps the groups it was issued with. + items: + maxLength: 128 + pattern: ^demo:[a-zA-Z0-9][a-zA-Z0-9:_-]*$ + type: string + maxItems: 8 + type: array + x-kubernetes-list-type: set revoked: default: false type: boolean diff --git a/config/crd/bases/room-pass.koudijs.dev_rooms.yaml b/config/crd/bases/room-pass.koudijs.dev_rooms.yaml index 2c2ba69..6bc6047 100644 --- a/config/crd/bases/room-pass.koudijs.dev_rooms.yaml +++ b/config/crd/bases/room-pass.koudijs.dev_rooms.yaml @@ -197,6 +197,64 @@ spec: maximum: 10000 minimum: 1 type: integer + question: + description: |- + Question asks everyone who joins to pick one answer, and the group of + that answer joins the audienceGroup in their token. Empty, nobody is + asked and the audienceGroup is their only group, as before. + + The participant chooses, so an answer is as unverified as a display + name: bind its group only to what anyone in the room may choose to have. + + The answer is kept on the Participant (spec.groups) at enrollment. + Changing the question later changes nothing for those already enrolled, + and nobody who enrolled before it existed is asked. Like any spec + change, editing it starts a fresh join-code epoch. Set it before the + event. + properties: + answers: + description: Answers are shown in this order, and every participant + picks one. + items: + properties: + group: + description: |- + Group is what picking it adds to the participant's groups, such as + demo:framework-svelte. It follows audienceGroup's rules, so the + apiserver's demo:-only containment rule covers it unchanged. + maxLength: 128 + pattern: ^demo:[a-zA-Z0-9][a-zA-Z0-9:_-]*$ + type: string + label: + description: Label is what the participant picks, such as + Svelte. + maxLength: 60 + minLength: 1 + pattern: ^[^<>\x00-\x1f\x7f]*$ + type: string + required: + - group + - label + type: object + maxItems: 8 + minItems: 2 + type: array + x-kubernetes-list-map-keys: + - group + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: answer labels must be unique + rule: self.all(a, self.exists_one(b, b.label == a.label)) + prompt: + description: Prompt is the question, shown above the answers. + maxLength: 120 + minLength: 1 + pattern: ^[^<>\x00-\x1f\x7f]*$ + type: string + required: + - answers + - prompt + type: object stopped: default: false description: Stopped permanently denies enrollment and new identity diff --git a/deploy/example/dex-config.yaml b/deploy/example/dex-config.yaml index f87af4f..fb8d776 100644 --- a/deploy/example/dex-config.yaml +++ b/deploy/example/dex-config.yaml @@ -29,7 +29,9 @@ connectors: # The only connector Room Pass works with. Its id is part of the identity # contract: it sets the callback path (/callback/room-pass), Room Pass's # CONNECTOR_ID default, and the connector the apiserver configuration - # contains. The header names are the ones Room Pass sends. + # contains. The header names are the ones Room Pass sends. It sends a Room's + # audienceGroup and the group of a participant's answer comma-separated, the + # default groupHeaderSeparator: leave that unset. - type: authproxy id: room-pass name: Join the room diff --git a/deploy/example/room.yaml b/deploy/example/room.yaml index 5f3dd98..68f319b 100644 --- a/deploy/example/room.yaml +++ b/deploy/example/room.yaml @@ -23,6 +23,16 @@ spec: # accentColor: "#f2a541" # backgroundColor: "#0f3d3e" # backgroundImage: /talks/my-talk/stage.jpg + # Optional: one question everyone answers when joining. The answer's group + # joins audienceGroup in their token. Participants choose it, so it carries + # no more weight than their name. Set it before the event too. + # question: + # prompt: Your favourite frontend framework? + # answers: + # - label: React + # group: demo:framework-react + # - label: Svelte + # group: demo:framework-svelte # Nobody can join after this. Move it before every event. endsAt: "2026-12-31T18:00:00Z" enrollment: Open diff --git a/docs/install.md b/docs/install.md index 01dc7ab..c2db8bc 100644 --- a/docs/install.md +++ b/docs/install.md @@ -193,6 +193,47 @@ The instructions, the issued address and the notice that a name is an unverified label stay as Room Pass writes them; `attributionNote` remains the one sentence an operator adds about identity. +### Ask one question at the door (optional) + +`spec.question` asks everyone who joins to pick one answer, and puts that +answer's group in their token beside `audienceGroup`. Bind a permission to one +answer's group and only part of the room has it, until you bind it to +`audienceGroup` as well. + +```yaml +spec: + audienceGroup: demo:my-talk + question: + prompt: Your favourite frontend framework? + answers: # 2 to 8, shown in this order + - label: React + group: demo:framework-react + - label: Vue + group: demo:framework-vue + - label: Angular + group: demo:framework-angular + - label: Svelte + group: demo:framework-svelte +``` + +Somebody who picks Svelte signs in with the groups `demo:my-talk` and +`demo:framework-svelte`. + +- **An answer is the participant's word.** Anyone can pick Svelte, so give an + answer's group only what anyone in the room could be allowed to have. Ask + nothing personal: the group travels in the token, into audit events and into + anything that mirrors them. +- **Answer groups follow `audienceGroup`'s rules**: `demo:`, then letters, + digits, `:`, `_` and `-`. The apiserver's containment rule (step 7) already + admits them, so its configuration does not change. Room Pass sends Dex the + groups comma-separated, the authproxy connector's default + `groupHeaderSeparator`; leave that unset. +- **The answer is kept on the Participant**, in `spec.groups`, and sent at every + sign-in. Nobody is asked twice. To move someone, edit their `spec.groups`: + their next sign-in carries it, while a token already issued keeps its groups. +- **Set it before you go on stage.** Editing it starts a fresh code epoch, and + anyone who joined before the question existed is not asked it. + ## 6. Connect your application Your application is an OIDC client of the issuer: diff --git a/docs/metrics.md b/docs/metrics.md index 3d05a82..70132bd 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -26,8 +26,9 @@ metrics. There is no separate telemetry pipeline or per-request Kubernetes scrap | `room_pass_handoff_slots_used` | How much of the in-memory transaction map is allocated? | | `room_pass_handoff_capacity` | What is the configured transaction limit? | -Enrollment result values are `enrolled`, `code_or_room_rejected`, `full` and -`storage_error` (which includes inability to generate/confirm enrollment). +Enrollment result values are `enrolled`, `code_or_room_rejected`, +`answer_rejected` (an answer the Room's question does not offer), `name_taken`, +`full` and `storage_error` (which includes inability to generate/confirm enrollment). `enrolled` is recorded only after a Participant create is confirmed, including recovery of a lost create response. Returning participants do not inflate it. It is a process-local counter, not the current number of Participants in Kubernetes. diff --git a/internal/server/metrics_test.go b/internal/server/metrics_test.go index ce34e58..0670a4c 100644 --- a/internal/server/metrics_test.go +++ b/internal/server/metrics_test.go @@ -19,10 +19,10 @@ func TestMetricsReflectOutcomesWithoutIdentityLabels(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := s.enrollParticipant(context.Background(), "WRONG", "private-name"); err == nil { + if _, err := s.enrollParticipant(context.Background(), "WRONG", "private-name", ""); err == nil { t.Fatal("invalid code accepted") } - if _, err := s.enrollParticipant(context.Background(), "BCDFGH", "private-name"); err != nil { + if _, err := s.enrollParticipant(context.Background(), "BCDFGH", "private-name", ""); err != nil { t.Fatal(err) } for _, path := range []string{"/healthz", "/unknown-private-name?code=SECRET", "/another-random-path"} { diff --git a/internal/server/server.go b/internal/server/server.go index 0745d67..72ae5d1 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -618,6 +618,81 @@ func lookOf(room *api.Room) *look { return l } +// demoGroup is audienceGroup's pattern, and so the pattern of every group a +// Room offers as an answer and a Participant keeps. TestGroupPatternIsTheCRDs +// keeps them equal. +var demoGroup = regexp.MustCompile(`^demo:[a-zA-Z0-9][a-zA-Z0-9:_-]*$`) + +const maxGroupLength = 128 + +func validGroup(g string) bool { return len(g) <= maxGroupLength && demoGroup.MatchString(g) } + +// questionOf is the question the join page asks, or nil when the Room asks +// none. An answer whose group escaped the schema is not offered, and a question +// left with fewer than two answers is not asked: nobody can be held at the door +// by a question they cannot answer. +func questionOf(room *api.Room) *api.RoomQuestion { + q := room.Spec.Question + if q == nil { + return nil + } + asked := &api.RoomQuestion{Prompt: q.Prompt} + for _, a := range q.Answers { + if validGroup(a.Group) { + asked.Answers = append(asked.Answers, a) + } + } + if len(asked.Answers) < 2 { + return nil + } + return asked +} + +// answerGroups is what enrollment stores for the answer a form carried. A Room +// that asks nothing ignores the field, so a forged one cannot add a group the +// Room never offered; a Room that asks takes only one of its own answers. +func answerGroups(room *api.Room, answer string) ([]string, error) { + q := questionOf(room) + if q == nil { + return nil, nil + } + for _, a := range q.Answers { + if a.Group == answer { + return []string{a.Group}, nil + } + } + return nil, errBadAnswer +} + +// answerLabel is what a returning participant picked, in the Room's words, or +// "" when the Room asks nothing they answered. +func answerLabel(room *api.Room, p *api.Participant) string { + if q := questionOf(room); q != nil { + for _, a := range q.Answers { + if contains(p.Spec.Groups, a.Group) { + return a.Label + } + } + } + return "" +} + +// groupHeader is the X-Remote-Group value: the Room's audience group, then the +// Participant's own. Dex's authproxy connector splits it on commas (its +// groupHeaderSeparator default), and no group the pattern admits holds one, so +// a stored group cannot smuggle in a second. One that escaped the schema is +// left out rather than sent. A Participant without groups sends exactly what +// every sign-in sent before Rooms could ask a question. +func groupHeader(room *api.Room, p *api.Participant) string { + groups := []string{room.Spec.AudienceGroup} + for _, g := range p.Spec.Groups { + if validGroup(g) && !contains(groups, g) { + groups = append(groups, g) + } + } + return strings.Join(groups, ",") +} + const nearBlack = "#111827" // onAccent is the text colour for a button filled with accent: white or @@ -665,7 +740,7 @@ var previewScriptSource = func() string { return "'sha256-" + base64.StdEncoding.EncodeToString(sum[:]) + "'" }() -var pageSource = `Join the room{{with .Look}}{{with .BackgroundImage}}{{end}}{{end}}
{{with .Look}}{{if .Picture}}{{.PictureAlt}}{{end}}{{end}}

{{.Title}}

{{with .Look}}{{with .Tagline}}

{{.}}

{{end}}{{end}}

{{.Message}}

{{if .Error}}{{end}}{{if .Form}}
{{if .Enrolled}}

You’re already enrolled as {{.EnrolledName}}. Continue with the same identity.

You are joining as {{.EnrolledEmail}}Room Pass built that address from your name, which is why there was nothing to fill in: it is never a real mailbox.{{with .AttributionNote}} {{.}}{{end}}

{{else}}{{if .Scanned}}

Room code {{.Scanned}}, from the code you scanned.

{{else}}{{end}}

You will appear as {{.Display}}You will join as {{.Email}}Room Pass builds both from your name, so there is nothing to fill in: spaces and accents are folded, and the address is never a real mailbox.{{with .AttributionNote}} {{.}}{{end}}

{{end}}
{{end}}{{if .Enrolled}}
{{end}}Your name is an unverified label, not a verified identity. It is shown to the application you are joining, together with the address above.
` +var pageSource = `Join the room{{with .Look}}{{with .BackgroundImage}}{{end}}{{end}}
{{with .Look}}{{if .Picture}}{{.PictureAlt}}{{end}}{{end}}

{{.Title}}

{{with .Look}}{{with .Tagline}}

{{.}}

{{end}}{{end}}

{{.Message}}

{{if .Error}}{{end}}{{if .Form}}
{{if .Enrolled}}

You’re already enrolled as {{.EnrolledName}}{{with .EnrolledAnswer}}, and you picked {{.}}{{end}}. Continue with the same identity.

You are joining as {{.EnrolledEmail}}Room Pass built that address from your name, which is why there was nothing to fill in: it is never a real mailbox.{{with .AttributionNote}} {{.}}{{end}}

{{else}}{{if .Scanned}}

Room code {{.Scanned}}, from the code you scanned.

{{else}}{{end}}

You will appear as {{.Display}}You will join as {{.Email}}Room Pass builds both from your name, so there is nothing to fill in: spaces and accents are folded, and the address is never a real mailbox.{{with .AttributionNote}} {{.}}{{end}}

{{with .Question}}
{{.Prompt}}{{range $i, $a := .Answers}}{{end}}
{{end}}{{end}}
{{end}}{{if .Enrolled}}
{{end}}{{if .Question}}Your name and your answer are unverified labels, not a verified identity. Both are shown to the application you are joining, together with the address above.{{else}}Your name is an unverified label, not a verified identity. It is shown to the application you are joining, together with the address above.{{end}}
` func (s *Server) join(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" && r.Method != "POST" { @@ -733,15 +808,20 @@ func (s *Server) join(w http.ResponseWriter, r *http.Request) { // enrolled. Someone returning to this page has no other way to see which // identity they are about to continue as -- and the name is the identity // here, so it is the one thing worth showing. - enrolledName, enrolledEmail := "", "" + enrolledName, enrolledEmail, enrolledAnswer := "", "", "" if enrolled { _, p, e := s.identity(r.Context(), ss) enrolled = e == nil if enrolled { enrolledName = p.Spec.DisplayName enrolledEmail = participantEmail(p) + enrolledAnswer = answerLabel(room, p) } } + // The answer a form came back with, so a refused join keeps it picked. It + // only ever selects one of the Room's own answers on the page; nothing + // renders it. + answer := r.FormValue("answer") // A code carried here by a scanned QR code. Only ever a prefill: the POST // below re-reads it from the form and Room Pass checks it against the // Room's rotating status exactly as it checks a typed one. @@ -816,7 +896,7 @@ func (s *Server) join(w http.ResponseWriter, r *http.Request) { // Email is what the script would compute for the name already in the // box, so a browser with script disabled and a form that came back with // a typed name both still show the address that is actually on offer. - _ = page.Execute(w, map[string]any{"Title": room.Spec.Title, "Look": lookOf(room), "AttributionNote": room.Spec.AttributionNote, "Message": heading, "Form": form, "CSRF": csrf, "Handoff": handoff, "Return": dest, "Enrolled": enrolled, "EnrolledName": enrolledName, "EnrolledEmail": enrolledEmail, "Scanned": pill, "Code": code, "Error": failure, "Name": name, "Display": displayPreview(name), "Email": emailPreview(name), "Focus": focus, "CodeInvalid": field == "code", "NameInvalid": field == "name"}) + _ = page.Execute(w, map[string]any{"Title": room.Spec.Title, "Look": lookOf(room), "AttributionNote": room.Spec.AttributionNote, "Message": heading, "Form": form, "CSRF": csrf, "Handoff": handoff, "Return": dest, "Enrolled": enrolled, "EnrolledName": enrolledName, "EnrolledEmail": enrolledEmail, "EnrolledAnswer": enrolledAnswer, "Scanned": pill, "Code": code, "Error": failure, "Name": name, "Display": displayPreview(name), "Email": emailPreview(name), "Question": questionOf(room), "Answer": answer, "Focus": focus, "CodeInvalid": field == "code", "NameInvalid": field == "name", "AnswerInvalid": field == "answer"}) } if r.Method == "GET" { render(200, notice, "", "", "") @@ -856,7 +936,7 @@ func (s *Server) join(w http.ResponseWriter, r *http.Request) { render(400, e.Error(), "name", code, typed) return } - ss, e = s.enrollParticipant(r.Context(), code, name) + ss, e = s.enrollParticipant(r.Context(), code, name, answer) if e != nil { field := "" switch { @@ -864,6 +944,8 @@ func (s *Server) join(w http.ResponseWriter, r *http.Request) { field = "code" case errors.Is(e, errNameTaken): field = "name" + case errors.Is(e, errBadAnswer): + field = "answer" } // The folded name goes back in the box, not the raw one: it is what // would have been stored, and it is what the participant has to @@ -912,6 +994,10 @@ var errBadCode = errors.New("That code is invalid or joining has closed. Check t // participant can actually change. var errNameTaken = errors.New("That name is already taken in this room. Please choose another.") +// And an answer the Room does not offer. The browser will not submit the form +// without one, so this is a forged form or a browser that skipped the check. +var errBadAnswer = errors.New("Please pick one of the answers.") + // maxTypedCodeLength matches the code field's maxlength, so a value that only // a forged form could carry is bounded before it goes back into the page. const maxTypedCodeLength = 24 @@ -934,7 +1020,7 @@ func typedCode(raw string) string { }, strings.ToValidUTF8(v, "")) } -func (s *Server) enrollParticipant(ctx context.Context, code, name string) (session, error) { +func (s *Server) enrollParticipant(ctx context.Context, code, name, answer string) (session, error) { result := "storage_error" defer func() { if s.metrics != nil { @@ -951,6 +1037,14 @@ func (s *Server) enrollParticipant(ctx context.Context, code, name string) (sess result = "code_or_room_rejected" return session{}, errBadCode } + // Checked against the Room read under the lock, the same one the code was + // accepted by: a question edited mid-join started a new code epoch, so the + // code and the answers a form carries always come from one version of it. + groups, e := answerGroups(room, answer) + if e != nil { + result = "answer_rejected" + return session{}, e + } ps := &api.ParticipantList{} if e = s.db.List(ctx, ps, client.InNamespace(room.Namespace)); e != nil { return session{}, errors.New("Room temporarily unavailable") @@ -965,7 +1059,7 @@ func (s *Server) enrollParticipant(ctx context.Context, code, name string) (sess result = "full" return session{}, errors.New("This room is full. Please ask the presenter.") } - p := &api.Participant{ObjectMeta: metav1.ObjectMeta{Name: participantPrefix + participantID(name), Namespace: room.Namespace, OwnerReferences: []metav1.OwnerReference{{APIVersion: api.GroupVersion.String(), Kind: "Room", Name: room.Name, UID: room.UID}}}, Spec: api.ParticipantSpec{RoomRef: api.RoomRef{Name: room.Name, UID: string(room.UID)}, DisplayName: name}} + p := &api.Participant{ObjectMeta: metav1.ObjectMeta{Name: participantPrefix + participantID(name), Namespace: room.Namespace, OwnerReferences: []metav1.OwnerReference{{APIVersion: api.GroupVersion.String(), Kind: "Room", Name: room.Name, UID: room.UID}}}, Spec: api.ParticipantSpec{RoomRef: api.RoomRef{Name: room.Name, UID: string(room.UID)}, DisplayName: name, Groups: groups}} if e = s.db.Create(ctx, p); e != nil { if !apierrors.IsAlreadyExists(e) { return session{}, errors.New("Enrollment could not be confirmed. Please retry.") @@ -1067,7 +1161,7 @@ func (s *Server) complete(w http.ResponseWriter, r *http.Request) { r.Header.Set("X-Remote-User", p.Spec.DisplayName) r.Header.Set("X-Remote-User-Name", p.Spec.DisplayName) r.Header.Set("X-Remote-User-Email", participantEmail(p)) - r.Header.Set("X-Remote-Group", room.Spec.AudienceGroup) + r.Header.Set("X-Remote-Group", groupHeader(room, p)) r.Header.Del("Cookie") r.Header.Del("Authorization") r.URL.Path = s.callbackPath() diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 82f6c1a..11a7d97 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -59,7 +59,7 @@ func TestEnrollmentConcurrencyAndLifecycle(t *testing.T) { wg.Add(1) go func(i int) { defer wg.Done() - if _, e := s.enrollParticipant(context.Background(), "bcd-fgh", fmt.Sprintf("Ada %d", i)); e == nil { + if _, e := s.enrollParticipant(context.Background(), "bcd-fgh", fmt.Sprintf("Ada %d", i), ""); e == nil { ok.Add(1) } }(i) @@ -68,7 +68,7 @@ func TestEnrollmentConcurrencyAndLifecycle(t *testing.T) { if ok.Load() != 100 { t.Fatalf("shared-room burst: %d", ok.Load()) } - if _, e := s.enrollParticipant(context.Background(), "BCDFGH", "overflow"); e == nil { + if _, e := s.enrollParticipant(context.Background(), "BCDFGH", "overflow", ""); e == nil { t.Fatal("cap bypassed") } ps := &api.ParticipantList{} @@ -155,7 +155,7 @@ func hidden(body, name string) string { // looking at the presenter's screen again. func TestARejectedJoinKeepsWhatWasTyped(t *testing.T) { s, _ := fixture(t, "http://dex.test") - if _, e := s.enrollParticipant(context.Background(), "BCDFGH", "Ada"); e != nil { + if _, e := s.enrollParticipant(context.Background(), "BCDFGH", "Ada", ""); e != nil { t.Fatal(e) } submit := func(b *browser, form url.Values) *httptest.ResponseRecorder { @@ -349,7 +349,7 @@ func TestStolenCrossHostLinkCannotEnroll(t *testing.T) { func TestCurrentRoomStateAndCookieTampering(t *testing.T) { s, db := fixture(t, "http://dex.test") ctx := context.Background() - ss, e := s.enrollParticipant(ctx, "BCDFGH", "Ada") + ss, e := s.enrollParticipant(ctx, "BCDFGH", "Ada", "") if e != nil { t.Fatal(e) } @@ -488,7 +488,7 @@ func TestIdentityRequiresCurrentEnrollment(t *testing.T) { t.Run(tc.name, func(t *testing.T) { s, db := fixture(t, "http://dex.test") ctx := context.Background() - ss, err := s.enrollParticipant(ctx, "BCDFGH", "Ada") + ss, err := s.enrollParticipant(ctx, "BCDFGH", "Ada", "") if err != nil { t.Fatal(err) } @@ -674,7 +674,7 @@ func TestScannedCodeIsAPrefillAndNothingMore(t *testing.T) { // That has to be worth nothing: the POST re-checks the value against // the Room's valid codes, so a made-up one fails exactly as a typed // made-up one fails. - if _, err := s.enrollParticipant(context.Background(), "ZZZZZZ", "Mallory"); err == nil { + if _, err := s.enrollParticipant(context.Background(), "ZZZZZZ", "Mallory", ""); err == nil { t.Fatal("a code that the Room never issued was accepted") } }) @@ -871,14 +871,14 @@ func TestNamesWithoutUsableCharactersAreRefused(t *testing.T) { // quietly seated at the first person's Participant with their own cookie. func TestASecondClaimOnANameIsRefused(t *testing.T) { s, db := fixture(t, "http://dex.test") - first, e := s.enrollParticipant(context.Background(), "BCDFGH", "Ada Demo") + first, e := s.enrollParticipant(context.Background(), "BCDFGH", "Ada Demo", "") if e != nil { t.Fatal(e) } if first.Name != "p-ada-demo" { t.Fatalf("participant object name: %q", first.Name) } - if _, e = s.enrollParticipant(context.Background(), "BCDFGH", "ada demo"); !errors.Is(e, errNameTaken) { + if _, e = s.enrollParticipant(context.Background(), "BCDFGH", "ada demo", ""); !errors.Is(e, errNameTaken) { t.Fatalf("a colliding name enrolled anyway: %v", e) } ps := &api.ParticipantList{} @@ -886,7 +886,7 @@ func TestASecondClaimOnANameIsRefused(t *testing.T) { if len(ps.Items) != 1 { t.Fatalf("participants after the refused claim: %d", len(ps.Items)) } - if _, e = s.enrollParticipant(context.Background(), "BCDFGH", "Ada Demo 2"); e != nil { + if _, e = s.enrollParticipant(context.Background(), "BCDFGH", "Ada Demo 2", ""); e != nil { t.Fatal(e) } } @@ -1182,3 +1182,237 @@ func TestAppearancePatternsAreTheCRDs(t *testing.T) { } } } + +// setQuestion replaces the fixture Room's question, as an operator's apply +// would. +func setQuestion(t *testing.T, s *Server, db client.Client, q *api.RoomQuestion) { + t.Helper() + room := &api.Room{} + if e := db.Get(context.Background(), s.cfg.Room, room); e != nil { + t.Fatal(e) + } + room.Spec.Question = q + if e := db.Update(context.Background(), room); e != nil { + t.Fatal(e) + } +} + +func frameworks() *api.RoomQuestion { + return &api.RoomQuestion{Prompt: "Your favourite frontend framework?", Answers: []api.RoomAnswer{ + {Label: "React", Group: "demo:framework-react"}, + {Label: "Vue", Group: "demo:framework-vue"}, + {Label: "Svelte", Group: "demo:framework-svelte"}, + }} +} + +// joinPage walks a browser from Dex's callback to the join page, as a login +// does, and returns the page. +func joinPage(t *testing.T, s *Server, b *browser) string { + t.Helper() + w := b.request(s, "GET", "https://login.test/callback/room-pass?state=dex-transaction", nil) + for i := 0; i < 3; i++ { + if w.Code != 303 { + t.Fatalf("binding step %d: %d %s", i, w.Code, w.Body.String()) + } + w = b.request(s, "GET", w.Header().Get("Location"), nil) + } + if w.Code != 200 { + t.Fatalf("join page: %d %s", w.Code, w.Body.String()) + } + return w.Body.String() +} + +// submit posts the form a page rendered, filled in with fields. +func submit(s *Server, b *browser, page string, fields url.Values) *httptest.ResponseRecorder { + form := url.Values{"csrf": {hidden(page, "csrf")}, "handoff": {hidden(page, "handoff")}, "return": {hidden(page, "return")}} + for k, v := range fields { + form[k] = v + } + return b.request(s, "POST", "https://demo.test/join", form) +} + +// dexRecorder stands in for Dex and keeps the group header of the identity +// Room Pass hands it. +func dexRecorder(t *testing.T) (url string, group func() string) { + t.Helper() + var mu sync.Mutex + got := "" + dex := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + got = r.Header.Get("X-Remote-Group") + mu.Unlock() + w.WriteHeader(204) + })) + t.Cleanup(dex.Close) + return dex.URL, func() string { mu.Lock(); defer mu.Unlock(); return got } +} + +func participantsOf(t *testing.T, db client.Client) []api.Participant { + t.Helper() + ps := &api.ParticipantList{} + if e := db.List(context.Background(), ps); e != nil { + t.Fatal(e) + } + return ps.Items +} + +// A Room without a question is every Room that existed before the field did, +// including the one a minor release rolls out to unattended. Nobody is asked +// anything, and an answer in a forged form adds no group: the identity Dex +// receives is exactly the one it always was. +func TestARoomWithoutAQuestionAddsNoGroup(t *testing.T) { + upstream, group := dexRecorder(t) + s, db := fixture(t, upstream) + for _, q := range []*api.RoomQuestion{nil, {Prompt: "Only one?", Answers: []api.RoomAnswer{{Label: "Yes", Group: "demo:yes"}}}} { + setQuestion(t, s, db, q) + body := newBrowser().request(s, "GET", "https://demo.test/join", nil).Body.String() + for _, absent := range []string{"Your name is an unverified label, not a verified identity. It is shown to the application you are joining, together with the address above.") { + t.Errorf("question %+v: the notice changed", q) + } + } + b := newBrowser() + page := joinPage(t, s, b) + w := submit(s, b, page, url.Values{"code": {"BCDFGH"}, "name": {"Ada"}, "answer": {"demo:yes"}}) + if w.Code != 303 { + t.Fatalf("join: %d %s", w.Code, w.Body.String()) + } + if w = b.request(s, "GET", w.Header().Get("Location"), nil); w.Code != 204 { + t.Fatalf("complete: %d %s", w.Code, w.Body.String()) + } + if got := group(); got != "demo:test" { + t.Errorf("X-Remote-Group = %q, want the audience group alone", got) + } + if ps := participantsOf(t, db); len(ps) != 1 || ps[0].Spec.Groups != nil { + t.Errorf("a forged answer was stored: %+v", ps) + } +} + +// The demo this was built for: everyone picks a framework at the door, and the +// framework's group arrives in their token beside the Room's, where RBAC can +// tell the Svelte people from the rest. +func TestTheAnswerBecomesAGroup(t *testing.T) { + upstream, group := dexRecorder(t) + s, db := fixture(t, upstream) + setQuestion(t, s, db, frameworks()) + b := newBrowser() + page := joinPage(t, s, b) + for _, want := range []string{ + `
Your favourite frontend framework?`, + ``, + `
`, + "Your name and your answer are unverified labels", + } { + if !strings.Contains(page, want) { + t.Errorf("the join page lacks %q:\n%s", want, page) + } + } + w := submit(s, b, page, url.Values{"code": {"BCDFGH"}, "name": {"Ada"}, "answer": {"demo:framework-svelte"}}) + if w.Code != 303 { + t.Fatalf("join: %d %s", w.Code, w.Body.String()) + } + ps := participantsOf(t, db) + if len(ps) != 1 || len(ps[0].Spec.Groups) != 1 || ps[0].Spec.Groups[0] != "demo:framework-svelte" { + t.Fatalf("the answer was not kept on the Participant: %+v", ps) + } + if w = b.request(s, "GET", w.Header().Get("Location"), nil); w.Code != 204 { + t.Fatalf("complete: %d %s", w.Code, w.Body.String()) + } + if got := group(); got != "demo:test,demo:framework-svelte" { + t.Errorf("X-Remote-Group = %q, want the audience group and the answer's", got) + } + // Returning, the participant is not asked again and sees what they picked: + // it is part of the identity they are about to continue as. + body := b.request(s, "GET", "https://demo.test/join", nil).Body.String() + if !strings.Contains(body, "You’re already enrolled as Ada, and you picked Svelte.") { + t.Errorf("the returning page hides the answer: %s", body) + } + if strings.Contains(body, "`, `value="BCD-FGH"`, `value="Ada"`, `value="demo:framework-react" required autofocus>`} { + if !strings.Contains(page, kept) { + t.Errorf("answer %q: the refused page lacks %q", answer, kept) + } + } + if strings.Contains(page, " checked") { + t.Errorf("answer %q: an answer the Room does not offer was checked", answer) + } + } + if ps := participantsOf(t, db); len(ps) != 0 { + t.Fatalf("a refused answer enrolled: %+v", ps) + } + // A join refused for something else keeps the answer picked, so a mistyped + // code is the only thing to fix. + w := submit(s, b, page, url.Values{"code": {"WRONG1"}, "name": {"Ada"}, "answer": {"demo:framework-vue"}}) + if w.Code != 403 || !strings.Contains(w.Body.String(), `value="demo:framework-vue" required checked>`) { + t.Fatalf("a wrong code dropped the answer: %d %s", w.Code, w.Body.String()) + } + if w = submit(s, b, w.Body.String(), url.Values{"code": {"BCDFGH"}, "name": {"Ada"}, "answer": {"demo:framework-vue"}}); w.Code != 303 { + t.Fatalf("a corrected form did not enroll: %d %s", w.Code, w.Body.String()) + } +} + +// The CRD refuses all of these. A Room or Participant that holds one anyway -- +// an older CRD, a schema edited by hand -- loses the group rather than putting +// it in front of Dex, which would split "demo:a,system:masters" into two. +func TestGroupsThatEscapedTheSchemaAreNotSent(t *testing.T) { + room := &api.Room{Spec: api.RoomSpec{AudienceGroup: "demo:test", Question: &api.RoomQuestion{Prompt: "?", Answers: []api.RoomAnswer{ + {Label: "Fine", Group: "demo:fine"}, + {Label: "Two", Group: "demo:a,system:masters"}, + {Label: "System", Group: "system:masters"}, + {Label: "Long", Group: "demo:" + strings.Repeat("x", 124)}, + }}}} + p := &api.Participant{Spec: api.ParticipantSpec{Groups: []string{"demo:fine", "demo:a,system:masters", "system:masters", "demo:test", "demo:fine", "demo:" + strings.Repeat("x", 124)}}} + if got := groupHeader(room, p); got != "demo:test,demo:fine" { + t.Errorf("groupHeader = %q", got) + } + // One usable answer is not a question anybody can be asked. + if q := questionOf(room); q != nil { + t.Errorf("a question with one usable answer is asked: %+v", q) + } + if groups, e := answerGroups(room, "demo:fine"); e != nil || groups != nil { + t.Errorf("answerGroups = %v, %v for a question that is not asked", groups, e) + } + room.Spec.Question.Answers = append(room.Spec.Question.Answers, api.RoomAnswer{Label: "Also fine", Group: "demo:also-fine"}) + if q := questionOf(room); q == nil || len(q.Answers) != 2 { + t.Errorf("questionOf = %+v, want the two usable answers", q) + } + if _, e := answerGroups(room, "system:masters"); !errors.Is(e, errBadAnswer) { + t.Errorf("an answer the page never offered was accepted: %v", e) + } +} + +// The server's group pattern is a second check on the CRDs', so it must be +// the same one. +func TestGroupPatternIsTheCRDs(t *testing.T) { + for file, want := range map[string]int{"rooms": 2, "participants": 1} { + crd, e := os.ReadFile("../../config/crd/bases/room-pass.koudijs.dev_" + file + ".yaml") + if e != nil { + t.Fatal(e) + } + if n := strings.Count(string(crd), "pattern: "+demoGroup.String()+"\n"); n != want { + t.Errorf("the %s CRD carries %s %d times, want %d", file, demoGroup, n, want) + } + } +} diff --git a/requirements.md b/requirements.md index 11f01ae..1d05430 100644 --- a/requirements.md +++ b/requirements.md @@ -92,6 +92,7 @@ spec: | `title` | Required, nonempty, bounded display text | | `attributionNote` | Optional, bounded text shown beside the issued address. The operator's sentence about what the application does with it; Room Pass only guarantees the address is unroutable | | `appearance` | Optional, mutable, display only: a tagline, a picture and its alt text, an accent colour, a background colour and a background image. Pictures are same-origin paths on the join host; colours are `#rrggbb`. It cannot reword the page's statements about identity | +| `question` | Optional, mutable: a prompt and 2–8 answers, each a label and a group under `audienceGroup`'s rules. Every joining participant picks one; its group is stored on the Participant and asserted beside `audienceGroup`. Self-selected, so never stronger than the display name. Without it, nobody is asked and no group is added | | `endsAt` | Required timestamp, editable by operators to accommodate schedule changes; shortening ends access sooner and extending can reopen an expired, non-stopped Room | | `enrollment` | `Open` or `Closed`; defaults to `Closed` | | `stopped` | Defaults to false; transition to true is irreversible for this object | @@ -191,6 +192,7 @@ spec: name: configbutler-demo uid: displayName: Ramon + groups: [demo:framework-svelte] # only when the Room asks a question revoked: false ``` @@ -199,8 +201,10 @@ a prefix plus the display name folded to a Kubernetes-safe identifier, so the id is guessable by design; its Kubernetes UID, not its name, binds the browser cookie to that exact enrollment. Room reference and display name are immutable; eligibility follows the Room’s current `endsAt`, so extending a talk does not require rewriting Participants. Revocation -is irreversible for the object. Derive the author email and group from server-owned -identity/Room data rather than accepting them in a participant payload. Audience users +is irreversible for the object. Derive the author email and groups from server-owned +identity/Room data rather than accepting them in a participant payload: an answer is +accepted only as one of the Room's own, and its group is copied from the Room. `groups` +is mutable, so an operator can move a participant; the next sign-in carries the change. Audience users cannot directly create, read, patch or list Participants through Kubernetes. A signed/encrypted HttpOnly cookie references the Room and Participant UIDs and expiry. @@ -255,7 +259,7 @@ The room code proves possession, not physical attendance or one-person-one-vote. | Participant ID | Derived by the server from the display name, unique within the event and immutable once enrolled; public, never a credential | | Display name | Participant-supplied, validated and escaped; never used as an authorization key | | Author email | Server-generated `@koudijs.dev.test`; never supplied by the participant, and shown on the join page while the name is typed | -| Group | Taken from event configuration, never from client input | +| Groups | `audienceGroup`, plus the group of the participant's answer when the Room asks a question. Both come from event configuration: a participant chooses among the Room's answers and never names a group | | Session | Signed/encrypted cookie referencing persisted Room and Participant UIDs; no separate Session CRD | Validate display names using the existing Git-safe rules where suitable: reject control diff --git a/test/browser/README.md b/test/browser/README.md index 78036b1..e470192 100644 --- a/test/browser/README.md +++ b/test/browser/README.md @@ -8,7 +8,8 @@ task browser This runs real Chromium through Traefik, Room Pass, Dex and the local OIDC demo client. The tests cover new enrollment, returning enrollment with the same Participant UID, invalid codes, tampered CSRF forms, secure enrollment-cookie flags, closed enrollment, -and a dressed Room whose pictures load from the join host under its CSP. A phone-sized viewport +a dressed Room whose pictures load from the join host under its CSP, and a Room's +question whose answer arrives in the token's groups and is accepted by Kubernetes. A phone-sized viewport exercises the room form without mocking browser headers or the login protocol. The test runner reads only `.local/kubeconfig`, never the current user diff --git a/test/browser/room-auth.spec.js b/test/browser/room-auth.spec.js index a5a07e0..1109751 100644 --- a/test/browser/room-auth.spec.js +++ b/test/browser/room-auth.spec.js @@ -311,3 +311,75 @@ test("a forged join code is still just a wrong code", async ({ page }) => { participants().filter((p) => p.spec.displayName === storedName), ).toHaveLength(0); }); + +// A Room's question, end to end: the browser will not join without an answer, +// the answer is kept on the Participant, Dex splits the group header into the +// token's groups, and the apiserver accepts a token carrying both. Each of those +// is a different program, and this is the only place they all run together. +test("a Room's question adds the picked answer to the token's groups", async ({ + page, +}, testInfo) => { + try { + patchRoom({ + question: { + prompt: "Your favourite frontend framework?", + answers: [ + { label: "React", group: "demo:framework-react" }, + { label: "Vue", group: "demo:framework-vue" }, + { label: "Svelte", group: "demo:framework-svelte" }, + ], + }, + }); + await roomSettled(); + await page.goto("/app/login"); + const question = page.getByRole("group", { + name: "Your favourite frontend framework?", + }); + await expect(question.getByRole("radio")).toHaveCount(3); + await page.getByLabel("Room code").fill(room().status.joinCode.code); + await page.getByLabel("Display name").fill(testName); + // Nothing picked: the browser holds the form back. + await page.getByRole("button", { name: "Continue", exact: true }).click(); + expect( + await question + .getByRole("radio", { name: "React" }) + .evaluate((r) => r.validity.valueMissing), + ).toBe(true); + expect( + participants().filter((p) => p.spec.displayName === storedName), + ).toHaveLength(0); + await question.getByRole("radio", { name: "Svelte" }).check(); + await page.screenshot({ + path: testInfo.outputPath("question.png"), + fullPage: true, + }); + await page.getByRole("button", { name: "Continue", exact: true }).click(); + await expect( + page.getByText(`Welcome, ${storedName}.`, { exact: true }), + ).toBeVisible(); + await expect( + page.getByText( + "Your groups: demo:room-pass-test, demo:framework-svelte", + { exact: true }, + ), + ).toBeVisible(); + const [p] = participants().filter((p) => p.spec.displayName === storedName); + expect(p.spec.groups).toEqual(["demo:framework-svelte"]); + await page + .getByRole("button", { name: "Write a message to Kubernetes" }) + .click(); + await expect(page.getByText(/^Kubernetes returned 201/)).toBeVisible(); + // Returning, nobody is asked twice; the page says what they picked. + await page.getByRole("link", { name: "Sign in again" }).click(); + await expect( + page.getByText( + `You’re already enrolled as ${storedName}, and you picked Svelte.`, + { exact: false }, + ), + ).toBeVisible(); + await expect(page.getByRole("radio")).toHaveCount(0); + } finally { + patchRoom({ question: null }); + await roomSettled(); + } +}); diff --git a/test/e2e/demo-client/main.go b/test/e2e/demo-client/main.go index 9ea0b31..1d7f559 100644 --- a/test/e2e/demo-client/main.go +++ b/test/e2e/demo-client/main.go @@ -28,7 +28,7 @@ import ( const origin = "https://demo.room-pass.test:18443" -type session struct{ Token, Name, State, Verifier string } +type session struct{ Token, Name, Groups, State, Verifier string } func id() string { b := make([]byte, 32) @@ -88,10 +88,14 @@ func main() { kp := x509.NewCertPool() kp.AppendCertsFromPEM(kubeCA) kube := &http.Client{Timeout: 10 * time.Second, Transport: &http.Transport{TLSClientConfig: &tls.Config{RootCAs: kp, MinVersion: tls.VersionTLS12}}} - page := template.Must(template.New("app").Parse(`Room Pass demo

Room Pass demo

{{if .Name}}

Welcome, {{.Name}}.

{{.Result}}

This uses your demo identity. Work resources are outside its permissions.

{{else}}

Join the room, then make a real Kubernetes change.

{{end}}

{{if .Name}}Sign in again{{else}}Join the demo{{end}}

`)) + page := template.Must(template.New("app").Parse(`Room Pass demo

Room Pass demo

{{if .Name}}

Welcome, {{.Name}}.

Your groups: {{.Groups}}

{{.Result}}

This uses your demo identity. Work resources are outside its permissions.

{{else}}

Join the room, then make a real Kubernetes change.

{{end}}

{{if .Name}}Sign in again{{else}}Join the demo{{end}}

`)) http.HandleFunc("/app/", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Cache-Control", "no-store") - w.Header().Set("Referrer-Policy", "no-referrer") + // same-origin, not no-referrer: under no-referrer a browser sends the + // write form's Origin as "null", and the check below refused every real + // browser while Go clients, which set Origin themselves, passed. Room + // Pass learned the same thing the same way. + w.Header().Set("Referrer-Policy", "same-origin") w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; form-action 'self'; frame-ancestors 'none'") s, e := get(r) switch r.URL.Path { @@ -130,11 +134,14 @@ func main() { http.Error(w, "Token invalid", 403) return } + // The groups are shown so the browser suite can read what a Room's + // question added, from the token Dex actually issued. var claims struct { - Name string `json:"name"` + Name string `json:"name"` + Groups []string `json:"groups"` } _ = verified.Claims(&claims) - set(w, session{Token: raw, Name: claims.Name, State: id()}) + set(w, session{Token: raw, Name: claims.Name, Groups: strings.Join(claims.Groups, ", "), State: id()}) http.Redirect(w, r, "/app/", 303) return case "/app/write": @@ -156,11 +163,11 @@ func main() { b, _ := io.ReadAll(io.LimitReader(resp.Body, 16384)) result := fmt.Sprintf("Kubernetes returned %d\n%s", resp.StatusCode, b) w.Header().Set("Content-Type", "text/html; charset=utf-8") - _ = page.Execute(w, map[string]string{"Name": s.Name, "CSRF": s.State, "Result": result}) + _ = page.Execute(w, map[string]string{"Name": s.Name, "Groups": s.Groups, "CSRF": s.State, "Result": result}) return case "/app/": w.Header().Set("Content-Type", "text/html; charset=utf-8") - _ = page.Execute(w, map[string]string{"Name": s.Name, "CSRF": s.State}) + _ = page.Execute(w, map[string]string{"Name": s.Name, "Groups": s.Groups, "CSRF": s.State}) return default: http.NotFound(w, r) diff --git a/test/integration/api_test.go b/test/integration/api_test.go index 38c0520..43424d5 100644 --- a/test/integration/api_test.go +++ b/test/integration/api_test.go @@ -3,6 +3,7 @@ package integration import ( "context" "os" + "reflect" "strings" "testing" "time" @@ -62,7 +63,20 @@ func TestAPISchemaAndReconcile(t *testing.T) { func(r *api.Room) { r.Spec.Appearance = &api.RoomAppearance{Picture: "//evil.test/logo.png"} }, func(r *api.Room) { r.Spec.Appearance = &api.RoomAppearance{BackgroundImage: `/a")`} }, func(r *api.Room) { r.Spec.Appearance = &api.RoomAppearance{Tagline: "bold"} }, - func(r *api.Room) { r.Spec.Appearance = &api.RoomAppearance{PictureAlt: strings.Repeat("x", 121)} }} { + func(r *api.Room) { r.Spec.Appearance = &api.RoomAppearance{PictureAlt: strings.Repeat("x", 121)} }, + // A question's answers are groups, so they follow audienceGroup's rules. + func(r *api.Room) { r.Spec.Question = question(answer("A", "system:masters"), answer("B", "demo:b")) }, + func(r *api.Room) { + r.Spec.Question = question(answer("A", "demo:a,system:masters"), answer("B", "demo:b")) + }, + func(r *api.Room) { r.Spec.Question = question(answer("A", "demo:a")) }, + func(r *api.Room) { r.Spec.Question = question(answer("A", "demo:a"), answer("B", "demo:a")) }, + func(r *api.Room) { r.Spec.Question = question(answer("Same", "demo:a"), answer("Same", "demo:b")) }, + func(r *api.Room) { r.Spec.Question = question(answer("A", "demo:a"), answer("B", "demo:b")) }, + func(r *api.Room) { + r.Spec.Question = question(answer("A", "demo:a"), answer("B", "demo:b")) + r.Spec.Question.Prompt = "" + }} { bad := room.DeepCopy() mutate(bad) if db.Update(ctx, bad) == nil { @@ -89,6 +103,14 @@ func TestAPISchemaAndReconcile(t *testing.T) { if e = db.Update(ctx, room); e != nil { t.Fatalf("appearance cannot be removed: %v", e) } + asked := question(answer("React", "demo:framework-react"), answer("Svelte", "demo:framework-svelte")) + room.Spec.Question = asked + if e = db.Update(ctx, room); e != nil { + t.Fatalf("question rejected: %v", e) + } + if e = db.Get(ctx, key, room); e != nil || room.Spec.Question == nil || !reflect.DeepEqual(*room.Spec.Question, *asked) { + t.Fatalf("question did not round-trip: %+v %v", room.Spec.Question, e) + } rec := &controller.Reconciler{Client: db, Room: key} if _, e = rec.Reconcile(ctx, ctrl.Request{NamespacedName: key}); e != nil { t.Fatal(e) @@ -114,10 +136,23 @@ func TestAPISchemaAndReconcile(t *testing.T) { if db.Update(ctx, room) == nil { t.Fatal("stop reversed") } - p := &api.Participant{ObjectMeta: metav1.ObjectMeta{Name: "p-test", Namespace: room.Namespace}, Spec: api.ParticipantSpec{RoomRef: api.RoomRef{Name: room.Name, UID: string(room.UID)}, DisplayName: "Ada"}} + // A Participant's groups follow the same rules as the answers they come from. + for _, groups := range [][]string{{"system:masters"}, {"demo:a,system:masters"}, {"demo:a", "demo:a"}} { + bad := &api.Participant{ObjectMeta: metav1.ObjectMeta{Name: "p-bad", Namespace: room.Namespace}, Spec: api.ParticipantSpec{RoomRef: api.RoomRef{Name: room.Name, UID: string(room.UID)}, DisplayName: "Bad", Groups: groups}} + if db.Create(ctx, bad) == nil { + t.Fatalf("invalid groups accepted: %v", groups) + } + } + p := &api.Participant{ObjectMeta: metav1.ObjectMeta{Name: "p-test", Namespace: room.Namespace}, Spec: api.ParticipantSpec{RoomRef: api.RoomRef{Name: room.Name, UID: string(room.UID)}, DisplayName: "Ada", Groups: []string{"demo:framework-svelte"}}} if e = db.Create(ctx, p); e != nil { t.Fatal(e) } + // An operator may move a participant to another group; their next sign-in + // carries it. + p.Spec.Groups = []string{"demo:framework-react"} + if e = db.Update(ctx, p); e != nil { + t.Fatalf("groups are not editable: %v", e) + } p.Spec.Revoked = true if e = db.Update(ctx, p); e != nil { t.Fatal(e) @@ -127,3 +162,9 @@ func TestAPISchemaAndReconcile(t *testing.T) { t.Fatal("revocation reversed") } } + +func question(answers ...api.RoomAnswer) *api.RoomQuestion { + return &api.RoomQuestion{Prompt: "Your favourite frontend framework?", Answers: answers} +} + +func answer(label, group string) api.RoomAnswer { return api.RoomAnswer{Label: label, Group: group} }